[COD-2349] feat(callgrind): track subprocesses across fork and exec - #25
Conversation
Greptile SummaryThis PR tracks benchmark subprocesses across fork and exec. The main changes are:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| callgrind/dump.c | Writes subprocess metadata per dump part and removes emitted records to prevent unbounded retention. |
| callgrind/main.c | Tracks forked children, resets child state, and forwards instrumentation state across execs. |
| callgrind/threads.c | Retains exited-thread data until it can be included in per-thread dumps. |
| coregrind/m_libcproc.c | Extends parent fork callbacks with the newly created child PID. |
Reviews (5): Last reviewed commit: "feat(callgrind): inherit instrumentation..." | Re-trigger Greptile
Merging this PR will not alter performance
Comparing Footnotes
|
ce72315 to
69b355e
Compare
5cae7e2 to
bf37129
Compare
69b355e to
002990c
Compare
bf37129 to
bb4fb76
Compare
002990c to
ec038a4
Compare
bb4fb76 to
febc434
Compare
art049
left a comment
There was a problem hiding this comment.
As discussed it would be simpler to have a new inherit mode that would resolve a file owned by the parent process declaring if instrumentation is enabled or not.
It will make the overall change in the logic of the codebase more isolated and easier to maintain.
Make per-thread dumps behave like OS threads. The valgrind core recycles ThreadId slots and callgrind keyed all per-thread state on that slot, so a new OS thread reusing a slot silently inherited the dead thread's identity and costs. Mid-run client-request dumps also only flushed the calling thread, deferring every other thread's history to the termination dump. - Key thread identity (BBCC lookup, "thread:" header, dump suffix) on a monotonic serial instead of the recycled ThreadId slot. - Retire exiting threads via a pre_thread_ll_exit hook: unwind, snapshot their name, move them to a retired list so their costs stay attributed across the rest of the run. Gated on separate_threads. - Flush every thread, live and retired, at each dump so per-thread deltas land under the part being dumped, in ascending serial order. Threads with a zero delta are skipped; if that skips a whole part (metadata dumps with instrumentation off, termination), force one empty section so the part and its trigger metadata survive. Key the combined-dump header on file state, not out_counter, so it is written even when leading parts are skipped. - Zero every thread on CALLGRIND_ZERO_STATS under separate_threads. - Emit the core's thread name as a "desc: Thread name:" line, picked up by the backend in COD-3197. Refs COD-3196 Co-Authored-By: Claude <noreply@anthropic.com>
When a benchmark spawns a subprocess, the resulting profile had no way to attribute that subprocess back to the exact benchmark (dump part) that spawned it, so the spawn tree could not be rebuilt. Track fork edges and emit them in the dump: on fork the parent records the new child pid against the part currently being measured, and each part's header lists the children spawned during it as "desc: Spawned pid:" lines (a desc field so kcachegrind and callgrind_annotate ignore it). A fork child restarts its part counter at 1 and drops the inherited edges so it only reports children it spawns itself, and zeroes cost so work before the fork stays attributed to the parent. To carry the child pid to the fork hook, VG_(atfork)'s parent callback now takes the pid of the just-created child; the pre/child callbacks are unchanged. All fork/clone sites (Linux, FreeBSD, Solaris, generic) pass it through. The child hook also re-stamps the in-flight fork syscall's start time from the child's clocks, since the thread CPU clock restarts in the child and the exit delta would otherwise underflow. Closes COD-2349 Co-Authored-By: Claude <noreply@anthropic.com>
A process spawned via exec under --trace-children=yes gets a fresh valgrind whose instrumentation state resets to --instr-atstart, so a benchmark reached through an exec chain (e.g. `cargo run`) was measured from the wrong point. A plain fork inherits the state with the address space, but an exec does not. Add --instr-atstart=inherit: like "no", except each process advertises its current instrumentation state by keeping <tmpdir>/callgrind-instr-<pid> in existence while enabled, and a fresh valgrind adopts the state advertised for its own PID. An exec keeps the PID, so the file maintained by the pre-exec image hands the state over; a fork child inherits the state with the address space and republishes it under its new PID. The file stores the process start time (stable across exec), so a file left behind by a killed process is rejected when its PID is reused. Closes COD-2349 Generated with AI Agent (Claude Code)
ec038a4 to
aece00f
Compare
Track subprocesses spawned by a benchmark so they can be measured and
attributed back to the benchmark that spawned them, across both
forkandexec.Two independent pieces, one per commit:
1. Forward instrumentation state across a traced exec. A process spawned
via
execunder--trace-children=yesgets a fresh valgrind whoseinstrumentation state resets to
--instr-atstart, so a benchmark reachedthrough an exec chain (e.g.
cargo run) was measured from the wrong point. Aplain
forkinherits the state with the address space, but an exec does not.This adds a
VG_(needs_child_exec_args)tool need: the core asks the tool forextra valgrind arguments while building the child's argv at exec time and
appends them after
VG_(args_for_valgrind)(later options win). Callgrind usesit to forward
--instr-atstart=<current state>. Wired into the exec argvconstruction on Linux (generic), Darwin, and Solaris.
2. Record spawned subprocesses per dump part. When a benchmark spawns a
subprocess, the profile had no way to attribute that subprocess to the exact
benchmark (dump part) that spawned it. On
forkthe parent now records the newchild pid against the part currently being measured, and each part's header
lists the children spawned during it as
desc: Spawned pid:lines (adescfield so kcachegrind and callgrind_annotate ignore it). A fork child restarts
its part counter at 1 and drops the inherited edges so it only reports children
it spawns itself, and zeroes cost so work before the fork stays attributed to
the parent.
To carry the child pid to the fork hook,
VG_(atfork)'s parent callback nowtakes the pid of the just-created child; the pre/child callbacks are unchanged.
All fork/clone sites (Linux, FreeBSD, Solaris, generic) pass it through. The
child hook also re-stamps the in-flight fork syscall's start time from the
child's clocks, since the thread CPU clock restarts in the child and the exit
delta would otherwise underflow.
The spawn edge is recorded parent→child rather than child→parent: at fork time
the parent already knows both the child pid and its own live part, so no spawn
identity needs to cross the exec boundary — only the instrumentation state
does. This is a change from the earlier squashed design that emitted
desc: Spawned by: <pid>:<part>from the child; see the COD-2349 discussion forthe full rationale.
Stacked on top of #24 (COD-3196, per-thread dump support), which this builds
on for the retired-thread handling in the fork/exec paths.
Behavior verified with fork/exec integration tests in the valgrind-helpers repo
(subprocess measured and recorded across fork, across a traced exec, through a
deep fork tree, and one child recorded per part across a two-part run).
Closes COD-2349