Eliminate ObligationCauseData

[nnethercote]

This makes Obligation two words bigger, but avoids allocating a lot of the time.

I previously tried this in #73983 and it didn't help much, but local timings look more promising now.

r? @ghost

[nnethercote]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit ddb96eb0b1587c670438b0e577eb709c90e29447 with merge 220db1ba07e4e16cdbe3889232735221018fe852...

[nnethercote]

@bors try

[bors]

⌛ Trying commit 748ccba835c1336c51462525d136e2314289a284 with merge 498fbcc6241318ce8d97f067af15d4fba425d18b...

[bors]

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

[rust-timer]

Queued 498fbcc6241318ce8d97f067af15d4fba425d18b with parent f7fd79a, future comparison URL.

[rust-timer]

Finished benchmarking commit (498fbcc6241318ce8d97f067af15d4fba425d18b): comparison url.

Summary: This change led to large relevant mixed results 🤷 in compiler performance.

• Large improvement in instruction counts (up to -3.8% on incr-unchanged builds of deep-vector)
• Small regression in instruction counts (up to 0.7% on incr-full builds of deeply-nested)

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

[nnethercote]

Lots more wins than losses among the instruction counts, plus it's a tiny reduction in number of lines of code. I think this is worth going ahead with.

r? @Mark-Simulacrum

[Mark-Simulacrum]

I agree that these wins exceed the regressions, so this is worth it.

I am wondering, though, if it makes sense to try to go further here. In particular, it seems to me that many ObligationCauseCode's are or could be <= one pointer in "size", in which case Lrc<...> is adding an allocation that's not really necessary.

For example, just by moving the discriminant into the Obligation (one extra byte), we'd be able to represent a lot of the variants inline -- trivially all of the ones that have no data attached, and in theory those which have data that's only usize wide (e.g., a single DefId). I don't off-hand know what the distribution there looks like -- this PR adds a comment suggesting this covers 60% of cases with just MiscObligation, but it's worth noting that we keep adding new obligation cause codes, so presumably that percentage is only going to decrease over time. I'm not sure what the distribution on the rest of the codes is like.

Further optimization seems fine to leave for future PRs, but I would like the comment on the Option discriminant hashing addressed; r=me with that done (another perf run may be in order).

[bors]

⌛ Testing commit 47d6bdfd7b00e28839d261f1c7a9de190560079f with merge d985d42bb4d0910b017e761acb7125427ce7aef2...

[bors]

💔 Test failed - checks-actions

[bors]

☔ The latest upstream changes (presumably #92099) made this pull request unmergeable. Please resolve the merge conflicts.

[nnethercote]

I rebased to fix the conflict. While there, I added a clean_code() function that trivially avoids some additional allocations relating to the parent_code field, that helps with #87012.

@bors r=Mark-simulacrum

[bors]

📌 Commit d7ca962282ccc09fad6c8193fa3a474e339097c5 has been approved by Mark-simulacrum

[nnethercote]

@bors r=Mark-Simulacrum

[bors]

📌 Commit f09b1fa has been approved by Mark-Simulacrum

[bors]

⌛ Testing commit f09b1fa with merge ed7a206...

[bors]

☀️ Test successful - checks-actions
Approved by: Mark-Simulacrum
Pushing ed7a206 to master...

[nnethercote]

So we could effectively split ObligationCauseCode in half: one part for all the simple variants, one part for all the complex variants.

I did some quick measurements and the potential for improvement here seems very low. The simple variants account for a small fraction (~5% or less) of the occurrences seen in practice (while compiling std).

[rust-timer]

Finished benchmarking commit (ed7a206): comparison url.

Summary: This change led to small relevant mixed results 🤷 in compiler performance.

• Small improvement in instruction counts (up to -0.9% on full builds of coercions)
• Small regression in instruction counts (up to 0.9% on full builds of wg-grammar)

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