chore: remove debug info in dev builds

[woshilapin]

First, most of the time, we do not debug our binaries: debug information is useless, until we need it for debugging. But it affects our compile time significantly (and that is more frequent than debugging).

Therefore, I propose to remove the debug information by default, and those who needs to debug the binary, can always turn it on again when needed. Removing debug information is configured with debug option https://doc.rust-lang.org/cargo/reference/profiles.html#debug.

A small benchmark for total build time,
using hyperfine, shows a 21% improvement in compile times.

❯ hyperfine \
    --parameter-list branch 'dev,wsl/editoast/chore/no-debug-info' \
    --prepare 'git switch {branch} && cargo clean' \
    'cargo build --workspace'

Benchmark 1: cargo build --workspace (branch = dev)
  Time (mean ± σ):     246.746 s ±  7.407 s    [User: 1196.443 s, System: 60.608 s]
  Range (min … max):   227.791 s … 256.083 s    10 runs

  Warning: Statistical outliers were detected. Consider re-running this benchmark on a quiet system without any interferences from other programs. It might help to use the '--warmup' or '--prepare' options.

Benchmark 2: cargo build --workspace (branch = wsl/editoast/chore/no-debug-info)
  Time (mean ± σ):     203.227 s ±  1.618 s    [User: 939.154 s, System: 51.359 s]
  Range (min … max):   201.509 s … 206.476 s    10 runs

Summary
  cargo build --workspace (branch = wsl/editoast/chore/no-debug-info) ran
    1.21 ± 0.04 times faster than cargo build --workspace (branch = dev)

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 36.90%. Comparing base (811a2ec) to head (1462895).
Report is 6 commits behind head on dev.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@             Coverage Diff              @@
##                dev    #8579      +/-   ##
============================================
+ Coverage     36.87%   36.90%   +0.03%     
  Complexity     2175     2175              
============================================
  Files          1279     1279              
  Lines        118883   118972      +89     
  Branches       3188     3188              
============================================
+ Hits          43836    43910      +74     
- Misses        73162    73177      +15     
  Partials       1885     1885              

Flag	Coverage Δ
core	74.87% <ø> (ø)
editoast	66.15% <ø> (+0.04%)	⬆️
front	15.84% <ø> (ø)
gateway	2.20% <ø> (ø)
osrdyne	2.71% <ø> (ø)
railjson_generator	87.49% <ø> (ø)
tests	72.93% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[emersion]

Hm, I'm a bit sad to see the stack trace go away in case of panics… Would debug = 1 be a good enough middle ground? (The default is debug = 2.)

[leovalais]

Great ! 🚀

On my machine (macOS, M1 Pro, 16Gb), I get a 16% improvement :)

Benchmark 1: cargo build --workspace (branch = dev)
  Time (mean ± σ):     117.344 s ±  0.804 s    [User: 496.570 s, System: 57.728 s]
  Range (min … max):   116.027 s … 118.518 s    10 runs

Benchmark 2: cargo build --workspace (branch = wsl/editoast/chore/no-debug-info)
  Time (mean ± σ):     100.904 s ±  0.705 s    [User: 417.180 s, System: 54.106 s]
  Range (min … max):   99.896 s … 102.012 s    10 runs

Summary
  cargo build --workspace (branch = wsl/editoast/chore/no-debug-info) ran
    1.16 ± 0.01 times faster than cargo build --workspace (branch = dev)

[leovalais]

@emersion I agree, the stack traces on panic are indeed useful. From https://doc.rust-lang.org/cargo/reference/profiles.html#debug:

debug = 

...
* "line-tables-only": line tables only. Generates the minimal amount of debug info for backtraces with filename/line number info, but not anything else, i.e. no variable or function parameter info.

I'll re-run the benchmark this afternoon with this setting (now it's lunch time 😁).

[leovalais]

Only 10% improvement with backtrace info:

Benchmark 1: cargo build --workspace (branch = dev)
  Time (mean ± σ):     116.566 s ±  0.666 s    [User: 487.292 s, System: 56.142 s]
  Range (min … max):   115.912 s … 117.970 s    10 runs

Benchmark 2: cargo build --workspace (branch = wsl/editoast/chore/no-debug-info)
  Time (mean ± σ):     105.710 s ±  0.713 s    [User: 429.094 s, System: 51.476 s]
  Range (min … max):   104.315 s … 106.561 s    10 runs

Summary
  cargo build --workspace (branch = wsl/editoast/chore/no-debug-info) ran
    1.10 ± 0.01 times faster than cargo build --workspace (branch = dev)

[woshilapin]

Just so we know what the impact is, here is a simple program tested with all the debug levels (with RUST_BACKTRACE=1).

The program is simply.

fn crash() {
    panic!("Hello, world!");
}
fn main() {
    crash()
}

With default debug level full.

thread 'main' panicked at panic-always/src/main.rs:2:5:
Hello, world!
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: my_panic_always_project::crash
             at ./src/main.rs:2:5
   3: my_panic_always_project::main
             at ./src/main.rs:5:5
   4: core::ops::function::FnOnce::call_once
             at /tmp/guix-build-rust-1.79.0.drv-0/rustc-1.79.0-src/library/core/src/ops/function.rs:250:5

With debug level limited

thread 'main' panicked at panic-always/src/main.rs:2:5:
Hello, world!
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: my_panic_always_project::crash
             at ./src/main.rs:2:5
   3: my_panic_always_project::main
             at ./src/main.rs:5:5
   4: core::ops::function::FnOnce::call_once
             at /tmp/guix-build-rust-1.79.0.drv-0/rustc-1.79.0-src/library/core/src/ops/function.rs:250:5

With debug level none

thread 'main' panicked at panic-always/src/main.rs:2:5:
Hello, world!
stack backtrace:
   0: rust_begin_unwind
   1: core::panicking::panic_fmt
   2: my_panic_always_project::crash
   3: my_panic_always_project::main
   4: core::ops::function::FnOnce::call_once

So for producing stacktrace, limited is essentially identical to full. But I'd argue that maybe none is still enough since the full path of the function is indicated so it's straight-forward to get back to the file and lines?

[emersion]

Good to see that "none" isn't literally "nothing at all good luck out there", ie. still includes function names at least.

But I'd argue that maybe none is still enough since the full path of the function is indicated so it's straight-forward to get back to the file and lines?

It depends. If there are multiple ways that a function could panic (e.g. multiple unwrap calls) then there is no way to tell which part is causing the panic.

It seems like there is no way to override the debug setting on the CLI. If we go forward with this, I'd recommend defining a new custom profile for dev with debug info enabled so that it's easy to compile with debug info without mutating files checked in in Git, e.g.

[profile.dev-with-debug]
inherits = "dev"
debug = true

[leovalais]

+1 on the other dev profile.

Unless I'm reading it wrong "none" still includes the line of the panic thread 'main' panicked at panic-always/src/main.rs:2:5:.
However, loosing the call site in previous calls could impact us in situations like:

fn panik(yes: bool) {
    if yes { panic!("oh"); }
}
fn main() {
    panik(false); // did the panic originate from here
    panik(true); // or here?
}

I'd suggest going for "none" since compilation times are quite impactful on the editoast dx and revert back to "limited" if we end up switching profiles a lot because of backtraces. Wdyt?

[woshilapin]

Thank you for all the feedback and suggestions. I created a new profile for debug information. I also added some documentation about that profile. Tell me if you like it.

[emersion]

Nice!

[emersion]

(Alternatively, we could only disable debug info when building inside Docker.)

[woshilapin]

In the end, I put limited thinking that most of our tests contains multiple panicking points (with the assert!()) and that a failing test will show a backtrace. I believe none would really impact too seriously the developer experience.

[flomonster]

🚀