From 8f4d384b6994e507cfe5ee70eb6a8504019f4ecf Mon Sep 17 00:00:00 2001 From: "John William Creighton (s243a)" Date: Tue, 15 Sep 2026 22:27:51 -0600 Subject: [PATCH 1/3] cpp_store fix-forward: mmap .idx (no heap slurp), real lmdb probe, rows_per_key gate Fixes Fable's review of the merged store-backend change (#4270). Fix-forward on a new branch; the merged commit is untouched. P2a -- mmap the .idx instead of slurping it into heap. The in-memory index was a whole-file std::string (idx_blob_): non-evictable ANONYMOUS heap, and the .idx is 39-97% of the store. Since auto routes >2x-RAM stores to indexed, under that pressure indexed tried to allocate ~1x MemAvailable of non-evictable heap -> bad_alloc -> caught by step_execution as goal failure -> SILENT under-answering. Now .idx is mmap'd (PROT_READ, MAP_PRIVATE) exactly like .data: page-cache-backed + evictable, in-memory binary search reads through idx_base_/idx_len_ (bounds- checked idx_u32/idx_u16), ifstream slurp kept only as the non-POSIX fallback. Also: the mmap path charges nothing to the D43 counters at open (was charging the full index size); the false "small and constant" comment is corrected; munmap + fd close in the destructor (close_indexed()). P2b -- auto "lmdb usable" probe now matches what the C++ build needs: not just that the npm module loads, but that system liblmdb links (#include + -llmdb) and, best-effort, that an already-built lmdb store under DIR/lmdb/* actually mdb_env_opens (catches MDB_INVALID). Falls back to indexed on any failure; message reworded to what is probed. rows_per_key gate -- auto now picks lmdb only when store>2xRAM AND rows_per_key >= UW_STORE_LMDB_MIN_ROWS_PER_KEY (default 2), computed cheaply from the UWIX .idx headers (n_records/n_keys, no scan). A ~1-row/key store (ABI symprov = 1.03) stays on indexed even when huge (lmdb buys nothing). Unknown rpk (pre- build, no .idx) skips the gate (size-only). P3 + nits: guard tellg()<0 in ensure_open (was resize(SIZE_MAX) on a failed retry) and close streams/mmaps on throw paths (close_indexed); close data_fd_/idx_fd_ right after a successful mmap; bench_crossover.sh now ASSERTS rows_found identical across backends per cell (was print-only); validate UW_STORE_LMDB_RAM_FACTOR is an integer (else warn+default 2); test_auto_select.sh stubs the lmdb-usable probe so it can never trigger a real npm install, and adds rows_per_key gate cases; noted stat/od are GNU/coreutils. Guardrails: 503 differential + 51 corpus + 122 ABI verify all 0 (built -O0/nice, one at a time); goldens re-baselined (plain 96529->99808, lmdb 96770->100049; +3279 each, gate-independent); mmap-indexed rows_found == lmdb on a scale-store cross-check; test_auto_select ALL PASS (incl rpk=1->indexed, rpk=3->lmdb). Frozen resolver/store/debian untouched. Docs updated (BACKEND_SELECTION.md: rule now with rows_per_key + real probe, storage note on the mmap fix, key-sort deferred). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RoXjhStCqoig6944pVNBGe --- .../abi/bench/BACKEND_SELECTION.md | 64 ++++--- .../pkg_resolver/abi/bench/bench_crossover.sh | 16 ++ examples/pkg_resolver/store/ensure_lmdb.sh | 88 ++++++++-- .../pkg_resolver/store/test_auto_select.sh | 90 +++++----- templates/targets/cpp_wam/runtime.h.mustache | 161 +++++++++++++----- tests/test_wam_cpp_templates.pl | 18 +- 6 files changed, 316 insertions(+), 121 deletions(-) diff --git a/examples/pkg_resolver/abi/bench/BACKEND_SELECTION.md b/examples/pkg_resolver/abi/bench/BACKEND_SELECTION.md index 55c58df32..5daf9e387 100644 --- a/examples/pkg_resolver/abi/bench/BACKEND_SELECTION.md +++ b/examples/pkg_resolver/abi/bench/BACKEND_SELECTION.md @@ -18,9 +18,10 @@ the decision doc: what we learned, the policy, and how to refine it. - lmdb only pulls ahead in one regime: **disk-bound (store > RAM) AND multi-row-per-key**, where its key-clustered leaves save cold seeks. The asymptotic win there is **≈ rows_per_key**. -- **Default policy (`auto`): pick lmdb iff `store_size > 2 × available_RAM` and - lmdb is usable, else indexed.** Deliberately conservative and **size-only** for - now (a rows-per-key refinement is deferred; see below). +- **Default policy (`auto`): pick lmdb iff `store_size > 2 × available_RAM` + AND `rows_per_key ≥ 2` AND lmdb is usable for the C++ lane, else indexed.** + Deliberately conservative; the `rows_per_key ≥ 2` gate keeps ~1-row/key stores + (ABI symprov = 1.03) on indexed even when huge, because lmdb buys nothing there. ## Theory: the cost model @@ -99,41 +100,60 @@ Implemented in `examples/pkg_resolver/store/ensure_lmdb.sh` ``` choose LMDB iff store_size_bytes > UW_STORE_LMDB_RAM_FACTOR × available_RAM_bytes - AND lmdb is usable (uw_ensure_lmdb succeeds) + AND rows_per_key >= UW_STORE_LMDB_MIN_ROWS_PER_KEY + AND lmdb is usable for the C++ lane else INDEXED ``` - `UW_STORE_LMDB_RAM_FACTOR` — the headroom factor, **default 2** (named constant, - tunable). 2× is deliberately conservative: the model puts the *onset* at ~1× - RAM and skew pushes it higher, so 2× only trips lmdb once the store clearly - exceeds RAM and disk-bound misses are unavoidable. + tunable; a non-integer value is rejected with a warning and treated as 2). 2× is + deliberately conservative: the model puts the *onset* at ~1× RAM and skew pushes + it higher, so 2× only trips lmdb once the store clearly exceeds RAM and + disk-bound misses are unavoidable. +- `UW_STORE_LMDB_MIN_ROWS_PER_KEY` — **default 2**. `rows_per_key` is aggregated + cheaply from the UWIX `.idx` headers (`n_records / n_keys`, no scan) across all + indexes in the store dir. Because the asymptotic lmdb win is ≈ rows_per_key, a + ~1-row/key store (ABI symprov = 1.03) never benefits — this gate keeps it on + indexed even above 2× RAM. When no `.idx` exists yet (pre-build), rows_per_key + is *unknown* and the gate is skipped (size-only for that first build). - `available_RAM` — `/proc/meminfo` `MemAvailable`, overridable with `UW_STORE_AVAIL_RAM_BYTES` (WSL2's `MemAvailable` balloons, so the override matters for tests/reproducibility). - `store_size` — the **built** indexed store (`*.data` + `*.idx`) when present, else the source **P/2 JSONL** (`*.jsonl`, excluding `cases.jsonl`) as a pre-build estimate. One consistent measure; documented here. -- **Fallback:** if the size rule wants lmdb but lmdb is not usable - (`uw_ensure_lmdb` fails / `MDB_INVALID`), it **WARNs loudly and uses indexed** — - safe because indexed is answer-identical and ~as fast up to the disk-bound - multi-row regime. It prints the chosen backend and the size-vs-`2×RAM` numbers. +- **`lmdb usable` for the C++ lane** is a real probe, not just "the npm module + loads": (1) `uw_ensure_lmdb` (the v1-format module used to *build* the store), + (2) system `liblmdb` links (`#include ` + `-llmdb`) — the C++ reader + needs it, and (3) best-effort: an already-built lmdb store under `DIR/lmdb/*` + actually `mdb_env_open`s (catches `MDB_INVALID` from a wrong page format). +- **Fallback:** if the rule wants lmdb but it is not usable, it **WARNs loudly and + uses indexed** — safe because indexed is answer-identical and ~as fast up to the + disk-bound multi-row regime. It prints the chosen backend and the numbers. - **Policy only, never answers.** Whichever backend is chosen returns identical rows. Proven: 503-case store differential + 51-case corpus stay **0 - divergences** (corpus verified through the auto path → indexed), and - `test_auto_select.sh` checks the rule returns lmdb above 2× and indexed below - (via `UW_STORE_AVAIL_RAM_BYTES`). + divergences** (corpus verified through the auto path → indexed); + `bench_crossover.sh` asserts `rows_found` identical across backends in every + cell; and `test_auto_select.sh` checks the size rule and the rows_per_key gate + (lmdb above 2× with rpk≥2, indexed below 2× or at rpk<2) via the RAM override. + +## Storage note: the index is mmap'd, not slurped + +The indexed backend **mmaps** both `.idx` and `.data` (`PROT_READ`, +`MAP_PRIVATE`), page-cache-backed and **evictable**. An earlier version slurped +the whole `.idx` into a heap `std::string` — but the `.idx` is **39-97% of the +store**, so that allocated ~that much *non-evictable anonymous* memory. Under the +exact pressure that routes a `> 2× RAM` store to indexed, the slurp risked +`bad_alloc` → caught as a goal failure → *silent under-answering*. mmap restores +the O(1)-heap, page-cache-friendly behavior (with an `ifstream` fallback for +non-POSIX). It also fixes the D43 counters (the mmap path charges nothing at open; +only actual record reads count). ## Future refinement (deferred, per the owner) -The size-only rule is conservative but coarse. Two cheap improvements, explicitly -deferred: +One cheap improvement remains, explicitly deferred: -1. **Fold in rows_per_key.** The asymptotic benefit is ≈ rows_per_key, so for - ~1-row-per-key stores lmdb is *never* worth it even above 2× RAM. The ABI - `symprov` store is **1.03 rows/key** (249,097 keys / 256,225 rows) — cheaply - computable from the `.idx` header (keys vs records) with no scan. A refined - selector should require `rows_per_key ≳ 2` in addition to the size trigger. -2. **Key-sort the indexed `.data`.** Indexed's only structural disadvantage is +1. **Key-sort the indexed `.data`.** Indexed's only structural disadvantage is source-order scatter (M scattered pages per multi-row key). Building `.data` in **key order** clusters a key's rows into ~1 page (measured: an 8-row/key store drops from 8 to **1.05** pages/key), erasing lmdb's disk-bound edge diff --git a/examples/pkg_resolver/abi/bench/bench_crossover.sh b/examples/pkg_resolver/abi/bench/bench_crossover.sh index ae1ec6195..f9a3e5726 100755 --- a/examples/pkg_resolver/abi/bench/bench_crossover.sh +++ b/examples/pkg_resolver/abi/bench/bench_crossover.sh @@ -147,6 +147,22 @@ for SCALE in $SCALES; do run_cell "$RES" lmdb lmdb "$OUT/bench_lmdb" "$LMDB" "$WLDIR" "$wl" "$R" "$ev" done; done; done echo "== wrote $RES ==" + # ASSERT answer-identity: for each (workload,R,evict) cell every backend must + # return the same rows_found (a backend choice must never change answers). + node -e ' +const fs=require("fs"); +const rows=fs.readFileSync(process.argv[1],"utf8").split("\n").filter(Boolean).map(JSON.parse); +const by={}; let bad=0; +for(const o of rows){const k=`${o.workload}|R${o.R}|ev${o.evict}`;(by[k]=by[k]||[]).push(o);} +for(const k of Object.keys(by)){ + const g=by[k], want=g[0].rows_found; + for(const o of g) if(o.rows_found!==want){ + console.error(` ROWS MISMATCH ${k}: ${o.label}=${o.rows_found} vs ${g[0].label}=${want}`); bad=1; + } +} +if(bad){console.error("== ROWS_FOUND NOT IDENTICAL ACROSS BACKENDS =="); process.exit(1);} +console.log("== rows_found identical across backends for every cell ("+Object.keys(by).length+" cells) =="); +' "$RES" done echo "store sizes:"; du -h "$OUT/idx/symprov.data" "$OUT/idx/symprov.idx" "$OUT/lmdb_v1/symprov/data.mdb" \ "$SCALE_DIR/pkg.data" "$SCALE_DIR/pkg.idx" "$PB/lmdb_pkg/data.mdb" 2>/dev/null | sed 's/^/ /' diff --git a/examples/pkg_resolver/store/ensure_lmdb.sh b/examples/pkg_resolver/store/ensure_lmdb.sh index 55126c6a2..738b9de34 100755 --- a/examples/pkg_resolver/store/ensure_lmdb.sh +++ b/examples/pkg_resolver/store/ensure_lmdb.sh @@ -79,10 +79,15 @@ uw_require_lmdb() { # backends return identical rows). Documented in # examples/pkg_resolver/abi/bench/BACKEND_SELECTION.md. # -# choose LMDB iff store_size_bytes > FACTOR * available_RAM_bytes AND lmdb -# is usable; else INDEXED. FACTOR (UW_STORE_LMDB_RAM_FACTOR, default 2) is a -# conservative headroom over the ~1x-RAM crossover onset. Size-only for now -# (a future refinement folds in rows_per_key -- see the doc). +# choose LMDB iff store_size_bytes > FACTOR * available_RAM_bytes +# AND rows_per_key >= MIN_RPK (lmdb buys ~nothing at ~1 row/key) +# AND lmdb is usable for the C++ lane (system liblmdb links, and a +# built lmdb store, if present, opens) +# else INDEXED. FACTOR (UW_STORE_LMDB_RAM_FACTOR, default 2) is conservative +# headroom over the ~1x-RAM crossover onset; MIN_RPK +# (UW_STORE_LMDB_MIN_ROWS_PER_KEY, default 2) reflects that the asymptotic lmdb +# win is ~= rows_per_key, so a 1-row/key store (e.g. ABI symprov, 1.03) never +# benefits. NOTE: stat/od are GNU/coreutils (Linux); fine for this lane. # --------------------------------------------------------------------------- # Sum the store bytes under DIR: the BUILT indexed store (*.data + *.idx) when @@ -103,6 +108,26 @@ uw_store_size_bytes() { # DIR -> bytes echo "$total" } +# Aggregate rows_per_key across the store's UWIX indexes (cheap: read the header +# of each *.idx -- n_keys at byte 8, n_records at byte 20, both u32 LE). Echoes a +# 2-dp float, or "" when no index exists yet (pre-build -> caller treats as +# unknown and does not gate on it). +uw_store_rows_per_key() { # DIR -> float | "" + local dir="${1:?usage: uw_store_rows_per_key DIR}" recs=0 keys=0 f had=0 nk nr + shopt -s nullglob + for f in "$dir"/*.idx; do + nk=$(od -An -tu4 -j8 -N4 "$f" 2>/dev/null | tr -d ' ') + nr=$(od -An -tu4 -j20 -N4 "$f" 2>/dev/null | tr -d ' ') + if [ -n "$nk" ] && [ -n "$nr" ]; then keys=$(( keys + nk )); recs=$(( recs + nr )); had=1; fi + done + shopt -u nullglob + if [ "$had" -eq 1 ] && [ "$keys" -gt 0 ]; then + awk "BEGIN{printf \"%.2f\", $recs/$keys}" + else + echo "" + fi +} + # Available RAM in bytes: UW_STORE_AVAIL_RAM_BYTES override (WSL2 MemAvailable # balloons, so an override matters for testing/reproducibility) else # /proc/meminfo MemAvailable. @@ -113,6 +138,36 @@ uw_available_ram_bytes() { echo $(( kb * 1024 )) } +# Is lmdb actually usable for the C++ store lane? Checks what the C++ build/read +# needs -- NOT just that the npm module loads: +# 1. uw_ensure_lmdb (the v1-format npm module, used to BUILD the store), +# 2. system liblmdb links ( + -llmdb) -- the C++ reader needs it, +# 3. best-effort: an already-built lmdb store under DIR/lmdb/* OPENS with +# vanilla liblmdb (catches MDB_INVALID from a wrong page format). +# Returns 0 if usable, non-zero otherwise. All output suppressed. +uw_lmdb_cpp_usable() { # [DIR] + local dir="${1:-}" cxx="${CXX:-g++}" + uw_ensure_lmdb >/dev/null 2>&1 || return 1 + printf '#include \nint main(){return 0;}\n' \ + | "$cxx" -x c++ -std=c++17 -O0 -o /dev/null -llmdb - >/dev/null 2>&1 || return 1 + # best-effort smoke-open of a built store (first lmdb sub-env under DIR/lmdb) + if [ -n "$dir" ]; then + local envdir="" + shopt -s nullglob + local d; for d in "$dir"/lmdb/*/ "$dir"/lmdb_v1/*/; do [ -f "$d/data.mdb" ] && { envdir="${d%/}"; break; }; done + shopt -u nullglob + if [ -n "$envdir" ]; then + local probe; probe="$(mktemp -d)/mdbprobe" + if printf '#include \nint main(int c,char**v){MDB_env*e;if(mdb_env_create(&e))return 1;int rc=mdb_env_open(e,v[1],MDB_RDONLY|MDB_NOTLS,0664);int ok=(rc==0);if(ok){MDB_txn*t;MDB_dbi d;if(mdb_txn_begin(e,0,MDB_RDONLY,&t)==0){if(mdb_dbi_open(t,0,0,&d)!=0)ok=0;mdb_txn_abort(t);}else ok=0;}mdb_env_close(e);return ok?0:2;}\n' \ + | "$cxx" -x c++ -std=c++17 -O0 -o "$probe" -llmdb - >/dev/null 2>&1; then + "$probe" "$envdir" >/dev/null 2>&1 || { rm -rf "$(dirname "$probe")"; return 1; } + fi + rm -rf "$(dirname "$probe")" + fi + fi + return 0 +} + # Resolve a requested backend (auto|indexed|lmdb) to a CONCRETE one. # Echoes indexed|lmdb on stdout; all diagnostics go to stderr so callers can # capture the choice with $(...). auto that wants lmdb but finds it unusable @@ -125,18 +180,27 @@ uw_resolve_store_backend() { # MODE DIR -> echoes indexed|lmdb *) echo "uw_resolve_store_backend: unknown backend '$mode' (auto|indexed|lmdb)" >&2; return 2 ;; esac local factor="${UW_STORE_LMDB_RAM_FACTOR:-2}" - local store ram threshold + local min_rpk="${UW_STORE_LMDB_MIN_ROWS_PER_KEY:-2}" + case "$factor" in ''|*[!0-9]*) echo "uw_store auto: UW_STORE_LMDB_RAM_FACTOR='$factor' is not a non-negative integer -> using 2" >&2; factor=2 ;; esac + local store ram threshold rpk store=$(uw_store_size_bytes "$dir") ram=$(uw_available_ram_bytes) threshold=$(( ram * factor )) - if [ "$store" -gt "$threshold" ]; then - if uw_ensure_lmdb >/dev/null 2>&1; then - echo "uw_store auto: store=${store}B > ${factor}x avail_RAM(${ram}B)=${threshold}B -> lmdb" >&2 - echo lmdb; return 0 - fi - echo "uw_store auto: store=${store}B > ${factor}x avail_RAM(${ram}B)=${threshold}B would pick lmdb, but lmdb is NOT usable (uw_ensure_lmdb failed / MDB_INVALID) -- FALLING BACK to indexed (answer-identical)" >&2 + if [ "$store" -le "$threshold" ]; then + echo "uw_store auto: store=${store}B <= ${factor}x avail_RAM(${ram}B)=${threshold}B -> indexed" >&2 echo indexed; return 0 fi - echo "uw_store auto: store=${store}B <= ${factor}x avail_RAM(${ram}B)=${threshold}B -> indexed" >&2 + # Above the size threshold: gate on rows_per_key (lmdb's asymptotic win ~= rpk). + rpk=$(uw_store_rows_per_key "$dir") + if [ -n "$rpk" ] && awk "BEGIN{exit !($rpk < $min_rpk)}"; then + echo "uw_store auto: store=${store}B > ${factor}x avail_RAM(${ram}B)=${threshold}B BUT rows_per_key=${rpk} < ${min_rpk} -> indexed (lmdb buys ~nothing at ~1 row/key)" >&2 + echo indexed; return 0 + fi + # Size + rows_per_key (or unknown rpk) favor lmdb; require it to be usable. + if uw_lmdb_cpp_usable "$dir"; then + echo "uw_store auto: store=${store}B > ${factor}x avail_RAM(${ram}B)=${threshold}B, rows_per_key=${rpk:-unknown} -> lmdb" >&2 + echo lmdb; return 0 + fi + echo "uw_store auto: store=${store}B > ${factor}x avail_RAM(${ram}B)=${threshold}B, rows_per_key=${rpk:-unknown} would pick lmdb, but lmdb is NOT usable for the C++ lane (missing liblmdb / npm module / MDB_INVALID store) -- FALLING BACK to indexed (answer-identical)" >&2 echo indexed; return 0 } diff --git a/examples/pkg_resolver/store/test_auto_select.sh b/examples/pkg_resolver/store/test_auto_select.sh index b023e2423..400e775cd 100755 --- a/examples/pkg_resolver/store/test_auto_select.sh +++ b/examples/pkg_resolver/store/test_auto_select.sh @@ -3,58 +3,72 @@ # Copyright (c) 2026 John William Creighton (@s243a) # # test_auto_select.sh -- unit test for the D43 auto store-backend policy -# (uw_resolve_store_backend in ensure_lmdb.sh). Proves the SIZE rule via the -# UW_STORE_AVAIL_RAM_BYTES override, independent of any real store or memory: -# - store <= FACTOR x avail_RAM -> indexed -# - store > FACTOR x avail_RAM -> lmdb (or indexed with a loud fallback if -# lmdb is not usable on this box) -# Also checks explicit modes pass through unchanged. Answers are unaffected -- a -# policy that only picks a backend cannot change rows. +# (uw_resolve_store_backend in ensure_lmdb.sh). Proves the SIZE rule and the +# rows_per_key gate via the UW_STORE_AVAIL_RAM_BYTES override, independent of any +# real store or real memory. It STUBS uw_lmdb_cpp_usable so the test can never +# trigger a real `npm install --build-from-source` or a compiler probe -- the +# lmdb-usability path is exercised separately by the real build. Answers are +# unaffected: a policy that only picks a backend cannot change rows. set -uo pipefail HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +ROOT="$(cd "$HERE/../../.." && pwd)" # shellcheck source=ensure_lmdb.sh source "$HERE/ensure_lmdb.sh" +# Stub the lmdb-usability probe (default: usable). TEST_LMDB_USABLE=1 -> usable. +uw_lmdb_cpp_usable() { [ "${TEST_LMDB_USABLE:-1}" = "1" ]; } + TMP="$(mktemp -d)"; trap 'rm -rf "$TMP"' EXIT -# A store dir whose only sizeable file is a 4096-byte P/2 JSONL (pre-build path). -head -c 4096 /dev/zero | tr '\0' 'x' > "$TMP/pkg.jsonl" -: > "$TMP/cases.jsonl" # must be ignored by the sizer (it is the query set) -SIZE="$(uw_store_size_bytes "$TMP")" +# A store dir whose only sizeable file is a 4096-byte P/2 JSONL (pre-build path, +# rows_per_key unknown -> not gated). +mkdir -p "$TMP/s" +head -c 4096 /dev/zero | tr '\0' 'x' > "$TMP/s/pkg.jsonl" +: > "$TMP/s/cases.jsonl" # must be ignored by the sizer (it is the query set) +SIZE="$(uw_store_size_bytes "$TMP/s")" fail=0 -check() { # desc expected actual - if [ "$2" = "$3" ]; then echo " PASS $1 (=$3)"; else echo " FAIL $1: expected $2 got $3"; fail=1; fi -} +check() { if [ "$2" = "$3" ]; then echo " PASS $1 (=$3)"; else echo " FAIL $1: expected $2 got $3"; fail=1; fi; } -echo "== auto store-backend policy (store_size=${SIZE}B, FACTOR=${UW_STORE_LMDB_RAM_FACTOR:-2}) ==" +echo "== auto policy (store_size=${SIZE}B, FACTOR=${UW_STORE_LMDB_RAM_FACTOR:-2}, MIN_RPK=${UW_STORE_LMDB_MIN_ROWS_PER_KEY:-2}) ==" check "sizer ignores cases.jsonl, counts pkg.jsonl" 4096 "$SIZE" -# Below threshold: avail_RAM huge -> 2x huge >> store -> indexed. -b_lo="$(UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP" 2>/dev/null)" -check "store <= 2x RAM -> indexed" indexed "$b_lo" - -# Above threshold: avail_RAM=1 -> 2x1=2 < 4096 -> size rule wants lmdb. -reason="$(UW_STORE_AVAIL_RAM_BYTES=1 uw_resolve_store_backend auto "$TMP" 2>&1 >/dev/null)" -b_hi="$(UW_STORE_AVAIL_RAM_BYTES=1 uw_resolve_store_backend auto "$TMP" 2>/dev/null)" -echo " info above-threshold decision: resolved=$b_hi" -echo " info reason: $reason" -if echo "$reason" | grep -q -- "-> lmdb"; then - check "store > 2x RAM AND lmdb usable -> lmdb" lmdb "$b_hi" -elif echo "$reason" | grep -q "FALLING BACK to indexed"; then - echo " PASS store > 2x RAM but lmdb unusable -> indexed (loud fallback; answer-identical)" - check " fallback resolved backend" indexed "$b_hi" -else - echo " FAIL above-threshold did not trigger the lmdb size rule"; fail=1 -fi +# Below threshold -> indexed (RAM huge). +check "store <= 2x RAM -> indexed" indexed \ + "$(UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" + +# Above threshold, rpk unknown (no .idx), lmdb usable -> lmdb. +check "store > 2x RAM, rpk unknown, usable -> lmdb" lmdb \ + "$(UW_STORE_AVAIL_RAM_BYTES=1 TEST_LMDB_USABLE=1 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" -# Explicit modes pass through unchanged (override auto). -check "explicit indexed passes through" indexed "$(uw_resolve_store_backend indexed "$TMP" 2>/dev/null)" -check "explicit lmdb passes through" lmdb "$(uw_resolve_store_backend lmdb "$TMP" 2>/dev/null)" +# Above threshold, lmdb NOT usable -> indexed (loud fallback). +check "store > 2x RAM but lmdb unusable -> indexed (fallback)" indexed \ + "$(UW_STORE_AVAIL_RAM_BYTES=1 TEST_LMDB_USABLE=0 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" -# Tunable factor: FACTOR=0 makes any non-empty store exceed the threshold. -b_f0="$(UW_STORE_LMDB_RAM_FACTOR=0 UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP" 2>&1 | tail -0; UW_STORE_LMDB_RAM_FACTOR=0 UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP" 2>/dev/null)" -if [ "$b_f0" = lmdb ] || [ "$b_f0" = indexed ]; then echo " PASS FACTOR override honored (resolved=$b_f0)"; else echo " FAIL FACTOR override"; fail=1; fi +# Explicit modes pass through. +check "explicit indexed passes through" indexed "$(uw_resolve_store_backend indexed "$TMP/s" 2>/dev/null)" +check "explicit lmdb passes through" lmdb "$(uw_resolve_store_backend lmdb "$TMP/s" 2>/dev/null)" + +# Invalid FACTOR falls back to 2 (and still resolves). +check "invalid FACTOR -> defaults, resolves" indexed \ + "$(UW_STORE_LMDB_RAM_FACTOR=abc UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" + +# rows_per_key gate (needs a real UWIX index; skip if the indexer is unavailable). +INDEX="$ROOT/scripts/js_wam/uw_fact_index.js" +if command -v node >/dev/null 2>&1 && [ -f "$INDEX" ]; then + mkdir -p "$TMP/rpk1" "$TMP/rpk3" + printf '["k1","v0"]\n["k2","v0"]\n' > "$TMP/rpk1/pkg.jsonl" + node "$INDEX" build "$TMP/rpk1/pkg.jsonl" "$TMP/rpk1/pkg" >/dev/null 2>&1 + printf '["k1","a"]\n["k1","b"]\n["k1","c"]\n["k2","a"]\n["k2","b"]\n["k2","c"]\n' > "$TMP/rpk3/pkg.jsonl" + node "$INDEX" build "$TMP/rpk3/pkg.jsonl" "$TMP/rpk3/pkg" >/dev/null 2>&1 + echo " info rpk1 rows_per_key=$(uw_store_rows_per_key "$TMP/rpk1") rpk3 rows_per_key=$(uw_store_rows_per_key "$TMP/rpk3")" + check "store > 2x RAM, rows_per_key=1 -> indexed (gate)" indexed \ + "$(UW_STORE_AVAIL_RAM_BYTES=1 TEST_LMDB_USABLE=1 uw_resolve_store_backend auto "$TMP/rpk1" 2>/dev/null)" + check "store > 2x RAM, rows_per_key=3 -> lmdb" lmdb \ + "$(UW_STORE_AVAIL_RAM_BYTES=1 TEST_LMDB_USABLE=1 uw_resolve_store_backend auto "$TMP/rpk3" 2>/dev/null)" +else + echo " SKIP rows_per_key gate (node / uw_fact_index.js unavailable)" +fi echo "== $([ $fail -eq 0 ] && echo ALL PASS || echo FAILURES) ==" exit $fail diff --git a/templates/targets/cpp_wam/runtime.h.mustache b/templates/targets/cpp_wam/runtime.h.mustache index 9019e2756..f349a6077 100644 --- a/templates/targets/cpp_wam/runtime.h.mustache +++ b/templates/targets/cpp_wam/runtime.h.mustache @@ -784,12 +784,7 @@ public: #ifdef WAM_CPP_ENABLE_LMDB close_lmdb(); #endif -#ifdef UW_SEEK_HAVE_MMAP - if (data_map_ && data_map_ != MAP_FAILED) - ::munmap(const_cast(data_map_), data_map_size_); - data_map_ = nullptr; - if (data_fd_ >= 0) { ::close(data_fd_); data_fd_ = -1; } -#endif + close_indexed(); } SeekFactSource(const SeekFactSource&) = delete; SeekFactSource& operator=(const SeekFactSource&) = delete; @@ -882,13 +877,22 @@ private: std::uint32_t keyblob_off_ = 0; std::uint32_t hits_off_ = 0; std::uint32_t n_records_ = 0; - // The whole .idx (sorted key table + key blob + hits blob) is slurped into - // RAM ONCE at open (idx_blob_). A keyed lookup is then an IN-MEMORY binary - // search over the key table plus exactly one positioned .data record read -- - // no per-probe seek+read syscalls. At symbol scale the .idx is ~18MB, so the - // resident cost is small and constant, and it is answer-identical to the old - // per-probe path. See lookup_offsets(). - std::string idx_blob_; + // The .idx (sorted key table + key blob + hits blob) is mmap'd at open + // (idx_map_) so a keyed lookup is an IN-MEMORY binary search over the key + // table plus one .data record read -- no per-probe seek+read syscalls. mmap + // (PROT_READ, MAP_PRIVATE) keeps the index PAGE-CACHE-BACKED and EVICTABLE: + // the .idx can be 39-97% of the store, so the earlier heap slurp allocated + // ~that much NON-evictable anonymous memory -- under the very memory pressure + // that routes a >2xRAM store to indexed, that risked bad_alloc -> caught as a + // goal failure -> silent under-answering. The ifstream slurp (idx_blob_) + // remains only as the non-POSIX / mmap-unavailable fallback. Reads go through + // idx_base_/idx_len_, which point at the map or the fallback blob. + std::string idx_blob_; // fallback storage only + int idx_fd_ = -1; + const unsigned char* idx_map_ = nullptr; + std::size_t idx_map_size_ = 0; + const unsigned char* idx_base_ = nullptr; // map or idx_blob_.data() + std::size_t idx_len_ = 0; // .data mmap (indexed backend). When mapped, a record is read in-place with // one page access (no read syscall, exact byte count) -- so a resident miss // costs ~one read like an lmdb mmap value fetch, not the two positioned reads @@ -1006,6 +1010,37 @@ private: return buf; } + // Bounds-checked little-endian reads over the resolved index base + // (idx_base_ = mmap or fallback blob). Return 0 on any out-of-range offset, + // matching the old seek_le_* behavior on a short buffer. + std::uint32_t idx_u32(std::size_t o) const { + if (!idx_base_ || o + 4 > idx_len_) return 0; + return le_u32_ptr(idx_base_ + o); + } + std::size_t idx_u16(std::size_t o) const { + if (!idx_base_ || o + 2 > idx_len_) return 0; + return le_u16_ptr(idx_base_ + o); + } + + // Release every indexed-backend handle/mapping and reset state so the source + // is safe to destroy AND a retry after a failed open starts clean. + void close_indexed() { +#ifdef UW_SEEK_HAVE_MMAP + if (idx_map_ && idx_map_ != MAP_FAILED) + ::munmap(const_cast(idx_map_), idx_map_size_); + idx_map_ = nullptr; idx_map_size_ = 0; + if (idx_fd_ >= 0) { ::close(idx_fd_); idx_fd_ = -1; } + if (data_map_ && data_map_ != MAP_FAILED) + ::munmap(const_cast(data_map_), data_map_size_); + data_map_ = nullptr; data_map_size_ = 0; + if (data_fd_ >= 0) { ::close(data_fd_); data_fd_ = -1; } +#endif + if (data_.is_open()) data_.close(); + if (idx_.is_open()) idx_.close(); + std::string().swap(idx_blob_); + idx_base_ = nullptr; idx_len_ = 0; + } + void ensure_open() { std::lock_guard guard(mu_); if (opened_) return; @@ -1013,43 +1048,79 @@ private: if (!data_.is_open()) throw std::runtime_error("seek store open failed (" + path_ + ".data)"); idx_.open(path_ + ".idx", std::ios::binary); - if (!idx_.is_open()) + if (!idx_.is_open()) { + close_indexed(); throw std::runtime_error("seek store open failed (" + path_ + ".idx)"); - // data size + } + // data size (guard a negative tellg: a failed stat/seek must not become a + // resize(SIZE_MAX) later -- report it clearly and leave no half-open state) data_.seekg(0, std::ios::end); - data_size_ = static_cast(data_.tellg()); + std::streamoff dpos = data_.tellg(); data_.seekg(0, std::ios::beg); + if (dpos < 0) { close_indexed(); throw std::runtime_error("seek store: cannot size " + path_ + ".data"); } + data_size_ = static_cast(dpos); g_fact_io_data_size.store(data_size_, std::memory_order_relaxed); - // Slurp the entire .idx into RAM once (single positioned read; the - // D43 counters see it as one read of the whole index). Every subsequent - // key probe reads from idx_blob_ in memory -- zero syscalls per probe. + // .idx size idx_.seekg(0, std::ios::end); - std::uint64_t idx_size = static_cast(idx_.tellg()); + std::streamoff ipos = idx_.tellg(); idx_.seekg(0, std::ios::beg); - idx_blob_ = fact_io_read(idx_, static_cast(idx_size), 0); - if (idx_blob_.size() < 24 || idx_blob_.compare(0, 4, "UWIX") != 0) + if (ipos < 0) { close_indexed(); throw std::runtime_error("seek store: cannot size " + path_ + ".idx"); } + std::uint64_t idx_size = static_cast(ipos); +#ifdef UW_SEEK_HAVE_MMAP + // mmap the .idx: page-cache-backed + evictable, so the in-memory binary + // search reads through the map with no per-probe syscall AND no + // non-evictable heap. Charges nothing to the D43 counters at open (mmap + // faults lazily; only actual record reads count). Fd is closed right + // after mapping -- the mapping stays valid. + idx_fd_ = ::open((path_ + ".idx").c_str(), O_RDONLY); + if (idx_fd_ >= 0 && idx_size > 0) { + void* m = ::mmap(nullptr, static_cast(idx_size), + PROT_READ, MAP_PRIVATE, idx_fd_, 0); + ::close(idx_fd_); idx_fd_ = -1; + if (m != MAP_FAILED) { + idx_map_ = static_cast(m); + idx_map_size_ = static_cast(idx_size); + idx_.close(); // header + probes read from the map + } + } else if (idx_fd_ >= 0) { + ::close(idx_fd_); idx_fd_ = -1; + } +#endif + if (idx_map_) { + idx_base_ = idx_map_; idx_len_ = idx_map_size_; + } else { + // Fallback (non-POSIX / mmap failed): slurp once. THIS path charges + // the full index size to the D43 counters (one read) -- the mmap path + // does not, which is the correct, lazy accounting. + idx_blob_ = fact_io_read(idx_, static_cast(idx_size), 0); + idx_base_ = reinterpret_cast(idx_blob_.data()); + idx_len_ = idx_blob_.size(); + } + if (idx_len_ < 24 || std::memcmp(idx_base_, "UWIX", 4) != 0) { + close_indexed(); throw std::runtime_error("seek store: bad index magic at " + path_ + ".idx"); - n_keys_ = seek_le_u32(idx_blob_, 8); - keyblob_off_ = seek_le_u32(idx_blob_, 12); - hits_off_ = seek_le_u32(idx_blob_, 16); - n_records_ = seek_le_u32(idx_blob_, 20); + } + n_keys_ = idx_u32(8); + keyblob_off_ = idx_u32(12); + hits_off_ = idx_u32(16); + n_records_ = idx_u32(20); std::string dh = fact_io_read(data_, 16, 0); - if (dh.size() < 16 || dh.compare(0, 4, "UWFI") != 0) + if (dh.size() < 16 || dh.compare(0, 4, "UWFI") != 0) { + close_indexed(); throw std::runtime_error("seek store: bad data magic at " + path_ + ".data"); + } #ifdef UW_SEEK_HAVE_MMAP // mmap the .data so read_record accesses each record in-place (one page - // fault, no read syscall). The ifstream data_ stays only as the fallback - // path; close it once mapped to avoid holding an extra fd. + // fault, no read syscall). Fd is closed right after mapping. data_fd_ = ::open((path_ + ".data").c_str(), O_RDONLY); if (data_fd_ >= 0 && data_size_ > 0) { void* m = ::mmap(nullptr, static_cast(data_size_), PROT_READ, MAP_PRIVATE, data_fd_, 0); + ::close(data_fd_); data_fd_ = -1; if (m != MAP_FAILED) { data_map_ = static_cast(m); data_map_size_ = static_cast(data_size_); - data_.close(); // reads now come from the map; fd no longer needed - } else { - ::close(data_fd_); data_fd_ = -1; // fall back to ifstream reads + data_.close(); // reads now come from the map } } else if (data_fd_ >= 0) { ::close(data_fd_); data_fd_ = -1; @@ -1058,12 +1129,12 @@ private: opened_ = true; } - // Byte-compare `target` against the key blob region [off, off+len) in - // idx_blob_ WITHOUT copying it out (string_view semantics, identical to + // Byte-compare `target` against the key blob region [off, off+len) in the + // mapped index (idx_base_) WITHOUT copying it out (identical to // seek_bytes_compare). Returns <0 / 0 / >0. int idx_key_compare(std::size_t off, std::size_t len, const std::string& target) const { - if (off + len > idx_blob_.size()) len = (off <= idx_blob_.size()) ? (idx_blob_.size() - off) : 0; - const char* k = idx_blob_.data() + off; + if (off + len > idx_len_) len = (off <= idx_len_) ? (idx_len_ - off) : 0; + const unsigned char* k = idx_base_ + off; std::size_t n = std::min(len, target.size()); for (std::size_t i = 0; i < n; ++i) { unsigned char ca = static_cast(k[i]); @@ -1076,10 +1147,10 @@ private: } // Binary search the sorted key table for `target`; return the .data offsets - // that key hits (empty if absent). The whole .idx is resident in idx_blob_ - // (loaded once at open), so every probe is an in-memory read -- NO per-probe - // seek+read syscall. Only the matching .data record read (read_record) still - // touches disk. Answer-identical to the old per-probe path. + // that key hits (empty if absent). The .idx is mmap'd (idx_base_), so every + // probe is an in-memory read of a page-cache-backed page -- NO per-probe + // seek+read syscall and NO non-evictable heap. Only the matching .data record + // read (read_record) still faults a page. Answer-identical to the old path. std::vector lookup_offsets(const std::string& target) { std::vector offs; std::int64_t lo = 0; @@ -1087,17 +1158,17 @@ private: while (lo <= hi) { std::int64_t mid = (lo + hi) >> 1; std::size_t pos = 24 + static_cast(mid) * 16; - std::uint32_t key_rel = seek_le_u32(idx_blob_, pos); - std::size_t key_len = seek_le_u16(idx_blob_, pos + 4); - std::size_t n_hits = seek_le_u16(idx_blob_, pos + 6); - std::uint32_t hits_rel = seek_le_u32(idx_blob_, pos + 8); + std::uint32_t key_rel = idx_u32(pos); + std::size_t key_len = idx_u16(pos + 4); + std::size_t n_hits = idx_u16(pos + 6); + std::uint32_t hits_rel = idx_u32(pos + 8); std::size_t key_off = static_cast(keyblob_off_) + key_rel; int c = idx_key_compare(key_off, key_len, target); if (c == 0) { std::size_t hoff = static_cast(hits_off_) + hits_rel; offs.reserve(n_hits); for (std::size_t i = 0; i < n_hits; ++i) - offs.push_back(seek_le_u32(idx_blob_, hoff + i * 4)); + offs.push_back(idx_u32(hoff + i * 4)); return offs; } if (c < 0) lo = mid + 1; else hi = mid - 1; diff --git a/tests/test_wam_cpp_templates.pl b/tests/test_wam_cpp_templates.pl index 4d8424b59..b9c39c743 100644 --- a/tests/test_wam_cpp_templates.pl +++ b/tests/test_wam_cpp_templates.pl @@ -65,10 +65,20 @@ % indexed rows_found == lmdb, and the deterministic read count fell to ~1 per % record = lmdb parity). The +4159 chars are gate-independent, so BOTH variants % grew by the SAME delta. -old_header_digest(plain, 96529, - '397f1dfb0762915d6ac2368ffb1422811e75a4dfb828ba7d29740848744df7a2'). -old_header_digest(lmdb, 96770, - '7d393ce8e085d607ef7a1d7f0eb8803c62cc0cccbb50a7bf9fd8739efeb4f4d4'). +% +% Re-baselined AGAIN (fix-forward: mmap the .idx, from plain 96529 / lmdb 96770): +% the indexed read path now mmaps .idx too (page-cache-backed + evictable) instead +% of slurping it into a non-evictable heap std::string -- the slurp was 39-97% of +% the store and risked bad_alloc under the very pressure that routes >2xRAM stores +% to indexed. Adds idx_map_/idx_base_/idx_len_ members, close_indexed(), idx_u32/ +% idx_u16, mmap in ensure_open with tellg<0 guards + fd-close-after-mmap, and +% lookup_offsets/idx_key_compare reading through idx_base_. Answer-identical (503 +% differential + 51 corpus + 122 ABI verify: 0 divergences). The +3279 chars are +% gate-independent, so BOTH variants grew by the SAME delta. +old_header_digest(plain, 99808, + '3c58c7afe2ec5d0279441b3ef87d4aaa7cd848fdf8d25502f3247957143e4f39'). +old_header_digest(lmdb, 100049, + '9648e59380a913af60281d11263771f8db6736a72741413d93c8cd947360a7de'). assert_old_header_bytes(Mode, Header) :- old_header_digest(Mode, Length, Digest), From b4883c91ec0709949299d6c776a390f369ad7162 Mon Sep 17 00:00:00 2001 From: "John William Creighton (s243a)" Date: Tue, 15 Sep 2026 22:59:43 -0600 Subject: [PATCH 2/3] cpp_store fix-forward P3s: idx_key_compare corrupt-.idx guard + real cold evict Second-reviewer residuals on PR #4271. 1. idx_key_compare: it formed `const unsigned char* k = idx_base_ + off` even when off > idx_len_ (a past-the-end pointer = UB, though never dereferenced), and an empty target with off>size fell through to `return 0` -- a spurious "match" yielding garbage offsets that read_record then bounds-rejects (dropped rows). Now handle out-of-range off BEFORE forming the pointer: if (off > idx_len_) return target.empty() ? 1 : -1; if (off + len > idx_len_) len = idx_len_ - off; Only reachable on a CORRUPT .idx; well-formed stores unaffected (503 differential + 51 corpus: 0 divergences). Goldens re-baselined (plain 99808->100151, lmdb 100049->100392; +343 each, gate-independent). 2. bench_main.cpp evict_file: it only called posix_fadvise(DONTNEED), a no-op on non-resident pages, so the "cold" numbers were cold-in-name-only. Now pre-faults the file (streaming read) BEFORE DONTNEED so eviction has resident pages to drop -- the cold lookups genuinely fault from disk. (bench-only; no goldens/gates.) Frozen resolver/store/debian untouched. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RoXjhStCqoig6944pVNBGe --- examples/pkg_resolver/abi/bench/bench_main.cpp | 10 ++++++++-- templates/targets/cpp_wam/runtime.h.mustache | 7 ++++++- tests/test_wam_cpp_templates.pl | 15 +++++++++++---- 3 files changed, 25 insertions(+), 7 deletions(-) diff --git a/examples/pkg_resolver/abi/bench/bench_main.cpp b/examples/pkg_resolver/abi/bench/bench_main.cpp index 9422427ea..04e231b89 100644 --- a/examples/pkg_resolver/abi/bench/bench_main.cpp +++ b/examples/pkg_resolver/abi/bench/bench_main.cpp @@ -51,8 +51,14 @@ static void evict_file(const std::string& p) { if (fd < 0) return; struct stat st{}; if (::fstat(fd, &st) == 0 && st.st_size > 0) { - // Read through it once to ensure it is cached, then drop it, so DONTNEED - // has resident pages to evict (DONTNEED is a no-op on non-resident pages). + // Pre-fault the file so its pages are actually RESIDENT, THEN drop them: + // POSIX_FADV_DONTNEED is a no-op on non-resident pages, so without the + // streaming read below the "cold" run would be cold-in-name-only. The + // sequential read forces the pages in; DONTNEED then evicts them, so the + // subsequent lookups genuinely fault from disk. + char buf[1 << 16]; + ssize_t n; + while ((n = ::read(fd, buf, sizeof(buf))) > 0) { /* force pages resident */ } ::posix_fadvise(fd, 0, st.st_size, POSIX_FADV_DONTNEED); } ::close(fd); diff --git a/templates/targets/cpp_wam/runtime.h.mustache b/templates/targets/cpp_wam/runtime.h.mustache index f349a6077..350dcccff 100644 --- a/templates/targets/cpp_wam/runtime.h.mustache +++ b/templates/targets/cpp_wam/runtime.h.mustache @@ -1133,7 +1133,12 @@ private: // mapped index (idx_base_) WITHOUT copying it out (identical to // seek_bytes_compare). Returns <0 / 0 / >0. int idx_key_compare(std::size_t off, std::size_t len, const std::string& target) const { - if (off + len > idx_len_) len = (off <= idx_len_) ? (idx_len_ - off) : 0; + // Only reachable with off > idx_len_ on a CORRUPT .idx (a key entry + // pointing past the blob). Handle it BEFORE forming the pointer: never + // build a past-the-end pointer (UB), and never report a spurious 0 + // "match" for an empty target (which would yield garbage offsets). + if (off > idx_len_) return target.empty() ? 1 : -1; + if (off + len > idx_len_) len = idx_len_ - off; const unsigned char* k = idx_base_ + off; std::size_t n = std::min(len, target.size()); for (std::size_t i = 0; i < n; ++i) { diff --git a/tests/test_wam_cpp_templates.pl b/tests/test_wam_cpp_templates.pl index b9c39c743..fb18b5a01 100644 --- a/tests/test_wam_cpp_templates.pl +++ b/tests/test_wam_cpp_templates.pl @@ -75,10 +75,17 @@ % lookup_offsets/idx_key_compare reading through idx_base_. Answer-identical (503 % differential + 51 corpus + 122 ABI verify: 0 divergences). The +3279 chars are % gate-independent, so BOTH variants grew by the SAME delta. -old_header_digest(plain, 99808, - '3c58c7afe2ec5d0279441b3ef87d4aaa7cd848fdf8d25502f3247957143e4f39'). -old_header_digest(lmdb, 100049, - '9648e59380a913af60281d11263771f8db6736a72741413d93c8cd947360a7de'). +% +% Re-baselined AGAIN (fix-forward P3: idx_key_compare corrupt-.idx guard, from +% plain 99808 / lmdb 100049): handle off > idx_len_ BEFORE forming the pointer +% (no past-the-end pointer; no spurious empty-target "match"). Only affects a +% corrupt index; well-formed stores unchanged (503 differential + 51 corpus: 0 +% divergences). The +343 chars are gate-independent, so BOTH variants grew by the +% SAME delta. +old_header_digest(plain, 100151, + 'fff20e7a4533ee4b1c7422f3648e3e73e714e96e3946e33445b7686d351c6f6d'). +old_header_digest(lmdb, 100392, + 'e3fedc576ce7d81b3a6a14583179c106c90fa4da9cd35eb8287f3c0c3defff4d'). assert_old_header_bytes(Mode, Header) :- old_header_digest(Mode, Length, Digest), From d6aac3a33f84d857061eb573e91f61cb02491e0b Mon Sep 17 00:00:00 2001 From: "John William Creighton (s243a)" Date: Wed, 16 Sep 2026 15:00:04 -0600 Subject: [PATCH 3/3] cpp_store fix-forward P2+P3s: fix dead lmdb probe (link order), LE idx read, rpk validation, content digest Kimi review of PR #4271. P2 (blocker) -- the lmdb usability probes were DEAD CODE. Both compiled as `... -llmdb -` (library BEFORE the stdin source), so the linker resolved liblmdb before seeing the object's mdb_* references -> undefined symbols -> link ALWAYS fails -> the `if` body was skipped and uw_lmdb_cpp_usable returned 0 ("usable") on every host. So missing system liblmdb AND a foreign/MDB_INVALID store were never detected: a pre-existing Symas-format store under $STORE/lmdb/ would make auto pick lmdb -> runtime mdb_env_open MDB_INVALID -> caught as goal failure -> silent under-answering. Fix: source BEFORE the library in both probes (`- -llmdb -o`), and make the link probe call a real symbol (mdb_version) so it is a genuine link test. Hand-verified: (a) with liblmdb present the probe now compiles+links (usable), (b) wrong order still fails to link (confirms the bug), (c) a deliberately-corrupt DIR/lmdb/pkg/data.mdb -> mdb_env_open MDB_INVALID -> uw_lmdb_cpp_usable returns non-zero -> auto falls back to indexed with the loud message. P3s: 1. Validate UW_STORE_LMDB_MIN_ROWS_PER_KEY (case ''|*[!0-9.]*|*.*.* -> default 2) like the RAM factor -- it is interpolated into an awk program, so a non-numeric value was awk injection / a silently-skipped gate. Test adds an injection case (no /tmp write, resolves cleanly). 2. uw_store_rows_per_key read the UWIX header with `od -tu4` (NATIVE-endian) but the header is little-endian -> byte-swapped garbage on a BE host. Now assemble each u32 explicitly LE from four -tu1 bytes (uw_le_u32_at); values verified (rpk1=1.00, rpk3=3.00 on this LE host, and correct on any host now). 3. bench_crossover.sh answer-identity assert compared rows_found COUNTS only. bench_main.cpp now accumulates an order-independent content digest (sum of a per-row FNV hash over every (a1,a2)), emitted as row_digest; the assert now requires rows_found AND row_digest to match across backends per cell. Verified indexed==lmdb digest on the scale store (skewed/uniform). Guardrails: differential 503/0 + corpus 51/0 (built -O0/nice, one at a time); runtime template UNTOUCHED this round -> byte-frozen goldens untouched; test_auto_select.sh ALL PASS (incl link-probe-independent stub, injection guard, LE rpk gate). Frozen resolver/store/debian untouched. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01RoXjhStCqoig6944pVNBGe --- .../pkg_resolver/abi/bench/bench_crossover.sh | 20 ++++++++---- .../pkg_resolver/abi/bench/bench_main.cpp | 27 +++++++++++++++- examples/pkg_resolver/store/ensure_lmdb.sh | 31 ++++++++++++++++--- .../pkg_resolver/store/test_auto_select.sh | 6 ++++ 4 files changed, 72 insertions(+), 12 deletions(-) diff --git a/examples/pkg_resolver/abi/bench/bench_crossover.sh b/examples/pkg_resolver/abi/bench/bench_crossover.sh index f9a3e5726..841f81433 100755 --- a/examples/pkg_resolver/abi/bench/bench_crossover.sh +++ b/examples/pkg_resolver/abi/bench/bench_crossover.sh @@ -148,20 +148,28 @@ for SCALE in $SCALES; do done; done; done echo "== wrote $RES ==" # ASSERT answer-identity: for each (workload,R,evict) cell every backend must - # return the same rows_found (a backend choice must never change answers). + # return the same rows_found AND the same row_digest -- an order-independent + # content hash of every (a1,a2) returned (emitted by bench_main.cpp). Matching + # counts alone would not catch a wrong row with the right cardinality; the + # digest makes this a true content check. node -e ' const fs=require("fs"); const rows=fs.readFileSync(process.argv[1],"utf8").split("\n").filter(Boolean).map(JSON.parse); const by={}; let bad=0; for(const o of rows){const k=`${o.workload}|R${o.R}|ev${o.evict}`;(by[k]=by[k]||[]).push(o);} for(const k of Object.keys(by)){ - const g=by[k], want=g[0].rows_found; - for(const o of g) if(o.rows_found!==want){ - console.error(` ROWS MISMATCH ${k}: ${o.label}=${o.rows_found} vs ${g[0].label}=${want}`); bad=1; + const g=by[k], want=g[0]; + for(const o of g){ + if(o.rows_found!==want.rows_found){ + console.error(` ROWS MISMATCH ${k}: ${o.label}=${o.rows_found} vs ${want.label}=${want.rows_found}`); bad=1; + } + if(o.row_digest!==want.row_digest){ + console.error(` DIGEST MISMATCH ${k}: ${o.label}=${o.row_digest} vs ${want.label}=${want.row_digest}`); bad=1; + } } } -if(bad){console.error("== ROWS_FOUND NOT IDENTICAL ACROSS BACKENDS =="); process.exit(1);} -console.log("== rows_found identical across backends for every cell ("+Object.keys(by).length+" cells) =="); +if(bad){console.error("== ANSWERS NOT IDENTICAL ACROSS BACKENDS =="); process.exit(1);} +console.log("== rows_found AND row_digest identical across backends for every cell ("+Object.keys(by).length+" cells) =="); ' "$RES" done echo "store sizes:"; du -h "$OUT/idx/symprov.data" "$OUT/idx/symprov.idx" "$OUT/lmdb_v1/symprov/data.mdb" \ diff --git a/examples/pkg_resolver/abi/bench/bench_main.cpp b/examples/pkg_resolver/abi/bench/bench_main.cpp index 04e231b89..9148f12a5 100644 --- a/examples/pkg_resolver/abi/bench/bench_main.cpp +++ b/examples/pkg_resolver/abi/bench/bench_main.cpp @@ -42,6 +42,22 @@ static const char* env_or(const char* name, const char* dflt) { return (v && *v) ? v : dflt; } +// FNV-1a 64 over raw bytes. +static inline std::uint64_t fnv1a(std::uint64_t h, const void* data, std::size_t n) { + const unsigned char* p = static_cast(data); + for (std::size_t i = 0; i < n; ++i) { h ^= p[i]; h *= 1099511628211ULL; } + return h; +} +// Hash one WAM Value (tag + all fields) so different-typed values never collide. +static std::uint64_t value_hash(std::uint64_t h, const Value& v) { + unsigned char tag = static_cast(v.tag); + h = fnv1a(h, &tag, 1); + h = fnv1a(h, v.s.data(), v.s.size()); + h = fnv1a(h, &v.i, sizeof(v.i)); + h = fnv1a(h, &v.f, sizeof(v.f)); + return h; +} + // Evict one file's pages from the OS page cache (no root needed): // posix_fadvise(DONTNEED) drops the clean cached pages for the inode, so the // next read faults from disk. This is the root-free "store not resident under @@ -107,6 +123,12 @@ int main(int argc, char** argv) { std::uint64_t rowsFound = 0; std::uint64_t lookups = 0; + // Order-independent content digest: sum a per-row FNV hash over EVERY (a1,a2) + // returned across the whole run. Commutative accumulation means row order + // (lmdb range-scan vs indexed hit-offset order) does not matter -- two runs + // agree iff they returned the same MULTISET of rows. This makes the + // cross-backend assert a true answer-identity check, not just a count match. + std::uint64_t rowDigest = 0; auto t0 = std::chrono::steady_clock::now(); for (int r = 0; r < R; ++r) { for (const std::string& k : keys) { @@ -114,6 +136,8 @@ int main(int argc, char** argv) { // encode_store_key (0x41 atom tag + utf8) before the keyed seek. auto rows = src.rows(std::optional(encode_store_key(Value::Atom(k)))); rowsFound += rows.size(); + for (const auto& pr : rows) + rowDigest += value_hash(value_hash(1469598103934665603ULL, pr.first), pr.second); ++lookups; } } @@ -124,7 +148,7 @@ int main(int argc, char** argv) { "{\"kind\":\"%s\",\"R\":%d,\"nkeys\":%zu,\"lookups\":%llu,\"rows_found\":%llu," "\"fact_io_bytes\":%llu,\"fact_io_reads\":%llu,\"fact_io_data_size\":%llu," "\"l1_hits\":%llu,\"l2_hits\":%llu,\"cache_misses\":%llu," - "\"l1_slots\":\"%s\",\"l2_cap\":\"%s\",\"evict\":%d,\"wall_ms\":%.3f}\n", + "\"l1_slots\":\"%s\",\"l2_cap\":\"%s\",\"evict\":%d,\"row_digest\":\"%016llx\",\"wall_ms\":%.3f}\n", kind.c_str(), R, keys.size(), (unsigned long long)lookups, (unsigned long long)rowsFound, (unsigned long long)fact_io_bytes(), (unsigned long long)fact_io_reads(), @@ -134,6 +158,7 @@ int main(int argc, char** argv) { env_or("UW_WAM_LMDB_L1_SLOTS", "default"), env_or("UW_WAM_LMDB_L2_CAP", "default"), evict ? 1 : 0, + (unsigned long long)rowDigest, wall_ms); return 0; } diff --git a/examples/pkg_resolver/store/ensure_lmdb.sh b/examples/pkg_resolver/store/ensure_lmdb.sh index 738b9de34..7c4de4944 100755 --- a/examples/pkg_resolver/store/ensure_lmdb.sh +++ b/examples/pkg_resolver/store/ensure_lmdb.sh @@ -108,6 +108,16 @@ uw_store_size_bytes() { # DIR -> bytes echo "$total" } +# Read a little-endian u32 at byte OFFSET of FILE (assembled from 4 bytes, so it +# is correct on any host endianness). Echoes the value or "" on short read. +uw_le_u32_at() { # FILE OFFSET -> u32 | "" + local f="$1" off="$2"; local -a b + # shellcheck disable=SC2207 + b=($(od -An -tu1 -j"$off" -N4 "$f" 2>/dev/null)) + [ "${#b[@]}" -eq 4 ] || { echo ""; return 0; } + echo $(( b[0] + b[1] * 256 + b[2] * 65536 + b[3] * 16777216 )) +} + # Aggregate rows_per_key across the store's UWIX indexes (cheap: read the header # of each *.idx -- n_keys at byte 8, n_records at byte 20, both u32 LE). Echoes a # 2-dp float, or "" when no index exists yet (pre-build -> caller treats as @@ -116,8 +126,11 @@ uw_store_rows_per_key() { # DIR -> float | "" local dir="${1:?usage: uw_store_rows_per_key DIR}" recs=0 keys=0 f had=0 nk nr shopt -s nullglob for f in "$dir"/*.idx; do - nk=$(od -An -tu4 -j8 -N4 "$f" 2>/dev/null | tr -d ' ') - nr=$(od -An -tu4 -j20 -N4 "$f" 2>/dev/null | tr -d ' ') + # UWIX header integers are LITTLE-ENDIAN (runtime uses seek_le_u32). od -tu4 + # would read NATIVE-endian -> byte-swapped garbage on a BE host, so assemble + # each u32 explicitly LE from four -tu1 bytes. + nk=$(uw_le_u32_at "$f" 8) + nr=$(uw_le_u32_at "$f" 20) if [ -n "$nk" ] && [ -n "$nr" ]; then keys=$(( keys + nk )); recs=$(( recs + nr )); had=1; fi done shopt -u nullglob @@ -148,8 +161,13 @@ uw_available_ram_bytes() { uw_lmdb_cpp_usable() { # [DIR] local dir="${1:-}" cxx="${CXX:-g++}" uw_ensure_lmdb >/dev/null 2>&1 || return 1 - printf '#include \nint main(){return 0;}\n' \ - | "$cxx" -x c++ -std=c++17 -O0 -o /dev/null -llmdb - >/dev/null 2>&1 || return 1 + # Genuine LINK test: the source calls a real symbol (mdb_version) so the linker + # must resolve liblmdb. The stdin source `-` MUST come BEFORE `-llmdb`: linkers + # resolve libraries in argument order, so `-llmdb -` leaves the object's mdb_* + # references undefined and the link ALWAYS fails -- which would make this probe + # dead code (function returns "usable" on every host). + printf '#include \nint main(){int a,b,c;(void)mdb_version(&a,&b,&c);return 0;}\n' \ + | "$cxx" -x c++ -std=c++17 -O0 - -llmdb -o /dev/null >/dev/null 2>&1 || return 1 # best-effort smoke-open of a built store (first lmdb sub-env under DIR/lmdb) if [ -n "$dir" ]; then local envdir="" @@ -159,7 +177,7 @@ uw_lmdb_cpp_usable() { # [DIR] if [ -n "$envdir" ]; then local probe; probe="$(mktemp -d)/mdbprobe" if printf '#include \nint main(int c,char**v){MDB_env*e;if(mdb_env_create(&e))return 1;int rc=mdb_env_open(e,v[1],MDB_RDONLY|MDB_NOTLS,0664);int ok=(rc==0);if(ok){MDB_txn*t;MDB_dbi d;if(mdb_txn_begin(e,0,MDB_RDONLY,&t)==0){if(mdb_dbi_open(t,0,0,&d)!=0)ok=0;mdb_txn_abort(t);}else ok=0;}mdb_env_close(e);return ok?0:2;}\n' \ - | "$cxx" -x c++ -std=c++17 -O0 -o "$probe" -llmdb - >/dev/null 2>&1; then + | "$cxx" -x c++ -std=c++17 -O0 - -llmdb -o "$probe" >/dev/null 2>&1; then "$probe" "$envdir" >/dev/null 2>&1 || { rm -rf "$(dirname "$probe")"; return 1; } fi rm -rf "$(dirname "$probe")" @@ -182,6 +200,9 @@ uw_resolve_store_backend() { # MODE DIR -> echoes indexed|lmdb local factor="${UW_STORE_LMDB_RAM_FACTOR:-2}" local min_rpk="${UW_STORE_LMDB_MIN_ROWS_PER_KEY:-2}" case "$factor" in ''|*[!0-9]*) echo "uw_store auto: UW_STORE_LMDB_RAM_FACTOR='$factor' is not a non-negative integer -> using 2" >&2; factor=2 ;; esac + # Validate min_rpk too: it is interpolated into an awk program below, so a + # non-numeric value would be awk-syntax injection / a silent gate skip. + case "$min_rpk" in ''|*[!0-9.]*|*.*.*) echo "uw_store auto: UW_STORE_LMDB_MIN_ROWS_PER_KEY='$min_rpk' is not numeric -> using 2" >&2; min_rpk=2 ;; esac local store ram threshold rpk store=$(uw_store_size_bytes "$dir") ram=$(uw_available_ram_bytes) diff --git a/examples/pkg_resolver/store/test_auto_select.sh b/examples/pkg_resolver/store/test_auto_select.sh index 400e775cd..dce438095 100755 --- a/examples/pkg_resolver/store/test_auto_select.sh +++ b/examples/pkg_resolver/store/test_auto_select.sh @@ -53,6 +53,12 @@ check "explicit lmdb passes through" lmdb "$(uw_resolve_store_backend lmd check "invalid FACTOR -> defaults, resolves" indexed \ "$(UW_STORE_LMDB_RAM_FACTOR=abc UW_STORE_AVAIL_RAM_BYTES=1000000000 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" +# Invalid MIN_ROWS_PER_KEY must not become awk injection or silently skip the +# gate: it defaults to 2 and still resolves (rpk unknown here -> lmdb, usable). +check "invalid MIN_ROWS_PER_KEY -> defaults, resolves" lmdb \ + "$(UW_STORE_LMDB_MIN_ROWS_PER_KEY='2); system("touch '"$TMP"'/pwn"' UW_STORE_AVAIL_RAM_BYTES=1 TEST_LMDB_USABLE=1 uw_resolve_store_backend auto "$TMP/s" 2>/dev/null)" +if [ -e "$TMP/pwn" ]; then echo " FAIL awk injection executed!"; rm -f "$TMP/pwn"; fail=1; else echo " PASS no awk injection from MIN_ROWS_PER_KEY"; fi + # rows_per_key gate (needs a real UWIX index; skip if the indexer is unavailable). INDEX="$ROOT/scripts/js_wam/uw_fact_index.js" if command -v node >/dev/null 2>&1 && [ -f "$INDEX" ]; then