Skip to content

fix: support perf jitdumps on macos#385

Open
not-matthias wants to merge 3 commits into
mainfrom
cod-2645-investigate-python_perf_jit_support-not-working-on-macos
Open

fix: support perf jitdumps on macos#385
not-matthias wants to merge 3 commits into
mainfrom
cod-2645-investigate-python_perf_jit_support-not-working-on-macos

Conversation

@not-matthias
Copy link
Copy Markdown
Member

No description provided.

@not-matthias not-matthias force-pushed the cod-2645-investigate-python_perf_jit_support-not-working-on-macos branch from 362e5a1 to 938d05e Compare June 1, 2026 14:27
@codspeed-hq
Copy link
Copy Markdown

codspeed-hq Bot commented Jun 1, 2026

Merging this PR will not alter performance

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

✅ 7 untouched benchmarks


Comparing cod-2645-investigate-python_perf_jit_support-not-working-on-macos (938d05e) with main (f79d57d)

Open in CodSpeed

@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Jun 1, 2026

Greptile Summary

This PR resolves the macOS JIT dump limitation for Python walltime profiling by updating the samply and linux-perf-data dependencies to versions that correctly handle perf JIT dumps on macOS, then removing the platform guard that was suppressing PYTHON_PERF_JIT_SUPPORT on macOS.

  • src/executor/helpers/env.rs: Drops the !cfg!(target_os = "macos") condition from the PYTHON_PERF_JIT_SUPPORT env var, enabling Python JIT profiling on macOS walltime runs. The accompanying FIXME(COD-2645) is removed, indicating the root cause in samply is fixed.
  • Cargo.toml / Cargo.lock: Switches linux-perf-data from mstange/linux-perf-data to an AvalancheHQ fork (rev effe486), and bumps the CodSpeedHQ samply fork to rev 75ae209d. The lock file now carries two linux-perf-data 0.13.0 entries — the registry copy (consumed by samply-symbols) and the git-fork copy (consumed by samply and codspeed-runner) — which is expected given how Cargo resolves distinct sources.

Confidence Score: 4/5

Safe to merge; the macOS JIT dump fix is straightforward and the dependency updates are properly locked in Cargo.lock.

The env.rs change is minimal and correct — it simply removes a platform guard that was explicitly tracking a now-fixed upstream issue. The dependency updates are pinned by rev and fully resolved in the lock file. The only observations are that both new git revs are short abbreviated SHAs rather than full 40-char SHAs, which is a consistency concern rather than a functional one.

Cargo.toml — the two new git revs use abbreviated SHAs; worth expanding to full SHAs to match the established pattern.

Important Files Changed

Filename Overview
src/executor/helpers/env.rs Removes the macOS exclusion for PYTHON_PERF_JIT_SUPPORT — previously disabled on macOS due to unresolved stack addresses; now enabled for all Walltime runs after the underlying samply/linux-perf-data fix.
Cargo.toml Switches linux-perf-data from mstange upstream to AvalancheHQ fork (rev effe486) and bumps samply CodSpeedHQ fork to rev 75ae209d; both revs are short 7-8 char abbreviated SHAs rather than full 40-char SHAs used previously.
Cargo.lock Lock file updated to reflect new dependency sources; two linux-perf-data 0.13.0 entries coexist — the registry version (for samply-symbols) and the AvalancheHQ git fork (for samply and codspeed-runner).

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[get_base_injected_env] --> B{mode == Walltime?}
    B -- Yes --> C[PYTHON_PERF_JIT_SUPPORT = 1]
    B -- No --> D[PYTHON_PERF_JIT_SUPPORT = 0]
    C --> E[Python JIT profiling enabled on Linux AND macOS]
    D --> F[Python JIT profiling disabled]

    subgraph "Before this PR (macOS guard)"
        G{mode == Walltime AND NOT macOS?}
        G -- Yes --> H[PYTHON_PERF_JIT_SUPPORT = 1]
        G -- No --> I[PYTHON_PERF_JIT_SUPPORT = 0]
    end

    subgraph "Dependency chain for JIT dump support"
        J[codspeed-runner] -->|uses| K[linux-perf-data 0.13.0\nAvalancheHQ git fork\nrev effe486d]
        J -->|uses| L[samply\nCodSpeedHQ fork\nrev 75ae209d]
        L -->|uses| K
        L -->|samply-symbols uses| M[linux-perf-data 0.13.0\ncrates.io registry]
    end
Loading

Reviews (1): Last reviewed commit: "chore: use samply with latest linux-perf..." | Re-trigger Greptile

Comment thread Cargo.toml
Comment on lines +49 to +51
linux-perf-data = { git = "https://github.com/AvalancheHQ/linux-perf-data.git", rev = "effe486", features = [
"zstd",
] } # unreleased main as of 2026-03-19
] } # unreleased branch as of 2026-06-01
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Both new git rev values are short abbreviated SHAs (7–8 chars) rather than the full 40-char SHA that was used previously (e.g. the old linux-perf-data rev was da5bce4b9fb724e84b1eea0cb6ab9c8a291bc676 and the old samply rev was e8b8dacc042978953a6db9616c72846f7b8d5062). The lock file resolves them correctly, but abbreviated revs leave a theoretical ambiguity window as the repository grows. Using the full SHA matches the pattern established in the rest of the codebase and the pinning rule intent.

Suggested change
linux-perf-data = { git = "https://github.com/AvalancheHQ/linux-perf-data.git", rev = "effe486", features = [
"zstd",
] } # unreleased main as of 2026-03-19
] } # unreleased branch as of 2026-06-01
linux-perf-data = { git = "https://github.com/AvalancheHQ/linux-perf-data.git", rev = "effe486d4a951d6655007a163b837e7c4a15848c", features = [
"zstd",
] } # unreleased branch as of 2026-06-01

Rule Used: Pin Cargo git dependencies to tag or rev, neve... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Comment thread Cargo.toml
which = "8.0.2"
crc32fast = "1.5.0"
samply = { git = "https://github.com/CodSpeedHQ/samply-codspeed", rev = "e8b8dacc042978953a6db9616c72846f7b8d5062" }
samply = { git = "https://github.com/CodSpeedHQ/samply-codspeed", rev = "75ae209d" }
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Same short-rev concern as linux-perf-data above — the full SHA resolved in the lock file is 75ae209d1f22272e046be816f785509ba3e0a163.

Suggested change
samply = { git = "https://github.com/CodSpeedHQ/samply-codspeed", rev = "75ae209d" }
samply = { git = "https://github.com/CodSpeedHQ/samply-codspeed", rev = "75ae209d1f22272e046be816f785509ba3e0a163" }

Rule Used: Pin Cargo git dependencies to tag or rev, neve... (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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.

1 participant