From 0a3d4a1e91e2b39bfcbbec4e493c1ec2cd01a011 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Tue, 21 Jul 2026 20:29:22 +0000 Subject: [PATCH 1/2] feat(valgrind): add per-thread dumps and opt-in subprocess measurement - Pass --separate-threads=yes to callgrind so each OS thread dumps its own cost sections (valgrind side fixed in COD-3196, parsed by the backend in COD-3197). - Add --track-simulation-subprocesses (CODSPEED_TRACK_SIMULATION_SUBPROCESSES, off by default), which runs valgrind with --instr-atstart=inherit instead of =no. A process reached through an exec then picks up the instrumentation state of the image it replaced, so a benchmark that spawns subprocesses has their cost measured and attributed to it through the ppid/spawn-part dump headers. Children also inherit LD_PRELOAD and the benchmark URI env, so exec-harness children additionally dump under the benchmark URI directly. - Drop the exec-harness guard that failed runs whose benchmark process spawned subprocesses under valgrind (TODO(COD-2163)): those runs now succeed either way, measuring the children when tracking is enabled and ignoring them otherwise. Refs COD-2349 Co-Authored-By: Claude --- crates/exec-harness/src/analysis/mod.rs | 47 ------------------------- src/cli/exec/mod.rs | 1 + src/cli/run/mod.rs | 2 ++ src/cli/shared.rs | 9 +++++ src/executor/config.rs | 8 +++++ src/executor/valgrind/measure.rs | 8 ++++- 6 files changed, 27 insertions(+), 48 deletions(-) diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 46d561914..394b8617c 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -6,7 +6,6 @@ use crate::BenchmarkCommand; use crate::constants; use crate::uri; use instrument_hooks_bindings::InstrumentHooks; -use std::path::PathBuf; use std::process::Command; mod ld_preload_check; @@ -65,8 +64,6 @@ pub fn perform_with_valgrind(commands: Vec) -> Result<()> { let status = child.wait().context("Failed to execute command")?; - bail_if_command_spawned_subprocesses_under_valgrind(child.id())?; - if !status.success() { bail!("Command exited with non-zero status: {status}"); } @@ -75,47 +72,3 @@ pub fn perform_with_valgrind(commands: Vec) -> Result<()> { Ok(()) } -/// Checks if the benchmark process spawned subprocesses under valgrind by looking for .out -/// files in the profile folder. -/// -/// The presence of .out files where is greater than the benchmark process pid indicates -/// that the benchmark process spawned subprocesses. This .out file will be almost empty, with a 0 -/// cost reported due to the disabled instrumentation. -/// -/// We currently do not support measuring processes that spawn subprocesses under valgrind, because -/// valgrind will not have its instrumentation in the new process. -/// The LD_PRELOAD trick that we use to inject our instrumentation into the benchmark process only -/// works for the first process. -/// -/// TODO(COD-2163): Remove this once we support nested processes under valgrind -fn bail_if_command_spawned_subprocesses_under_valgrind(pid: u32) -> Result<()> { - let Some(profile_folder) = std::env::var_os("CODSPEED_PROFILE_FOLDER") else { - debug!("CODSPEED_PROFILE_FOLDER is not set, skipping subprocess detection"); - return Ok(()); - }; - - let profile_folder = PathBuf::from(profile_folder); - - // Bail if any .out where > pid of the benchmark process exists in the profile - // folder, which indicates that the benchmark process spawned subprocesses. - for entry in std::fs::read_dir(profile_folder)? { - let entry = entry?; - let file_name = entry.file_name(); - let file_name = file_name.to_string_lossy(); - - if let Some(stripped) = file_name.strip_suffix(".out") { - if let Ok(subprocess_pid) = stripped.parse::() { - if subprocess_pid > pid { - bail!( - "The codspeed CLI in CPU Simulation mode does not support measuring processes that spawn other processes yet.\n\n\ - Please either:\n\ - - Use the walltime measurement mode, or\n\ - - Benchmark a process that does not create subprocesses" - ) - } - } - } - } - - Ok(()) -} diff --git a/src/cli/exec/mod.rs b/src/cli/exec/mod.rs index 5ae0692af..1d85d7975 100644 --- a/src/cli/exec/mod.rs +++ b/src/cli/exec/mod.rs @@ -92,6 +92,7 @@ fn build_orchestrator_config( fair_sched: args.shared.experimental.experimental_fair_sched, cycle_estimation: args.shared.cycle_estimation, exclude_allocations: args.shared.exclude_allocations, + track_simulation_subprocesses: args.shared.track_simulation_subprocesses, }) } diff --git a/src/cli/run/mod.rs b/src/cli/run/mod.rs index d7f9089e9..312b5d22f 100644 --- a/src/cli/run/mod.rs +++ b/src/cli/run/mod.rs @@ -71,6 +71,7 @@ impl RunArgs { base: None, cycle_estimation: true, exclude_allocations: true, + track_simulation_subprocesses: false, profiler_run_args: ProfilerRunArgs { enable_profiler: false, enable_perf: None, @@ -133,6 +134,7 @@ fn build_orchestrator_config( fair_sched: args.shared.experimental.experimental_fair_sched, cycle_estimation: args.shared.cycle_estimation, exclude_allocations: args.shared.exclude_allocations, + track_simulation_subprocesses: args.shared.track_simulation_subprocesses, }) } diff --git a/src/cli/shared.rs b/src/cli/shared.rs index ae684bf62..532ad3d8d 100644 --- a/src/cli/shared.rs +++ b/src/cli/shared.rs @@ -131,6 +131,15 @@ pub struct ExecAndRunSharedArgs { )] pub exclude_allocations: bool, + /// Measure the subprocesses spawned by the benchmarked process in simulation mode. + #[arg( + long, + env = "CODSPEED_TRACK_SIMULATION_SUBPROCESSES", + default_value_t = false, + action = clap::ArgAction::Set + )] + pub track_simulation_subprocesses: bool, + #[command(flatten)] pub profiler_run_args: ProfilerRunArgs, diff --git a/src/executor/config.rs b/src/executor/config.rs index b0748d34e..1f9aed4f1 100644 --- a/src/executor/config.rs +++ b/src/executor/config.rs @@ -95,6 +95,9 @@ pub struct OrchestratorConfig { pub cycle_estimation: bool, /// Signal the backend to exclude memory allocation time from simulation results. pub exclude_allocations: bool, + /// Inherit valgrind's instrumentation state across a traced exec, so the cost of + /// subprocesses spawned by a benchmark is measured too. + pub track_simulation_subprocesses: bool, } /// Per-execution configuration passed to executors. @@ -132,6 +135,9 @@ pub struct ExecutorConfig { pub cycle_estimation: bool, /// Signal the backend to exclude memory allocation time from simulation results. pub exclude_allocations: bool, + /// Inherit valgrind's instrumentation state across a traced exec, so the cost of + /// subprocesses spawned by a benchmark is measured too. + pub track_simulation_subprocesses: bool, } #[derive(Debug, Clone, PartialEq)] @@ -203,6 +209,7 @@ impl OrchestratorConfig { fair_sched: self.fair_sched, cycle_estimation: self.cycle_estimation, exclude_allocations: self.exclude_allocations, + track_simulation_subprocesses: self.track_simulation_subprocesses, } } } @@ -237,6 +244,7 @@ impl OrchestratorConfig { fair_sched: false, cycle_estimation: true, exclude_allocations: true, + track_simulation_subprocesses: false, } } } diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index b42fa0ea2..e8527ca64 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -26,7 +26,6 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec Vec { if config.cycle_estimation { @@ -44,6 +49,7 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec { args.push("--tool=tracegrind".to_string()); From 9ab21d0e44a211ba2c1a606125ad474601d90e88 Mon Sep 17 00:00:00 2001 From: Guillaume Lagrange Date: Mon, 27 Jul 2026 10:41:18 +0200 Subject: [PATCH 2/2] chore: stop skipping rustup wrapper for valgrind This was a bandaid fix that is better fixed in other ways, since we should now be able to do codpseed exec -m simulation -- cargo run --- crates/exec-harness/src/analysis/mod.rs | 1 - src/executor/valgrind/measure.rs | 15 +-------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index 394b8617c..8bb4eaf44 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -71,4 +71,3 @@ pub fn perform_with_valgrind(commands: Vec) -> Result<()> { Ok(()) } - diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index e8527ca64..44d176c7f 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -56,8 +56,7 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec