-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Introduce -Zembed-metadata
to allow omitting full metadata from rlibs and dylibs
#137535
Conversation
This comment has been minimized.
This comment has been minimized.
(The test failure is the one situation not yet implemented, which is described in the PR description - |
That's by design,
In #120855 the heuristics were a temporary measure to enable split metadata in bootstrap, while cargo still doesn't support it. |
Interesting. So if I understood it correctly, you would like to remove all heuristics from rustc (so no lookup of On the other hand, it would essentially double the length of the rustc command line, because for every library that you depend on, Cargo would have to pass both |
Yes.
My impression was that it is already the case with pipelined builds, but apparently it's indeed only |
Some changes occurred in compiler/rustc_codegen_ssa This PR modifies cc @jieyouxu |
Ok, rebased and removed the heuristics. I added an error message when the full metadata is not found, but I'm not sure if we should emit the error eagerly (left a comment in code). Could you please take a look at fe29fec? Thank you! |
This comment has been minimized.
This comment has been minimized.
rust-lang/compiler-team#851 was accepted, so I think that we can go forward with this. @petrochenkov What do you think about renaming the flag to |
Seems ok to me. |
-Zsplit-metadata
to split metadata out of rlibs/dylibs-Zembed-metadata
to allow omitting full metadata from rlibs and dylibs
Ok, changed it to @rustbot ready |
This comment has been minimized.
This comment has been minimized.
Created a tracking issue and fixed tests. |
r=me with the metadata version updated. |
This comment has been minimized.
This comment has been minimized.
rust-analyzer is developed in its own repository. If possible, consider making this change to rust-lang/rust-analyzer instead. cc @rust-lang/rust-analyzer |
@bors r=petrochenkov rollup=never Marked as rollup=never, because this modifies metadata version. |
☀️ Test successful - checks-actions |
What is this?This is an experimental post-merge analysis report that shows differences in test outcomes between the merged PR and its parent PR.Comparing ed20157 (parent) -> 8c35f4a (this PR) Test differencesShow 43 test diffsStage 1
Stage 2
Additionally, 2 doctest diffs were found. These are ignored, as they are noisy. Job group index
Job duration changes
How to interpret the job duration changes?Job durations can vary a lot, based on the actual runner instance |
Finished benchmarking commit (8c35f4a): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary -1.7%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (secondary 3.9%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeResults (secondary 0.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Bootstrap: 773.019s -> 774.818s (0.23%) |
This is a continuation of #120855 (I was mentored by @bjorn3 to move it forward). Most of the original code was written by bjorn3, I tried to clean it up a bit and add some documentation and tests.
This PR introduces a new unstable compiler flag called
-Zembed-metadata=[no|yes]
, with the default beingyes
(see #57076 for context). When set tono
, rustc will only store a small metadata stub inside rlibs/dylibs instead of the full metadata, to keep their size smaller. It should be used in combination with--emit=metadata
, so that the users of such a compiled library can still read the metadata from the corresponding.rmeta
file. This comment shows an example of binary/artifact size wins that can be achieved using this approach.Contrary to #120855, this PR only introduces the new flag, along with a couple of run-make tests and documentation, but does not yet use it in bootstrap to actually compile rustc. I plan to do that as a follow-up step (along with integration in Cargo, which should ideally just always pass this flag to reduce the size of target directories).
Fixes #23366
Closes #29511
Fixes #57076
Another attempt of #93945 and #120855.
r? @petrochenkov