Skip to content

Build a new incr comp session dir from scratch every time - #159287

Open
bjorn3 wants to merge 1 commit into
rust-lang:mainfrom
bjorn3:incr_comp_session_from_scratch
Open

Build a new incr comp session dir from scratch every time#159287
bjorn3 wants to merge 1 commit into
rust-lang:mainfrom
bjorn3:incr_comp_session_from_scratch

Conversation

@bjorn3

@bjorn3 bjorn3 commented Jul 14, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member

View all comments

Rather than copying the old incr comp dir and then modifying it. This saves a copy/hardlink for files that are modified. And it removes the need for accurate work product tracking to avoid accumulating cruft, which is non-trivial. We don't accurately track the pre-LTO bitcode files for ThinLTO for example.

Part of rust-lang/compiler-team#908

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 14, 2026
@bjorn3 bjorn3 mentioned this pull request Jul 14, 2026
Closed
21 tasks
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3 bjorn3 added the A-incr-comp Area: Incremental compilation label Jul 14, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch 2 times, most recently from bf1a4e3 to 50ba28a Compare July 21, 2026 09:35
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 50ba28a to 68660f2 Compare July 21, 2026 14:00
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 68660f2 to 1f2a9e3 Compare July 21, 2026 14:43
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 1f2a9e3 to e1094a1 Compare July 23, 2026 10:32
@bjorn3 bjorn3 mentioned this pull request Jul 24, 2026
Open
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from e1094a1 to d9dd28d Compare July 28, 2026 13:39
@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from d9dd28d to decf8a0 Compare August 5, 2026 18:45
@bjorn3
bjorn3 marked this pull request as ready for review August 5, 2026 18:45
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 5, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Aug 5, 2026
@rustbot

rustbot commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

r? @nnethercote

rustbot has assigned @nnethercote.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, incremental
  • compiler, incremental expanded to 75 candidates
  • Random selection from 18 candidates

@bjorn3 bjorn3 added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 13, 2026
@bjorn3 bjorn3 mentioned this pull request Aug 19, 2026
Open
@lewisl9029

lewisl9029 commented Sep 3, 2026

Copy link
Copy Markdown

Fwiw, I think this could also fix a nasty cache corruption bug I ran into when a rustc compile gets interrupted with incremental and opt-level >= 1.

I had my agent whip up a quick repro here for anyone interested in digging deeper: https://github.com/lewisl9029/rustc-incremental-lto-repro

TL;DR: it can either show cryptic errors that don't match the source, or (worse) successfully build a new binary that doesn't match the source, depending on what gets mixed up in the cache from unintentionally overwriting hardlinks pointing to previous finalized session state without a new finalized session taking its place (due to getting interrupted, even from just a ctrl+c, no SIGKILL needed). Rebuilding doesn't fix the issue either, only a cargo clean will. Originally encountered on 1.94.1, but still repros on 1.98.0.

@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3 bjorn3 mentioned this pull request Sep 10, 2026
Open
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 6d15619 to 4f85d86 Compare September 10, 2026 10:21
@rustbot

This comment has been minimized.

Sign in to view
@bjorn3

bjorn3 commented Sep 10, 2026

Copy link
Copy Markdown
Member Author

Blocked on #162602 and resolving some review comments.

@rustbot blocked

@rustbot rustbot added S-blocked Status: Blocked on something else such as an RFC or other implementation work. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 10, 2026
@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 4f85d86 to 12df0c5 Compare September 16, 2026 09:50
@rustbot

This comment has been minimized.

Sign in to view
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@rust-bors

This comment has been minimized.

Sign in to view
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 12df0c5 to 3194c7f Compare September 18, 2026 09:02
@rustbot

This comment has been minimized.

Sign in to view
@rustbot rustbot added the A-run-make Area: port run-make Makefiles to rmake.rs label Sep 18, 2026
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from be97457 to 6bae246 Compare September 18, 2026 09:17
@bjorn3 bjorn3 mentioned this pull request Sep 18, 2026
Merged
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch 2 times, most recently from c77153b to 49c263e Compare September 18, 2026 12:45
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 21, 2026
@JonathanBrouwer
Rollup merge of rust-lang#162952 - bjorn3:incr_comp_locking_change, r…
9e850b1 
…=khyperia

Depend on lockfiles to prevent GC of the current session

Instead of manually skipping it right before the remove_dir. This will be simpler once we stop immediately copying from the old incr comp dir. Previously this wasn't possible due to the usage of process-based fcntl locking, but I added proper fd-based locking recently in rust-lang#162602.

Will probably simplify rust-lang#159287 a bit.
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Sep 21, 2026
@JonathanBrouwer
Rollup merge of rust-lang#162952 - bjorn3:incr_comp_locking_change, r…
04ebe2a 
…=khyperia

Depend on lockfiles to prevent GC of the current session

Instead of manually skipping it right before the remove_dir. This will be simpler once we stop immediately copying from the old incr comp dir. Previously this wasn't possible due to the usage of process-based fcntl locking, but I added proper fd-based locking recently in rust-lang#162602.

Will probably simplify rust-lang#159287 a bit.
rust-bors Bot pushed a commit that referenced this pull request Sep 21, 2026
@bors
Unrolled build for #162952 in rollup 163095
854c002 
Rollup merge of #162952 - bjorn3:incr_comp_locking_change, r=khyperia

Depend on lockfiles to prevent GC of the current session

Instead of manually skipping it right before the remove_dir. This will be simpler once we stop immediately copying from the old incr comp dir. Previously this wasn't possible due to the usage of process-based fcntl locking, but I added proper fd-based locking recently in #162602.

Will probably simplify #159287 a bit.
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 49c263e to 71716be Compare September 21, 2026 11:25
@rustbot

rustbot commented Sep 21, 2026

Copy link
Copy Markdown
Collaborator

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.

@bjorn3
Build a new incr comp session dir from scratch every time
9c951e2 
Rather than copying the old incr comp dir and then modifying it. This
saves a copy/hardlink for files that are modified. And it removes the
need for accurate work product tracking to avoid accumulating cruft,
which is non-trivial. We don't accurately track the pre-LTO bitcode
files for ThinLTO for example.
@bjorn3
bjorn3 force-pushed the incr_comp_session_from_scratch branch from 71716be to 9c951e2 Compare September 21, 2026 11:40
bjorn3
bjorn3 commented Sep 21, 2026
View reviewed changes
Comment thread compiler/rustc_incremental/src/persist/fs.rs
Comment on lines -83 to -90
//! The exact same scheme is also used when reading the metadata hashes file
//! from an extern crate. When a crate is compiled, the hash values of its
//! metadata are stored in a file in its session directory. When the
//! compilation session of another crate imports the first crate's metadata,
//! it also has to read in the accompanying metadata hashes. It thus will access
//! the finalized session directory of all crates it links to and while doing
//! so, it will also place a read lock on that the respective session directory
//! so that it won't be deleted while the metadata hashes are loaded.

@bjorn3 bjorn3 Sep 21, 2026 •
edited by rustbot
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This section has long been outdated. We don't ever access the incr comp cache for other crates nowadays. Instead we just read the crate metadata like usual through eval_always queries.

View changes since the review

@bjorn3

bjorn3 commented Sep 21, 2026

Copy link
Copy Markdown
Member Author

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Blocked on something else such as an RFC or other implementation work. labels Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nnethercote nnethercote

Labels

A-incr-comp Area: Incremental compilation A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. A-run-make Area: port run-make Makefiles to rmake.rs 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.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@bjorn3 @rust-log-analyzer @rustbot @lewisl9029 @nnethercote