Add runner options to enable valgrind subprocess tracking - #493
Add runner options to enable valgrind subprocess tracking#493GuillaumeLagrange wants to merge 2 commits into
Conversation
- 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 <noreply@anthropic.com>
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
5ce8ad4 to
9ab21d0
Compare
Greptile SummaryAdds opt-in Valgrind subprocess measurement support.
Confidence Score: 4/5The PR is not yet safe to merge because enabling subprocess tracking passes an unsupported Valgrind option value and prevents the benchmark from launching. The subprocess-tracking path still emits Files Needing Attention: src/executor/valgrind/measure.rs
|
| Filename | Overview |
|---|---|
| src/cli/shared.rs | Adds the shared opt-in CLI and environment configuration for simulation subprocess tracking. |
| src/executor/config.rs | Propagates subprocess-tracking configuration from orchestration into each execution. |
| src/executor/valgrind/measure.rs | Changes Valgrind argument construction to support subprocess and per-thread simulation output. |
| crates/exec-harness/src/analysis/mod.rs | Removes the post-execution guard that rejected benchmarks spawning Valgrind-traced subprocesses. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
A["CLI flag / environment variable"] --> B["OrchestratorConfig"]
B --> C["ExecutorConfig"]
C --> D["Valgrind argument construction"]
D --> E["Instrument benchmark subprocesses"]
E --> F["Combined simulation artifacts"]
Reviews (2): Last reviewed commit: "chore: stop skipping rustup wrapper for ..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
Related: CodSpeedHQ/valgrind-codspeed#25
Not to be merged until valgrind-codspeed has been released and required version has been bumped