[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
69b355e to
002990c
Compare
002990c to
ec038a4
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.
aece00f to
4d05dec
Compare
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
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
4d05dec to
c6a3e0b
Compare
Unable to generate the flame graphsThe performance report has correctly been generated, but there was an internal error while generating the flame graphs for this run. We're working on fixing the issue. Feel free to contact us on Discord or at support@codspeed.io if the issue persists. |
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