Skip to content

Don't store current-session side effects in OnDiskCache#154252

Merged
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
Zalathar:on-disk-cache
Mar 24, 2026
Merged

Don't store current-session side effects in OnDiskCache#154252
rust-bors[bot] merged 4 commits intorust-lang:mainfrom
Zalathar:on-disk-cache

Conversation

@Zalathar
Copy link
Member

This PR is a series of related cleanups to OnDiskCache, which is responsible for loading query return values (and side effects) that were serialized during the previous incremental-compilation session.

The primary change is to move the current_side_effects field out of OnDiskCache and into QuerySystem. That field was awkward because it was the only part of OnDiskCache state related to serializing the current compilation session, rather than loading values from the previous session.

The other commits should hopefully be straightforward.

r? nnethercote (or compiler)

@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Mar 23, 2026
Copy link
Contributor

@nnethercote nnethercote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with this part of the code, but the changes seem simple enough.

View changes since this review

file_index_to_file: Lock<FxHashMap<SourceFileIndex, Arc<SourceFile>>>,

// A map from dep-node to the position of the cached query result in
// A map from dep-node to the position of its disk-cached query return value in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"map from a dep-node"

// A map from dep-node to the position of any associated `QuerySideEffect` in
// `serialized_data`.
prev_side_effects_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
side_effects_index: FxHashMap<SerializedDepNodeIndex, AbsoluteBytePos>,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

@nnethercote
Copy link
Contributor

r=me with the comment nits fixed.

@rustbot
Copy link
Collaborator

rustbot commented Mar 24, 2026

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

These lists can be considered part of the encoder state, and bundling them
inside the encoder is certainly more convenient than passing them around
separately.
Every other part of `OnDiskCache` deals with loading information from the
_previous_ session, except for this one field.

Moving it out to `QuerySystem` makes more sense, because that's also where
query return values are stored (inside the caches in their vtables).
This assertion makes the method body a lot messier, and seems very unlikely to
ever usefully fail.
@Zalathar
Copy link
Member Author

Zalathar commented Mar 24, 2026

Rebased over a trivial conflict.

Instead of applying the comment suggestions as-is, I ended up rewriting the two comments to hopefully be clearer (diff).

@nnethercote
Copy link
Contributor

@bors r+

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 24, 2026

📌 Commit ee15154 has been approved by nnethercote

It is now in the queue for this repository.

@rust-bors rust-bors bot added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 24, 2026
rust-bors bot pushed a commit that referenced this pull request Mar 24, 2026
Rollup of 10 pull requests

Successful merges:

 - #153964 (Fix `doc_cfg` not working as expected on trait impls)
 - #153979 (Rename various query cycle things.)
 - #154132 (Add missing num_internals feature gate to coretests/benches)
 - #154153 (core: Implement `unchecked_funnel_{shl,shr}`)
 - #154236 (Clean up query-forcing functions)
 - #154252 (Don't store current-session side effects in `OnDiskCache`)
 - #154017 ( Fix invalid add of duplicated call locations for the rustdoc scraped examples feature)
 - #154163 (enzyme submodule update)
 - #154264 (Update books)
 - #154282 (rustc-dev-guide subtree update)
@rust-bors rust-bors bot merged commit 39343f5 into rust-lang:main Mar 24, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 24, 2026
@Zalathar Zalathar deleted the on-disk-cache branch March 24, 2026 08:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants