Skip to content

Commit

Permalink
Fix Python 3.11 native sampling (#635)
Browse files Browse the repository at this point in the history
In Python 3.11, merging native frames works a bit differently. We have to detect if a frame is an "entry frame". Once we hit a native frame that corresponds to python, we need to keep merging python frames until we hit the entry frame.

Co-authored-by: Ben Frederickson <[email protected]>
Co-authored-by: Ben Frederickson <[email protected]>
  • Loading branch information
3 people authored Nov 17, 2023
1 parent 9348e93 commit 2f8cfdd
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,7 @@ fn record_samples(pid: remoteprocess::Pid, config: &Config) -> Result<(), Error>
short_filename: None,
line: 0,
locals: None,
is_entry: true,
});
}

Expand Down
13 changes: 12 additions & 1 deletion src/native_stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,16 @@ impl NativeStack {
// if we have a corresponding python frame for the evalframe
// merge it into the stack. (if we're out of bounds a later
// check will pick up - and report overall totals mismatch)
if python_frame_index < frames.len() {

// Merge all python frames until we hit one with `is_entry`.
while python_frame_index < frames.len() {
merged.push(frames[python_frame_index].clone());

if frames[python_frame_index].is_entry {
break;
}

python_frame_index += 1;
}
python_frame_index += 1;
}
Expand Down Expand Up @@ -141,6 +149,7 @@ impl NativeStack {
short_filename: None,
module: None,
locals: None,
is_entry: true,
});
});

Expand Down Expand Up @@ -281,6 +290,7 @@ impl NativeStack {
short_filename: None,
module: Some(frame.module.clone()),
locals: None,
is_entry: true,
})
}
None => Some(Frame {
Expand All @@ -290,6 +300,7 @@ impl NativeStack {
line: 0,
short_filename: None,
module: Some(frame.module.clone()),
is_entry: true,
}),
}
}
Expand Down
7 changes: 7 additions & 0 deletions src/python_interpreters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ pub trait FrameObject {
fn code(&self) -> *mut Self::CodeObject;
fn lasti(&self) -> i32;
fn back(&self) -> *mut Self;
fn is_entry(&self) -> bool;
}

pub trait CodeObject {
Expand Down Expand Up @@ -157,6 +158,9 @@ macro_rules! PythonCommonImpl {
fn back(&self) -> *mut Self {
self.f_back
}
fn is_entry(&self) -> bool {
true
}
}

impl Object for $py::PyObject {
Expand Down Expand Up @@ -357,6 +361,9 @@ impl FrameObject for v3_11_0::_PyInterpreterFrame {
fn back(&self) -> *mut Self {
self.previous
}
fn is_entry(&self) -> bool {
self.is_entry
}
}

impl Object for v3_11_0::PyObject {
Expand Down
1 change: 1 addition & 0 deletions src/speedscope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,7 @@ mod tests {
short_filename: None,
line: 0,
locals: None,
is_entry: true,
};

let trace = stack_trace::StackTrace {
Expand Down
6 changes: 6 additions & 0 deletions src/stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ pub struct Frame {
pub line: i32,
/// Local Variables associated with the frame
pub locals: Option<Vec<LocalVariable>>,
/// If this is an entry frame. Each entry frame corresponds to one native frame.
pub is_entry: bool,
}

#[derive(Debug, Hash, Eq, PartialEq, Ord, PartialOrd, Clone, Serialize)]
Expand Down Expand Up @@ -158,13 +160,16 @@ where
None
};

let is_entry = frame.is_entry();

frames.push(Frame {
name,
filename,
line,
short_filename: None,
module: None,
locals,
is_entry,
});
if frames.len() > 4096 {
return Err(format_err!("Max frame recursion depth reached"));
Expand Down Expand Up @@ -278,6 +283,7 @@ impl ProcessInfo {
short_filename: None,
line: 0,
locals: None,
is_entry: true,
}
}
}
Expand Down

0 comments on commit 2f8cfdd

Please sign in to comment.