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

Adding new crate-type target increases size of another crate-type target #55942

Closed
tiras-j opened this issue Nov 13, 2018 · 11 comments
Closed

Adding new crate-type target increases size of another crate-type target #55942

tiras-j opened this issue Nov 13, 2018 · 11 comments
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code.

Comments

@tiras-j
Copy link

tiras-j commented Nov 13, 2018

Problem
Adding an additional crate-type listing appears to affect the size of other crate types, specifically moving from crate-type = ["cdylib", "staticlib"] --> crate-type = ["cdylib", "staticlib", "rlib"] appears to increase the size of the "cdylib" output (in this case a Linux .so ELF) by 100% (2MB -> 4MB) which is suspiciously close to the size of the "rlib" output.

The motivation is to provide benchmarking (via criterion) which of course involves depending on the the core crate to build benchmark functions against. This dependency requires a Rust-style output target, not an arbitrary binary that "cdylib" generates. Adding the "rlib" target does in fact allow the benchmarking but the side-effect of the dramatically increased binary size is not viable, and this size increase appears through the standard cargo build --release process itself.

Steps
Hard to provide concise steps - but basically add an "rlib" type target for an existing "cdylib" crate that exports an some FFI? If this is a me problem, I'd love to hear how to fix this!

Notes

Output of cargo version:
cargo 1.30.0 (36d96825d 2018-10-24)

Also - rustc 1.30.0 stable as the toolchain.

@alexcrichton
Copy link
Member

The compiler internally has a range of heuristics for what symbols need to be exported and what can be internalized, and internalizing symbols can often make binaries much smaller because LLVM and/or the linker can remove a bunch of unused code.

The compiler also, however, attempts to handle generating multiple artifacts at once, attempting to reduce the amount of work done as much as possible. That means that the compiler has historically made some trade-offs where if multiple crate types are requested the output may not be as optimized as if only one crate type were requested. (it prioritizes producing correct output for each crate-type rather than optimized output for crate-type)

That being said, this may still be fixable! Specifically for code size issues and cdylib we should already pass a whitelist of symbols to export to the linker, and the presence of an rlib may accidentally make that whitelist too big or something like that.

In any case if you can gist an example project that would definitely be quite helpful! It doesn't perhaps have to be the exact same project, even something small that only differs by a few bytes may be useful for tracking down the root cause and fixing this issue upstream.

@alexcrichton alexcrichton transferred this issue from rust-lang/cargo Nov 14, 2018
@alexcrichton
Copy link
Member

I've also transferred this issue to rust-lang/rust because I believe this is largely a compiler issue rather than a Cargo issue

@alexcrichton alexcrichton added the I-heavy Issue: Problems and improvements with respect to binary size of generated code. label Nov 14, 2018
@tiras-j
Copy link
Author

tiras-j commented Nov 14, 2018

I see! Yea I was speculating it might be compiler being a bit too clever in optimizing build times by re-using some incremental steps that were much larger for the rlib target (of course that was very hand wavy speculation since I know very little about the rustc internals). I wonder if changing the order of the crate-type targets would have an impact? I'll try to throw together a simple project that demonstrates this behavior - hopefully it shouldn't be too hard.

In the mean-time do you have any thoughts on the problem I was originally trying to tackle - namely conditionally building a Rust lib (rlib or lib, just not cdylib etc) when running benchmark suites so that I can properly extern crate <mycrate> in the external benchmarks? The workaround I currently am using is running the benchmark against the cdylib by mapping the exported FFI symbols (which in reality is a better "true" benchmark in some ways). I'm doing this with a conditional rustc-link-search=target/release in an empty build script - but it requires that the release binary be pre-built (since it doesn't seem that any intermediate build from the cargo bench command generates full cdylib outputs that I can find?)

I'd be happy with either conditionally building a lib/rlib crate-type for cargo bench or having a proper link path to a binary that's built as part of the bench target that I can reliably link against instead of relying on the binary being pre-built with a cargo build --release before running the cargo bench.

@alexcrichton
Copy link
Member

The "best" way to do that today is to split the crate in two, one for the rlib and one for the staticlib/cdylib interface. The latter crate isn't benchmarked/tested (not by Cargo at least) but the rlib crate can be benchmarked/tested. This isn't a great setup, but is the most likely to work for now!

AFAIK the ordering to the compiler doesn't affect anything, but something may have snuck in over time!

@tiras-j
Copy link
Author

tiras-j commented Nov 14, 2018

I see - that should be simple enough to put together! I'll still try to throw together a quick project that demonstrates this behavior to link here just for future investigation.

Thanks for the help!

@tiras-j
Copy link
Author

tiras-j commented Nov 26, 2018

@alexcrichton I was looking at this again today and I'm almost certain it has to do with "rlib" targets interfering with LTO on the "cdylib" target. I have a tiny example project with this behavior I could share if needed (perhaps by pushing it to github and linking? Not sure the best way to share something like that).

But basically adding the "rlib" target seems to cause the DSO to be generated at the same size as a non-LTO release (might be coincidence, but it's certainly something).

@alexcrichton
Copy link
Member

@tiras-j that sounds right to me! I think the bug here is that there's something like get_all_exported_symbols somewhere in the compiler, and the presence of an rlib makes that much larger than it would otherwise be.

@tiras-j
Copy link
Author

tiras-j commented Nov 27, 2018

@alexcrichton looking at the lto.rs here definitely seems that LTO is abandoned if any target is a non-LTO target. However I'm not seeing that diag message compiling with --verbose but I also know very little about hacking on the compiler so I'm probably not looking in the right places.

@alexcrichton
Copy link
Member

@tiras-j oh right yeah, Cargo will silently turn off LTO even though you request it if it sees rlib in the crate-types. It does that because of that line in the compiler, it knows it will fail

@tiras-j
Copy link
Author

tiras-j commented Nov 28, 2018

@alexcrichton well that certainly explains what I was seeing with the binary size changes when adding an rlib target! I suppose we can close this ticket now, however it would be nice if this side-effect was made more visible to the user in some way - particularly since it's likely anyone building cdylib targets with LTO enabled will probably care quite a bit about binary size. Manifesting that diag message would likely be enough - but I'm not sure where that message is going during the compilation, as I don't see it shown even when running cargo with --verbose. Any thoughts?

@alexcrichton
Copy link
Member

Ah right this was previously opened as #51009, so yeah let's close in favor of that.

It does also seem like some sort of diagnostic here would be useful!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-heavy Issue: Problems and improvements with respect to binary size of generated code.
Projects
None yet
Development

No branches or pull requests

2 participants