Remove WithCachedTypeInfo::stable_hash.

[nnethercote]

We store a stable hash value in the most common interned values (e.g. types, predicates, regions). This is 16 bytes of data.

• In non-incremental builds it's a straightforward performance loss: the stable hash isn't computed or used, and the 16 bytes of space goes to waste (but it still gets hashed when interning).
• In incremental builds it avoids some hashing but doesn't seem to actually be a genuine performance win, and the complexity doesn't seem worth it.

r? @oli-obk

[nnethercote]

@bors try @rust-timer queue

[rust-bors]

☀️ Try build successful (CI)
Build commit: aef2982 (aef29827df2a13132c3307473c209a99baedf470, parent: bd1e7c79485d6a4113bb11dd98cb8b415cd4a55e)

[rust-timer]

Finished benchmarking commit (aef2982): 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.7%	[0.2%, 1.4%]	117
Regressions ❌ (secondary)	0.7%	[0.1%, 2.8%]	71
Improvements ✅ (primary)	-0.4%	[-0.8%, -0.2%]	33
Improvements ✅ (secondary)	-0.3%	[-0.8%, -0.0%]	24
All ❌✅ (primary)	0.5%	[-0.8%, 1.4%]	150

Max RSS (memory usage)

Results (primary -1.5%, 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)	2.9%	[2.5%, 3.3%]	2
Improvements ✅ (primary)	-1.5%	[-2.7%, -0.6%]	28
Improvements ✅ (secondary)	-2.4%	[-4.0%, -1.4%]	18
All ❌✅ (primary)	-1.5%	[-2.7%, -0.6%]	28

Cycles

Results (secondary -3.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)	-	-	0
Improvements ✅ (primary)	-	-	0
Improvements ✅ (secondary)	-3.8%	[-6.5%, -2.0%]	3
All ❌✅ (primary)	-	-	0

Binary size

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

Bootstrap: 495.503s -> 489.042s (-1.30%)
Artifact size: 394.22 MiB -> 394.04 MiB (-0.05%)

[rustbot]

Pull request author cannot be assigned as reviewer.

Please choose another assignee.

[nnethercote]

Perf is pretty interesting here.

• icounts perf is worse overall, due to worse results for the incremental cases. (Non-incremental icounts are improved, which makes sense.)
• cycles and wall-time are both similar: very few results above the threshold; not much of a signal there. If you click "show non-relevant results" it's a mix but there is more green than red.
• max-rss results are great, with lots of green. This makes sense because some very common interned types have gotten 16-20 bytes smaller. Faults are also improved, if that's meaningful.
• bootstrap is down 1.3%. Not sure if that's big enough to be meaningful.
• Artifact size is slightly down.

I think this is one of those cases where icounts is misleading, and overall the perf here is slightly positive. Combine that with it being a code simplification, and I'm inclined to pursue this.

@rustbot label: +perf-regression-triaged

[rustbot]

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

[rustbot]

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

[panstromek]

The bootstrap time might be noise again. Base commit had a similarly big regression, and the next one on main is down by the same amount again.

I guess this might also make the wall-time numbers tricky to analyze. It depends on whether the source of noise is something on the machine or something in the artifact. If it's on the machine, then those wall-time improvements might also be noise.

[bjorn3]

Why do these hashes change? Is there some impurity in the hashing?

View changes since the review

[nnethercote]

It's because impl<T: HashStable<Hcx>, Hcx> HashStable<Hcx> for WithCachedTypeInfo<T> has changed from a two-step hash to a one-step hash. (Search for "hash that hash" in the diff.) The old code would hash the T to create a Fingerprint, and then hash that Fingerprint. The new code just hashes T.

(To double check this, I just tried reinstating the two-step hash and the symbol names reverted to their previous values.)