Implement synchronization scheme for incr. comp. directory #35718
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
|
cc @nikomatsakis @alexcrichton @brson @rust-lang/compiler |
| bug!("Trying initialize already active IncrCompSession.") | ||
| } | ||
| IncrCompSession::Finalized { .. } => { | ||
| bug!("Trying initialize already finalized IncrCompSession.") |
There was a problem hiding this comment.
nit: Both this bug! and the one before it should be "Trying to initialize.."
michaelwoerister
force-pushed
the
incr-comp-dir-locking
branch
from
15f3787
to
d86ad10
Compare
|
OK, I've implemented the read-write-lock approach. That seems like a clear improvement. I did not, however, move the lock file out of the directory. It made cleanup code more complicated in a few places and having it in the directory should be just as correct. I think, I also addressed all the nits. |
|
|
||
| // Like std::fs::create_dir_all, except handles concurrent calls among multiple | ||
| // threads or processes. | ||
| pub fn create_dir_racy(path: &Path) -> io::Result<()> { |
There was a problem hiding this comment.
is this just copied from compiletest?
There was a problem hiding this comment.
It's moved here from rustc_incremental::persist::util which doesn't exist anymore.
|
ok, r=me modulo nits |
michaelwoerister
force-pushed
the
incr-comp-dir-locking
branch
2 times, most recently
from
14b9dc8
to
d1d8258
Compare
|
@bors r=nikomatsakis |
|
📌 Commit d1d8258 has been approved by |
|
@alexcrichton @eddyb This seems to be a restriction purely in the Windows API (i.e. |
|
@michaelwoerister You could do it conditionally, since you already know the length. |
|
@eddyb yep. |
|
@bors r- Tidy fails. Feel free to re-r+ when fixed. |
|
☔ The latest upstream changes (presumably #35984) made this pull request unmergeable. Please resolve the merge conflicts. |
… on some 32bit systems.
By using "s-" instead of "sess-" as a prefix and encoding numbers as base36 instead of base16.
michaelwoerister
force-pushed
the
incr-comp-dir-locking
branch
from
bf29d72
to
bcd2f90
Compare
|
@alexcrichton Care to take a look at those last two commits and r+ if they look good to you? |
|
@bors: r+ |
|
📌 Commit bcd2f90 has been approved by |
|
⌛ Testing commit bcd2f90 with merge 5a8ca4b... |
|
@bors: retry force clean
|
…crichton Implement synchronization scheme for incr. comp. directory This PR implements a copy-on-write-based synchronization scheme for the incremental compilation cache directory. For technical details, see the documentation at the beginning of `rustc_incremental/persist/fs.rs`. The PR contains unit tests for some functions but for testing whether the scheme properly handles races, a more elaborate test setup would be needed. It would probably involve a small tool that allows to manipulate the incremental compilation directory in a controlled way and then letting a compiler instance run against directories in different states. I don't know if it's worth the trouble of adding another test category to `compiletest`, but I'd be happy to do so. Fixes #32754 Fixes #34957
This PR implements a copy-on-write-based synchronization scheme for the incremental compilation cache directory. For technical details, see the documentation at the beginning of
rustc_incremental/persist/fs.rs.The PR contains unit tests for some functions but for testing whether the scheme properly handles races, a more elaborate test setup would be needed. It would probably involve a small tool that allows to manipulate the incremental compilation directory in a controlled way and then letting a compiler instance run against directories in different states. I don't know if it's worth the trouble of adding another test category to
compiletest, but I'd be happy to do so.Fixes #32754
Fixes #34957