Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Binary size regression in dylibs #92013

Closed
Nashenas88 opened this issue Dec 16, 2021 · 6 comments · Fixed by #92029
Closed

Binary size regression in dylibs #92013

Nashenas88 opened this issue Dec 16, 2021 · 6 comments · Fixed by #92029
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.

Comments

@Nashenas88
Copy link
Contributor

Nashenas88 commented Dec 16, 2021 •
edited
Loading

The fuchsia nightly toolchain rolls caught a binary size regression due to commit f9e77f2

Limiting to just binary changes in the rust repo, following the instructions here: https://fuchsia.dev/fuchsia-src/development/build/rust_toolchain?hl=en, we can see that on average the standard library is 3.5 times larger:

Click to expand 
 $ diff -u <(cd ~/fuchsia-rust-without && du -a | sort -k2) <(cd install/fuchsia-rust && du -a | sort -k2)
--- /proc/self/fd/11    2021-12-16 23:41:12.360226537 +0000
+++ /proc/self/fd/18    2021-12-16 23:41:12.360226537 +0000
@@ -1,12 +1,12 @@
-1135076        .
-100040 ./bin
+1139492        .
+100048 ./bin
 27808  ./bin/cargo
 8312   ./bin/cargo-clippy
 9484   ./bin/cargo-fmt
-10252  ./bin/clippy-driver
+10256  ./bin/clippy-driver
 12     ./bin/rustc
 9928   ./bin/rust-demangler
-13064  ./bin/rustdoc
+13068  ./bin/rustdoc
 21164  ./bin/rustfmt
 4      ./bin/rust-gdb
 4      ./bin/rust-gdbgui
@@ -14,30 +14,26 @@
 20     ./etc
 16     ./etc/bash_completion.d
 12     ./etc/bash_completion.d/cargo
-1026028        ./lib
-38272  ./lib/.build-id
-4932   ./lib/.build-id/1f
-380    ./lib/.build-id/1f/33eefa63da5e0d
-4548   ./lib/.build-id/1f/33eefa63da5e0d.debug
-14044  ./lib/.build-id/52
-1360   ./lib/.build-id/52/119c59550c0d88
-12680  ./lib/.build-id/52/119c59550c0d88.debug
-14304  ./lib/.build-id/c2
-1292   ./lib/.build-id/c2/d10ba0e45111ca
-13008  ./lib/.build-id/c2/d10ba0e45111ca.debug
-4988   ./lib/.build-id/dd
-332    ./lib/.build-id/dd/4b1b684bd04567
-4652   ./lib/.build-id/dd/4b1b684bd04567.debug
+1030436        ./lib
+34908  ./lib/.build-id
+13012  ./lib/.build-id/48
+13008  ./lib/.build-id/48/eb8f061585f3df.debug
+4552   ./lib/.build-id/57
+4548   ./lib/.build-id/57/8eb4ac89ce0ac7.debug
+4656   ./lib/.build-id/ad
+4652   ./lib/.build-id/ad/516bd46661c2b2.debug
+12684  ./lib/.build-id/c3
+12680  ./lib/.build-id/c3/b4a43e34abcca0.debug
 8456   ./libexec
 8452   ./libexec/cargo-credential-1password
-75384  ./lib/libLLVM-13-rust-1.59.0-nightly.so
-120696 ./lib/librustc_driver-8d166f3d4b6e5d80.so
+75380  ./lib/libLLVM-13-rust-1.59.0-nightly.so
+120708 ./lib/librustc_driver-fe9dbf77676f2946.so
 13356  ./lib/libstd-3f5a6058990e33c2.so
 4764   ./lib/libtest-48b5b7c796fdec12.so
 4      ./lib/runtime.json
-773548 ./lib/rustlib
-136060 ./lib/rustlib/aarch64-fuchsia
-136056 ./lib/rustlib/aarch64-fuchsia/lib
+781312 ./lib/rustlib
+139940 ./lib/rustlib/aarch64-fuchsia
+139936 ./lib/rustlib/aarch64-fuchsia/lib
 304    ./lib/rustlib/aarch64-fuchsia/lib/libaddr2line-90c91cd254aac157.rlib
 48     ./lib/rustlib/aarch64-fuchsia/lib/libadler-8684584fc827b6cd.rlib
 5016   ./lib/rustlib/aarch64-fuchsia/lib/liballoc-62373237ba162112.rlib
@@ -60,21 +56,21 @@
 8      ./lib/rustlib/aarch64-fuchsia/lib/librustc_std_workspace_core-7b9c68ff961c0793.rlib
 12     ./lib/rustlib/aarch64-fuchsia/lib/librustc_std_workspace_std-59a7bdc55614e72c.rlib
 26016  ./lib/rustlib/aarch64-fuchsia/lib/libstd-be07c02dc031f481.rlib
-1292   ./lib/rustlib/aarch64-fuchsia/lib/libstd-be07c02dc031f481.so
+4712   ./lib/rustlib/aarch64-fuchsia/lib/libstd-be07c02dc031f481.so
 164    ./lib/rustlib/aarch64-fuchsia/lib/libstd_detect-bbd4c7f2ff882d73.rlib
 12664  ./lib/rustlib/aarch64-fuchsia/lib/libtest-76f50ba48ac29a1e.rlib
-332    ./lib/rustlib/aarch64-fuchsia/lib/libtest-76f50ba48ac29a1e.so
+792    ./lib/rustlib/aarch64-fuchsia/lib/libtest-76f50ba48ac29a1e.so
 148    ./lib/rustlib/aarch64-fuchsia/lib/libunicode_width-0a583143af34670b.rlib
 124    ./lib/rustlib/aarch64-fuchsia/lib/libunwind.a
 52     ./lib/rustlib/aarch64-fuchsia/lib/libunwind-c427f07ef33d9f9d.rlib
-161076 ./lib/rustlib/aarch64-unknown-linux-gnu
-161072 ./lib/rustlib/aarch64-unknown-linux-gnu/lib
+161080 ./lib/rustlib/aarch64-unknown-linux-gnu
+161076 ./lib/rustlib/aarch64-unknown-linux-gnu/lib
 312    ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libaddr2line-37f44d9d37c20730.rlib
 48     ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libadler-2caee81c613175ce.rlib
 5068   ./lib/rustlib/aarch64-unknown-linux-gnu/lib/liballoc-fe7cef42d77d414d.rlib
 12     ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libcfg_if-cf5e4577938aedb7.rlib
 5068   ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libcompiler_builtins-05086e81dbe54bf6.rlib
-51424  ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libcore-b44def38eb7dae47.rlib
+51428  ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libcore-b44def38eb7dae47.rlib
 2120   ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libgetopts-5eeb5287d17a8d26.rlib
 6864   ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libgimli-308bcc12df2355e2.rlib
 1180   ./lib/rustlib/aarch64-unknown-linux-gnu/lib/libhashbrown-1bc7c700b83c21f3.rlib
@@ -1916,14 +1912,14 @@
 8580   ./lib/rustlib/wasm32-unknown-unknown/lib/libtest-530f59db49fb2773.rlib
 144    ./lib/rustlib/wasm32-unknown-unknown/lib/libunicode_width-d6f6eb05caa4b08c.rlib
 8      ./lib/rustlib/wasm32-unknown-unknown/lib/libunwind-0ce2d83679176881.rlib
-141800 ./lib/rustlib/x86_64-fuchsia
-141796 ./lib/rustlib/x86_64-fuchsia/lib
+145680 ./lib/rustlib/x86_64-fuchsia
+145676 ./lib/rustlib/x86_64-fuchsia/lib
 304    ./lib/rustlib/x86_64-fuchsia/lib/libaddr2line-dd556d58a6a98686.rlib
 48     ./lib/rustlib/x86_64-fuchsia/lib/libadler-f331a1913229a0c9.rlib
 4984   ./lib/rustlib/x86_64-fuchsia/lib/liballoc-543c62796e6ba4f7.rlib
 12     ./lib/rustlib/x86_64-fuchsia/lib/libcfg_if-41b6d6da5c5d8f78.rlib
 4496   ./lib/rustlib/x86_64-fuchsia/lib/libcompiler_builtins-a606301fb5da8fdf.rlib
-56820  ./lib/rustlib/x86_64-fuchsia/lib/libcore-da0ced80ec6ab2c8.rlib
+56816  ./lib/rustlib/x86_64-fuchsia/lib/libcore-da0ced80ec6ab2c8.rlib
 1976   ./lib/rustlib/x86_64-fuchsia/lib/libgetopts-e19d8aa129e1788d.rlib
 6828   ./lib/rustlib/x86_64-fuchsia/lib/libgimli-5b1712acc4689b1a.rlib
 1164   ./lib/rustlib/x86_64-fuchsia/lib/libhashbrown-e26bc67a99f1c0d5.rlib
@@ -1940,10 +1936,10 @@
 8      ./lib/rustlib/x86_64-fuchsia/lib/librustc_std_workspace_core-185860a1c4fa1525.rlib
 12     ./lib/rustlib/x86_64-fuchsia/lib/librustc_std_workspace_std-f3d442e17b28bc30.rlib
 25860  ./lib/rustlib/x86_64-fuchsia/lib/libstd-4b0aaefa59d0d7cc.rlib
-1360   ./lib/rustlib/x86_64-fuchsia/lib/libstd-4b0aaefa59d0d7cc.so
+4784   ./lib/rustlib/x86_64-fuchsia/lib/libstd-4b0aaefa59d0d7cc.so
 448    ./lib/rustlib/x86_64-fuchsia/lib/libstd_detect-27929f8c582fe1d2.rlib
 12516  ./lib/rustlib/x86_64-fuchsia/lib/libtest-3f645ae7dc0007c0.rlib
-380    ./lib/rustlib/x86_64-fuchsia/lib/libtest-3f645ae7dc0007c0.so
+840    ./lib/rustlib/x86_64-fuchsia/lib/libtest-3f645ae7dc0007c0.so
 148    ./lib/rustlib/x86_64-fuchsia/lib/libunicode_width-618f3c1ed4dee7a0.rlib
 52     ./lib/rustlib/x86_64-fuchsia/lib/libunwind-6aef5eac0c7d634e.rlib
 136    ./lib/rustlib/x86_64-fuchsia/lib/libunwind.a

I'm currently assuming that our setting of debuginfo-level-std = 2 in the config of fuchsia rust builds is what causes this to be so noticeable.

The effect on fuchsia is very noticeable:

$ compare-sizes.py ~/elf_sizes.json ~/with_changes_elf_sizes.json
../../../rust/install/fuchsia-rust/lib/rustlib/x86_64-fuchsia/lib/libstd-4b0aaefa59d0d7cc.so: blob = 2,785,280 (84.4%)
obj/build/images/sizes/fuchsia.zbi/bootfs/lib/libstd-4b0aaefa59d0d7cc.so: zbi = 3,506,176 (71.6%)

Total differences:
blob: 2785280 of 3301376 (84.4%)
zbi : 3506176 of 4898816 (71.6%)

The binary size increases on our debug builds are large enough to prevent the binaries from fitting on the devices we're debugging.

CC @tmandry
@rustbot modify labels: +regression-from-stable-to-nightly -regression-untriaged
@Nashenas88 Nashenas88 added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Dec 16, 2021
@rustbot rustbot added regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. I-prioritize Issue: Indicates that prioritization has been requested for this issue. and removed regression-untriaged Untriaged performance or correctness regression. labels Dec 16, 2021
@tmandry
Copy link
Member

tmandry commented Dec 17, 2021

The regression comes from the inclusion of the .rmeta section in the libstd dylib for the fuchsia target. This section is quite large (3.4 MB, more than twice the size of the original binary).

I'm actually confused about how this was working before. Clearly we need rustc metadata to compile against libstd or any other crate. The old DSO doesn't appear to have a section for metadata, but was compiling fine. Was this being shoved in another section (and somehow much smaller)? Or was rustc finding metadata from another libstd file, perhaps the rlib?

cc #91604 @nikic @nagisa

@tmandry tmandry changed the title Binary size regression Binary size regression in dylibs Dec 17, 2021
@nikic
Copy link
Contributor

nikic commented Dec 17, 2021

The .rmeta section should not be getting included in dylibs (those should use .rustc with compressed metadata). This patch wasn't supposed to change handling of .rmeta at all. I must have broken something while refactoring, though I don't immediately spot what.

@nikic
Copy link
Contributor

nikic commented Dec 17, 2021

Or did you mean .rustc above? I just checked the libstd.so from a local (non-fuchsia) build and I see a .rustc section, but not an .rmeta section.

@nikic
Copy link
Contributor

nikic commented Dec 17, 2021

Though I see now that .rustc is marked as CONTENTS, ALLOC, LOAD, DATA rather than CONTENTS, READONLY, that's not right.

@nikic
Copy link
Contributor

nikic commented Dec 17, 2021

Okay, that's because the section is created with SectionKind::Data, which assigns default flags: https://github.com/gimli-rs/object/blob/fc6289cfa81e193736dba3d0cfde11501139d992/src/write/elf/object.rs#L694 Probably it should use a debug section like .rmeta does.

@nikic nikic mentioned this issue Dec 17, 2021
Merged
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 18, 2021
@matthiaskrgr
Rollup merge of rust-lang#92029 - nikic:section-flags-fix, r=davidtwco
b8d0fa7 
Use debug section for .rustc

For a data section, the object crate will set SHF_ALLOC by default, which is exactly what we don't want. Use a debug section instead, the same as we do for .rmeta.

I checked with `objdump -h` that this produces the right flags for ELF.

Fixes rust-lang#92013.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this issue Dec 18, 2021
@matthiaskrgr
Rollup merge of rust-lang#92029 - nikic:section-flags-fix, r=davidtwco
af5b8bc 
Use debug section for .rustc

For a data section, the object crate will set SHF_ALLOC by default, which is exactly what we don't want. Use a debug section instead, the same as we do for .rmeta.

I checked with `objdump -h` that this produces the right flags for ELF.

Fixes rust-lang#92013.
@bors bors closed this as completed in 9415c67 Dec 19, 2021
@rustbot rustbot removed the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Dec 19, 2021
@tmandry
Copy link
Member

tmandry commented Dec 20, 2021

Thanks for the fix! and sorry yes, I did mean .rustc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Explicitly set no ELF flags for .rustc section nikic/rust
4 participants
@nikic @Nashenas88 @tmandry @rustbot