-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
rust: Parallelize build better by starting next Rust target already once metadata (rmeta) of all dependencies is available #11695
Comments
(sorry, re-commenting, I accidentally posted the previous one incomplete) The idea I had requires a wrapper command implemented in C (since python's startup cost is too high). The wrapper command will have three modes:
What we'd do is:
|
I'm not sure to understand how it works with cargo, please correct me if I got it wrong:
In theory rustc invocation could be split in 2 invocations, the first stops after creating the json file and the 2nd do the actual compilation/link, but in practice that 2nd invocation redo too much of the processing already done in 1st invocation? So we need 2 ninja targets, one for rlib and one for json.
|
Almost. The JSON part is parseable stdout status reporting from rustc, which could tell us when the metadata file is available so we don't have to do inotify or similar. The metadata file itself is a binary file. |
That means that for every rustc process we need to spawn 3 other processes, that's still significant overhead especially on Windows, no? I guess the alternative is adding in ninja support for a process to notify ninja that some dep files are already available before the process exit. |
I'm currious if C++20 modules has a similar situation. |
What's the practical effect of this? How much would we actually be paying to offset the increased parallelism opportunity? BTW it would be great if rustc had an option to share this information :/ something vaguely like a precompiled header or, with the way rust seems to be headed, a background daemon. Which is what this proposal essentially is, isn't it? A background daemon but implemented with flag files instead of the process table. |
I believe this is only for the link stage, so it's 3 extra processes per link. |
There's only a link stage for Rust targets (unless you want to split it into depinfo, metadata, link and duplicate work between them). |
I mean, we can't do this because rustc can't read in rmeta for the crate under compilation, only for other crates. I thought there was talk about supporting this though from the rustc side.
We've actually had requests to support having rmeta and object files separate, to avoid linking the stdlib into every rlib, which would make the actualy in-meson implementation a lot cleaner (and will likely be needed to support gcc-rs). We could implement this today, since an rlib is (currently) just an archive with the rmeta as the first file, and then the object file(s) after that. |
@xclaesse has some WIP code for this at ninja-build/ninja#2287 and #11707 . For my test project this reduces compilation time by 8% with This makes sense because we'd start compiling the next target while LLVM is busy, and optimizations are mostly done in LLVM and taking up time there. |
Describe the bug
rustc
is currently called bymeson
to only produce anrlib
ordylib
via--emit link
. Another kind of outputrustc
can produce ismetadata
.The metadata contains everything that is needed for compiling the next targets that depend on this one, and
cargo
uses this to parallelize the build better. Basically, you can already start compiling the next target while LLVM is doing its things (optimizer, etc) on the previous one still.In case of
cargo
this is done by enabling the JSON status output ofrustc
, which then notifies once the metadata is available or the linking is done. Theoretically both steps can also be done independently by callingrustc
twice but that will be slower as a lot of work has to be duplicated between both runs.It's not clear to me how this could be integrated into
meson
but @nirbheek said he has some ideas.CC @dcbaker
The text was updated successfully, but these errors were encountered: