Caching the stable hash of Ty within itself

[oli-obk]

Instead of computing stable hashes on types as needed, we compute it during interning.

This way we can, when a hash is requested, just hash that hash, which is significantly faster than traversing the type itself.

We only do this for incremental for now, as incremental is the only frequent user of stable hashing.

As a next step we can try out

• moving the hash and TypeFlags to Interner, so projections and regions get the same benefit (tho regions are not nested, so maybe that's not a good idea? Would be nice for dedup tho)
• start comparing types via their stable hash instead of their address?

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 85ce0ec1f2ad3b14f46b87a663330217830e13e4 with merge 8350f52e17c0d97bcd0b122bd425c51eefd7be22...

[bors]

☀️ Try build successful - checks-actions
Build commit: 8350f52e17c0d97bcd0b122bd425c51eefd7be22 (8350f52e17c0d97bcd0b122bd425c51eefd7be22)

[rust-timer]

Queued 8350f52e17c0d97bcd0b122bd425c51eefd7be22 with parent c651ba8, future comparison URL.

[rust-timer]

Finished benchmarking commit (8350f52e17c0d97bcd0b122bd425c51eefd7be22): comparison url.

Summary: This benchmark run shows 131 relevant improvements 🎉 but 103 relevant regressions 😿 to instruction counts.

• Average relevant regression: 1.2%
• Average relevant improvement: -1.4%
• Largest improvement in instruction counts: -11.6% on incr-unchanged builds of clap-rs check
• Largest regression in instruction counts: 8.2% on full builds of issue-46449 check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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 led 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-review -S-waiting-on-perf +perf-regression

[michaelwoerister]

Fingerprint caching has to take into account that there can be different hash settings. See #92278, for example. I think we'll want to get rid of these settings eventually (or somehow assert that they are fully determined by the type that's being hashed) but until then we have to expect that there can be multiple fingerprints per value.

I'm wondering if, instead of caching hashes per type, it would be better to build fingerprint caching into a HashStable impl for the new Interned type, so it's handled in a uniform way.

[oli-obk]

I'm wondering if, instead of caching hashes per type, it would be better to build fingerprint caching into a HashStable impl for the new Interned type, so it's handled in a uniform way.

Jup, that was my plan. Possibly along with type flags which are often also shared there

[michaelwoerister]

It seems like an overall performance win. The regression all seem to be in non-incremental mode, in which we usually don't compute these fingerprints (except for legacy symbol mangling).

[oli-obk]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit 8b0440a with merge a432345cd8a7c2f366eb9e829f27092c074862d7...

[bors]

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

[rust-timer]

Queued a432345cd8a7c2f366eb9e829f27092c074862d7 with parent 7ccfe2f, future comparison URL.

[jackh726]

I'm interested to hear your thoughts on what this will look like in an eventual rustc_type_ir @oli-obk

I think the wins here a pretty solid, but I'm a bit uneasy on encoding something specific to the rustc encoding into the Ty representation.

[oli-obk]

I am working locally on moving this information into the interner, but that has a bigger diff and I want to do it separately from this PR anyway so we can have clear perf numbers.

[bors]

💥 Test timed out

[rust-log-analyzer]

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

[oli-obk]

@bors retry timeout

[bors]

⌛ Testing commit 5875d7b with merge 8d6f527...

[bors]

☀️ Test successful - checks-actions
Approved by: michaelwoerister
Pushing 8d6f527 to master...

[rust-timer]

Finished benchmarking commit (8d6f527): comparison url.

Summary: This benchmark run shows 120 relevant improvements 🎉 but 12 relevant regressions 😿 to instruction counts.

• Arithmetic mean of relevant regressions: 0.9%
• Arithmetic mean of relevant improvements: -1.2%
• Arithmetic mean of all relevant changes: -1.0%
• Largest improvement in instruction counts: -9.6% on incr-unchanged builds of clap-rs check
• Largest regression in instruction counts: 1.3% on incr-full builds of deeply-nested check

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

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

[oli-obk]

@rustbot label: +perf-regression-triaged

The improvements outweigh the regressions significantly both in number of tests and in the magnitude

[nnethercote]

I am working locally on moving this information into the interner, but that has a bigger diff and I want to do it separately from this PR anyway so we can have clear perf numbers.

I am currently working on using Interned more widely, e.g. for Allocation (#94597), Layout and AdtDef (in progress), and List (still to be done). Are fingerprints appropriate for those?

I also have some concerns about memory usage. Types like TyS and PredicateS account for a decent chunk of our peak memory usage, and increases like 40 to 56 bytes are considerable, esp. for something that isn't used in non-incremental builds...

[oli-obk]

Considering my experiments in #94487 it does not seem like this is generally useful within Interned yet. I can write up a doc of all the experiments that I have planned and then we can talk about whether it makes sense to continue with caching stable hashes.

Wrt memory usage, https://perf.rust-lang.org/compare.html?start=4ce3749235fc31d15ebd444b038a9877e8c700d7&end=8d6f527530f4ba974d922269267fe89050188789&stat=max-rss looks just like any PR to me. Some ups, some downs, no concrete change in maximum memory usage