Skip to content

Conversation

not-matthias
Copy link
Member

This PR adds support for DWARF unwinding in Python by:

  • Enabling the PYTHON_PERF_JIT_SUPPORT feature via env var
  • Dumping the generated JIT dump info
  • Converting it into our UnwindInfo structure which is used for unwinding

@not-matthias not-matthias force-pushed the cod-992-perf-works-in-github-action-but-not-locally branch 3 times, most recently from 801b78a to 03cfd8a Compare September 10, 2025 10:36
@not-matthias not-matthias marked this pull request as ready for review September 10, 2025 17:46
@not-matthias not-matthias force-pushed the cod-992-perf-works-in-github-action-but-not-locally branch from 1b98e09 to 1028e92 Compare September 10, 2025 17:47
@not-matthias not-matthias requested review from art049, Copilot and GuillaumeLagrange and removed request for art049 and Copilot September 10, 2025 17:48
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for DWARF unwinding in Python by enabling Python JIT support through environment variables and processing JIT dump files. The changes include extracting JIT symbols, converting them to perf maps, and generating unwind data for stack unwinding.

  • Enables PYTHON_PERF_JIT_SUPPORT environment variable for walltime mode
  • Adds JIT dump parsing functionality to extract symbols and unwind data
  • Modifies perf call graph mode to include larger stack size for DWARF unwinding

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/run/runner/wall_time/perf/jit_dump.rs New module for parsing JIT dump files and converting to symbols/unwind data
src/run/runner/wall_time/perf/mod.rs Integrates JIT dump harvesting and adds Python module detection
src/run/runner/wall_time/perf/perf_map.rs Exposes Symbol struct and adds helper methods for module symbols
src/run/runner/wall_time/perf/unwind_data.rs Adds custom Debug implementation and test cases for unwind data
src/run/runner/helpers/env.rs Sets PYTHON_PERF_JIT_SUPPORT environment variable and fixes debug check
src/run/mod.rs Adds PartialEq derive to RunnerMode enum

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@art049 art049 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

Comment on lines +239 to +249
for map in exe_maps.iter().sorted_by_key(|m| m.address.0) {
let (base_addr, end_addr) = map.address;
debug!(
" {:016x}-{:016x} {:08x} {:?} {:?} ",
base_addr, end_addr, map.offset, map.pathname, map.perms,
);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this be too much logs?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't add that much (around 40-50 lines). However, it's one of the most important things needed for debugging any perf related issues (e.g. if we fail to resolve the symbol we can look at which module it belongs to).

And since we only enable it when running with CODSPEED_LOG=debug it shouldn't be an issue.

@not-matthias not-matthias force-pushed the cod-992-perf-works-in-github-action-but-not-locally branch from 1028e92 to 942970d Compare September 11, 2025 16:40
@not-matthias not-matthias force-pushed the cod-992-perf-works-in-github-action-but-not-locally branch from 942970d to 3323c5a Compare September 12, 2025 10:39
@not-matthias not-matthias merged commit 3323c5a into main Sep 12, 2025
9 checks passed
@not-matthias not-matthias deleted the cod-992-perf-works-in-github-action-but-not-locally branch September 12, 2025 10:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants