Turns out we can work around rust's current two-level symbol visibility, which causes std library symbols to be public in .so output, but running the aggregrate crate through lto when creating the static library we link into libxul.

Passing -Clto tells the rust compiler to run the entire set of crates through the llvm optimizer. This has the practical effect of removing a bunch of otherwise exported std library symbols. It resulst in a modest increase in binary size in unoptimized builds but a significant decrease in the size of optimized builds. Review commit: https://reviewboard.mozilla.org/r/49479/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/49479/

Thanks to Alex Crichton for the pointer. Nathan, do we want this for all builds, or just optimized builds?

Comment on attachment 8746585 [details] MozReview Request: Bug 1268547 - Enable lto linking our final rust crate. r?froydnj https://reviewboard.mozilla.org/r/49479/#review46351 I wonder why the size goes up in non-optimized builds? That's kind of weird. Please fix the typos in the commit message before landing.

Sadly, this gives multiple symbol warnings linking the gtests libxul.so. Not sure why we didn't trigger this before. 15:13:09 INFO - librul.a(rul.0.o): In function `unix::notbsd::WEXITSTATUS::h661938ca194ff7cb0rb': 15:13:09 INFO - rul.0.rs:(.text._ZN4unix6notbsd11WEXITSTATUS20h661938ca194ff7cb0rbE+0x0): multiple definition of `unix::notbsd::WEXITSTATUS::h661938ca194ff7cb0rb' 15:13:09 INFO - ../librul.a(rul.0.o):rul.0.rs:(.text._ZN4unix6notbsd11WEXITSTATUS20h661938ca194ff7cb0rbE+0x0): first defined here [...and many more...] Alex, any idea what's going on?

(In reply to Nathan Froyd [:froydnj] from comment #4) > I wonder why the size goes up in non-optimized builds? That's kind of weird. This *may* be because of how we re-codegen the entire standard library. Nothing is stripped as no optimizations are performed, and I guess LLVM's codegen is worse at lower opt-levels than it is at higher opt-levels? This is surprising as well to me, though, albeit a rarely encountered use case. (In reply to Ralph Giles (:rillian) needinfo me from comment #5) > Alex, any idea what's going on? Is Gecko linking multiple crates compiled with `-C lto`? Or perhaps linking rlibs plus a crate compiled with `-C lto`? The object file in the staticlib produced will contain all upstream dependencies, so if two crates are linked you'll get duplicate symbols.

(In reply to Alex Crichton [:acrichto] from comment #6) > Is Gecko linking multiple crates compiled with `-C lto`? Or perhaps linking > rlibs plus a crate compiled with `-C lto`? Looks like this is the cause. We compile one 'librul.a' from the all the rlibs for xul, and another 'librul.a' for the gtests, and link them both into the gtest version of libxul.so. If I manually put both crates a single unified staticlib, the builds succeeds. So we need to clean up our crate generation.

This will be a single-line change once we get cargo support in bug 1231764.

(In reply to Nathan Froyd [:froydnj] from comment #8) > This will be a single-line change once we get cargo support in bug 1231764. e.g. https://treeherder.mozilla.org/#/jobs?repo=try&revision=25a7ec2bf234

Nothing complicated here. This depends on building libxul via Cargo, which just landed on inbound this morning.

Comment on attachment 8777404 [details] [diff] [review] turn on LTO support for the Rust parts of libxul Review of attachment 8777404 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/library/rust/Cargo.toml @@ +35,2 @@ > debug-assertions = false > codegen-units = 1 I mentioned this somewhere else, but the cargo docs claim codegen-units is ignored if lto is true. It might be clearer to omit codegen-units when lto is set.

(In reply to Chris Manchester (:chmanchester) from comment #11) > I mentioned this somewhere else, but the cargo docs claim codegen-units is > ignored if lto is true. It might be clearer to omit codegen-units when lto > is set. Sorry about that; I remember your previous advice, but I clearly failed to apply it to this new context. :)

Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/ee73a5429f54 turn on LTO support for the Rust parts of libxul; r=chmanchester

Nice incremental installer size win from this: == Change summary for alert #2323 (as of August 08 2016 07:35 UTC) == Summary of tests that improved: installer size summary osx-10-7 opt: 111891020.83 -> 110739836.83 (1.03% better) installer size summary linux32 opt: 56733463.33 -> 56129677.08 (1.06% better) installer size summary linux32 pgo: 60078857.58 -> 59453749.58 (1.04% better) installer size summary linux64 pgo: 59887845 -> 59308565.17 (0.97% better) For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=2323

(In reply to William Lachance (:wlach) from comment #15) > Nice incremental installer size win from this: > > == Change summary for alert #2323 (as of August 08 2016 07:35 UTC) == > > Summary of tests that improved: > > installer size summary osx-10-7 opt: 111891020.83 -> 110739836.83 (1.03% > better) > installer size summary linux32 opt: 56733463.33 -> 56129677.08 (1.06% > better) > installer size summary linux32 pgo: 60078857.58 -> 59453749.58 (1.04% > better) > installer size summary linux64 pgo: 59887845 -> 59308565.17 (0.97% better) > > For up to date results, see: > https://treeherder.mozilla.org/perf.html#/alerts?id=2323 Cool, thanks for the pointer! Do we not track Windows installer size at all?

(In reply to Nathan Froyd [:froydnj] from comment #16) > Cool, thanks for the pointer! Do we not track Windows installer size at all? We do but not seeing any wins there (afaict): https://treeherder.mozilla.org/perf.html#/graphs?series=%5Bmozilla-inbound,83688d879936c1a2bf175a6652d63f69daf217b9,1,2%5D

Do we want to uplift that?

(In reply to Sylvestre Ledru [:sylvestre] from comment #18) > Do we want to uplift that? We could, but it would require uplifting bug 1231764 as well, and possibly another Mac-specific bug or two (we upgraded the version of Rust we used on Mac builds around the same time this landed, and I don't know whether the older version would a) work with -C lto; or b) produce the same size wins). I could imagine doing it for Aurora, but doing this on Beta seems a bit risky to me.

OK, quite some work for a small gain, let it side the train :)