johanjoensson / impurityModel / 35728744800

94%

Build Type

push

github

Committed by johanjoensson

Commit Message

test: cover the block merge primitives, and correct a hazard that was false

Four things the adversarial review made necessary.

**A benchmark for `block_inner`/`count_rows_in`.** A 4.0x and a 7.2x regression in these
shipped with `pytest -m benchmark` green, because nothing in the suite touched them. The
assertion is a RATIO, not a time: the perf tests here deliberately avoid absolute durations
as machine-dependent, and the defect was allocation rather than speed. `count_rows_in` is
the same linear merge over the same support with no per-row allocation, so `block_inner`
should cost a small multiple of it -- that multiple being its width^2 multiply-accumulate,
not malloc traffic. Measured 3.8-3.9x with views against ~13x when the join materialized
keys; the bound sits at 8.

**Hazard 6 of `flat_key_store.md` was false, and expensively so.** It said
`IMPURITYMODEL_BUILD=debug` is the only build that catches an out-of-bounds read in this
layer. `debug` enables CYTHON's boundscheck, which does not reach the C++ layer at all: the
heap-buffer-overflow write in `from_columns` shipped with the four-leg debug gate green on
every commit containing it, and only ASan found it. Believing that claim is what made a
green debug gate feel like evidence the indexing was sound, and I repeated it in commit
messages. Marked false in place rather than quietly edited.

**A missing `except +`.** `ManyBodyBlockState(vector[Key], vector[Value], size_t)` packs its
keys through the flat store now, which rejects a zero-chunk key -- but the declaration had
no `except +`, so such a throw would cross the Cython boundary as `std::terminate`: SIGABRT
with no traceback, and from a `nogil` block no rank output either. Found by auditing after
an unexplained SIGABRT in one gate run; that run is NOT demonstrably this bug (no call site
is inside `nogil`, and it did not reproduce), but the hazard is real and was mine.

**A pre-existing flake, fixed because it cost a gate cycle.**
`test_the_... (continued)

Coverage Stats

6 of 28 new or added lines in 2 files covered. (21.43%)

112 existing lines in 9 files now uncovered.

35544 of 38779 relevant lines covered (91.66%)

1.83 hits per line