Skip to content

Commit ec6a293

Browse files
committed
fix: only set PYTHONPERFSUPPORT for exec-harness benchmarks
Setting PYTHONPERFSUPPORT=1 in every mode (#577) activates CPython's perf trampoline in all Python processes the runner starts. pytest-codspeed crashes with it enabled and the run exits with status 139. Remove it from the shared runner env and set it only in exec-harness, which needs the perf map to name Python frames in its profiles. exec-harness now exposes runtime_env::env(), which apply() uses. The memtrack interpreter tests use it as a dev-dependency instead of copying the env and the node wrapper flags by hand.
1 parent 90cc8e9 commit ec6a293

6 files changed

Lines changed: 28 additions & 24 deletions

File tree

‎Cargo.lock‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎crates/exec-harness/src/lib.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ use std::io::{self, BufRead};
55
pub mod analysis;
66
pub mod constants;
77
pub mod prelude;
8-
mod runtime_env;
8+
pub mod runtime_env;
99
mod uri;
1010
pub mod walltime;
1111

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,23 +1,37 @@
11
//! Runtime env for benchmark processes started by exec-harness: the shared
2-
//! [`runner_shared::runtime_env::env`] vars plus the `node` wrapper on `PATH`.
2+
//! [`runner_shared::runtime_env::env`] vars, `PYTHONPERFSUPPORT`, and the
3+
//! `node` wrapper on `PATH`.
34
45
use crate::MeasurementMode;
6+
use std::ffi::OsString;
57

68
mod node;
79

8-
const PATH_ENV: &str = "PATH";
10+
/// Env vars to set on benchmark processes for `mode`.
11+
pub fn env(mode: MeasurementMode) -> anyhow::Result<Vec<(&'static str, OsString)>> {
12+
let mut env: Vec<(&'static str, OsString)> = runner_shared::runtime_env::env(mode)
13+
.into_iter()
14+
.map(|(key, value)| (key, value.into()))
15+
.collect();
16+
// Makes CPython write `/tmp/perf-<pid>.map` (or jitdump when
17+
// `PYTHON_PERF_JIT_SUPPORT=1`) so profiles can name Python frames. Only set
18+
// for exec-harness: integrations with their own harness (pytest-codspeed)
19+
// crash with the perf trampoline enabled.
20+
env.push(("PYTHONPERFSUPPORT", "1".into()));
21+
let path = std::env::var_os("PATH").unwrap_or_default();
22+
env.push(("PATH", node::path_with_node_wrapper(&path)?));
23+
Ok(env)
24+
}
925

10-
/// Applies the runtime env and the node wrapper to the current process, so
11-
/// every child inherits them. Existing values are overwritten: `mode` may come
12-
/// from a CLI flag that differs from an inherited `CODSPEED_RUNNER_MODE`.
26+
/// Applies [`env`] to the current process, so every child inherits it.
27+
/// Existing values are overwritten: `mode` may come from a CLI flag that
28+
/// differs from an inherited `CODSPEED_RUNNER_MODE`.
1329
///
1430
/// # Safety
1531
/// Must be called while the process is single-threaded.
1632
pub(crate) unsafe fn apply(mode: MeasurementMode) -> anyhow::Result<()> {
17-
for (key, value) in runner_shared::runtime_env::env(mode) {
33+
for (key, value) in env(mode)? {
1834
unsafe { std::env::set_var(key, value) };
1935
}
20-
let path = std::env::var_os(PATH_ENV).unwrap_or_default();
21-
unsafe { std::env::set_var(PATH_ENV, node::path_with_node_wrapper(&path)?) };
2236
Ok(())
2337
}

‎crates/memtrack/Cargo.toml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ bindgen = "0.72"
4949

5050
[dev-dependencies]
5151
tempfile = { workspace = true }
52+
exec-harness = { path = "../exec-harness" }
5253
rstest = { workspace = true }
5354
test-log = { workspace = true }
5455
insta = { workspace = true, features = ["json", "redactions"] }

‎crates/memtrack/tests/interpreter_tests.rs‎

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,16 @@ mod shared;
44
use memtrack::TrackerOptions;
55
use runner_shared::artifacts::{MemtrackEvent, MemtrackEventKind};
66
use runner_shared::measurement_mode::MeasurementMode;
7-
use runner_shared::runtime_env;
87
use std::path::Path;
98
use std::process::Command;
109

1110
const ALLOCATION_SIZE: u64 = 2_000_001;
12-
const NODE_PROGRAM: &str = "node";
13-
/// Writes /tmp/perf-<pid>.map.
14-
const NODE_PERF_MAP_FLAG: &str = "--perf-basic-prof";
1511

16-
/// Applies the memory-mode runtime env the runner injects, and makes Node
17-
/// emit its perf map like exec-harness's `node` wrapper does in memory mode.
12+
/// Runs `program` with the env exec-harness sets for memory-mode benchmarks.
1813
fn memory_mode_command(program: &str, fixture_path: &str) -> anyhow::Result<Command> {
1914
let mut command = Command::new(program);
20-
if program == NODE_PROGRAM {
21-
command.arg(NODE_PERF_MAP_FLAG);
22-
}
2315
command.arg(fixture_path);
24-
command.envs(runtime_env::env(MeasurementMode::Memory));
16+
command.envs(exec_harness::runtime_env::env(MeasurementMode::Memory)?);
2517
Ok(command)
2618
}
2719

@@ -87,7 +79,7 @@ fn test_python_allocation_has_jit_symbol() -> anyhow::Result<()> {
8779
#[test_with::env(GITHUB_ACTIONS)]
8880
#[test_log::test]
8981
fn test_node_allocation_has_jit_symbol() -> anyhow::Result<()> {
90-
let command = memory_mode_command(NODE_PROGRAM, &fixture("node_alloc.js"))?;
82+
let command = memory_mode_command("node", &fixture("node_alloc.js"))?;
9183
let (events, thread_handle) = shared::track_command(command, stack_capture())?;
9284

9385
assert_perf_map_symbol(allocation_pid(&events), "JS:~allocation ");

‎crates/runner-shared/src/runtime_env/python.rs‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,21 +2,17 @@ use crate::measurement_mode::MeasurementMode;
22

33
const PYTHONHASHSEED: &str = "PYTHONHASHSEED";
44
const PYTHON_PERF_JIT_SUPPORT: &str = "PYTHON_PERF_JIT_SUPPORT";
5-
const PYTHONPERFSUPPORT: &str = "PYTHONPERFSUPPORT";
65

76
/// Python env:
87
/// - `PYTHONHASHSEED=0`: deterministic hashing in every mode.
98
/// - `PYTHON_PERF_JIT_SUPPORT`: perf jitdump for walltime profiles.
109
/// FIXME(COD-2645): Keep this disabled on macOS. Enabling it causes
1110
/// many unresolved addresses on the stack when profiling with samply.
12-
/// - `PYTHONPERFSUPPORT=1`: `/tmp/perf-<pid>.map` so valgrind and memory flamegraphs can
13-
/// name Python frames. When `PYTHON_PERF_JIT_SUPPORT=1` is also set, CPython uses jitdump.
1411
pub(super) fn env(mode: MeasurementMode) -> Vec<(&'static str, String)> {
1512
let perf_jit = mode == MeasurementMode::Walltime && !cfg!(target_os = "macos");
1613
let perf_jit_value = if perf_jit { "1" } else { "0" };
1714
vec![
1815
(PYTHONHASHSEED, "0".to_owned()),
1916
(PYTHON_PERF_JIT_SUPPORT, perf_jit_value.to_owned()),
20-
(PYTHONPERFSUPPORT, "1".to_owned()),
2117
]
2218
}

0 commit comments

Comments
 (0)