Skip to content

Commit 345f77b

Browse files
lvaroquiclaude
andcommitted
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: <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 <noreply@anthropic.com>
1 parent 3a766f1 commit 345f77b

7 files changed

Lines changed: 83 additions & 12 deletions

File tree

‎crates/exec-harness/src/analysis/mod.rs‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,15 +25,20 @@ pub fn perform(commands: Vec<BenchmarkCommand>, mode: MeasurementMode) -> Result
2525
}
2626

2727
hooks.start_benchmark().unwrap();
28-
let status = cmd.status();
28+
let result = cmd.spawn().and_then(|mut child| {
29+
let pid = child.id();
30+
child.wait().map(|status| (pid, status))
31+
});
2932
hooks.stop_benchmark().unwrap();
30-
let status = status.context("Failed to execute command")?;
33+
let (pid, status) = result.context("Failed to execute command")?;
3134

3235
if !status.success() {
3336
bail!("Command exited with non-zero status: {status}");
3437
}
3538

36-
hooks.set_executed_benchmark(&name_and_uri.uri).unwrap();
39+
hooks
40+
.set_executed_benchmark_for_pid(pid, &name_and_uri.uri)
41+
.unwrap();
3742
}
3843

3944
Ok(())

‎crates/instrument-hooks-bindings/src/lib.rs‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -58,10 +58,15 @@ mod linux_impl {
5858

5959
#[inline(always)]
6060
pub fn set_executed_benchmark(&self, uri: &str) -> Result<(), u8> {
61-
let pid = std::process::id() as i32;
61+
self.set_executed_benchmark_for_pid(std::process::id(), uri)
62+
}
63+
64+
/// Declares a benchmark that ran in the process `pid` rather than in this one.
65+
#[inline(always)]
66+
pub fn set_executed_benchmark_for_pid(&self, pid: u32, uri: &str) -> Result<(), u8> {
6267
let c_uri = CString::new(uri).map_err(|_| 1u8)?;
6368
let result = unsafe {
64-
ffi::instrument_hooks_set_executed_benchmark(self.0, pid, c_uri.as_ptr())
69+
ffi::instrument_hooks_set_executed_benchmark(self.0, pid as i32, c_uri.as_ptr())
6570
};
6671
if result == 0 { Ok(()) } else { Err(result) }
6772
}
@@ -184,6 +189,10 @@ mod other_impl {
184189
Ok(())
185190
}
186191

192+
pub fn set_executed_benchmark_for_pid(&self, _pid: u32, _uri: &str) -> Result<(), u8> {
193+
Ok(())
194+
}
195+
187196
pub fn set_integration(&self, _name: &str, _version: &str) -> Result<(), u8> {
188197
Ok(())
189198
}
@@ -213,6 +222,11 @@ mod tests {
213222
let hooks = InstrumentHooks::instance("test_integration", "1.0.0");
214223
assert!(!hooks.is_instrumented() || hooks.start_benchmark().is_ok());
215224
assert!(hooks.set_executed_benchmark("test_uri").is_ok());
225+
assert!(
226+
hooks
227+
.set_executed_benchmark_for_pid(std::process::id() + 1, "test_uri")
228+
.is_ok()
229+
);
216230
assert!(hooks.set_integration("test_integration", "1.0.0").is_ok());
217231
let start = InstrumentHooks::current_timestamp();
218232
let end = start + 1_000_000; // Simulate 1ms later

‎src/binary_pins.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ pub const VALGRIND_CODSPEED_VERSION: Version = Version::new(3, 26, 0);
1313
/// the .deb is repackaged without a new upstream valgrind release. Appears in
1414
/// the .deb package version (`3.26.0-0codspeed3`) and in `valgrind --version`
1515
/// output (`valgrind-3.26.0.codspeed3`).
16-
pub const VALGRIND_CODSPEED_ITERATION: u32 = 7;
16+
pub const VALGRIND_CODSPEED_ITERATION: u32 = 8;
1717
/// Suffix appended to `VALGRIND_CODSPEED_VERSION` to form the .deb package version.
1818
static VALGRIND_DEB_REV: LazyLock<String> =
1919
LazyLock::new(|| format!("0codspeed{VALGRIND_CODSPEED_ITERATION}"));
@@ -93,16 +93,16 @@ impl ValgrindTarget {
9393
fn sha256(self) -> &'static str {
9494
match (self.distro_version, self.arch) {
9595
(DistroVersion::Ubuntu2204, Arch::Amd64) => {
96-
"658a64049b6a1f5bec9c038b8036f9a61fee83011a909b649e32a82edab4ba99"
96+
"9836998817cd6d12e86bf60a230634f9f11594a1d21fcbd580a47b693ec5b6d1"
9797
}
9898
(DistroVersion::Ubuntu2404, Arch::Amd64) => {
99-
"ea1788e43cfd75b8af84dd496eb265307c6ab6d048cb7bc8ba4bc564dbbd5aba"
99+
"edee86d0d61b46435ea3f4a73ffb222fcf8d605a9e3901638f280197c3c3d602"
100100
}
101101
(DistroVersion::Ubuntu2204, Arch::Arm64) => {
102-
"6db07f15ce23e3cfda00bda85aac0ea3b8a2261c00d529ed856ceb7f3ba207f6"
102+
"453e2455a3a924c2b2e93201336fb9b7904f1154bf9304bf910da884cee27052"
103103
}
104104
(DistroVersion::Ubuntu2404, Arch::Arm64) => {
105-
"0bccdaf2da7202ff4fe15e8301641c3a0abd46879c03713150cb5f21d6c9baaf"
105+
"1884667fe9da4f991476b97108d021341b0456ebc5185430a1748e57f4cccc52"
106106
}
107107
}
108108
}

‎src/executor/valgrind/measure.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,13 @@ fn get_valgrind_args(tool: &SimulationTool, config: &ExecutorConfig) -> Vec<Stri
6363
children_skip_patterns.join(",")
6464
));
6565

66+
// Internal subcommands run through the runner's own executable, which may
67+
// carry file capabilities that valgrind refuses to run by default. They
68+
// do not need those capabilities.
69+
if let Ok(self_exe) = crate::cli::self_exe() {
70+
args.push(format!("--allow-file-caps={}", self_exe.display()));
71+
}
72+
6673
if config.fair_sched {
6774
args.push("--fair-sched=yes".to_string());
6875
}

‎tests/docker/run.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ memory=()
9797
walltime=()
9898
while read -r name; do
9999
case $name in
100-
*Simulation*) simulation+=("$name") ;;
100+
*Simulation* | simulation_*) simulation+=("$name") ;;
101101
*Memory* | memory_*) memory+=("$name") ;;
102102
*) walltime+=("$name") ;;
103103
esac

‎tests/executors.rs‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -269,6 +269,51 @@ fn walltime_exec_harness() {
269269
.success();
270270
}
271271

272+
/// exec-harness measures the command from its own process, so its part
273+
/// must declare the pid of the spawned process that ran the benchmark.
274+
#[test]
275+
fn simulation_exec_harness_declares_benchmark_pid() {
276+
let profile_folder = tempfile::tempdir().unwrap();
277+
let _guard = Mode::Simulation.acquire();
278+
codspeed()
279+
.args(["exec", "--mode", "simulation", "--skip-upload"])
280+
.arg("--profile-folder")
281+
.arg(profile_folder.path())
282+
.args(["--", "sh", "-c", "ls > /dev/null"])
283+
.assert()
284+
.success();
285+
286+
let profiles: Vec<(String, String)> = std::fs::read_dir(profile_folder.path())
287+
.unwrap()
288+
.map(|entry| entry.unwrap().path())
289+
.filter(|path| path.extension().is_some_and(|ext| ext == "out"))
290+
.map(|path| {
291+
let pid = path.file_stem().unwrap().to_string_lossy().into_owned();
292+
(pid, std::fs::read_to_string(&path).unwrap())
293+
})
294+
.collect();
295+
296+
let declarations: Vec<(&str, &str)> = profiles
297+
.iter()
298+
.flat_map(|(pid, content)| {
299+
content
300+
.lines()
301+
.filter_map(|line| line.strip_prefix("desc: Benchmark pid: "))
302+
.map(move |benchmark_pid| (pid.as_str(), benchmark_pid))
303+
})
304+
.collect();
305+
let [(harness_pid, benchmark_pid)] = declarations.as_slice() else {
306+
panic!("expected exactly one benchmark pid declaration, got {declarations:?}");
307+
};
308+
309+
assert_ne!(harness_pid, benchmark_pid);
310+
assert!(
311+
profiles.iter().any(|(pid, _)| pid == benchmark_pid),
312+
"benchmark pid {benchmark_pid} has no profile among {:?}",
313+
profiles.iter().map(|(pid, _)| pid).collect::<Vec<_>>()
314+
);
315+
}
316+
272317
/// Prepends `prefix` to `var` and checks the benchmark still sees it.
273318
fn memory_forwards_path_like(var: &str, prefix: &str) {
274319
let value = match std::env::var(var).unwrap_or_default() {

0 commit comments

Comments
 (0)