From f5164db51334b7c2d9b51bf1b8129d637d5e703c Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 11 Apr 2023 14:43:03 +0200 Subject: [PATCH 1/2] Compile with `panic = "abort"` This PR sets `panic = "abort"` for both debug and release builds. This cuts down the `rerun` binary size in release builds from 29.9 MB to 22.7 MB - a 25% reduction! ## Details The default panic behavior in Rust is to unwind the stack. This leads to a lot of extra code bloat, and some missed opportunities for optimization. The benefit is that one can let a thread die without crashing the whole application, and one can use `std::panic::catch_unwind` as a kind of try-catch block. We don't make use of these features at all (at least not intentionally), and so are paying a cost for something we don't need. I would also argue that a panic SHOULD lead to a hard crash unless you are building an Erlang-like robust actor system where you use defensive programming to protect against programmer errors (all panics are programmer errors - user errors should use `Result`). --- Cargo.toml | 4 +++- clippy.toml | 2 ++ crates/rerun/src/crash_handler.rs | 8 ++++++-- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 76c8577f1c6a..b543d56845f1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -89,7 +89,8 @@ wgpu-hal = { version = "0.15.4", default-features = false } [profile.dev] -opt-level = 1 # Make debug builds run faster +opt-level = 1 # Make debug builds run faster +panic = "abort" # This leads to better optimizations and smaller binaries (and is the default in Wasm anyways). # Optimize all dependencies even in debug builds (does not affect workspace packages): [profile.dev.package."*"] @@ -97,6 +98,7 @@ opt-level = 2 [profile.release] # debug = true # good for profilers +panic = "abort" # This leads to better optimizations and smaller binaries (and is the default in Wasm anyways). [profile.bench] debug = true diff --git a/clippy.toml b/clippy.toml index c72661614556..4da41d009fbc 100644 --- a/clippy.toml +++ b/clippy.toml @@ -27,6 +27,8 @@ disallowed-methods = [ "std::thread::spawn", # Use `std::thread::Builder` and name the thread "sha1::Digest::new", # SHA1 is cryptographically broken + + "std::panic::catch_unwind", # We compile with `panic = "abort"` ] # https://rust-lang.github.io/rust-clippy/master/index.html#disallowed_names diff --git a/crates/rerun/src/crash_handler.rs b/crates/rerun/src/crash_handler.rs index e73ffd62b006..d6764e5054d4 100644 --- a/crates/rerun/src/crash_handler.rs +++ b/crates/rerun/src/crash_handler.rs @@ -30,8 +30,6 @@ fn install_panic_hook(_build_info: BuildInfo) { format!("{file}:{}", location.line()) }); - // `panic_info.message` is unstable, so this is the recommended way of getting - // the panic message out. We need both the `&str` and `String` variants. let msg = panic_info_message(panic_info); if let Some(msg) = &msg { @@ -90,10 +88,16 @@ fn install_panic_hook(_build_info: BuildInfo) { std::thread::sleep(std::time::Duration::from_secs(1)); // Give analytics time to send the event } } + + // We compile with `panic = "abort"`, but we don't want to report the same problem twice, so just exit: + std::process::exit(102); })); } fn panic_info_message(panic_info: &std::panic::PanicInfo<'_>) -> Option { + // `panic_info.message` is unstable, so this is the recommended way of getting + // the panic message out. We need both the `&str` and `String` variants. + #[allow(clippy::manual_map)] if let Some(msg) = panic_info.payload().downcast_ref::<&str>() { Some((*msg).to_owned()) From f8b98d839018e836e181ec34b28fab42b1640809 Mon Sep 17 00:00:00 2001 From: Emil Ernerfeldt Date: Tue, 11 Apr 2023 15:07:27 +0200 Subject: [PATCH 2/2] Quiet clippy --- crates/rerun/src/crash_handler.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/crates/rerun/src/crash_handler.rs b/crates/rerun/src/crash_handler.rs index d6764e5054d4..b0650b3e72cb 100644 --- a/crates/rerun/src/crash_handler.rs +++ b/crates/rerun/src/crash_handler.rs @@ -90,6 +90,7 @@ fn install_panic_hook(_build_info: BuildInfo) { } // We compile with `panic = "abort"`, but we don't want to report the same problem twice, so just exit: + #[allow(clippy::exit)] std::process::exit(102); })); }