From d098635da23a20993cec38fbae149ba56e2a2098 Mon Sep 17 00:00:00 2001 From: agent Date: Tue, 10 Nov 2026 07:44:04 -0400 Subject: [PATCH] bench: the preflight observes the load mode it asked for, not just prints it (engine #1668) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `--dispatch` path pins `-lm`, validates it against llama-bench's own set, prints it in the header, and has arms asserting the flag reached the bench's argv. Nothing checked that the run **did** it: a bench that ignored `-lm` (the flag is a default, not a contract) produced a tally the header then mislabelled, which is the same defect the pin exists to prevent — one variable measured, two reported. llama.cpp puts a model it mmaps in `/proc//maps` and a model it only reads is not there, so the mode is observable rather than trusted. The bench now runs in the background and the preflight polls that process's maps while it lives; a mode whose mapping disagrees with the flag is refused: -lm none that actually mmapped is refused rc=1 -lm mmap that only read is refused rc=1 observed mmap agrees with -lm mmap rc=0 observed read agrees with -lm none rc=0 Only the unambiguous modes are asserted (`mmap`/`mmap+mlock` map, `none`/`dio` do not); `mlock` alone is left unasserted because whether it maps depends on the loader, and a gate that guessed would fail a correct run. An observed mapping is trusted on its own, but an absent one only when the process was seen alive after the sample — a bench that maps and exits between polls must not be reported as a mismatch, and if it finishes before any sample the line is a WARN, not a failure. Test: five arms in tools/tests/test-bench-preflight.sh driving the real script against the fake world, with a fake bench that really maps or really reads the model (FAKE_BENCH_LM makes it disagree with `-lm`). 21 passed, 0 failed; against the unpatched script the three mismatch/reason arms fail and the two agreement arms still pass, which is what makes them controls rather than decoration. Refs #1668 --- tools/bench-preflight.sh | 31 +++++++++++++++++- tools/tests/test-bench-preflight.sh | 50 +++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/tools/bench-preflight.sh b/tools/bench-preflight.sh index be3c23f18..ea3fc8789 100755 --- a/tools/bench-preflight.sh +++ b/tools/bench-preflight.sh @@ -167,7 +167,36 @@ case "$lm" in *) fatal "PREFLIGHT_LM='$lm' is not a llama-bench load mode (none|mmap|mlock|mmap+mlock|dio); the mode is part of the measurement, so it is not guessed (engine #1668)" ;; esac GGML_HRX_LOG_DISPATCH=1 "$bench" -m "$model" -p "$tokens" -n 0 -ngl 99 --device HRX0 -r 3 -lm "$lm" \ - >/dev/null 2>"$log" + >/dev/null 2>"$log" & +bench_pid=$! +# The mode is REQUESTED above and OBSERVED here (engine #1668): llama.cpp puts a model it mmaps in `/proc//maps` and a +# model it only reads is not there, so the run can be checked against the flag instead of trusted to honour it. A bench that +# ignores `-lm` yields a number the header mislabels, which is the same defect as an unstated mode -- one variable measured, +# two reported. +mapped=no; alive=0 +while :; do + grep -qF -- "$(basename "$model")" "/proc/$bench_pid/maps" 2>/dev/null && { mapped=yes; break; } + kill -0 "$bench_pid" 2>/dev/null || break + alive=$((alive + 1)) + sleep 0.1 +done +bench_rc=0 +wait "$bench_pid" || bench_rc=$? +[ "$bench_rc" -eq 0 ] || fatal "the bench exited $bench_rc; see $log" +# Only the modes whose mapping is unambiguous are asserted: `mmap`/`mmap+mlock` map, `none`/`dio` do not, and `mlock` alone is +# not asserted because whether it maps depends on the loader -- a gate that guessed would fail a correct run. An OBSERVED +# mapping is trusted on its own; an ABSENT one is only trusted when the process was seen alive after the sample, since a +# bench that maps and exits between polls would otherwise be reported as a mismatch. +if [ "$mapped" = no ] && [ "$alive" -eq 0 ]; then + printf ' WARN the bench finished before the load mode could be observed; -lm %s not checked\n' "$lm" +else + case "$lm:$mapped" in + mmap:no|mmap+mlock:no) + fatal "the load mode is -lm $lm but the model is not in /proc/$bench_pid/maps; the run is not the measurement the header claims (engine #1668)" ;; + none:yes|dio:yes) + fatal "the load mode is -lm $lm but the model IS in /proc/$bench_pid/maps; the run is not the measurement the header claims (engine #1668)" ;; + esac +fi lines=$(grep -c 'ggml_hrx dispatch:' "$log" || true) # A silent bench is the failure this flag exists to catch: an empty tally looks like "nothing declined" diff --git a/tools/tests/test-bench-preflight.sh b/tools/tests/test-bench-preflight.sh index 76d26d5c8..24bb741d1 100755 --- a/tools/tests/test-bench-preflight.sh +++ b/tools/tests/test-bench-preflight.sh @@ -167,5 +167,55 @@ else printf 'FAIL %-38s header does not name the load mode\n%s\n' "tally header states -lm" "$(cat "$tmp/lastout")"; fail=$((fail+1)) fi +# --dispatch must also OBSERVE the load mode, not only request it (engine #1668): llama.cpp puts a model it mmaps in +# `/proc//maps` and a model it only reads is not there. The bench below really maps or really reads the model and then +# sleeps, so the observation has something to see; FAKE_BENCH_LM lets a case make the bench ignore `-lm`, which is the defect +# the check exists for (a number the header mislabels). +head -c 65536 /dev/urandom > "$tmp/models/big.gguf" # non-empty: mmap of a zero-length file fails +cat > "$tmp/mapbench" <<'PY' +#!/usr/bin/env python3 +import mmap, os, sys, time +a = sys.argv[1:] +lm = a[a.index('-lm') + 1] if '-lm' in a else 'mmap' +model = a[a.index('-m') + 1] +mode = os.environ.get('FAKE_BENCH_LM') or lm # a case sets this to make the bench disagree with -lm +handle = keep = None +if mode in ('mmap', 'mmap+mlock'): + handle = open(model, 'rb') + keep = mmap.mmap(handle.fileno(), 0, access=mmap.ACCESS_READ) +else: + open(model, 'rb').read(4096) +sys.stderr.write('ggml_hrx dispatch: common.mul_mat.q4_k_q8_1_x4_prefill\n') +sys.stderr.write('| qwen3-4b | 2.30 GiB | 4B | HRX | 99 | 1 | pp512 | 3181.97 t/s |\n') +time.sleep(0.7) +assert keep is None or len(keep) == 65536 # the mapping must outlive the sleep +PY +chmod +x "$tmp/mapbench" + +lm_case() { # lm_case [FAKE_BENCH_LM] + local want=$1 name=$2 lm=$3 fake_lm=${4:-} root out rc got + root=$(fake 3600 0 60) + out=$(PREFLIGHT_LM=$lm FAKE_BENCH_LM=$fake_lm PREFLIGHT_MODEL=$tmp/models/big.gguf \ + patched "$root" --dispatch "$tmp/mapbench" 512 2>&1); rc=$? + got=ok; [ $rc -ne 0 ] && got=no + if [ "$got" = "$want" ]; then + printf 'ok %-38s rc=%d\n' "$name" "$rc"; pass=$((pass+1)) + else + printf 'FAIL %-38s rc=%d want=%s\n%s\n' "$name" "$rc" "$want" "$out"; fail=$((fail+1)) + fi + printf '%s' "$out" > "$tmp/lastlm" +} + +lm_case ok "observed mmap agrees with -lm mmap" mmap +lm_case ok "observed read agrees with -lm none" none +lm_case no "-lm none that actually mmapped is refused" none mmap +lm_case no "-lm mmap that only read is refused" mmap none +# The refusal has to say what it saw, or the reader cannot tell a mapping mismatch from a bench that never started. +if grep -q 'is not the measurement the header claims' "$tmp/lastlm"; then + printf 'ok %-38s names the mismatch\n' "the refusal names its reason"; pass=$((pass+1)) +else + printf 'FAIL %-38s missing reason\n%s\n' "the refusal names its reason" "$(cat "$tmp/lastlm")"; fail=$((fail+1)) +fi + printf '\nbench-preflight test: %d passed, %d failed\n' "$pass" "$fail" [ "$fail" -eq 0 ]