-
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
Reduce rustbuild bloat caused by serde_derive #94844
Conversation
☔ The latest upstream changes (presumably #94845) made this pull request unmergeable. Please resolve the merge conflicts. |
This reduces binary size from 10.1MiB (6.2MiB for just rustbuild code) to 9.7MiB (5.8MiB for just rustbuild code). This also reduces compile time from ~6.1s for incr recompilation to ~5.6s. There is still a lot of unnecessary code due to the toml crate monomorphizing every deserialization impl 5 times.
This reduces binary size from 9.7MiB (5.8MiB for just rustbuild code) to 9.3MiB (5.3MiB for just rustbuild code). This doesn't reduce compile time in a statistically significant way.
b983ba5
to
dca8ff5
Compare
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.
Generally seems OK, though it does seem unfortunate to lose the nicer ergonomics offered by serde-derive. Ultimately we're not really using them so it's probably OK.
I am a little surprised to see that the dependency on serde derive isn't entirely dropped -- is that because it's still being used by metadata or something else? Do you expect to drop that eventually?
It is still used in a couple of other places like cargo artifact message deserialization and toolstate serialization/deserialization. I want to remove these too, but I focused on config deserialization first as it has the best cost vs benefit ratio. |
FWIW, if we care about disk space, it's probably worth comparing against an optimized build (at least -O1) of bootstrap and see what the size vs. time tradeoff is. But regardless, this seems good to me -- r=me if you don't want to iterate further in this PR. |
My main motivation for reducing this bloat is actually reducing compilation time. Smaller binaries generally take less time to codegen and less time to link. Reducing the total disk usage is a nice side-effect. By the way final stats for text section size: 4.9MiB for just rustbuild code. 8.8MiB for the whole binary. @bors r=Mark-Simulacrum |
📌 Commit a0163f7 has been approved by |
…askrgr Rollup of 5 pull requests Successful merges: - rust-lang#93292 (Implement `BITS` constant for non-zero integers) - rust-lang#94777 (Update armv7-unknown-linux-uclibceabi platform support page.) - rust-lang#94816 (Add `Atomic*::get_mut_slice`) - rust-lang#94844 (Reduce rustbuild bloat caused by serde_derive) - rust-lang#94907 (Omit stdarch test crates from the rust-src component) Failed merges: r? `@ghost` `@rustbot` modify labels: rollup
This reduces the size of the
.text
section from 10.1MiB (6.2MiB for just rustbuild code) to 9.3MiB (5.3MiB for just rustbuild code).This also reduces compile time from ~6.1s for incr recompilation to ~5.6s.
r? @Mark-Simulacrum