Repository navigation
fix(ci): read the process table without forking ps in the exec tracker (D-1008) - #295
Merged
Merged
Conversation
…ing ps (D-1008) The exec funnel's descendant tracker forked ps -A up to three times per snapshot, and the process identity marker and Linux group probe forked ps again. A no-op funnel job cost about 31 ms; it now costs about 2.4 ms. Snapshots read /proc on Linux and sysctl kern.proc.all on darwin, one read per call; ps stays as the fallback. Markers written by older builds (ps lstart text) are still compared through ps.
…l stable, bind start ticks to boot KillAttemptGroup compared only the new st: identity, so a marker written by an older build (ps lstart text) looked like a reused PID and its group was left running. Both kill paths now share sameProcess, which reads the live identity in the marker's own form and treats an unreadable one as unknown, not different. The tracker snapshot repeats (at most three passes) while a pass finds new processes, as the ps version did, so forks during the read are caught. Linux start ids carry the boot id so ticks cannot match across a reboot. Adds identity, legacy-marker, reused-PID and nested-descendant tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
D-1008: the exec funnel tracker forked
ps -Aup to three times per snapshot. It now reads the process table from/proc(Linux) orsysctl kern.proc.all(darwin) and falls back topsonly if that fails. Per-job process identity is the fork-freest:<start>form (Linux adds the boot id); legacyps lstartmarkers are still compared.Review fixes on top of the Codex branch
KillAttemptGroupcompared only the new identity form, so a marker written by an older build looked like a reused PID: its group was left running and the marker removed. Both kill paths now use onesameProcesshelper that reads the live identity in the marker's own form; an unreadable one is "unknown", not "different".Verified
go vet,go test ./internal/exec/... ./internal/hygiene/pass on darwin (local, mm6) and linux (ps1). Existing cancel/timeout/detached-grandchild teardown tests unchanged and green.Still open
planRevisionstill makes about 9 funnel git calls, not merged into one invocation, and the cibench ratio still misses (below), so D-1008 stays open.cibench (V15 / v1.4)
Measured after merge on ps1 (linux/amd64, 16 vCPU),
cibench -runs 20 -warmup 2, harness fromp7/P7-CI-46(#282) merged onto main at8c7550c1in a scratch worktree, load average 4.4 at start. Numbers are from theperfbench/v1result.ci.exec_overheadp50ci.e2e_tiny.v14p50ci.e2e_tiny.v15p50ci.e2e_medium.v14/v15p50A first run under load average 34 gave the same tiny medians (v1.4 295.0, V15 2206.2 ms), so the figure is not load noise.
Result: the per-job funnel overhead dropped about 3x and an strace of one tiny V15 run shows zero
psexecs, but the end-to-end V15 time did not improve and is higher than the CI-46 baseline (main has moved since revision8ecef39e). The ratio check still fails (ci.e2e_tiny.v15limit 577 ms), so P7-CI-46 cannot drop its ratio exception on this PR alone and D-1008 stays open. The remaining cost is not the tracker: an strace of one tiny run shows 26 git, 28 sh, repeateddocker info(plus buildx/compose plugin metadata execs, 5 times) and 3go versionexecs besides the checks themselves.