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

Big file size regression on Windows #99143

Closed
BlaCoiso opened this issue Jul 11, 2022 · 16 comments · Fixed by #99252
Closed

Big file size regression on Windows #99143

BlaCoiso opened this issue Jul 11, 2022 · 16 comments · Fixed by #99252
Labels
C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.

Comments

@BlaCoiso
Copy link
Contributor

Binary sizes on the latest nightlies are significantly larger than on stable and some previous nightly versions. This is caused by some extra sections in the binary and also an increase in the text and data sections
Stable (1.62.0):

nth paddr          size vaddr          vsize perm name
――――――――――――――――――――――――――――――――――――――――――――――――――――――
0   0x00000400  0x19e00 0x140001000  0x1a000 -r-x .text
1   0x0001a200   0x7600 0x14001b000   0x8000 -r-- .rdata
2   0x00021800    0x200 0x140023000   0x1000 -rw- .data
3   0x00021a00   0x1200 0x140024000   0x2000 -r-- .pdata
4   0x00022c00    0x400 0x140026000   0x1000 -r-- .reloc

Nightly (1.64.0-nightly c396bb3):

nth paddr          size vaddr          vsize perm name
――――――――――――――――――――――――――――――――――――――――――――――――――――――
0   0x00000400  0x7d200 0x140001000  0x7e000 -r-x .text
1   0x0007d600  0x29c00 0x14007f000  0x2a000 -r-- .rdata
2   0x000a7200    0x200 0x1400a9000   0x1000 -rw- .data
3   0x000a7400   0x5c00 0x1400aa000   0x6000 -r-- .pdata
4   0x000ad000    0xc00 0x1400b0000   0x1000 -r-- .debug_a
5   0x000adc00  0x9d000 0x1400b1000  0x9d000 -r-- .debug_i
6   0x0014ac00   0x9000 0x14014e000   0x9000 -r-- .debug_a_1
7   0x00153c00  0x68c00 0x140157000  0x69000 -r-- .debug_r
8   0x001bc800  0xe8e00 0x1401c0000  0xe9000 -r-- .debug_s
9   0x002a5600  0x5a000 0x1402a9000  0x5a000 -r-- .debug_p
10  0x002ff600    0x200 0x140303000   0x1000 -r-- .debug_p_1
11  0x002ff800  0x3a200 0x140304000  0x3b000 -r-- .debug_l
12  0x00339a00  0x21400 0x14033f000  0x22000 -r-- .reloc

Code

I tried this code:

fn main() {}

I expected to see this happen: small binary size (143 360 bytes on 1.62.0)

Instead, this happened: extremely large binary size (3 517 952 bytes)

Version it worked on

It most recently worked on: Rust 1.64.0 (nightly) ddcbba0

Version with regression

rustc --version --verbose:

rustc 1.64.0-nightly (c396bb3b8 2022-07-10)
binary: rustc
commit-hash: c396bb3b8a16b1f2762b7c6078dc3e023f6a2493
commit-date: 2022-07-10
host: x86_64-pc-windows-msvc
release: 1.64.0-nightly
LLVM version: 14.0.6
@BlaCoiso BlaCoiso added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Jul 11, 2022
@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 Jul 11, 2022
@ehuss
Copy link
Contributor

ehuss commented Jul 11, 2022

This seems to be due to #98350, cc @pcwalton.

@eddyb
Copy link
Member

eddyb commented Jul 12, 2022

How was this built? cargo build (i.e. without --release) would enable debuginfo, but plain rustc hello.rs wouldn't.

My best guess (and why I'm asking the above) is that you already have a .pdb next to the .exe, but #98350 caused you to also have DWARF debuginfo in the .exe (and that part is the regression).

EDIT: turns out it's the standard library, I forgot we compile it w/ debuginfo until #99143 (comment)

@BlaCoiso
Copy link
Contributor Author

BlaCoiso commented Jul 12, 2022

How was this built? cargo build (i.e. without --release) would enable debuginfo, but plain rustc hello.rs wouldn't.

This was built with rustc +nightly main.rs -C opt-level=3 -C target-cpu=native, although the same thing happens with cargo +nightly build --release

@BlaCoiso
Copy link
Contributor Author

Just checked, this regressed between nightly-2022-07-09 and nightly-2022-07-10

@eddyb

This comment was marked as outdated.

@ehuss
Copy link
Contributor

ehuss commented Jul 12, 2022

I already ran cargo-bisect-rustc, that's how I came to the conclusion in #99143 (comment) that it was #98350. Sorry if that wasn't clear.

@eddyb
Copy link
Member

eddyb commented Jul 12, 2022

My bad - also, I was probably wasting everyone's time by not trying to borrow someone else's windows laptop.

So what's really going on is that even at -C debuginfo=0 we link debuginfo from the standard library - I bet that's why there's a .pdb at all in the first place! (which should've clued me in)

> ls $HOME/.rustup/toolchains/nightly-2022-07-09-x86_64-pc-windows-msvc/lib/rustlib/x86_64-pc-windows-msvc/lib/libstd-*.rlib
-a----         7/12/2022   9:52 PM       17610844 libstd-3aa3a0da9afb34c9.rlib

> ls $HOME/.rustup/toolchains/nightly-2022-07-10-x86_64-pc-windows-msvc/lib/rustlib/x86_64-pc-windows-msvc/lib/libstd-*.rlib
-a----         7/12/2022   9:55 PM       19936364 libstd-3aa3a0da9afb34c9.rlib

So yeah, it's #98350 via core/alloc/std being always compiled with debuginfo and statically linked into every binary (and -C prefer-dynamic does seem to avoid this, as one might expect).

@apiraino
Copy link
Contributor

apiraino commented Jul 13, 2022

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium +I-heavy

@rustbot rustbot added I-heavy Issue: Problems and improvements with respect to binary size of generated code. P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jul 13, 2022
@flip1995
Copy link
Member

Regarding prioritization:

This kills the Clippy Windows CI, because it is running out of disk space: rust-lang/rust-clippy#9174 It increases the size of clippy_driver from 17MB to 1GB: rust-lang/rust-clippy#9174 (comment)

I don't think Clippy is/will be the only project affected by this.

@eddyb
Copy link
Member

eddyb commented Jul 14, 2022

Btw as per #98350 (comment) a quick fix would be to make the (unconditional after #98350) LLVMRustAddModuleFlag call for Dwarf Version be conditional on !is_like_msvc (basically the opposite of the Code View one).

This should be perfectly backwards compatible as all MSVC targets had None in their target spec, for the DWARF version, before #98350 (so they never got the LLVM Dwarf Version set).

If someone can make a PR and test it on windows, I'll r+ p=10 it just to get it out sooner.

@lqd
Copy link
Member

lqd commented Jul 14, 2022

I have access to a windows machine and can do that, but I wonder about the debuginfo sizes:

$ cargo clean && cargo +stable build -q && ls -lh target/debug/helloworld.*
-rw-r--r-- 1 lqd 197121  117 Jul 14 19:40 target/debug/helloworld.d
-rwxr-xr-x 2 lqd 197121 156K Jul 14 19:40 target/debug/helloworld.exe*
-rw-r--r-- 2 lqd 197121 1.2M Jul 14 19:40 target/debug/helloworld.pdb

$ cargo clean && cargo +nightly-2022-07-08 build -q && ls -lh target/debug/helloworld.*
-rw-r--r-- 1 lqd 197121  117 Jul 14 19:41 target/debug/helloworld.d
-rwxr-xr-x 2 lqd 197121 157K Jul 14 19:41 target/debug/helloworld.exe*
-rw-r--r-- 2 lqd 197121 1.3M Jul 14 19:41 target/debug/helloworld.pdb

$ cargo clean && cargo +nightly build -q && ls -lh target/debug/helloworld.*
-rw-r--r-- 1 lqd 197121  117 Jul 14 19:41 target/debug/helloworld.d
-rwxr-xr-x 2 lqd 197121 3.4M Jul 14 19:41 target/debug/helloworld.exe*
-rw-r--r-- 2 lqd 197121 2.7M Jul 14 19:41 target/debug/helloworld.pdb

$ cargo clean && cargo +stage1 build -q && ls -lh target/debug/helloworld.*
-rw-r--r-- 1 lqd 197121  117 Jul 14 19:41 target/debug/helloworld.d
-rwxr-xr-x 2 lqd 197121 167K Jul 14 19:41 target/debug/helloworld.exe*
-rw-r--r-- 2 lqd 197121 596K Jul 14 19:41 target/debug/helloworld.pdb

stage1 is my local build of your suggestion. I wonder if #98350 also somehow impacted the stdlib debuginfo or what's in that pdb: it would be half the size it was before the PR landed, on nightly-2022-07-08 or stable.

I can open the PR if it unblocks clippy and possibly others. We could also revert #98350 if we don't want to risk removing those 600K of debuginfo.

Actually, cc windows and debugging experts @wesleywiser @michaelwoerister for an opinion on the above. edit2: Sorry for the useless ping, that was indeed it. But maybe it could be useful anyways: for the debugging initiative, it could be interesting to have tests that would have caught this regression.

(edit1: maybe I did something silly in this test above in that maybe debuginfo-std-level is not the same as a nightly)

@eddyb
Copy link
Member

eddyb commented Jul 14, 2022

Since you're doing a local build, that will be highly impacted by debuginfo-level-std AFAICT.

rust/src/ci/run.sh

Lines 72 to 75 in 24699bc

if [ "$DEPLOY$DEPLOY_ALT" = "1" ]; then
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --enable-llvm-static-stdcpp"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --set rust.remap-debuginfo"
RUST_CONFIGURE_ARGS="$RUST_CONFIGURE_ARGS --debuginfo-level-std=1"

Okay yeah looks like dist builds always set debuginfo-level-std=1.

@lqd
Copy link
Member

lqd commented Jul 14, 2022

Indeed, forcing this setting as @eddyb mentions, and doing the same build as #99143 (comment) instead of a debug build, it's looking good now.

I'll open a PR.

@eddyb
Copy link
Member

eddyb commented Jul 14, 2022

cc @rust-lang/wg-debugging Before this is concluded, I wanted to bring up that we could've likely avoided this bug if we had "debuginfo tests" (in the sense of introspecting the debuginfo generated, without involved a debugger) and they were always looking for both:

  • DWARF (either embedded or split), via llvm-dwarfdump
  • CodeView in PDBs, llvm-pdbutil's dump/pretty/pdb2yaml subcommands
    • speaking of which, we should ship llvm-pdbutil with CI LLVM and/or the rustup llvm-tools component, I only happen to have it in a local LLVM build

(The only test that even uses llvm-dwarfdump seems to be run-make/remap-path-prefix-dwarf, and it only checks that certain strings are contained and others aren't, in a very coarse way)

Because what would've then happened in #98350 is any CodeView-only test would start also having DWARF being found by llvm-dwarfdump, and fail because no DWARF was expected.


One complication though, and you might already have guessed it by the section names in the original issue description, but @lqd just tried to use llvm-dwarfdump on the .exes w/ DWARF sections and couldn't get anything out of it other than --show-section-sizes (which presumably just looks for anything starting with .debug_).

We believe PE is causing the symbol names to be truncated, as per MS docs:

An 8-byte, null-padded UTF-8 encoded string. If the string is exactly 8 characters long, there is no terminating null. For longer names, this field contains a slash (/) that is followed by an ASCII representation of a decimal number that is an offset into the string table. Executable images do not use a string table and do not support section names longer than 8 characters. Long names in object files are truncated if they are emitted to an executable file.

However, I don't believe we would ever actually rely on dumping debuginfo from a final executable (on any platform), because that statically links std so unless we can rely on -C prefer-dynamic to bypass that, the amount of debuginfo would be too big to reasonably handle in a test.

Thankfully, @lqd was able to confirm that llvm-dwarfdump on the MSVC nightly's libstd-*.rlib does show the DWARF data that shouldn't be there (and the section names are untruncated), so the (much likelier) approach of testing .rlibs (or even individual .os?) would've detected this.

@eddyb
Copy link
Member

eddyb commented Jul 14, 2022

Out of curiosity, I also checked the MinGW target (-windows-gnu) which does use DWARF:

(click to expand llvm-dwarfdump output for -windows-gnu rustc.exe/rustc_driver-*.dll)
$ rustup toolchain install nightly-x86_64-pc-windows-gnu
error: DEPRECATED: future versions of rustup will require --force-non-host to install a non-host toolchain as the default.
warning: toolchain 'nightly-x86_64-pc-windows-gnu' may not be able to run on this system.
warning: If you meant to build software to target that platform, perhaps try `rustup target add x86_64-pc-windows-gnu` instead?
info: syncing channel updates for 'nightly-x86_64-pc-windows-gnu'
info: latest update on 2022-07-14, rust version 1.64.0-nightly (87588a2af 2022-07-13)
info: downloading component 'cargo'
info: downloading component 'clippy'
info: downloading component 'rust-docs'
info: downloading component 'rust-mingw'
info: downloading component 'rust-std'
info: downloading component 'rustc'
info: downloading component 'rustfmt'
info: installing component 'cargo'
info: installing component 'clippy'
info: installing component 'rust-docs'
 18.7 MiB /  18.7 MiB (100 %)   4.4 MiB/s in  5s ETA:  0s
info: installing component 'rust-mingw'
info: installing component 'rust-std'
 28.1 MiB /  28.1 MiB (100 %)   8.0 MiB/s in  3s ETA:  0s
info: installing component 'rustc'
 69.5 MiB /  69.5 MiB (100 %)   7.4 MiB/s in  9s ETA:  0s
info: installing component 'rustfmt'

  nightly-x86_64-pc-windows-gnu installed - (rustc does not exist)

$ llvm-dwarfdump --show-section-sizes ~/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/bin/rustc.exe
----------------------------------------------------
file: /home/eddy/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/bin/rustc.exe
----------------------------------------------------
SECTION         SIZE (b)
--------------  --------
.debug_aranges        80 (0.15%)
.debug_info         7657 (14.01%)
.debug_abbrev        303 (0.55%)
.debug_line          546 (1.00%)
.debug_frame          72 (0.13%)

 Total Size: 8658  (15.84%)
 Total File Size: 54656
----------------------------------------------------
$ llvm-dwarfdump --show-section-sizes ~/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/bin/rustc_driver-*.dll 
----------------------------------------------------
file: /home/eddy/.rustup/toolchains/nightly-x86_64-pc-windows-gnu/bin/rustc_driver-3ff33b521bc8cb0c.dll
----------------------------------------------------
SECTION          SIZE (b)
---------------  --------
.debug_aranges        128 (0.00%)
.debug_pubnames    363860 (0.16%)
.debug_pubtypes        18 (0.00%)
.debug_info        198952 (0.09%)
.debug_abbrev         789 (0.00%)
.debug_line         92971 (0.04%)
.debug_frame           48 (0.00%)
.debug_str         556999 (0.24%)
.debug_loc            329 (0.00%)
.debug_ranges      255808 (0.11%)

 Total Size: 1469902  (0.64%)
 Total File Size: 230200250
----------------------------------------------------

So they have untrucated section names - a quick check in a hex viewer confirms that it is the COFF format described in the docs (/ + an ASCII decimal encoding of a string table offset):

image

The docs also said that it's not meant for (PE) "images" (.exe/.dll), and MSVC does truncate section names, but maybe most tools that don't need to know about those sections will just see the weird name that starts with / and simply assume that it's the actual name, and nothing will break?

@bors bors closed this as completed in c2f428d Jul 14, 2022
@apiraino
Copy link
Contributor

This kills the Clippy Windows CI, because it is running out of disk space

thanks for the ping @flip1995 I have re-read this issue and you're right, this is much more impacting than I (mistakenly) understood. Even if the issue should be now solved I'll bump up the prioritization for historical records to P-critical to signal that it must have not progressed in the release cycle.

@rustbot label P-critical -P-medium

@apiraino apiraino added P-critical Critical priority and removed P-medium Medium priority labels Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-heavy Issue: Problems and improvements with respect to binary size of generated code. P-critical Critical priority regression-from-stable-to-nightly Performance or correctness regression from stable to nightly.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants