[experiment] Benchmark -Zthreads=sync on rustc-perf benchmarks

[petrochenkov]

View all comments

[rustbot]

r? @oli-obk

rustbot has assigned @oli-obk.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

• Owners of files modified in this PR: compiler
• compiler expanded to 73 candidates
• Random selection from 19 candidates

[petrochenkov]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 345cec8 (345cec89010122da419ecb12aeeeb347c73ffe39)
Base parent: f28ac76 (f28ac764c36004fa6a6e098d15b4016a838c13c6)

[rust-timer]

Finished benchmarking commit (345cec8): comparison URL.

Overall result: no relevant changes - no action needed

Benchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up.

@rustbot label: -S-waiting-on-perf -perf-regression

Instruction count

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

Max RSS (memory usage)

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

Cycles

Results (primary -2.3%, secondary 12.8%)

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)	12.8%	[2.4%, 23.1%]	2
Improvements ✅ (primary)	-2.3%	[-2.3%, -2.3%]	1
Improvements ✅ (secondary)	-	-	0
All ❌✅ (primary)	-2.3%	[-2.3%, -2.3%]	1

Binary size

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

Bootstrap: 510.446s -> 503.041s (-1.45%)
Artifact size: 353.05 MiB -> 353.51 MiB (0.13%)

[petrochenkov]

This is surprising, the "fast path" for -Zthreads=1 is not actually faster than -Zthreads=sync (aka -Zthreads=1 with enabled synchronization).
Or at least it is not faster withing the usual noise (I do see some red below the threshold).

This may be an opportunity to simplify things.

[Zoxc]

I assume the perf suite doesn't pass -Z threads for most benchmarks, so this change wouldn't affect it?

[petrochenkov]

Ah, right, parse_threads is only called if there is an option, I'll re-benchmark.

[rustbot]

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

[petrochenkov]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: 52eea4b (52eea4bc01ed8c427114c7c51fc60a92b897c7ed)
Base parent: 73100ee (73100eefe2afb831c4964f579c95beeb27b86e28)

[rust-timer]

Finished benchmarking commit (52eea4b): 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)	1.6%	[0.4%, 24.8%]	238
Regressions ❌ (secondary)	3.4%	[0.2%, 19.8%]	247
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-0.5%	[-0.6%, -0.4%]	13
All ❌✅ (primary)	1.6%	[0.4%, 24.8%]	238

Max RSS (memory usage)

Results (primary 17.2%, secondary 19.0%)

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

	mean	range	count
Regressions ❌ (primary)	17.2%	[2.4%, 31.4%]	259
Regressions ❌ (secondary)	19.7%	[2.0%, 43.4%]	308
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-1.9%	[-2.6%, -1.3%]	9
All ❌✅ (primary)	17.2%	[2.4%, 31.4%]	259

Cycles

Results (primary 7.8%, secondary 12.0%)

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

	mean	range	count
Regressions ❌ (primary)	7.8%	[1.1%, 37.8%]	241
Regressions ❌ (secondary)	12.7%	[1.8%, 36.3%]	277
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-5.0%	[-8.4%, -3.5%]	11
All ❌✅ (primary)	7.8%	[1.1%, 37.8%]	241

Binary size

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

Bootstrap: 505.309s -> 525.313s (3.96%)
Artifact size: 353.63 MiB -> 355.81 MiB (0.62%)

[petrochenkov]

24.83% on helloworld is something that we clearly need to investigate.

[lqd]

20% on helloworld is 5ms

[bjorn3]

A fixed 5ms cost is still expensive if you have a ton of small deoendencies to compile.

[lqd]

If you only do maybe, otherwise it will be amortized by the first big crate that takes advantage of parallelism.

[Zoxc]

I wonder if sharding is the cause of the helloworld slowdown. Probably should test helloworld on #153826.

[bjorn3]

From a quick look CtxtInterners::new seems to be 10x slower, now taking 5% of total time excluding link time.