Skip to content

Replace per-crate trait impl queries with merged foreign impl indexes - #107

Draft
xmakro wants to merge 1 commit into
perf/foreign-impls-basefrom
perf/foreign-impls-index
Draft

Replace per-crate trait impl queries with merged foreign impl indexes#107
xmakro wants to merge 1 commit into
perf/foreign-impls-basefrom
perf/foreign-impls-index

Conversation

@xmakro

@xmakro xmakro commented Aug 10, 2026

Copy link
Copy Markdown
Owner

Prototype: replace the per-crate trait impl queries with two merged foreign impl indexes.

Currently every CrateMetadata eagerly decodes its trait impl index and incoherent impl index at crate load time, and trait_impls_of then issues one implementations_of_trait query per loaded crate for every foreign trait it is asked about. Each of those calls also forces a crate_hash read for dependency tracking. On a leaf check of the cargo benchmark (301 loaded crates) that comes out to 63210 implementations_of_trait executions (226 foreign traits times roughly 280 crates), about 161ms inside the providers plus untimed query machinery, and 63k dep graph nodes with edges, about 5 percent of the whole graph.

This change:

  • Removes the eager trait_impls and incoherent_impls decoding from CrateMetadata::new.
  • Adds foreign_trait_impls_index, a single query that walks all crates once and records, per trait, where each crate's impl list lives in the metadata blob (crate, position, length). The impl lists themselves are not decoded. Dependency tracking mirrors the extern provider macro: the provider reads crates(()) and crate_hash(cnum) for every crate, so it re-executes exactly when a dependency changes and is green on unchanged incremental builds.
  • Adds foreign_implementations_of_trait(trait_id), which decodes just that trait's impl lists via the index. trait_impls_of now makes one query per trait instead of one per crate and trait.
  • Same treatment for incoherent impls via foreign_incoherent_impls (fully decoded, the data is tiny).
  • Deletes the implementations_of_trait and crate_incoherent_impls queries.
  • The one pre-freeze user, the Default suggestion in resolver diagnostics, now scans the direct extern crates through the cstore without going through the query system, because forcing crates(()) during resolution would freeze the crate store while crates can still be loaded (behavior preserved, verified by the existing suggestion tests).

Impl ordering is preserved everywhere (crate loading order, encoding order within a crate), so query results are bit identical to before.

An earlier version of this decoded all impl lists eagerly inside the merged query. That won on dependency-heavy crates but regressed small crates badly (helloworld check full +9 percent, incr-full +19 percent) because it decoded every sysroot impl list up front. The index/list split removes those regressions while keeping the wins.

Perf (local dist-fidelity harness: stage2, thin LTO, jemalloc, from-scratch builds on both sides, instructions:u, full primary+secondary suite, 551 comparable cells):

Geomean by profile, all scenarios: check -0.135%, debug -0.098%, opt -0.069%. Every profile/scenario geomean is negative or flat; check incr-unchanged -0.31% (n=45), check incr-patched println -0.28% (n=22).

Dependency-heavy benchmarks:

benchmark cell change
large-workspace check incr-patched println -1.04%
eza check incr-patched printlns -1.01%
large-workspace check incr-full -1.00%
cargo check incr-patched println -0.90%
cargo check incr-unchanged -0.57%
ripgrep check incr-unchanged -0.64%
image check incr-unchanged -0.53%
cargo check incr-full -0.39%
hyper check incr-full -0.31%
diesel check incr-unchanged -0.29%

Worst remaining regressions are confined to two secondary stress benchmarks (coercions incr-full +0.63%, helloworld isolated cells up to +0.39%). Self-profile shows the new queries execute in about 110us total on coercions with 33us of result hashing, so those cells are codegen layout noise rather than added work.

Correctness: x check clean for compiler, rustdoc and clippy; tests/ui/coherence, tests/ui/traits and the Default suggestion tests pass (1731 tests). The first draft deadlocked in resolver diagnostics by freezing the cstore mid-resolution, which is what motivated the untracked path described above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant