Skip to content

Commit

Permalink
make it easy to resolve symbols by frame (#526)
Browse files Browse the repository at this point in the history
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<String> = 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<String> = 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.
  • Loading branch information
fraillt committed Mar 9, 2024
1 parent da34864 commit 4dea00c
Showing 1 changed file with 53 additions and 43 deletions.
96 changes: 53 additions & 43 deletions src/capture.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,6 @@ use serde::{Deserialize, Serialize};
pub struct Backtrace {
// Frames here are listed from top-to-bottom of the stack
frames: Vec<BacktraceFrame>,
// 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() {
Expand Down Expand Up @@ -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<BacktraceSymbol> {
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.
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand All @@ -216,48 +233,28 @@ 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<Vec<BacktraceFrame>> for Backtrace {
fn from(frames: Vec<BacktraceFrame>) -> Self {
Backtrace {
frames,
actual_start_index: 0,
}
Backtrace { frames }
}
}

impl From<crate::Frame> for BacktraceFrame {
fn from(frame: crate::Frame) -> BacktraceFrame {
fn from(frame: crate::Frame) -> Self {
BacktraceFrame {
frame: Frame::Raw(frame),
symbols: None,
}
}
}

// we don't want implementing `impl From<Backtrace> for Vec<BacktraceFrame>` on purpose,
// because "... additional directions for Vec<T> can weaken type inference ..."
// more information on https://github.com/rust-lang/backtrace-rs/pull/526
impl Into<Vec<BacktraceFrame>> for Backtrace {
fn into(self) -> Vec<BacktraceFrame> {
self.frames
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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);
Expand All @@ -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()?;
Expand Down

0 comments on commit 4dea00c

Please sign in to comment.