-
Notifications
You must be signed in to change notification settings - Fork 12.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
rustbuild: Both stage0 and stage1 std are rebuilt spuriously #68149
Comments
Reproduction script: ./x.py test --stage 1 src/test/ui --test-args mytest
rm -rf build/x86_64-pc-windows-gnu/{stage1*,stage0-sysroot}
./x.py test --stage 1 src/test/ui --test-args mytest
./x.py test --stage 1 src/test/ui --test-args mytest -vvvvvv You might have to run it multiple times to cause the spurious rebuild in the last command. |
For some reason
|
I think @eddyb was hitting this at some point as well, though we weren't able to determine quite why. |
I haven't encountered these rebuilds before, but recently (since December?) they happen quite often, like every second or third build, and that's pretty inconvenient given the long builds. Perhaps something changed in |
I don't think we've changed backtrace itself recently. It would perhaps be handy to check if this still happens before 87540bd (which changed how/when we clear out std's target directory). However, I don't expect it would have, as that would cause all of std to be rebuilt, not just backtrace. Maybe backtrace is unique in being a dependency of both std and the compiler (for example?). I'm not sure what could be causing this odd behavior :/ |
I'm pretty sure the rebuilds appeared before 87540bd (and I hoped that PR would fix them, but it didn't). |
I suspect #67077 to be the cause, at least based on timing. |
I've been hitting similar (this?) issue with x86_64-pc-windows-gnu when doing |
Hm, so #67077 in theory at least wouldn't have affected backtrace specifically, I think, though there may have been knock on effects. One thing that might be helpful is getting a log with |
Update: this reproduces on I'll try |
Reproduced on
|
I was able to reproduce this once, but I'm having trouble reproducing it again. When I did reproduce it, I noticed that the |
binary-dep-depinfo should not include backtrace into libcore, I think, since we don't pass I don't know why we're including dep-lib-backtrace in the libcore dep file though, that does sound like a bug. I guess this is then likely an upstream bug in some sense? I think leaving it here for tracking at least for now makes sense though. |
It's the other way around. I'm not too familiar with the compiler, so it's a little slow for me to figure out. I seem to recall being told that writing the |
We currently emit a dependency on both the rmeta and rlib files (presuming they were found during building, I guess?) with binary-dep-depinfo. That might be in error! I did so as the most pessimistic option when initially adding that feature, but perhaps I was too eager in pulling in dependencies. If I'm understanding correctly, that may mean that if libcore's codegen completes before backtrace is built, we'll pull in the libcore rlib as a dependency of backtrace, and then when backtrace is checked for freshness in the future, we'll notice that it's actually been built before the rlib (which we finished writing after libbacktrace started). Is there ever a case where there's a dependency on rlibs directly? Maybe we should only emit rmeta/dylib filepaths in binary-dep-depinfo? |
For reference, here's what I have in that
|
Hm, so it looks like in practice we read metadata from the first of the three that we find -- so in some sense, we don't actually depend on all three, but rather rlib/rmeta/dylib, in that order, when we're loading up dependency information. I guess there's not really any tracking for which one we've used metadata from, but in practice, we should probably only emit one of these (probably rmeta/rlib/dylib, in that order) that we have set as the CrateSource. |
This is actually something that I wanted to try to change (#64882 (comment)) and read from the rmeta first. |
Even if |
I was able to record exactly what happens. The timeline is: T1. The result is that I'm not sure how to fix this. One solution would be to change the |
I like the idea of inverting rustc's preference, using only the rmeta rather than both the rmeta and rlib if they're available. Other than that though I don't know of a great way to solve this :( I may be missing something though, is this rustc-specific? This seems like a bug that everyone should be hitting naively which makes it quite serious? |
It is only with |
Ah I see, makes sense! Here's another possible question and/or solution. When Cargo builds libbacktrace it's presumably only passing |
Backtrace only sees |
I think this is our best bet -- furthermore, I wonder if there's some wins to be had in terms of compile times. It seems like we are currently loading libcore's rmeta/rlib for all crates out there, which presumably is not great (more IO, etc.). I can pursue a PR to that effect soon. |
@ehuss aha ok that makes sense as well. Then yeah I think your suggestion is indeed the only thing I can think of as well! |
Hm, actually, in going to do this, it sounds like we can't quite just emit one of them, per @petrochenkov's comment "Even if rmeta is used for reading metadata, rlib can still be used for reading object code, in that case both should indeed be in dependencies?" -- right? |
When building an rlib we never use contents of upstream rlibs. When building dynamically linked artifacts, though (or executables) we do use upstream object code. (this is basically pipelining) |
Aha, okay, so it sounds like a partial fix would be to:
Does that sound right? Basically, we emit "worst case" for either situation which I think might mean that for dylibs this bug persists -- since they'll start compilation before the rlib is finished -- but I don't know how much we can do about it under the current model. |
That should be right yeah, and I think the dylib bug won't be an issue because Cargo won't start libstd until all subsequent rlib compilations are finished (e.g. it doesn't pipeline it) |
Great! Okay, then I'll try to get that impl up. (I presume the "dylib bug won't be an issue" also applies to dylibs in the wild, right?) |
That's the theory at least! |
I'm not sure when exactly it happens, but it reproduces roughly like this:
./x.py test --stage 1 src/test/ui --test-args mytest
, the build starts fromstage0 std
or later and succeeds../x.py test --stage 1 src/test/ui --test-args mytest
again, should be a trivial build, but the build starts fromstage0 std
orstage1 std
again and succeeds../x.py test --stage 1 src/test/ui --test-args mytest
again, trivial build happens.The target is x86_64-pc-windows-gnu.
The issue exists for several weeks, so it's a regression.
cc @Mark-Simulacrum
The text was updated successfully, but these errors were encountered: