Computing crate_hash from metadata encoding instead of HIR (implements #94878) (very draft)

[Daniel-B-Smith]

View all comments

Adds a parallel process of hashing along with the metadata encoding. It does add metadata encoding in a few places to ensure that the hash was available. Currently, the metadata encoding always generates both the metadata and the hash even if only one is needed.

Known issue: one test failure due to #137108 (pre-existing repr(simd) projection bug. The new metadata pass trips some MIR validation. This will need to catch the issue earlier.

Local profiling on laptop shows roughly neutral (~0.5%), requesting CI perf run for precise measurement

[nnethercote]

@bors try @rust-timer queue

[mejrs]

@bors try cancel

[rust-bors]

❗ There is currently no try build in progress on this PR.

[mejrs]

@bors try @rust-timer queue

[Daniel-B-Smith]

The latest update reverts crate_hash to use the HIR hash in cases where metadata is not otherwise generated. That fixed the large performance regression for compilations that depend heavily on cases where metadata is not generated.

The big performance open question right now is whether or not the less invasive metadata hashing strategy is fast enough. Hashing the bytes of each item one at a time has substantial overhead relative to hashing all of the bytes as they are flushed to disk. The latter is a much more invasive change to FileEncoder, so I'm holding off until it's clear that is needed.

[rust-bors]

☀️ Try build successful (CI)
Build commit: ec2b0e4 (ec2b0e4e0b51187e1b740843ecf29588eaa02a0f, parent: 52b6e2c208b73276ccb36ec0b68456913a801c96)

[rust-timer]

Finished benchmarking commit (ec2b0e4): 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.9%	[0.2%, 2.8%]	152
Regressions ❌ (secondary)	0.8%	[0.2%, 3.9%]	89
Improvements ✅ (primary)	-1.3%	[-5.5%, -0.2%]	5
Improvements ✅ (secondary)	-0.3%	[-0.6%, -0.0%]	9
All ❌✅ (primary)	0.9%	[-5.5%, 2.8%]	157

Max RSS (memory usage)

Results (primary -4.2%, 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)	5.2%	[3.5%, 6.9%]	2
Improvements ✅ (primary)	-4.2%	[-4.2%, -4.2%]	1
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-4.2%	[-4.2%, -4.2%]	1

Cycles

Results (primary -2.4%, 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)	2.8%	[2.8%, 2.8%]	1
Regressions ❌ (secondary)	4.7%	[4.2%, 5.3%]	4
Improvements ✅ (primary)	-5.0%	[-7.3%, -2.6%]	2
Improvements ✅ (secondary)	-2.2%	[-2.9%, -1.5%]	2
All ❌✅ (primary)	-2.4%	[-7.3%, 2.8%]	3

Binary size

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

Bootstrap: 487.959s -> 489.335s (0.28%)
Artifact size: 393.44 MiB -> 393.45 MiB (0.00%)

[Kobzol]

@bors try @rust-timer queue

[Daniel-B-Smith]

For posterity, the big change behind the ongoing perf run is to hash the metadata as it is being flushed to disk instead of as each individual writes into the buffer. That reduces the hashing overhead substantially.

[rust-bors]

☀️ Try build successful (CI)
Build commit: 34256d3 (34256d3644315502924b8c0bd4c52aecb65b8136, parent: 03c609abb6638f9d7f49f34326d4137d07f5cd61)

[rust-timer]

Finished benchmarking commit (34256d3): 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.5%	[0.2%, 1.0%]	46
Regressions ❌ (secondary)	0.9%	[0.1%, 3.9%]	31
Improvements ✅ (primary)	-0.6%	[-5.4%, -0.2%]	13
Improvements ✅ (secondary)	-0.4%	[-1.6%, -0.0%]	18
All ❌✅ (primary)	0.3%	[-5.4%, 1.0%]	59

Max RSS (memory usage)

Results (primary 0.5%, secondary -1.0%)

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

	mean	range	count
Regressions ❌ (primary)	5.6%	[5.6%, 5.6%]	1
Regressions ❌ (secondary)	-	-	0
Improvements ✅ (primary)	-4.5%	[-4.5%, -4.5%]	1
Improvements ✅ (secondary)	-1.0%	[-1.0%, -1.0%]	1
All ❌✅ (primary)	0.5%	[-4.5%, 5.6%]	2

Cycles

Results (primary -2.6%, 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)	2.2%	[2.2%, 2.2%]	1
Regressions ❌ (secondary)	3.8%	[2.5%, 4.8%]	5
Improvements ✅ (primary)	-7.3%	[-7.3%, -7.3%]	1
Improvements ✅ (secondary)	-1.3%	[-1.3%, -1.3%]	1
All ❌✅ (primary)	-2.6%	[-7.3%, 2.2%]	2

Binary size

Results (secondary -0.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)	-	-	0
Improvements ✅ (secondary)	-0.1%	[-0.1%, -0.1%]	1
All ❌✅ (primary)	-	-	0

Bootstrap: 487.089s -> 487.591s (0.10%)
Artifact size: 393.44 MiB -> 393.40 MiB (-0.01%)