Closed jrose-signal closed 2 years ago
Instead of making the dependency optional could this make the dependency unused unless explicitly asked for? That way elimination of dead code would mean it would get removed if you don't actually do anything that requires it. This may involve shuffling a few things around but it shouldn't be too too bad in theory.
That would be nice for compatibility! But it would require changing clients to conditionally call different methods, even when they're not particularly thinking about demangling. log-panics, for example, just Debug-prints the Backtrace object without looking at individual Symbols at all.
I understand if this change isn't appropriate for the crate! Just wanted to make it available.
Hm ok so coming back to this I would prefer to not have this in this form. I think it's reasonable to expect this so long as consumers of this crate follow a particular pattern (e.g. don't print the symbol) but otherwise once the demangled version is printed I think it's reasonable to have it pulled in. Otherwise I would prefer to keep it where it's still here by default but will get gc'd if you don't use it.
Saves on ~40KB of code size if you don't need symbol demangling. Unfortunately, this would be a breaking change (because it changes the behavior of
--no-default-features
).This came out of some size micro-optimizing in signalapp/libsignal-client's libsignal-jni package. We use log-panics, which pulls in backtrace, which pulls in addr2line, gimli, and rustc-demangle. It's possible we'll want to cut out using debug info altogether, in which case maybe we'd switch to something lighter like libunwind, but meanwhile skipping the demangling would still be a reasonable savings.
cargo bloat
unscientifically suggests rustc-demangle contributes ~40 KB, with backtrace itself at ~100KB, addr2line at ~50KB, and gimli at ~30KB. Removing rustc-demangle isn't a huge* win, but since it turned out to be this straightforward I figured I'd make the PR anyway. I'm not sure how to write automated tests for it, though.* because I'm running it on the built-for-host library, rather than the library we ship on Android.