From 4dea00c98ff120834c3071919320b9ed51129c39 Mon Sep 17 00:00:00 2001 From: Mindaugas Vinkelis Date: Sat, 9 Mar 2024 08:01:56 +0200 Subject: [PATCH] make it easy to resolve symbols by frame (#526) Hello, Resolving backtrace (at least for the first time) is very slow, but this is not something that can be solved (I guess). However, not all frames are equally useful, and at least in the code base that I'm working on, we simply ignore bunch of them. We have code something like this: ```rust let mut bt = Backtrace::new_unresolved(); // ... bt.resolve(); let bt: Vec = bt.frames() .iter() .flat_map(BacktraceFrame::symbols) .take_while(is_not_async_stuff) .map(format_symbols) .collect(); ``` For us, it takes around 700~1500ms to resolve whole backtrace. With this PR we're able to write like this: ```rust let bt: Vec = bt.frames_mut() .iter_mut() .flat_map(BacktraceFrame::symbols_resolved) .take_while(is_not_async_stuff) .map(format_symbols) .collect(); ``` And it takes around 25ms to resolve the frames that we actually need. I'm aware of low level utils `backtrace::trace` and `backtrace::resolve_frame`, but this would basically mean that I would basically need to reimplement Backtrace, including Serialize and Deserialize capability, which is too much. --- src/capture.rs | 96 ++++++++++++++++++++++++++++---------------------- 1 file changed, 53 insertions(+), 43 deletions(-) diff --git a/src/capture.rs b/src/capture.rs index a7ee9383..4cb39806 100644 --- a/src/capture.rs +++ b/src/capture.rs @@ -26,9 +26,6 @@ use serde::{Deserialize, Serialize}; pub struct Backtrace { // Frames here are listed from top-to-bottom of the stack frames: Vec, - // The index we believe is the actual start of the backtrace, omitting - // frames like `Backtrace::new` and `backtrace::trace`. - actual_start_index: usize, } fn _assert_send_sync() { @@ -86,6 +83,27 @@ impl Frame { } => module_base_address.map(|addr| addr as *mut c_void), } } + + /// Resolve all addresses in the frame to their symbolic names. + fn resolve_symbols(&self) -> Vec { + let mut symbols = Vec::new(); + let sym = |symbol: &Symbol| { + symbols.push(BacktraceSymbol { + name: symbol.name().map(|m| m.as_bytes().to_vec()), + addr: symbol.addr().map(|a| a as usize), + filename: symbol.filename().map(|m| m.to_owned()), + lineno: symbol.lineno(), + colno: symbol.colno(), + }); + }; + match *self { + Frame::Raw(ref f) => resolve_frame(f, sym), + Frame::Deserialized { ip, .. } => { + resolve(ip as *mut c_void, sym); + } + } + symbols + } } /// Captured version of a symbol in a backtrace. @@ -172,23 +190,22 @@ impl Backtrace { fn create(ip: usize) -> Backtrace { let mut frames = Vec::new(); - let mut actual_start_index = None; trace(|frame| { frames.push(BacktraceFrame { frame: Frame::Raw(frame.clone()), symbols: None, }); - if frame.symbol_address() as usize == ip && actual_start_index.is_none() { - actual_start_index = Some(frames.len()); + // clear inner frames, and start with call site. + if frame.symbol_address() as usize == ip { + frames.clear(); } + true }); + frames.shrink_to_fit(); - Backtrace { - frames, - actual_start_index: actual_start_index.unwrap_or(0), - } + Backtrace { frames } } /// Returns the frames from when this backtrace was captured. @@ -202,7 +219,7 @@ impl Backtrace { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn frames(&self) -> &[BacktraceFrame] { - &self.frames[self.actual_start_index..] + self.frames.as_slice() } /// If this backtrace was created from `new_unresolved` then this function @@ -216,41 +233,18 @@ impl Backtrace { /// This function requires the `std` feature of the `backtrace` crate to be /// enabled, and the `std` feature is enabled by default. pub fn resolve(&mut self) { - for frame in self.frames.iter_mut().filter(|f| f.symbols.is_none()) { - let mut symbols = Vec::new(); - { - let sym = |symbol: &Symbol| { - symbols.push(BacktraceSymbol { - name: symbol.name().map(|m| m.as_bytes().to_vec()), - addr: symbol.addr().map(|a| a as usize), - filename: symbol.filename().map(|m| m.to_owned()), - lineno: symbol.lineno(), - colno: symbol.colno(), - }); - }; - match frame.frame { - Frame::Raw(ref f) => resolve_frame(f, sym), - Frame::Deserialized { ip, .. } => { - resolve(ip as *mut c_void, sym); - } - } - } - frame.symbols = Some(symbols); - } + self.frames.iter_mut().for_each(BacktraceFrame::resolve); } } impl From> for Backtrace { fn from(frames: Vec) -> Self { - Backtrace { - frames, - actual_start_index: 0, - } + Backtrace { frames } } } impl From for BacktraceFrame { - fn from(frame: crate::Frame) -> BacktraceFrame { + fn from(frame: crate::Frame) -> Self { BacktraceFrame { frame: Frame::Raw(frame), symbols: None, @@ -258,6 +252,9 @@ impl From for BacktraceFrame { } } +// we don't want implementing `impl From for Vec` on purpose, +// because "... additional directions for Vec can weaken type inference ..." +// more information on https://github.com/rust-lang/backtrace-rs/pull/526 impl Into> for Backtrace { fn into(self) -> Vec { self.frames @@ -312,6 +309,20 @@ impl BacktraceFrame { pub fn symbols(&self) -> &[BacktraceSymbol] { self.symbols.as_ref().map(|s| &s[..]).unwrap_or(&[]) } + + /// Resolve all addresses in this frame to their symbolic names. + /// + /// If this frame has been previously resolved, this function does nothing. + /// + /// # Required features + /// + /// This function requires the `std` feature of the `backtrace` crate to be + /// enabled, and the `std` feature is enabled by default. + pub fn resolve(&mut self) { + if self.symbols.is_none() { + self.symbols = Some(self.frame.resolve_symbols()); + } + } } impl BacktraceSymbol { @@ -368,11 +379,10 @@ impl BacktraceSymbol { impl fmt::Debug for Backtrace { fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result { - let full = fmt.alternate(); - let (frames, style) = if full { - (&self.frames[..], PrintFmt::Full) + let style = if fmt.alternate() { + PrintFmt::Full } else { - (&self.frames[self.actual_start_index..], PrintFmt::Short) + PrintFmt::Short }; // When printing paths we try to strip the cwd if it exists, otherwise @@ -383,7 +393,7 @@ impl fmt::Debug for Backtrace { let mut print_path = move |fmt: &mut fmt::Formatter<'_>, path: crate::BytesOrWideString<'_>| { let path = path.into_path_buf(); - if !full { + if style == PrintFmt::Full { if let Ok(cwd) = &cwd { if let Ok(suffix) = path.strip_prefix(cwd) { return fmt::Display::fmt(&suffix.display(), fmt); @@ -395,7 +405,7 @@ impl fmt::Debug for Backtrace { let mut f = BacktraceFmt::new(fmt, style, &mut print_path); f.add_context()?; - for frame in frames { + for frame in &self.frames { f.frame().backtrace_frame(frame)?; } f.finish()?;