[experiment] Try to make index vec smaller when possible

[panstromek]

View all comments

This is very ugly and unsound, but I want to check CI and perf to see whether there's at least something to gain.

Notes

• this roundtrips every mutation through vec, which might mask any gains if it's not optimized out properly. I hope I'm gonna see at least some change to see whether it's worth cleaning up.

TODO

• not sure about Dyn[Send|Sync]
• Why I had to change the lifetimes in ProjectedMaybeOwner? (Failed in borrowck in rustc_middle)
• proper desc
• proper commits

[rust-bors]

☀️ Try build successful (CI)
Build commit: 76803df (76803df0625aa385701a24c186e4526355371fd5, parent: a1e52fc1cf67929a7c01ed9c037520e276ec98fd)

[rust-timer]

Finished benchmarking commit (76803df): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.2%, 0.7%]	155
Regressions ❌ (secondary)	0.4%	[0.1%, 1.0%]	107
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.9%, -0.4%]	7
All ❌✅ (primary)	0.4%	[0.2%, 0.7%]	155

Max RSS (memory usage)

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

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.0%, -0.8%]	4
Improvements ✅ (secondary)	-1.2%	[-2.0%, -0.3%]	2
All ❌✅ (primary)	-0.9%	[-1.0%, -0.8%]	4

Cycles

Results (primary 2.2%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	2.2%	[2.2%, 2.2%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	2.2%	[2.2%, 2.2%]	1

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 519.113s -> 519.594s (0.09%)
Artifact size: 400.82 MiB -> 400.82 MiB (-0.00%)

[rust-bors]

☀️ Try build successful (CI)
Build commit: e8c5fee (e8c5feedca5bbedf67ca37e9d86d33007371e1f6, parent: 1f087276ba7a16d74ec04739ecf582ae495b3b79)

[rust-timer]

Finished benchmarking commit (e8c5fee): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.4%	[0.2%, 0.7%]	162
Regressions ❌ (secondary)	0.4%	[0.2%, 0.8%]	118
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.9%, -0.4%]	7
All ❌✅ (primary)	0.4%	[0.2%, 0.7%]	162

Max RSS (memory usage)

Results (primary -0.9%, secondary -1.9%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-0.9%	[-1.7%, -0.5%]	8
Improvements ✅ (secondary)	-1.9%	[-2.0%, -1.8%]	5
All ❌✅ (primary)	-0.9%	[-1.7%, -0.5%]	8

Cycles

Results (secondary 3.0%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.0%	[2.2%, 4.1%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 526.872s -> 519.342s (-1.43%)
Artifact size: 401.48 MiB -> 400.98 MiB (-0.12%)

[panstromek]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 3ca59f1 (3ca59f14bda8dc4e0fd83c358768803dd5de2cea)
Base parent: 4c9d2bf (4c9d2bfe4ad7a65669098754964aaebe0ec1ced2)

[rust-timer]

Finished benchmarking commit (3ca59f1): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.4%]	67
Regressions ❌ (secondary)	0.4%	[0.2%, 1.3%]	32
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.1%]	3
Improvements ✅ (secondary)	-0.3%	[-0.8%, -0.1%]	24
All ❌✅ (primary)	0.3%	[-0.4%, 0.4%]	70

Max RSS (memory usage)

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

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.6%	[2.1%, 3.2%]	2
Improvements ✅ (primary)	-1.6%	[-3.0%, -0.8%]	6
Improvements ✅ (secondary)	-1.8%	[-2.8%, -0.6%]	5
All ❌✅ (primary)	-1.6%	[-3.0%, -0.8%]	6

Cycles

Results (secondary -2.6%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-2.6%	[-2.6%, -2.6%]	1
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 488.059s -> 489.607s (0.32%)
Artifact size: 393.71 MiB -> 393.84 MiB (0.03%)

[panstromek]

I'm a bit frustrated by the results tbh. The last two commits which avoid the ub_checks in as_slice, as_slice_mut and _into_vec fixed the majority of the regression in bitflags check incr-unchanged in my local cachegrind runs (1000x less instructions added to the baseline), but the perf run here looks almost identical (even though it improved slightly).

I tried to remove one more bounds check in ast lowering, let's see.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 84df2e4 (84df2e4cc26e177dd75d2f922bcdfc2782c1d9b0)
Base parent: c397dae (c397dae808f70caebab1fc4e11b3edf7e59f58c7)

[rust-timer]

Finished benchmarking commit (84df2e4): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.5%]	73
Regressions ❌ (secondary)	0.4%	[0.2%, 0.7%]	29
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.2%]	4
Improvements ✅ (secondary)	-0.3%	[-0.9%, -0.1%]	27
All ❌✅ (primary)	0.3%	[-0.4%, 0.5%]	77

Max RSS (memory usage)

Results (primary -0.2%, secondary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	4.3%	[4.3%, 4.3%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-1.4%	[-2.1%, -0.6%]	4
Improvements ✅ (secondary)	-2.4%	[-2.6%, -2.2%]	4
All ❌✅ (primary)	-0.2%	[-2.1%, 4.3%]	5

Cycles

Results (secondary -2.4%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	2.4%	[2.4%, 2.4%]	1
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.3%	[-6.0%, -2.4%]	5
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 485.944s -> 485.892s (-0.01%)
Artifact size: 393.37 MiB -> 393.88 MiB (0.13%)

[panstromek]

Detailed results show that incr_comp_persist_dep_graph the biggest slowdown, but that's probably spurious, because that one just renames a file, so I assume that's mostly noise.

Based on results from other queries and local profiling with cachegrind, I see the problem is often in places that iterate over the vec and index into it in each iteration through [] operator. That implicitly unpacks the vec into a slice in each iteration, so maybe the optimizer struggles to optimize some of that that when len is 32 bits and therefore vec/slice fields correspondence is not so straightforward anymore?

I tried to change one of those loops to create those slices before the loop explicitly to test this hypothesis.

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 2e6fea9 (2e6fea92ac4d36e0c16dd83c49b77487f04c9119)
Base parent: c397dae (c397dae808f70caebab1fc4e11b3edf7e59f58c7)

[rust-timer]

Finished benchmarking commit (2e6fea9): comparison URL.

Overall result: ❌✅ regressions and improvements - please read:

Benchmarking means the PR may be perf-sensitive. It's automatically marked not fit for rolling up. Overriding is possible but disadvised: it risks changing compiler perf.

Next, please: If you can, justify the regressions found in this try perf run in writing along with @rustbot label: +perf-regression-triaged. If not, fix the regressions and do another perf run. Neutral or positive results will clear the label automatically.

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

Instruction count

Our most reliable metric. Used to determine the overall result above. However, even this metric can be noisy.

	mean	range	count
Regressions ❌ (primary)	0.3%	[0.2%, 0.5%]	74
Regressions ❌ (secondary)	0.4%	[0.2%, 0.7%]	27
Improvements ✅ (primary)	-0.3%	[-0.4%, -0.2%]	3
Improvements ✅ (secondary)	-0.3%	[-0.9%, -0.2%]	26
All ❌✅ (primary)	0.3%	[-0.4%, 0.5%]	77

Max RSS (memory usage)

Results (primary -2.7%, secondary -3.1%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-2.7%	[-6.3%, -1.2%]	4
Improvements ✅ (secondary)	-3.1%	[-6.2%, -2.2%]	5
All ❌✅ (primary)	-2.7%	[-6.3%, -1.2%]	4

Cycles

Results (secondary 1.5%)

A less reliable metric. May be of interest, but not used to determine the overall result above.

	mean	range	count
Regressions ❌ (primary)	-	-	0
Regressions ❌ (secondary)	3.9%	[2.7%, 5.7%]	7
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-4.2%	[-6.8%, -2.5%]	3
All ❌✅ (primary)	-	-	0

Binary size

This perf run didn't have relevant results for this metric.

Bootstrap: 485.944s -> 488.554s (0.54%)
Artifact size: 393.37 MiB -> 393.43 MiB (0.01%)