Enable strip in release mode by default

[SuperCuber]

I'd expect build --release to apply all optimisations, and one of them would be file size.

I tried two programs, a trivial "hello rust" and a trivial "hello panic". Both of their sizes were 3.8M which is pretty crazy for something that is supposed to be optimised... So I tried strip --strip-alling the executables.

Hello rust went to 372K Hello panic went to 364K

That's a pretty huge change... The only difference in functionality I found is that RUST_BACKTRACE=1 shows a lot of <unknown>s on striped files which is to be expected, but I think that's not a real issue because the whole point of --release is that it's not supposed to be debugged, there is the normal build for that...

For now I will be strip --strip-alling all of my released executables but I really don't see a reason why build --release doesn't do it. I'd be happy to be enlightened if I missed anything though!

[ghost]

I think this should only change if panic = 'abort' to avoid the backtrace getting wiped.

[SuperCuber]

@cedenday I don't see an issue with backtraces being wiped in --release if it's used in the way it needs to be used though. Also it gives the basic information of where the panic was (file and line number) after striping so it should give enough information to rebuild the project in debug mode and reproduce + backtrace the issue.

[alexcrichton]

Thanks for the report! I think this is mostly due to the addition of debuginfo into the standard library a few versions ago, so the lion's share of strip is stripping out that debugging information. This does make sense to me to do by default! I'm not sure what the best place for that would be, though. We probably can't literally run strip because it's not very cross-platform so this would probably go into rustc assuming LLVM can handle it, but that'd likely involve a good amount of work.

[mati865]

@alexcrichton maybe LLVM's opt --strip-debug could be handy? https://llvm.org/docs/CommandGuide/opt.html

For ld/lld --strip-all can be used and it gives the best results. In my tests on Arch Linux (fairly recent toolchains) RUSTFLAGS="-C link-arg=-s" cargo build --release gave ~3% better results than running strip -s on release binary.

[matklad]

@mati865 we have a nightly rustc flag for that: https://github.com/rust-lang/rust/pull/49212. I think the idea is, eventually, to enable stripping for --release by default and flip the flag's meaning to disable strip.

[mati865]

@matklad that's nice but it would be even better if there was option to use --strip-all (-s) instead of --strip-debug (-S).

[jcbritobr]

Hello. When this will be solved?

[kornelski]

cc #3483

[LifeIsStrange]

Maybe useful : https://www.phoronix.com/scan.php?page=news_item&px=LLVM-Strip

[garrett-hopper]

Should this be closed now that cargo-features = ["strip"] works on nightly?

See: https://github.com/rust-lang/cargo/issues/3483#issuecomment-631395566

[jcbritobr]

Should this be closed now that cargo-features = ["strip"] works on nightly?

See: #3483 (comment)

I think not. --release flag is explicit.

[ckcr4lyf]

FYI: With Rust 1.59.0, this can be specified in Cargo.toml as:

[profile.release]
strip = true

and this will strip them in the compile process, rather than needing to explicitly call strip afterwards.

Ref: https://blog.rust-lang.org/2022/02/24/Rust-1.59.0.html#creating-stripped-binaries

[edmorley]

Should this issue be renamed to be Enable strip in release mode by default, or a new issue filed for that and this one closed out? :-)

[jcbritobr]

Should this issue be renamed to be Enable strip in release mode by default, or a new issue filed for that and this one closed out? :-)

seems acceptable for me.

[SuperCuber]

Gotchu :)

[epage]

Note that there are plans to shrink the amount of debug info by default, see #11958 for some talk of this.

[Kobzol]

I would like to propose to change Cargo defaults so that debug = 0 implies strip = "debuginfo". This has been discussed on Zulip several times, most recently here.

Motivation

Currently, when debug = 0 is used in a Cargo profile, Cargo promises that there will be no debuginfo in the resulting binary. However, this is not true, because the standard library contains debuginfo, and this debuginfo is propagated into the resulting binary by default, unless it is stripped.

This means that every binary built by Cargo with its default profile options will contain the stdlib debug symbols, which take about 3-4 MiB on x64 Linux. So, today, no Rust (Linux) binary built with the default profile options can be smaller than 4 MiB! Of course, the symbols can be removed via stripping, but not everyone knows about this.

Notably, when someone new to Rust tries to build a helloworld binary in release mode and then finds out that it takes more than 4 MiB, they might be discouraged and consider Rust binaries to be "bloated".

An example using the latest (1.74.1) stable rustc:

Debug symbols stripped	Helloworld release size
Yes	446 KiB
No	4,5 MiB

That's a big difference! Defaults do matter.

What exactly is being proposed

When a Cargo profile should not produce debuginfo (debug is set to 0), and the strip configuration option is not explicitly selected, we should set strip to "debuginfo", to make sure that here will indeed be no debug symbols. Notably, this will have an effect on the default release Cargo profile.

cargo pseudocode:

profile.strip = profile.strip.or_else(|| if profile.debug == "0" { Some("debuginfo") } else { None });

Here is a proposed implementation.

Note that this would also apply to proc macros and build scripts, not just binaries (unless we explicitly do this only for binaries).

Advantages

• Rust release binaries will be smaller by ~3-4 MiB by default.
• We will uphold our promise from the Cargo documentation and actually avoid producing debug symbols when debug is set to 0.
• On Linux, this actually makes compilation slightly faster, since the linker has less work to do. Although this is relevant only for really small crates.

Disadvantages

• Stack traces of release/debug = 0 builds will contain less information (notably, it will not contain stdlib line numbers). Here is an example of the difference. However, I think that this is not an issue:
  • When the user requests that there will be no debuginfo, it should be the expected behavior… that there will be no debuginfo. The fact that there were (some) debug symbols before was basically just an oversight caused by the way we distribute the standard library. If you want debug symbols, you should just set debug = 1 (or some other value that produces debug symbols).
  • It's unlikely that seeing a bunch of line numbers in stdlib (usually in panic/Result/etc. code) is helpful on its own, without seeing line numbers from the actual application and its dependencies.
• On macOS, debug symbols are stripped not via the linker, but by invoking a separate strip binary. This can have some performance overhead, but it seems that it might not be large - one experiment showed around a 1% slowdown when compiling cargo itself. ehuss has also mentioned some problems with finding the strip binary on macOS.

Unresolved questions/concerns

• Should we do this on all platforms by default, or do it in a platform specific way? For example, we might not do it by default on macOS due to the mentioned issues. It depends whether we see this as a performance optimization (smaller binaries by default and slightly faster compilation on Linux) or as a "correctness fix" (cargo should do what it promises in its documentation, and which it doesn't do currently), which might warran a small perf. hit on some platforms.
• (Note that this is not strictly related to this proposed change, but more to the behavior of the strip option). When at least one dependency wants to produce debug symbols using Cargo overrides, we should probably either not apply strip = "debuginfo" by default, or we should make sure that strip will not actually strip everything when this situation happens. However, it seems to me that overriding debuginfo with overrides currently does not even work. When I tried this:

[dependencies]
serde_json = "1.0.108"

[profile.release]
debug = 1

[profile.release.package.serde_json]
debug = 0

then debug symbols were generated for serde_json. Conversely, when I set debug = 0 for release and debug = 1 for serde_json, then debug symbols were not generated for serde_json. So it seems that overrides for debug symbols don't currently work, and therefore this concern seems orthogonal to this proposal.

[weihanglo]

Thank you, @Kobzol. Going to kick off an FCP.

@rfcbot fcp merge

Propose to accept this proposal https://github.com/rust-lang/cargo/issues/4122#issuecomment-1868318860 for all platforms. For concerns on macOS, we had issues like https://github.com/rust-lang/cargo/issues/11641, which shows the symptom of incompatible strip. To minimize a potential maintenance burden, we could document a troubleshooting process somewhere to let people help themselves.

[rfcbot]

Team member @weihanglo has proposed to merge this. The next step is review by the rest of the tagged team members:

• [ ] @Eh2406
• [x] @Muscraft
• [x] @arlosi
• [x] @ehuss
• [x] @epage
• [x] @joshtriplett
• [x] @weihanglo

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[ehuss]

However, it seems to me that overriding debuginfo with overrides currently does not even work.

What did you use to determine if debuginfo was generated? I tested, and saw that the rlib was generated without debuginfo.

Here is a proposed implementation.

Just FYI, this implementation does not look correct to me. That will only override the value if the user has manually specified a [profile] table in Cargo.toml. I would expect it to take effect if the user did not specify a profile table.

[Kobzol]

What did you use to determine if debuginfo was generated? I tested, and saw that the rlib was generated without debuginfo.

I haven't checked the rlibs, but the final binary contained debug sections, and there were line numbers in stack traces.

// main.rs
use serde::Serialize;
use serde_json::Serializer;

struct S {
    s: u32
}

impl Serialize for S {
    fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error> where S: serde::Serializer {
        panic!("Foo")
    }
}

fn main() {
    println!("{}", serde_json::to_string(&S { s: 0 }).unwrap());
}

// Cargo.toml
[package]
name = "debuginfo"
version = "0.1.0"
edition = "2018"

[dependencies]
serde_json = "1.0.108"
serde = "1.0.193"

[profile.release]
debug = X

[profile.release.package.serde_json]
debug = Y

Here are the results for various values of X and Y (running cargo build/run --release):

X	Y	Binary size	serde_json line numbers in stack trace
0	0	4535096	No
0	1	4535096	No
0	Unset	4535096	No
1	0	4543453	Yes
1	1	4543453	Yes
1	Unset	4543453	Yes

It seems to me that the override doesn't really do anything, at least for the final binary (not sure about the intermediate rlibs).

Edit: yeah, it affects the rlibs, but not the final binary.

Just FYI, this implementation does not look correct to me. That will only override the value if the user has manually specified a [profile] table in Cargo.toml. I would expect it to take effect if the user did not specify a profile table.

Yeah, it was just something that I cooked up in 5 minutes :sweat_smile: I'm sure that the actual implementation will be a bit different, but I wanted to provide some "proof-of-concept" code to make it clear what I'm proposing.

[ehuss]

You have to be careful about whether you are using something that involves generics or #[inline]. Generics will get instantiated in the crate where they are used (not defined), and thus will use the codegen options from the instantiating crate.

I would recommend instead of using complex dependencies like serde_json to instead just use local test crates. For example:

// foo
fn main() { bar::zzz(); }

// bar
pub fn zzz() { panic!("test from bar"); }

Doing that, you should be able to see the expected changes in behavior.

[Kobzol]

You have to be careful about whether you are using something that involves generics or #[inline]. Generics will get instantiated in the crate where they are used (not defined), and thus will use the codegen options from the instantiating crate.

Ah, good point, I didn't realize that all the used code from serde_json is generic and/or inlined, I expected that something would be codegenned in the library itself. With the simple example that you have posted it indeed works as expected.

When strip = "debuginfo" is used, it strips debuginfo from the library too. I think that's expected behavior. However, when strip is unset, we should probably detect if any dependency has debug symbols enabled, and in that case don't apply the proposed strip = "debuginfo" default, to preserve the manually overridden debug symbols (and thus also keep libstd debug info, since we can't really strip in a granular way, AFAIK).

[rfcbot]

:bell: This is now entering its final comment period, as per the review above. :bell:

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[Kobzol]

Implementation PR: https://github.com/rust-lang/cargo/pull/13257.

[retep998]

I consider it to be a process failure that this was proposed and implemented without a single consideration or mention of Windows. That something so dependent on toolchain specifics could go through without even considering all tier1 platforms is embarrassing.

[Kobzol]

The problem with backtraces wasn't caused by this change, it only exposed a pre-existing issue, which has been existing in Rust for a long time (4 years, https://github.com/rust-lang/rust/commit/a6c2f73b6e0d24af5396355886b26bb23885c37e). It's just that no one has noticed it until now, since it started appearing more often. That probably means that not many people use msvc targets on Windows combined with debuginfo stripping.

That something so dependent on toolchain specifics could go through without even considering all tier1 platforms is embarrassing.

I think that's unnecessarily strong wording :) We have a lot of tests for tier 1 platforms, of course, but we didn't have a test for this specific situation.

[retep998]

The problem with backtraces wasn't caused by this change, it only exposed a pre-existing issue, which has been existing in Rust for a long time (4 years, rust-lang/rust@a6c2f73). It's just that no one has noticed it until now, since it started appearing more often. That probably means that not many people use msvc targets on Windows combined with debuginfo stripping.

Nobody on Windows MSVC should ever have a reason to consider stripping as binary files never contain debuginfo to begin with. That PR from 4 years ago was definitely wrong to have stripping affect PDB file generation given we previously made the decision to always generate PDB files regardless of whether debuginfo is enabled.

I think that's unnecessarily strong wording :) We have a lot of tests for tier 1 platforms, of course, but we didn't have a test for this specific situation.

Then we should definitely add tests to ensure backtraces work on Windows MSVC in cargo release builds.

[Kobzol]

Yeah, we should add tests to https://github.com/rust-lang/rust/pull/115120.