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.

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

executable grew by 30mb after libdeno merge #3599

Closed
ry opened this issue Jan 5, 2020 · 5 comments · Fixed by #3645
Closed

executable grew by 30mb after libdeno merge #3599

ry opened this issue Jan 5, 2020 · 5 comments · Fixed by #3645
Assignees
Labels
bug Something isn't working correctly

Comments

@ry
Copy link
Member

ry commented Jan 5, 2020

Screen Shot 2020-01-05 at 9 37 04 AM

Not sure why but there has been no fundamental change in our asset management - so this should be solvable.

@ry ry added the bug Something isn't working correctly label Jan 5, 2020
@ry
Copy link
Member Author

ry commented Jan 6, 2020

If we look at the largest symbols comparing v0.28.1 and the master branch

gpu ~/src/deno> nm --demangle --debug-syms --print-size --size-sort `which deno`| tail
0000000000012a00 000000000002b728 r .dynstr
0000000000000000 0000000000030e10 N .debug_pubnames
000000000257c880 0000000000033e08 r v8::internal::blob_data
0000000000000000 000000000003c83b N .debug_loc
0000000000000000 0000000000058e9a N .debug_line
0000000000000000 00000000000598b0 N .debug_ranges
0000000000000000 00000000000b3c10 N .debug_str
0000000002601c18 00000000000bcfc8 r .eh_frame
0000000000000000 00000000000bf4b4 N .debug_info
000000000003f680 000000000012e940 r .rela.dyn
gpu ~/src/deno> nm --demangle --debug-syms --print-size --size-sort target/release/deno | tail
00000000024a6000 0000000000025000 r GFp_nistz256_precomputed
0000000000000000 0000000000030e10 N .debug_pubnames
000000000256d100 0000000000036b58 r v8::internal::blob_data
0000000000000000 000000000003b05c N .debug_loc
0000000000002e78 00000000001304b8 r .rela.dyn
0000000002641408 0000000000180b3c r .eh_frame
0000000000000000 0000000000258d73 N .debug_str
0000000000000000 000000000083fcb0 N .debug_ranges
0000000000000000 00000000009c07e2 N .debug_line
0000000000000000 000000000136d032 N .debug_info

we see that master has some extra very large symbols:

0000000000000000 00000000009c07e2 N .debug_line
0000000000000000 000000000136d032 N .debug_info

Here 0x136d032 and 0x9c07e2 represent the size of the section. 0x136d032 = 19.4 MiB and 0x9c07e2 = 9.75 MiB. So it looks like we've found that these debug symbols account for the added size in the binary. Indeed, if we call strip on the executable, we get back down to the original size.

@ry
Copy link
Member Author

ry commented Jan 8, 2020

Also worth noting that this issue is only on linux.

@piscisaureus
Copy link
Member

piscisaureus commented Jan 8, 2020

Some notes from the investigation so far:

  • The pre-rusty build has a .debug_info section too; it was just a lot smaller.
  • The currently open rust issue about this seems to be Trivial dependencies on large crates pull in massive amounts of debuginfo rust-lang/rust#56068. In summary:
    • Rust relies on the --gc-sections linker flag to remove dead code from the final executable.
    • This has no effect on what's in the .debug_info section, which remains full of debug info for unused object files.
    • The creator if the above issue contributed a patch to lld to fix this, but this was later reverted.
  • I personally was able to use ld.lld (the llvm linker) to build deno and tell it to strip debug info.
    • This does reduce the size of the executable 87mb to 46mb.
    • The magic incantation I used, which was possible because I had a linux build of rusty_v8 in ~/d/v8l:
      RUSTFLAGS="-Clinker=$HOME/d/v8l/target/debug/build/rusty_v8-432ac2fac61ecd18/out/clang/bin/clang -Clink-arg=-fuse-ld=$HOME/d/v8l/target/debug/build/rusty_v8-432ac2fac61ecd18/out/clang/bin/ld.lld -Clink-arg=-Wl,--strip-debug" cargo build --release --locked.
    • This of course removes all debug info from the file, which may not be desirable.

@piscisaureus
Copy link
Member

piscisaureus commented Jan 9, 2020

Over 24 hours of trial and error later, it looks like the following combination of flags reduces the size back to what it was, and the debug info that now remains in the release binary is actually more useful than it was.

Note that this final attempt tried 3 flags and had cross-language LTO enabled.
I still have to figure out which of these changes we should land and which ones are unnecessary.

diff --git a/config/compiler/BUILD.gn b/config/compiler/BUILD.gn
index 2354f1b..eff3d52 100644
--- a/config/compiler/BUILD.gn
+++ b/config/compiler/BUILD.gn
@@ -588,7 +588,7 @@ config("compiler") {
     cflags += [ "-flto=thin" ]

     if (target_os != "chromeos") {
-      cflags += [ "-fsplit-lto-unit" ]
+      cflags += [ "-fno-split-lto-unit" ]
     }

     if (thin_lto_enable_optimizations) {
@@ -2416,7 +2416,12 @@ config("minimal_symbols") {
     # flag, so we can use use -g1 for pnacl and nacl-clang compiles.
     # gcc nacl is is_nacl && !is_clang, pnacl and nacl-clang are && is_clang.
     if (!is_nacl || is_clang) {
-      cflags += [ "-g1" ]
+      cflags += [
+        "-g1",
+        "-gno-inline-line-tables",
+        "-fno-force-dwarf-frame",
+        "-fdebug-types-section",
+      ]
     }
     ldflags = []
     if (is_android && is_clang) {
RUSTFLAGS="-Clinker-plugin-lto -Clinker=$CLANG_DIR/bin/clang -Clink-arg=-fuse-ld=$CLANG_DIR/bin/ld.lld  -Clink-arg=-Wl,--thinlto-cache-dir=`pwd`/target/release/gn_out/thinlto-cache -Clink-arg=-Wl,-O5 -Clink-arg=-Wl,--icf=safe -Clink-arg=-Wl,--compress-debug-sections=zlib" cargo build --release --vv

@piscisaureus
Copy link
Member

v0.22.0:

piscisaureus@guru:~/d/denol$ ls -l ~/.deno/bin/deno
-rwxrwxrwx 1 piscisaureus piscisaureus 49987512 Oct 29 23:50 /home/piscisaureus/.deno/bin/deno

v0.28.1:

piscisaureus@guru:~/d/denol$ ls -l target/release/deno
-rwxrwxrwx 2 piscisaureus piscisaureus 50091656 Jan 10 00:19 target/release/deno

piscisaureus added a commit that referenced this issue Jan 10, 2020
This reduces the size of the Linux binary to 54MB.

That's not quite as low as it can go, but to squeeze out those last
megabytes we'd have to make some drastic changes (use a different
linker), which is not worth the time and effort.

Closes: #3599
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants