-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Move metadata generation before analysis #64112
Conversation
It's a false dependency. The result isn't used and there are no relevant side-effects.
They each have two call sites, and the sequencing is almost identical in both cases.
`Compiler::register_plugins()` calls `passes::register_plugins()`, which calls `Compiler::dep_graph_future()`. This is the only way in which a `passes` function calls a `Compiler` function. This commit moves the `dep_graph_future()` call out of `passes::register_plugins()` and into `Compiler::register_plugins()`, which is a more sensible spot for it. This will delay the loading of the dep graph slightly -- from the middle of plugin registration to the end of plugin registration -- but plugin registration is fast enough (especially compared to expansion) that the impact should be neglible.
`Compiler::compile()` is different to all the other `Compiler` methods because it lacks a `Queries` entry. It only has one call site, which is in a test that doesn't need its specific characteristics. This patch removes `Compile::compile()` and replaces the call to it in the test with a call to `Compile::codegen_and_link()`, which is similar enough for the test's purposes.
The goal of this commit is to improve the effectiveness of pipelining, because metadata generation must complete before dependent crates can begin compiling. Unfortunately it's not much of a win. The biggest speed-up I have seen is 1.07x, vs 1.79x for the original pipelining implementation. And it slightly slows down the common case when an error occurs during analysis, because metadata generation now precedes analysis. Currently three tests fail with the change, due to bounds violations. These tests are currently `git rm`'d, so that subsequent tests can run.
Unfortunately it's not much of a win. The biggest speed-up I have seen is 1.07x, vs 1.79x for the original pipelining implementation. And it slightly slows down the common case when an error occurs during analysis, because metadata generation now precedes analysis. Currently three tests fail with the change, due to bounds violations. These tests are currently The first five commits above are from #64016, which is awaiting review. Here are timings for the multi-crate rustc-perf benchmarks that currently compile on my Linux box. For each configuration there are three measurements:
You can see that the additional improvements from the new, aggressive pipelining are paltry compared to the improvements from the existing pipelining. Because of all this, my current inclination is to abandon the effort, but I'm posting it here in case somebody has an idea to improve it. |
The small improvements make sense; analysis takes a lot less time than code generation, so the amount of additional overlap is relatively small. |
My reading of the diff is that metadata generation still happens after lowering and more importantly after cfg stripping/macro expansion. Can you confirm? |
That is correct. I couldn't push it any earlier because |
Thanks. The numbers for |
What about doing them in parallel? Of course, it might still be slightly slower on hardware with not enough cores available, but at least there won't be any regression on sufficiently "good" hardware. |
Doesn't metadata generation needed data that is produced only very late in the process (like optimized MIR and, potentially, the list of exported monomorphizations)? The query system should take care of producing this data on demand but with what's expected to be contained in RLIBs at the moment, it seems hard to effectively move metadata generation more towards the front. |
@alexcrichton said on Zulip:
This may be true. Understanding the ordering of things in rustc is surprisingly hard. I did spend enough time looking at (and refactoring!) I am still inclined to abandon this effort. I tried what I set out to try, got a result that makes sense (very slight speedups, but much smaller than the original pipelining because analysis is much quicker than codegen), and suggestions for further work are more speculative. If somebody else wants to experiment with it further, please do so. |
The goal of this commit is to improve the effectiveness of pipelining, because metadata generation must complete before dependent crates can begin compiling.
r? @ghost