Build rustc and codegen backends with -Zembed-metadata=no - #151061
Conversation
This comment has been minimized.
This comment has been minimized.
Build rustc and codegen backends with -Zno-embed-metadata
|
And the baseline to compare the disk space against for try builds: @bors try |
Build rustc and codegen backends with -Zno-embed-metadata
This comment has been minimized.
This comment has been minimized.
|
r? @Kobzol |
6e25c8e to
b6605c3
Compare
|
For the try build it saves 2GB or 4% of the total space the build takes (46GB -> 44GB). Hoped it would be a bit more, but still not bad I think. And maybe it gives a higher relative saving when not collecting PGO profiles? @rustbot ready |
|
Let's do a full try build to see the codegen backend savings (we can compare them with the parent commit, should be enough), and also run perf., just in case. @bors try jobs=dist-x86_64-linux @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Build rustc and codegen backends with -Zno-embed-metadata try-job: dist-x86_64-linux
|
For cg_clif the |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (2cbe7f7): comparison URL. Overall result: no relevant changes - no action neededBenchmarking this pull request means it may be perf-sensitive – we'll automatically label it not fit for rolling up. You can override this, but we strongly advise not to, due to possible changes in compiler perf. @bors rollup=never Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (secondary -1.4%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 476.337s -> 475.884s (-0.10%) |
|
Before: After: And no other components seem to have changed their size. Yeah, pretty nice, a solid ~10%! |
This comment has been minimized.
This comment has been minimized.
|
I can't reproduce the CI failure locally. Could it be related to rust-lang/cargo#16503? |
|
Possibly, although my first guess is that we are not copying rustc .rmeta files somewhere in bootstrap. |
|
Maybe we could retry this after #151225? |
14017fc to
158bff0
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| if matches!(mode, Mode::Std) { | ||
| cargo.arg("-Zno-embed-metadata"); | ||
| } | ||
|
|
There was a problem hiding this comment.
this looks like what I came up with while upgrading to 1.94.0 in alpinelinux https://git.alpinelinux.org/aports/commit/?id=e688d5dcba35af7386e9b9f24527ea98dad4040d
There was a problem hiding this comment.
In this PR, the -Zno-embed-metadata is just moved and also enabled by rustc and codegen backends.
without this we would need to keep the lib*.rmeta files in the main rust
package and have it grow more than twice in size and there would be less
reason of keeping the then much smaller rustc-dev subpackage split
You only need to keep the .rmeta files for the standard library. Without -Zno-embed-metadata they are still present, just embedded in the .rlib and .so files. If anything removing -Zno-embed-metadata increases the size of the package you create as now both libstd.rlib and libstd.so contain a separate copy of the crate metadata rather than sharing a single .rmeta file.
This comment has been minimized.
This comment has been minimized.
158bff0 to
14dbf7e
Compare
|
This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed. Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers. |
This comment has been minimized.
This comment has been minimized.
|
Managed to reproduce it locally. Seems to be working now. I wasn't able to unconditionally pass |
cb2e7ce to
b61abad
Compare
|
@rustbot ready |
While this won't change much for rustc and rustc-dev, it should make codegen backends a bit smaller by omitting their crate metadata. In addition it should reduce disk usage while compiling.
The run-make-support build code doesn't use run_cargo. Previously this would be skipped by the |
b61abad to
5e75882
Compare
|
Hmm, it's not just compiletest, rustdoc also breaks if we apply this everywhere: |
|
Looks like someone decided to add a dependency on rustc_public to rustc_driver. That would have to be the other way around for rustc_public to function without manually linking in rustc_driver as user of rustc_public. That said, I don't understand how that error you got would be triggered. Both librustc_public.rmeta and librustc_public.rlib should be included in the sysroot. |
|
Sorry, I tried this in a fresh checkout and didn't port your |
View all comments
While this won't change much for rustc and rustc-dev, it should make codegen backends a bit smaller by omitting their crate metadata. In addition it should reduce disk usage while compiling.
Follow up to #145343