-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Emit metadata files earlier #60385
Emit metadata files earlier #60385
Conversation
r? @oli-obk (rust_highfive has picked a reviewer for you, use r? to override) |
r? @nnethercote ...because this code isn't ready for review yet. |
r? @ghost |
☔ The latest upstream changes (presumably #60006) made this pull request unmergeable. Please resolve the merge conflicts. |
This commit separates metadata encoding (`tcx.encode_metadata`) from the creation of the metadata module (which is now handled by `write_compressed_metadata`, formerly `write_metadata`). The metadata encoding now occurs slightly earlier in the pipeline, at the very start of code generation within `start_codegen`. Metadata *writing* still occurs near the end of compilation; that will be moved forward in subsequent commits.
This change simplifies things for the subsequent commit.
cf952d1
to
cc77611
Compare
I have moved metadata encoding and writing to the start of You hinted that it might be possible to move it even earlier, before analysis. I haven't done that because it's fiddly (even moving it as far as I did was fiddly) and I didn't want to do it without having a clearer idea of whether it would work. Also, writing metadata before doing analysis would penalize the speed of compiler invocations that end in compile errors (which are common), so it's not a clear win. Anyway, take a look, see what you think about what I've done so far. |
The commit moves metadata writing from `link_binary` to `encode_metadata` (and renames the latter as `encode_and_write_metadata`). This is at the very start of code generation.
cc77611
to
38dffeb
Compare
@bors: r+ Looks great to me! I think it's actually a good point that we should wait to commit metadata until the crate has passed most static analysis anyway, in which case writing it just as codegen starts I think is at the very least a great start for now. |
📌 Commit 38dffeb has been approved by |
Does it make sense to move it earlier only for non-check builds or only for release builds? |
If people religiously used |
…xcrichton Emit metadata files earlier This will make cargo pipelining much more effective.
Rollup of 7 pull requests Successful merges: - #59634 (Added an explanation for the E0704 error.) - #60348 (move some functions from parser.rs to diagostics.rs) - #60385 (Emit metadata files earlier) - #60428 (Refactor `eval_body_using_ecx` so that it doesn't need to query for MIR) - #60437 (Ensure that drop order of `async fn` matches `fn` and that users cannot refer to generated arguments.) - #60439 (doc: Warn about possible zombie apocalypse) - #60452 (Remove Context and ContextKind) Failed merges: r? @ghost
Fair enough, though it seems reasonable to only promise really fast builds if one uses check... |
@@ -110,12 +110,13 @@ impl ExtraBackendMethods for LlvmCodegenBackend { | |||
ModuleLlvm::new_metadata(tcx, mod_name) | |||
} | |||
|
|||
fn write_metadata<'b, 'gcx>( | |||
fn write_compressed_metadata<'b, 'gcx>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this name is misleading, it should probably be {inject,embed,include,etc.}_compressed_metadata
.
This will make cargo pipelining much more effective.