Apply "polymorphization at home" to RawVec #126793

saethlin · 2024-06-21T16:32:10Z

The idea here is to move all the logic in RawVec into functions with explicit size and alignment parameters. This should eliminate all the fussing about how tweaking RawVec code produces large swings in compile times.

This uncovered rust-lang/rust-clippy#12979, so I've modified the relevant test in a way that tries to preserve the spirit of the test without tripping the ICE.

saethlin · 2024-06-21T17:29:42Z

@bors try @rust-timer queue

Monomorphize RawVec The idea here is to move all the logic in RawVec into functions with explicit size and alignment parameters. This should eliminate all the fussing about how tweaking RawVec code produces large swings in compile times. r? `@ghost`

bors · 2024-06-21T17:30:55Z

⌛ Trying commit ec16141 with merge 7646794...

library/alloc/src/raw_vec.rs

scottmcm · 2024-06-21T18:27:20Z

library/alloc/src/raw_vec.rs

@@ -462,85 +657,26 @@ impl<T, A: Allocator> RawVec<T, A> {
    // of the code that doesn't depend on `T` as possible is in functions that
    // are non-generic over `T`.
    fn grow_amortized(&mut self, len: usize, additional: usize) -> Result<(), TryReserveError> {
+        const { assert!(mem::size_of::<T>() % mem::align_of::<T>() == 0) };


nit: is this checking anything? That's definitionally true for all types, AFAIK...

🤷 This was in the original code. There's also a wrong comment in current_memory about types where size != stride, such types do not exist in Rust.

#83706 added this check by going through Layout::array
#107167 partially undid it since it previously caused a small perf regression but left the const assert in its place to move the check to compile time

I've deleted all these const assertions. I don't know if that was the right move here? I'm not opposed to adding it by some other means, but it seems a bit silly to have an assertion if it's true by definition.

library/alloc/src/raw_vec.rs

scottmcm · 2024-06-21T18:53:18Z

library/alloc/src/raw_vec.rs

+}
+
+struct MonoRawVec<A: Allocator = Global> {
+    ptr: Unique<u8>,


YMMV: Maybe make yourself an opaque type (like the one in

rust/library/core/src/ptr/metadata.rs

Lines 160 to 166 in 5ced3da

extern "C" {

/// Opaque type for accessing vtables.

///

/// Private implementation detail of `DynMetadata::size_of` etc.

/// There is conceptually not actually any Abstract Machine memory behind this pointer.

type VTable;

}

) to make sure you don't accidentally use the u8-ness of this anywhere?

bors · 2024-06-21T19:12:23Z

☀️ Try build successful - checks-actions
Build commit: 7646794 (764679442556a000f69f7c7f3f3b0080d4dae76a)

rust-timer · 2024-06-21T20:43:53Z

Finished benchmarking commit (7646794): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Benchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf.

Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please fix the regressions and do another perf run. If the next run shows neutral or positive results, the label will be automatically removed.

@bors rollup=never
@rustbot label: -S-waiting-on-perf +perf-regression

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.9%	[2.9%, 2.9%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.3%	[-5.0%, -0.2%]	82
Improvements ✅ (secondary)	-1.8%	[-4.4%, -0.3%]	11
All ❌✅ (primary)	-1.3%	[-5.0%, 2.9%]	83

Max RSS (memory usage)

Results (primary -0.6%, secondary -4.9%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	4.4%	[1.4%, 7.4%]	5
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.1%	[-8.4%, -2.2%]	7
Improvements ✅ (secondary)	-4.9%	[-5.5%, -4.3%]	2
All ❌✅ (primary)	-0.6%	[-8.4%, 7.4%]	12

Cycles

Results (primary -2.2%, secondary -3.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	2.6%	[2.6%, 2.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.4%	[-4.5%, -1.2%]	34
Improvements ✅ (secondary)	-3.2%	[-4.7%, -2.6%]	5
All ❌✅ (primary)	-2.2%	[-4.5%, 2.6%]	35

Binary size

Results (primary -2.0%, secondary -0.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.1%	[0.1%, 2.5%]	11
Regressions ❌ (secondary)	0.1%	[0.1%, 0.2%]	4
Improvements ✅ (primary)	-2.3%	[-7.5%, -0.0%]	105
Improvements ✅ (secondary)	-0.6%	[-4.6%, -0.0%]	47
All ❌✅ (primary)	-2.0%	[-7.5%, 2.5%]	116

Bootstrap: 694.089s -> 690.601s (-0.50%)
Artifact size: 326.85 MiB -> 327.00 MiB (0.05%)

Kobzol · 2024-06-21T21:01:09Z

Woah, this looks awesome! Truly a "we have polymorphization at home" moment.

tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir

saethlin · 2024-06-22T04:09:02Z

@bors try @rust-timer queue

Apply "polymorphization at home" to RawVec The idea here is to move all the logic in RawVec into functions with explicit size and alignment parameters. This should eliminate all the fussing about how tweaking RawVec code produces large swings in compile times. This uncovered rust-lang/rust-clippy#12979, so I've modified the relevant test in a way that tries to preserve the spirit of the test without tripping the ICE. try-job: x86_64-msvc

bors · 2024-08-11T22:04:24Z

⌛ Testing commit 28a8301 with merge ba33d7b...

bors · 2024-08-12T00:31:36Z

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing ba33d7b to master...

bors · 2024-08-12T00:32:34Z

👀 Test was successful, but fast-forwarding failed: 422 Update is not a fast forward

saethlin · 2024-08-12T00:34:57Z

rust-lang/homu#75

@bors r=scottmcm

bors · 2024-08-12T00:34:59Z

💡 This pull request was already approved, no need to approve it again.

This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
There's another pull request that is currently being tested, blocking this pull request: Use FnSig instead of raw FnDecl for ForeignItemKind::Fn, fix ICE for Fn trait error on safe foreign fn #128792

bors · 2024-08-12T00:35:00Z

📌 Commit 28a8301 has been approved by scottmcm

It is now in the queue for this repository.

bors · 2024-08-12T01:47:09Z

⌛ Testing commit 28a8301 with merge 13f8a57...

rust-timer · 2024-08-12T01:48:47Z

Finished benchmarking commit (ba33d7b): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.2%, 1.6%]	98
Regressions ❌ (secondary)	0.4%	[0.1%, 1.2%]	125
Improvements ✅ (primary)	-1.0%	[-3.5%, -0.2%]	51
Improvements ✅ (secondary)	-1.3%	[-2.9%, -0.3%]	13
All ❌✅ (primary)	-0.0%	[-3.5%, 1.6%]	149

Max RSS (memory usage)

Results (primary -0.5%, secondary 0.6%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.1%	[1.8%, 4.4%]	8
Regressions ❌ (secondary)	2.3%	[1.0%, 4.9%]	4
Improvements ✅ (primary)	-3.7%	[-7.0%, -0.9%]	9
Improvements ✅ (secondary)	-2.8%	[-4.1%, -1.4%]	2
All ❌✅ (primary)	-0.5%	[-7.0%, 4.4%]	17

Cycles

Results (primary -1.3%, secondary 0.5%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.5%	[1.1%, 2.5%]	4
Regressions ❌ (secondary)	2.3%	[1.6%, 2.7%]	6
Improvements ✅ (primary)	-2.0%	[-5.8%, -1.0%]	17
Improvements ✅ (secondary)	-2.2%	[-3.0%, -1.4%]	4
All ❌✅ (primary)	-1.3%	[-5.8%, 2.5%]	21

Binary size

Results (primary -1.6%, secondary -0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.0%, 3.6%]	33
Regressions ❌ (secondary)	0.2%	[0.1%, 3.4%]	38
Improvements ✅ (primary)	-2.5%	[-6.8%, -0.1%]	81
Improvements ✅ (secondary)	-1.4%	[-4.5%, -0.1%]	16
All ❌✅ (primary)	-1.6%	[-6.8%, 3.6%]	114

Bootstrap: 756.29s -> 754.361s (-0.26%)
Artifact size: 341.47 MiB -> 341.52 MiB (0.02%)

bors · 2024-08-12T04:13:58Z

☀️ Test successful - checks-actions
Approved by: scottmcm
Pushing 13f8a57 to master...

rust-timer · 2024-08-12T05:30:36Z

Finished benchmarking commit (13f8a57): comparison URL.

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.5%	[0.2%, 1.5%]	89
Regressions ❌ (secondary)	0.4%	[0.1%, 1.3%]	96
Improvements ✅ (primary)	-1.1%	[-3.8%, -0.2%]	54
Improvements ✅ (secondary)	-1.2%	[-2.9%, -0.3%]	13
All ❌✅ (primary)	-0.1%	[-3.8%, 1.5%]	143

Max RSS (memory usage)

Results (primary -0.8%, secondary 0.4%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	3.4%	[2.4%, 4.8%]	6
Regressions ❌ (secondary)	3.4%	[2.0%, 5.7%]	3
Improvements ✅ (primary)	-3.6%	[-6.0%, -1.8%]	9
Improvements ✅ (secondary)	-2.6%	[-4.2%, -1.5%]	3
All ❌✅ (primary)	-0.8%	[-6.0%, 4.8%]	15

Cycles

Results (primary -1.1%, secondary -1.2%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	1.7%	[0.8%, 2.1%]	6
Regressions ❌ (secondary)	6.1%	[6.1%, 6.1%]	1
Improvements ✅ (primary)	-1.7%	[-3.5%, -0.8%]	25
Improvements ✅ (secondary)	-2.6%	[-3.7%, -1.7%]	5
All ❌✅ (primary)	-1.1%	[-3.5%, 2.1%]	31

Binary size

Results (primary -1.6%, secondary -0.3%)

This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.

	mean	range	count
Regressions ❌ (primary)	0.8%	[0.0%, 3.6%]	33
Regressions ❌ (secondary)	0.2%	[0.1%, 3.4%]	38
Improvements ✅ (primary)	-2.5%	[-6.8%, -0.1%]	82
Improvements ✅ (secondary)	-1.4%	[-4.5%, -0.1%]	16
All ❌✅ (primary)	-1.6%	[-6.8%, 3.6%]	115

Bootstrap: 755.478s -> 752.51s (-0.39%)
Artifact size: 341.47 MiB -> 339.31 MiB (-0.63%)

rylev · 2024-08-13T21:20:37Z

Given the comments here, I think this regression can be marked triaged.

@rustbot label: +perf-regression-triaged

Mark-Simulacrum · 2024-08-19T23:47:52Z

Seconding, but also noting that this seems to have been a good memory usage improvement (probably due to smaller code footprint?) that held up as the week progressed. Good work!

…try> Enable Alignment::new_unchecked precondition check Similar to what happened with rust-lang#126556, I think this has become palatable since rust-lang#126793. r? `@ghost`

Apply "polymorphization at home" to RawVec The idea here is to move all the logic in RawVec into functions with explicit size and alignment parameters. This should eliminate all the fussing about how tweaking RawVec code produces large swings in compile times. This uncovered rust-lang/rust-clippy#12979, so I've modified the relevant test in a way that tries to preserve the spirit of the test without tripping the ICE.

…orkingjubilee Enable Alignment::new_unchecked precondition check Similar to what happened with rust-lang#126556, I think this has become palatable since rust-lang#126793.

…ilee Enable Alignment::new_unchecked precondition check Similar to what happened with rust-lang/rust#126556, I think this has become palatable since rust-lang/rust#126793.

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jun 21, 2024

This comment has been minimized.

Sign in to view

saethlin force-pushed the mono-rawvec branch from 29af9bb to ec16141 Compare June 21, 2024 17:29

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 21, 2024

This comment has been minimized.

Sign in to view

scottmcm reviewed Jun 21, 2024

View reviewed changes

library/alloc/src/raw_vec.rs Outdated Show resolved Hide resolved

scottmcm reviewed Jun 21, 2024

View reviewed changes

library/alloc/src/raw_vec.rs Outdated Show resolved Hide resolved

scottmcm reviewed Jun 21, 2024

View reviewed changes

This comment has been minimized.

Sign in to view

saethlin mentioned this pull request Jun 21, 2024

Enable -Zshare-generics for inline(never) functions #123244

Merged

rustbot added perf-regression Performance regression. and removed S-waiting-on-perf Status: Waiting on a perf run to be completed. labels Jun 21, 2024

scottmcm reviewed Jun 21, 2024

View reviewed changes

tests/mir-opt/pre-codegen/vec_deref.vec_deref_to_slice.PreCodegen.after.panic-unwind.mir Outdated Show resolved Hide resolved

saethlin force-pushed the mono-rawvec branch from ec16141 to 389c3ea Compare June 22, 2024 02:35

This comment has been minimized.

Sign in to view

rustbot added A-testsuite Area: The testsuite used to check the correctness of rustc T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. labels Jun 22, 2024

saethlin changed the title ~~Monomorphize RawVec~~ Apply "polymorphization at home" to RawVec Jun 22, 2024

This comment has been minimized.

Sign in to view

rustbot added the S-waiting-on-perf Status: Waiting on a perf run to be completed. label Jun 22, 2024

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 11, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 12, 2024

bors merged commit 13f8a57 into rust-lang:master Aug 12, 2024
7 checks passed

rustbot added this to the 1.82.0 milestone Aug 12, 2024

bors mentioned this pull request Aug 12, 2024

liballoc: introduce String, Vec const-slicing #128399

Merged

rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 13, 2024

saethlin deleted the mono-rawvec branch August 13, 2024 21:30

GrigorenkoPV mentioned this pull request Aug 21, 2024

Add #[track_caller] to allocating methods of Vec & VecDeque #126557

Merged

saethlin mentioned this pull request Aug 23, 2024

Enable Alignment::new_unchecked precondition check #129488

Merged

valenting mentioned this pull request Sep 16, 2024

rust-url's debugger_visualizer tests are broken in rustc nightly-2024-08-13 #130431

Closed

joshuataylor mentioned this pull request Sep 22, 2024

Error: Invalid Value Object vadimcn/codelldb#1142

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply "polymorphization at home" to RawVec #126793

Apply "polymorphization at home" to RawVec #126793

saethlin commented Jun 21, 2024 •

edited

Loading

This comment has been minimized.

saethlin commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 21, 2024

This comment has been minimized.

scottmcm Jun 21, 2024

saethlin Jun 21, 2024

the8472 Jun 21, 2024 •

edited

Loading

saethlin Jul 12, 2024

scottmcm Jun 21, 2024

bors commented Jun 21, 2024

This comment has been minimized.

rust-timer commented Jun 21, 2024

Kobzol commented Jun 21, 2024

This comment has been minimized.

saethlin commented Jun 22, 2024

This comment has been minimized.

bors commented Aug 11, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

saethlin commented Aug 12, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

rust-timer commented Aug 12, 2024

bors commented Aug 12, 2024

rust-timer commented Aug 12, 2024

rylev commented Aug 13, 2024

Mark-Simulacrum commented Aug 19, 2024

	extern "C" {
	/// Opaque type for accessing vtables.
	///
	/// Private implementation detail of `DynMetadata::size_of` etc.
	/// There is conceptually not actually any Abstract Machine memory behind this pointer.
	type VTable;
	}

Apply "polymorphization at home" to RawVec #126793

Apply "polymorphization at home" to RawVec #126793

Conversation

saethlin commented Jun 21, 2024 • edited Loading

This comment has been minimized.

saethlin commented Jun 21, 2024

This comment has been minimized.

bors commented Jun 21, 2024

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

the8472 Jun 21, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bors commented Jun 21, 2024

This comment has been minimized.

rust-timer commented Jun 21, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Kobzol commented Jun 21, 2024

This comment has been minimized.

saethlin commented Jun 22, 2024

This comment has been minimized.

bors commented Aug 11, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

saethlin commented Aug 12, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

bors commented Aug 12, 2024

rust-timer commented Aug 12, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

bors commented Aug 12, 2024

rust-timer commented Aug 12, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

rylev commented Aug 13, 2024

Mark-Simulacrum commented Aug 19, 2024

saethlin commented Jun 21, 2024 •

edited

Loading

the8472 Jun 21, 2024 •

edited

Loading