Apply "polymorphization at home" to RawVec by saethlin · Pull Request #126793 · rust-lang/rust

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

    // 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

bors · 2024-08-10T17:42:38Z

💔 Test failed - checks-actions

saethlin · 2024-08-10T18:15:55Z

@bors try

bors · 2024-08-10T18:17:06Z

⌛ Trying commit c68ffad with merge 75ca9d8...

bors · 2024-08-10T19:13:32Z

💔 Test failed - checks-actions

saethlin · 2024-08-11T18:46:31Z

@bors try

bors · 2024-08-11T18:47:42Z

⌛ Trying commit 730355a with merge b3e67d4...

bors · 2024-08-11T20:32:14Z

☀️ Try build successful - checks-actions
Build commit: b3e67d4 (b3e67d48fc41e60a17a658be1b751f00e64c685b)

saethlin · 2024-08-11T20:53:15Z

After some slow trial-and-error, I think I've fixed the debuginfo tests.
(there's a fair chance this PR will conflict with my debuginfo PRs)

@bors r=scottmcm

bors · 2024-08-11T20:53:18Z

📌 Commit 28a8301 has been approved by scottmcm

It is now in the queue for this repository.

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!

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

This comment has been minimized.

Sign in to view

Fix debuginfo providers/tests

28a8301

bors mentioned this pull request Aug 12, 2024

liballoc: introduce String, Vec const-slicing #128399

Merged

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

tgross35 mentioned this pull request Sep 20, 2025

Fix using SIMD for basic loop iteration with sufficient capacity #146819

Closed

scottmcm mentioned this pull request Nov 20, 2025

See if this is the time we can remove layout::size_align #149109

Merged

	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;
	}

Uh oh!

Conversation

saethlin commented Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

saethlin commented Jun 21, 2024

Uh oh!

This comment has been minimized.

bors commented Jun 21, 2024

Uh oh!

This comment has been minimized.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

the8472 Jun 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jun 21, 2024

Uh oh!

This comment has been minimized.

rust-timer commented Jun 21, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

Kobzol commented Jun 21, 2024

Uh oh!

Uh oh!

This comment has been minimized.

saethlin commented Jun 22, 2024

Uh oh!

This comment has been minimized.

bors commented Aug 10, 2024

Uh oh!

saethlin commented Aug 10, 2024

Uh oh!

bors commented Aug 10, 2024

Uh oh!

This comment has been minimized.

bors commented Aug 10, 2024

Uh oh!

saethlin commented Aug 11, 2024

Uh oh!

bors commented Aug 11, 2024

Uh oh!

bors commented Aug 11, 2024

Uh oh!

saethlin commented Aug 11, 2024

Uh oh!

bors commented Aug 11, 2024

Uh oh!

bors commented Aug 11, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

saethlin commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

bors commented Aug 12, 2024

Uh oh!

rust-timer commented Aug 12, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

bors commented Aug 12, 2024

Uh oh!

rust-timer commented Aug 12, 2024

Overall result: ❌✅ regressions and improvements - ACTION NEEDED

Uh oh!

saethlin commented Jun 21, 2024 •

edited

Loading

the8472 Jun 21, 2024 •

edited

Loading