From e6d09c73ece83698fd28c9a7e82eac2213e5fa52 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 | 12 ++++++- 6 files changed, 31 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..4a757f74b 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, + simulation_track_subprocess: args.shared.simulation_track_subprocess, }) } diff --git a/src/cli/run/mod.rs b/src/cli/run/mod.rs index e5444d49b..a218155c7 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: false, + simulation_track_subprocess: 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, + simulation_track_subprocess: args.shared.simulation_track_subprocess, }) } diff --git a/src/cli/shared.rs b/src/cli/shared.rs index 3767bbf35..14f3bfda4 100644 --- a/src/cli/shared.rs +++ b/src/cli/shared.rs @@ -135,6 +135,15 @@ pub struct ExecAndRunSharedArgs { )] pub exclude_allocations: bool, + /// Measure the subprocesses spawned by the benchmarked process in simulation mode. + #[arg( + long, + env = "CODSPEED_SIMULATION_TRACK_SUBPROCESS", + default_value_t = false, + action = clap::ArgAction::Set + )] + pub simulation_track_subprocess: bool, + #[command(flatten)] pub profiler_run_args: ProfilerRunArgs, diff --git a/src/executor/config.rs b/src/executor/config.rs index 0f7dfe24a..07f99c820 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 simulation_track_subprocess: 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 simulation_track_subprocess: 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, + simulation_track_subprocess: self.simulation_track_subprocess, } } } @@ -237,6 +244,7 @@ impl OrchestratorConfig { fair_sched: false, cycle_estimation: true, exclude_allocations: false, + simulation_track_subprocess: false, } } } diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index b42fa0ea2..c8201d725 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,11 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec { args.push("--tool=tracegrind".to_string()); From e425c8d5980395bf86833f38bd2bed6f86795a96 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 c8201d725..d9e5b2470 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -60,8 +60,7 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec