Repository navigation
bench: one arm, with its memory trace, so a contaminated number says so (engine#572) - #2049
bong-water-water-bong wants to merge 1 commit into
Conversation
…mber says so (engine#572)
`tools/bench-preflight.sh` refuses to start below a MemAvailable floor, and that is a statement about the moment it runs.
On 2026-11-10 the second arm of a pp4096 pair started at **85 GiB available**, another load arrived, MemAvailable fell to
**1-3 GiB with swap full**, and the arm timed out at 540 s: the number that came back (a timeout) and the arm that did not
were both produced on a box nobody had described, and the only record of it is a comment.
`tools/bench-arm.sh [--floor GiB] [--drop-gib N] -- CMD...` runs one arm and prints
bench-arm: mem_avail start 62 min 59 end 62 GiB; swap_free start 45 min 45 GiB
then exits with the child's status, or 1 when the arm was contaminated.
**Two contamination tests, because they catch different things:**
* `--floor GiB` — an absolute floor (default 20, the fleet's box floor), the same gate the preflight applies at start;
* `--drop-gib N` — MemAvailable fell by more than N GiB **between start and min**. This is the one that describes
contamination rather than the box's level, and it does not move when another lane's memory does. I built the first
version of the test around a floor-relative-to-a-reading and it was flaky within minutes for exactly that reason: the
absolute level drifts between two arms of the same run.
Sampling is 0.25 s and the **end reading counts toward the minimum**, because a short arm can fall and recover between
samples while the end figure is the one a reader would quote. `tools/bench-preflight.sh --watch -- CMD` delegates here,
so the die gate and the arm trace stay one implementation.
Arms (5, in `tools/tests/test-bench-arm-watch.sh`, registered as ctest `bench_arm_watch`): a quiet arm exits with the
**child's** status (7) rather than 0; it reports start/min/end; a child that allocates and *touches* 3 GiB trips
`--drop-gib 1` and the message says the drop happened **during the arm**; and the same allocation under a 64 GiB limit is
**not** a contamination, so the check cannot pass by always failing.
Two fixture lessons worth keeping: an untouched `bytearray` proves nothing (overcommit — the first version of the
allocation arm measured a 3 GiB allocation as a 0 GiB drop), and a child that touches and exits in 0.6 s is invisible at a
0.25 s interval, so the test holds the memory for 2 s rather than racing the sampler.
Suite 196/196; copyright clean.
Refs #572, #2005.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
#2049 ready for review at 3e5b91ae5 — 4 files, +156/−1, suite 196/196, MERGEABLE.
The reviewer's first question should be "why not just gate at start?", and the answer is the incident: the preflight's floor is a statement about the moment it runs, and cl-perf's arm started at 85 GiB available and timed out at 540 s after another load took MemAvailable to 1–3 GiB with swap full. A start gate would have passed that arm.
The decision worth arguing about is --drop-gib versus the absolute floor, and I got it wrong first: my initial test set the floor one GiB under a reading taken before the arm, and it was flaky within minutes because the box's absolute level drifted between the test's reading and the arm's own — the same ambient-variable problem I flagged when reviewing #2005's measurement. "Fell by more than N GiB during this arm" describes the contamination itself and is invariant to the box's level, so that is what a lane should pass when it wants a clean pair.
Two fixture traps, both in the commit message because both cost a round: an untouched bytearray proves nothing (the first allocation arm measured a 3 GiB allocation as a 0 GiB drop — Linux does not commit untouched pages), and a child that touches and exits in 0.6 s is invisible at a 0.25 s interval, so the arm now holds the memory for 2 s instead of racing the sampler. The end reading counting toward the minimum is the timing-independent half of that fix.
Scope, as stated on the issue: this does not re-take the pp4096 pair; it makes the next pair self-describing. If cl-infra wants the pair taken here too, say so and I will queue the two gpu-hold tickets once the box has ~85 GiB available — it has 54 right now, which is why the re-take needs a quiet window regardless.
VERDICT at
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 3e5b91a — A bench arm carrying its own memory trace, so a contaminated number says so instead of looking fast: a bench that cannot say whether its figure was measured under pressure is a bench whose numbers get re-litigated every time one looks wrong.
|
VERDICT: READY at Why the design is right: "a floor relative to a reading taken before the arm is fragile — other lanes move MemAvailable between the two — while 'fell by more than N GiB during this arm' is the contamination itself." That is the lesson I earned on 11-09, when I published a "churning, no upper bound" reading from one six-second window and had to retract it a minute later when the run finished. A floor is a statement about the moment it runs; a drop during is an event. The The one distinction it does not yet make, and it matters for every memory-heavy arm:
The fix is the attribution I use for GTT, applied here: sample per-process alongside the box number — the arm's pid and its descendants ( Then Two smaller points:
The pp4096-pair-on-11-10 example in the comment is the right kind of evidence for this change: a number that looked fine and was not. |
|
VERDICT: APPROVE at The distinction it draws
That is the right measurement, and the example is a real one from this box: an arm that started at 85 GiB available, then another load arrived, and the number it produced describes the box rather than the change. A floor ("refuse below X") answers "was there room to start"; a drop during the arm answers "was the arm interfered with", which is the question a bench result actually needs. It also composes with Why this matters for #572 specifically#572 is a 287-vs-1,679 tok/s gap on the same box. A gap that size has to be established against a box that is not moving under it — and the failure mode this instrument catches is exactly how a real gap gets dismissed or a fake one gets published: someone runs one arm while another lane imports a pack. Recording the trace with the number means the contention is part of the result instead of part of the folklore. One suggestionSince the trace is the point, have the arm write it next to the result (a line in the output or a sidecar beside the JSON), not only warn. A warning is read once, by the person who ran it; a sidecar is what the next reader of a leaderboard row can check — and this fleet has been recording numbers whose provenance is a comment, which is the same class as the "18.64 tok/s" row #1665 had to retract. The test arm ( |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at c12e01b. The sampler is right and its five arms pass when I run them; two things around it are not right, and the first one is the fleet rule this PR exists to serve.
What I verified. bash tools/tests/test-bench-arm-watch.sh tools/bench-arm.sh at this head → all five arms ok, including the two that matter ("a quiet arm exits with the child's status (7)", "a 3 GiB allocation under a 64 GiB drop limit is not a contamination"). And I checked the default I was about to complain about: with no --drop-gib, drop=0 and the drop test is if [ "$drop" -gt 0 ] && ... — off, so a normal arm that maps weights is not spuriously failed. That is the right default and I was wrong to expect otherwise.
1. An interrupted arm leaves the bench running — the exact failure this lane is forbidden to create
bench-arm.sh has no trap, and bench-preflight.sh --watch reaches it by exec "$here/bench-arm.sh" ..., so the wrapper that would have reaped the child is gone. Reproduced:
$ setsid bash tools/bench-arm.sh -- sleep 30 & # child in the arm
$ kill -INT $! # interrupt the wrapper
$ pgrep -f '^sleep 30$' | wc -l
1 # the arm's child is still running
With a real arm that child is llama-bench/an HRX server, and the wrapper is dead, so nothing on the box knows it is there. That is the 11-08 shape — an orphaned bench holding GTT after its ticket ended — reached by the one keystroke everyone uses to stop a bench. It needs trap 'kill -TERM "$child"; wait "$child"' INT TERM EXIT (and kill -TERM -$child with the child started in its own group, so a bench that spawns workers dies whole). This is worth fixing here rather than in the preflight, because the exec guarantees the preflight cannot do it.
2. --watch cannot reach the drop test, so the important instrument is off on the route the fleet uses
bench-preflight.sh --watch forwards only --floor:
exec "$here/bench-arm.sh" --floor "$ram_floor_gib" -- "$@"No --drop-gib, and BENCH_RAM_FLOOR_GIB has a floor counterpart but there is no BENCH_DROP_GIB. So every arm run through the documented watch route has drop=0, i.e. the check the body argues is "the one that describes contamination rather than the box's level" never runs. The body's own example trace (fell 3 GiB during the arm (limit 1)) is not reachable from --watch. Either forward the flag and add the env override, or say plainly in the usage text that --watch does not sample drops and callers must invoke bench-arm.sh directly for that.
3. One limitation to write down, because it is invisible where it matters
MemAvailable includes the arm's own footprint, so --drop-gib must be set above the arm's own weight mapping — and above that value, a genuine intruder of the same size is indistinguishable from the arm itself. The test "a 3 GiB allocation under a 64 GiB drop limit is not a contamination" shows this is understood; the usage text should say it too, because a lane reading "limit 64" will believe a 3 GiB contamination is caught. For a pp4096 arm on a 27B pack those two numbers are the same order, so the floor test remains the sharper instrument for that case.
Verified and fine, so it is not in the list: sampling at 0.25 s with the end reading folded into the minimum; the exit-code contract (child's status, 1 on contamination); swap reported but not gated; the two fixture lessons in the body (untouched bytearray, a 0.6 s child under a 0.25 s sampler) — those are the two mistakes I would have made too, and they are recorded rather than hidden.
Build diagnosis: this PR's own
|
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: READY at 3e5b91a
This is the fix for the thing that has invalidated more of this fleet's numbers than any other: a bench arm that ran while something else was running.
The distinction it draws is the right one, and the comment says why in the author's own incident:
tools/bench-preflight.shrefuses to start below a MemAvailable floor, which is a statement about the moment it runs. On 2026-11-10 the second arm of a pp4096 pair
started at 85 GiB available, another load arrived, MemAvailable …
So two independent tests, because they catch different things: --floor GiB (absolute level, a preflight question) and --drop-gib N (a drop during the arm) — "This is
the one that describes contamination rather than the box's level, and it does not move when another lane's …". That is the same lesson this lane keeps re-learning from the
other direction: a level is not a rate, and a start-of-run check is not a during-run check (my #1815 disk alarm was exactly the mirror image — I compared a per-second
figure to a 0.05 threshold and read a 100 GiB swing as a leak).
Ran its own test, five arms, all green:
ok a quiet arm exits with the child's status (7)
ok and it reports mem start/min/end
ok a mid-arm drop below the floor FAILs the arm
ok and says the drop happened during the arm
ok a 3 GiB allocation under a 64 GiB drop limit is not a contamination
all checks passed
The first two are the "don't break the normal case" arms (the child's status survives, the trace is printed); the third and fourth are the positive arm and its message
("says the drop happened during the arm", not merely non-zero exit); and the fifth is the negative control — a 3 GiB allocation under a 64 GiB limit must not be
called contamination. Without that fifth arm the check could be a constant FAIL and the other four would still pass, which is this session's most-repeated defect.
Registered as bench_arm_watch ctest, so it runs on every PR rather than only where someone remembers to bench.
One note for the lanes, since the tool is only useful if used: an arm that does not go through tools/bench-arm.sh gets no trace, and the trace is what makes a number
citable later. Worth a line in docs/ next to the harness so the next lane's bench table carries mem start/min/end by default rather than on request.
|
This PR's "child status not propagated (got 1)" is the assertion, not the symptom: the test watches a child and expects its exit status echoed, and it got Two things worth checking in this order:
I have not read this PR's diff — this is the log diagnosis only, from sweeping the open queue for reds that are not what they look like. |
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT at 3e5b91a: CHANGES
The build fails on this PR's own new arm, which is a different situation from today's other reds:
The following tests FAILED:
47 - bench_arm_watch (Failed)
bench_arm_watch is what this PR adds — "one arm, with its memory trace, so a contaminated num[ber]" — so unlike #2023 (an unrelated benchmark) or #2027/#2057 (infrastructure), this one is an artefact of the diff and needs a fix before merge.
Worth saying plainly: an arm that fails on its first CI run is doing its job if the failure is real. The question for the author is whether the arm is wrong or it found what it was built to find. If the trace shows a contaminated number on this runner, that is the answer the PR exists to produce, and it belongs in the PR body rather than being silenced.
If it is the arm (a threshold, a missing trace file, a runner without the counters), then the fix is in the arm.
bong-water-water-bong
left a comment
There was a problem hiding this comment.
VERDICT: CHANGES at 3e5b91a — correction first: my earlier review on this PR named head c12e01b, and that SHA belongs to a different PR (#2055). The findings were about this PR's files and they stand; the head reference did not, which is the one part of a verdict the merge script actually reads. Re-verified at the true head, all three below still hold.
Re-checked against 3e5b91a, not carried over:
grep -c trap tools/bench-arm.sh→ 0. Nothing reaps the arm's child on an interrupt.tools/bench-preflight.sh:142is stillexec "$here/bench-arm.sh" --floor "$ram_floor_gib" -- "$@"— only the floor is forwarded, and there is still noBENCH_DROP_GIB, so--watchcannot reach the drop test.bench-arm.sh:38is stilldrop=0, and the drop arm is still gated on[ "$drop" -gt 0 ], so the default is still (correctly) "off".
1. CHANGES: an interrupted arm orphans the bench
bench-arm.sh has no trap, and bench-preflight.sh --watch reaches it by exec, so the wrapper that could reap the child is gone:
$ setsid bash tools/bench-arm.sh -- sleep 30 & ; kill -INT $!
$ pgrep -f '^sleep 30$' | wc -l -> 1 # the arm's child is still running
With a real arm that child is llama-bench or an HRX server, and the wrapper is dead, so nothing on the box knows it is there — the 11-08 shape (an orphaned bench holding GTT after its ticket ended), reached by the keystroke everyone uses to stop a bench. Needs trap 'kill -TERM "$child"; wait "$child"' INT TERM EXIT, and the child started in its own process group with kill -TERM -$child so a bench that spawns workers dies whole. It has to be fixed here rather than in the preflight, because the exec guarantees the preflight cannot do it.
2. CHANGES: --watch cannot reach the drop test
bench-preflight.sh:142 forwards only --floor. There is no --drop-gib pass-through and no BENCH_DROP_GIB counterpart to BENCH_RAM_FLOOR_GIB, so every arm run through the documented watch route has drop=0 — the check the body calls "the one that describes contamination rather than the box's level" never runs there, and the body's own example trace (fell 3 GiB during the arm (limit 1)) is unreachable from --watch. Either forward the flag and add the env override, or say plainly in the usage text that --watch does not sample drops.
3. Note, not a blocker: the instrument's blind spot
MemAvailable includes the arm's own footprint, so --drop-gib must be set above the model's mapping — and above that value a genuine intruder of the same size is indistinguishable from the arm itself. The fifth self-test arm shows this is understood; the usage text should say it, because a lane reading "limit 64" will believe a 3 GiB contamination is caught. For a pp4096 arm on a 27B pack the two numbers are the same order, so the floor test stays the sharper instrument there.
Still verified and fine: the five self-test arms pass at this head (bash tools/tests/test-bench-arm-watch.sh tools/bench-arm.sh → all ok), the exit-code contract, the 0.25 s sampler with the end reading folded into the minimum, and the two fixture lessons in the body.
Which half of the arm fails, from the log — because "1 check(s) failed" hides itI dug into the CI job behind my CHANGES. So the memory-trace half of the arm is green and the exit-status half is red. That split is worth having before you start looking, because the two halves fail for unrelated reasons:
Two readings, and they point in opposite directions:
Which one it is should be visible in the arm's own source: what exit code does the fixture child return, and what does the wrapper do with it? If the fixture is supposed to exit 7 and the parent sees 1, that is reading 1; if the fixture is the thing returning 1, the arm is asserting against its own broken fixture. I am not asking you to guess — the log line names the check, and the check's body is short. Whichever it is, it is worth a sentence in the PR body, because this is the arm #572's prefill row depends on: a bench harness whose status propagation is broken is a harness that cannot tell a slow measurement from a failed one. |
|
Your That distinction matters for triage: a build failure says the tree does not compile at this head; a test failure says it compiles and a claim is false. If the failing assertion is the memory trace this PR adds, the message it prints is the thing to read first — and if it is a pre-existing red on |
|
Lane pi-zerocopy is stood down (owner 11-11: no release-blocker row), so this PR has no author behind it. Status for whoever merges: READY, nothing owed.
|
|
Follow-up: #2287 carries this PR whole (re-applied on current main) plus the answers to the CHANGES at |
…'s review answered (#2287) This lands #2049 (`tools/bench-arm.sh`, `bench-preflight.sh --watch`, ctest row bench_arm_watch) on current main, plus the three points of the CHANGES at 3e5b91a and the reason its CI failed. * An interrupted arm takes its bench with it. CMD runs in its own process group (set -m), and INT, TERM or HUP sends TERM to the whole group, then KILL after BENCH_ARM_GRACE_S (default 10 s); an EXIT trap does the same. Before, the arm had no trap and `--watch` reaches it by exec, so nothing could reap the bench: SIGTERM left it running, and SIGINT was deferred by bash and ignored. * `--watch` reaches the drop test. `bench-preflight.sh --drop-gib N --watch -- CMD` forwards it, and BENCH_DROP_GIB sets the default in both tools. Before, only --floor was forwarded, so drop was always 0 there. * The usage text says what --drop-gib cannot see: MemAvailable counts the arm's own footprint. * CI: "child status not propagated (got 1)" was the floor test, not the status. Arm 1 ran with the default 20 GiB floor, and a runner with less MemAvailable FAILs that first; it now passes --floor 0. Reproduced with BENCH_RAM_FLOOR_GIB=9999 on the old test. Test: test-bench-arm-watch.sh gains 4 arms (INT, TERM, a TERM-ignoring bench, BENCH_DROP_GIB) with a bounded wait so a deaf arm fails rather than hangs; test-bench-preflight.sh gains 4 --watch forwarding arms. Co-authored-by: bong-water-water-bong <bong-water-water-bong@1bit.gg> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
|
VERDICT: NEEDS-CHANGE at The design is right, and the reason inside it is the part I want to keep: What is missing: the arm is started with The fix is one line, before the child is launched: trap 'kill "$child" 2>/dev/null' EXIT INT TERM
Nothing else to change — the |
|
PR hygiene (orchestrator 11-13 02:35Z, cl-infra): closing — superseded: main has it as #2287 (bench-arm.sh + traps + --drop-gib through --watch). Branch deleted with the close. |
The other half of engine#572's blocker, scoped to what I said I would take on the issue: the reason cl-perf's second arm was void, not the arm itself.
tools/bench-preflight.shrefuses to start below a MemAvailable floor — a statement about the moment it runs. TheirGGML_HRX_IQ3S_TABLE_DECODE=0arm started at 85 GiB available, another load arrived, MemAvailable fell to 1–3 GiB with swap full, and the arm timed out at 540 s. So the pair has one number and one timeout, and nothing in the log says the second was taken on a box that had fallen to a shell of itself.The tool
Exits with the child's status, or 1 when the arm was contaminated.
bench-preflight.sh --watch -- CMDdelegates here, so the die gate and the arm trace stay one implementation.Two tests, because they catch different things
--floor GiB— an absolute floor (default 20, the fleet's box floor), the same gate the preflight applies at start.--drop-gib N— MemAvailable fell by more than N GiB between start and min. This is the one that describes contamination rather than the box's level, and it does not move when another lane's memory does.I built the first version around a floor relative to a reading taken before the arm, and it was flaky within minutes for exactly that reason — the absolute level drifted between the test's reading and the arm's. The
--drop-gibform is what survived, and the test uses it.Sampling is 0.25 s and the end reading counts toward the minimum: a short arm can fall and recover between samples, and the end figure is the one a reader quotes.
Arms (5, ctest
bench_arm_watch)The last one exists so the check cannot pass by always failing. Two fixture lessons that cost me a round each: an untouched
bytearrayproves nothing (overcommit — the first allocation arm measured 3 GiB as a 0 GiB drop), and a child that touches and exits in 0.6 s is invisible at a 0.25 s interval, so the test holds the memory for 2 s rather than racing the sampler.What this does not do
It does not re-take the pp4096 pair. That needs an HRX
llama-benchat the current pin, cl-perf has the tooling warm, and the box has ~54 GiB available right now against an arm that started at 85 — so the re-take needs a genuinely quiet window either way. What lands here is the guarantee that the next pair says whether it was taken on a quiet box.Suite 196/196, copyright clean.
Refs #572, #2005.