Return an indexmap in all_local_trait_impls query

[pierwill]

The data structure previously used here required that DefId be Ord. As part of #90317, we do not want DefId to implement Ord.

[rust-highfive]

r? @cjgillot

(rust-highfive has picked a reviewer for you, use r? to override)

[pierwill]

@rustbot author

[pierwill]

I tried this with FxHashMap and it caused ui test failures.

[pierwill]

@bors try @rust-timer queue

[rust-timer]

Awaiting bors try build completion.

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

[bors]

⌛ Trying commit f5fe6cd with merge fdfba66d9cef8e970a1c68a4b6fef10362925a7f...

[bors]

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

[rust-timer]

Queued fdfba66d9cef8e970a1c68a4b6fef10362925a7f with parent 8cdb3cd, future comparison URL.

[pierwill]

@rustbot ready

[rust-timer]

Finished benchmarking commit (fdfba66d9cef8e970a1c68a4b6fef10362925a7f): comparison url.

Summary: This benchmark run did not return any relevant results. 32 results were found to be statistically significant but too small to be relevant.

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.

@bors rollup=never
@rustbot label: +S-waiting-on-review -S-waiting-on-perf -perf-regression

[michaelwoerister]

IndexMap is fine as long as it is constructed in a deterministic way. Do we know if that is the case here?

[pierwill]

IndexMap is fine as long as it is constructed in a deterministic way. Do we know if that is the case here?

I'm not sure. Is this where the IndexMap would end up being constructed?

rust/compiler/rustc_middle/src/hir/mod.rs

Line 109 in f5fe6cd

providers.all_local_trait_impls = |tcx, ()| &tcx.resolutions(()).trait_impls;

In that case, looks like the determinism would depend on rustc_middle::ty::context::TyCtxt::resolutions(). Where is that defined? I'm still learning how the query system works.

[cjgillot]

resolutions is defined from tcx.untracked_resolutions, which is just data taken from the resolver and plugged as start conditions for the query system. Everything in resolutions is always checked by the query system (the query is both eval_always and no_hash), so it would not crash if it were non-deterministic. In this case, trait_impls is filled in rustc_resolve::late, which walks the AST in-order, so it is actually deterministic.

[cjgillot]

@bors r+ rollup=iffy

[bors]

📌 Commit f5fe6cd has been approved by cjgillot

[bors]

⌛ Testing commit f5fe6cd with merge 7cd14d2...

[bors]

☀️ Test successful - checks-actions
Approved by: cjgillot
Pushing 7cd14d2 to master...

[rust-timer]

Finished benchmarking commit (7cd14d2): comparison url.

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

• Average relevant regression: 0.6%
• Average relevant improvement: -0.9%
• Largest improvement in instruction counts: -1.1% on incr-unchanged builds of ctfe-stress-4 check
• Largest regression in instruction counts: 0.8% on incr-unchanged builds of cranelift-codegen 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