search

rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.29k stars 12.58k forks source link

Serious regression in binary size when using LTO #43054

Closed japaric closed 7 years ago

japaric commented 7 years ago

STR

$ git clone https://github.com/japaric/2wd

$ cd 2wd/firmware

$ git rev-parse HEAD
3750a542d77a37c099f9cfffe066f7369d6ec9de

$ xargo build --release

$ arm-none-eabi-size arm-none-eabi-size target/thumbv7m-none-eabi/release/firmware
   text    data     bss     dec     hex filename
  10118      24      20   10162    27b2 firmware

With version: rustc 1.20.0-nightly (067971139 2017-07-02) (nightly-2017-07-03).

This is a 200%+ increase in binary size compared to an older nightly:

$ arm-none-eabi-size arm-none-eabi-size target/thumbv7m-none-eabi/release/firmware
   text    data     bss     dec     hex filename
   3057      24      20    3101     c1d firmware

With version: rustc 1.20.0-nightly (05b579766 2017-07-01) (nightly-2017-07-02).

The number gets worst with simpler programs. I've seen increases of up to 600% (1KB -> 7KB).

The compiler seems to have lost the ability to optimize away the formatting machinery -- the program shown here doesn't do any formatting on panics. Compare the disassembly of the normal program vs the one of the "bloated" program.

PRs between the good and bad version:

cc @alexcrichton @brson @est31

est31 commented 7 years ago

PR #42938 by me looks like the most suspicious suspect. It stores the column numbers of each possible panic location in addition to the line number. Its possible that there is some additional effect that bloats up this generally very tiny increase to the reported increase.

est31 commented 7 years ago

Okay, found the culprit. It was my PR indeed. It broke the ABI of panic_fmt.

@japaric I can reproduce the sizes you reported locally. 

$ arm-none-eabi-size target/thumbv7m-none-eabi/release/firmware
   text    data     bss     dec     hex filename
  10102      24      20   10146    27a2 target/thumbv7m-none-eabi/release/firmware

If I change the function signature in lang_items.rs in cortex-m-rt to the following (one line change, addition of col param):

#[lang = "panic_fmt"]
#[linkage = "weak"]
unsafe extern "C" fn panic_fmt(
    _args: ::core::fmt::Arguments,
    _file: &'static str,
    _line: u32,
    _col: u32,
) -> ! {

Then its back to normal levels:

$ arm-none-eabi-size target/thumbv7m-none-eabi/release/firmware
   text    data     bss     dec     hex filename
   3065      24      20    3109     c25 target/thumbv7m-none-eabi/release/firmware

So, summarizing I'd say its legal breakage. Lang items are unstable so this is okay to do. Maybe we should do some messaging to alert people? Optimally rustc could simply deny compilation if the abi doesn't match.

nagisa commented 7 years ago

Duplicate of https://github.com/rust-lang/rust/issues/9307 then, I think.

japaric commented 7 years ago

@est31 Thanks for checking. 👍 Indeed changing the ABI fixes the problem.

Optimally rustc could simply deny compilation if the abi doesn't match.

I certainly would have preferred a compilation error over silent bloat.

Closing as a duplicate of #9307.