From 227ce462b9c0f7f398227718abfec5ea311ec40f Mon Sep 17 00:00:00 2001 From: Luc Varoqui Date: Thu, 1 Oct 2026 16:50:27 +0200 Subject: [PATCH 1/2] feat(instrument-hooks): declare benchmarks run in another process Add set_executed_benchmark_for_pid, which passes an explicit pid to instrument_hooks_set_executed_benchmark instead of the calling process' own. set_executed_benchmark keeps its behavior and delegates to it. Bump instrument-hooks, whose valgrind instrument now writes a "Benchmark pid: " desc line in the dump part when that pid is not the calling process'. This also brings thread-safe C API exports and the callgrind_toggle_collect helper. Refs COD-3722 Co-Authored-By: Claude Opus 5.5 --- crates/exec-harness/src/analysis/mod.rs | 11 +++-- .../instrument-hooks | 2 +- crates/instrument-hooks-bindings/src/lib.rs | 18 +++++++- src/executor/valgrind/measure.rs | 7 +++ tests/docker/run.sh | 2 +- tests/executors.rs | 45 +++++++++++++++++++ 6 files changed, 78 insertions(+), 7 deletions(-) diff --git a/crates/exec-harness/src/analysis/mod.rs b/crates/exec-harness/src/analysis/mod.rs index feed1ab21..9d12738ee 100644 --- a/crates/exec-harness/src/analysis/mod.rs +++ b/crates/exec-harness/src/analysis/mod.rs @@ -25,15 +25,20 @@ pub fn perform(commands: Vec, mode: MeasurementMode) -> Result } hooks.start_benchmark().unwrap(); - let status = cmd.status(); + let result = cmd.spawn().and_then(|mut child| { + let pid = child.id(); + child.wait().map(|status| (pid, status)) + }); hooks.stop_benchmark().unwrap(); - let status = status.context("Failed to execute command")?; + let (pid, status) = result.context("Failed to execute command")?; if !status.success() { bail!("Command exited with non-zero status: {status}"); } - hooks.set_executed_benchmark(&name_and_uri.uri).unwrap(); + hooks + .set_executed_benchmark_for_pid(pid, &name_and_uri.uri) + .unwrap(); } Ok(()) diff --git a/crates/instrument-hooks-bindings/instrument-hooks b/crates/instrument-hooks-bindings/instrument-hooks index b9ddb5bc6..d38f44cbc 160000 --- a/crates/instrument-hooks-bindings/instrument-hooks +++ b/crates/instrument-hooks-bindings/instrument-hooks @@ -1 +1 @@ -Subproject commit b9ddb5bc654b2e6fa13eb18efcd3a45e7ecda0bb +Subproject commit d38f44cbc992b58920440d8c80f6c1222d6657e2 diff --git a/crates/instrument-hooks-bindings/src/lib.rs b/crates/instrument-hooks-bindings/src/lib.rs index 8ea620cb8..09a031dd7 100644 --- a/crates/instrument-hooks-bindings/src/lib.rs +++ b/crates/instrument-hooks-bindings/src/lib.rs @@ -58,10 +58,15 @@ mod linux_impl { #[inline(always)] pub fn set_executed_benchmark(&self, uri: &str) -> Result<(), u8> { - let pid = std::process::id() as i32; + self.set_executed_benchmark_for_pid(std::process::id(), uri) + } + + /// Declares a benchmark that ran in the process `pid` rather than in this one. + #[inline(always)] + pub fn set_executed_benchmark_for_pid(&self, pid: u32, uri: &str) -> Result<(), u8> { let c_uri = CString::new(uri).map_err(|_| 1u8)?; let result = unsafe { - ffi::instrument_hooks_set_executed_benchmark(self.0, pid, c_uri.as_ptr()) + ffi::instrument_hooks_set_executed_benchmark(self.0, pid as i32, c_uri.as_ptr()) }; if result == 0 { Ok(()) } else { Err(result) } } @@ -184,6 +189,10 @@ mod other_impl { Ok(()) } + pub fn set_executed_benchmark_for_pid(&self, _pid: u32, _uri: &str) -> Result<(), u8> { + Ok(()) + } + pub fn set_integration(&self, _name: &str, _version: &str) -> Result<(), u8> { Ok(()) } @@ -213,6 +222,11 @@ mod tests { let hooks = InstrumentHooks::instance("test_integration", "1.0.0"); assert!(!hooks.is_instrumented() || hooks.start_benchmark().is_ok()); assert!(hooks.set_executed_benchmark("test_uri").is_ok()); + assert!( + hooks + .set_executed_benchmark_for_pid(std::process::id() + 1, "test_uri") + .is_ok() + ); assert!(hooks.set_integration("test_integration", "1.0.0").is_ok()); let start = InstrumentHooks::current_timestamp(); let end = start + 1_000_000; // Simulate 1ms later diff --git a/src/executor/valgrind/measure.rs b/src/executor/valgrind/measure.rs index 36fbdc0d6..e48405e26 100644 --- a/src/executor/valgrind/measure.rs +++ b/src/executor/valgrind/measure.rs @@ -63,6 +63,13 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec /dev/null"]) + .assert() + .success(); + + let profiles: Vec<(String, String)> = std::fs::read_dir(profile_folder.path()) + .unwrap() + .map(|entry| entry.unwrap().path()) + .filter(|path| path.extension().is_some_and(|ext| ext == "out")) + .map(|path| { + let pid = path.file_stem().unwrap().to_string_lossy().into_owned(); + (pid, std::fs::read_to_string(&path).unwrap()) + }) + .collect(); + + let declarations: Vec<(&str, &str)> = profiles + .iter() + .flat_map(|(pid, content)| { + content + .lines() + .filter_map(|line| line.strip_prefix("desc: Benchmark pid: ")) + .map(move |benchmark_pid| (pid.as_str(), benchmark_pid)) + }) + .collect(); + let [(harness_pid, benchmark_pid)] = declarations.as_slice() else { + panic!("expected exactly one benchmark pid declaration, got {declarations:?}"); + }; + + assert_ne!(harness_pid, benchmark_pid); + assert!( + profiles.iter().any(|(pid, _)| pid == benchmark_pid), + "benchmark pid {benchmark_pid} has no profile among {:?}", + profiles.iter().map(|(pid, _)| pid).collect::>() + ); +} + /// Prepends `prefix` to `var` and checks the benchmark still sees it. fn memory_forwards_path_like(var: &str, prefix: &str) { let value = match std::env::var(var).unwrap_or_default() { From d778a71db82d518784fbf829ce69befa415c5ced Mon Sep 17 00:00:00 2001 From: Luc Varoqui Date: Wed, 7 Oct 2026 11:43:30 +0200 Subject: [PATCH 2/2] tmp: use valgrind-codspeed release for tests --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5507377a6..ca7dda4f3 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -55,8 +55,9 @@ jobs: # No cache for now here, but we could cache either docker volumes or mounted host directories to cache build artifacts - run: just test-integ # Runs against an unreleased valgrind-codspeed branch, tag or full commit SHA - # env: - # CODSPEED_VALGRIND_REF: my-valgrind-branch + # TODO: remove this commit once valgrind-codspeed is released + env: + CODSPEED_VALGRIND_REF: cod-3722-ignore-process-spawning-overhead-in-exec-harness-simulation exec-harness-tests: runs-on: ubuntu-latest