perf(compare): hoist e_minmax into a dense array — store −71%, run_dada −20% (#147) - #148
Merged
Conversation
#146 left the store as the whole serial block inside b_compare (23-30% of run_dada, a flat 18.4-19.6 ns/raw-visit across a 5x range in pool size). Its commit path is not the cost -- stores are 0.102% of raw-visits -- so the loop is ~entirely a scan over two arrays, and #147 asked which of them binds before anything gets built against it. Seven arms isolate the two accesses and model both proposed levers: dropping CompCost from the result element (48 B -> 16 B) and hoisting e_minmax out of the 160-byte Raw into a dense Vec<f64>. Laptop, nraw = 1,225,523 (soil 16S R1): both48 6.77 ns/visit today both16 6.45 lever A -4.7% dense48 1.80 lever B -73% dense16 1.74 A+B comps48 1.52 sequential stream alone comps16 1.52 ditto, 3x fewer bytes -- IDENTICAL stride 5.65 strided read alone The strided read is 83% of the loop, and comps48 == comps16 shows why lever A buys almost nothing: the sequential stream is fully prefetched and nowhere near a bandwidth limit, so removing 32 B/element removes traffic that was not costing anything. #147 proposed A first on a traffic argument; the traffic argument was measuring the wrong quantity. Absolute ns are laptop figures and about 3x faster than production -- the ratio is the claim, and it wants confirming on the bench node. One bug worth recording: the first draft reported 574 GB/s for comps48, which is impossible. The single-array arms auto-vectorised while the paired arms could not, so the benchmark was comparing SIMD width rather than access pattern. Every arm now reads through black_box; the cost is a per-iteration constant identical across arms. Refs #147. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b_compare's serial store is the whole serial block inside compare after #146 -- 23-30% of run_dada at a flat 18.4-19.6 ns/raw-visit -- and it is ~entirely a scan: stores fire on 0.102% of raw-visits, so the commit path is ~1.2s of a 220s loop. The scan reads two things per raw-visit: the map's result element (sequential) and Raw::e_minmax. Raw is 160 bytes, so consecutive e_minmax reads land 2.5 cache lines apart -- a full 64-byte line pulled to use 8 bytes of it. examples/store_bandwidth measured that strided read as 83% of the scan on both a laptop and the EPYC bench node. Moving e_minmax to a dense Vec<f64> on B, indexed by raw index, makes those reads contiguous. Same parallel-array pattern as B::raw_cluster (#132). Measured by the benchmark, soil-16S-sized (nraw = 1,225,523), EPYC: both48 6.98 ns/visit today dense48 2.64 this change -62% Local gate: 20-sample MiSeq pooled, 20/20 outputs byte-identical, and the store falls 7.5 -> 3.9 ns/raw. Cluster confirmation still owed -- the benchmark runs 2.8x cheaper than production on both machines, and whether that deficit is additive or multiplicative decides whether this returns -6% or -17% of run_dada. Planning against the low end. Semantics preserved exactly: seeded to the same -999.0 sentinel Raw::with_qual used, and deliberately NOT reset by B::init, which never reset the field either. Dropped from Raw::reset_for_iteration because B::new re-seeds the array from scratch. Refs #147. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both are merged and neither had a Performance entry. #139's numbers are the post-#143 re-measurement (64 threads, two reps per arm), not the originals from its own PR -- the fold shrank the denominator those fractions were taken against, so the carry's share grew from -28.8/-32.0% to -45/-48% on soil 16S without the absolute saving changing. #147's entry waits on measurement rather than prediction. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Records the #147 result: b_compare's serial store was walking a 160-byte Raw to read one f64, and the tell was a per-unit cost that did not move with working-set size. Adds docs/findings/compare-store-scan.md, links it from the findings index, mkdocs nav, and the forward-pointer at the end of compare-serial-fold.md, and adds the CHANGELOG entry now that both pools have measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 18, 2026
Merged
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.
Closes the first half of #147:
b_compare's serial store.The tell
#143's fold
left the store as the entire remaining serial block, at a flat
18.4–19.6 ns/raw-visit across a 5× range in pool size. A per-unit cost that
does not move when the working set moves five-fold is not a residency problem.
It is a fixed per-access price — a cache miss on every access.
The arithmetic settles what the store is before any run: on soil 16S R1 it
walks 11,888,798,623 raw-visits and commits 12,082,545 of them, 0.102%. So
99.9% of the store is the scan, and the commit path can be ignored.
The measurement
examples/store_bandwidth(first commit) decomposes the scan into its componentstreams at the real pool size and store rate. Every read goes through
black_box— the first draft reported 574 GB/s because the single-array armsauto-vectorised while the paired arms could not.
Reproduced on two machines and under both
releaseandrelease-native:the strided
Rawread is 83–87% of the scan; the result vector, a cleansequential stream, is nearly free.
Rawis 160 bytes.e_minmaxis anf64. Each read pulled a 64-byte line touse 8 bytes of it, in an order that defeats the prefetcher.
The change
e_minmaxmoves offRawinto aVec<f64>onB, parallel to theB::raw_clusterarray that is already there. Seeded to the same-999.0sentinel
Raw::with_qualused to set, and — like the field it replaces —deliberately not reset by
B::init.Result
Two replicates per arm, both reads, exclusive node (compute-5-6, EPYC 7713),
64 threads,
numactl --interleave=all.mainrun_dadaR1run_dadaR2Soil ITS2 returns −69% of store time and −15.2% of
run_dadaon both reads.Correctness: all 300 per-sample outputs across the six 16S runs are
byte-identical once the embedded build hash is excluded; 240/240 on ITS2.
Controls:
b_shufflesits at 112.5–115.3 s across every 16S arm andreconcileat 40.2–41.2 ns/comp. The untouched phases did not move, which iswhat rules out a whole-run effect presenting as a phase win.
Occupancy — the observable that prompted this: effective cores
(
(map thread-busy + serial wall) / run_dada) go 31.4 → 38.4 on R1 and29.4 → 36.5 on R2, of 64.
A note on the synthetic
store_bandwidthbracketed this at −6 to −17% ofrun_dadaand I recommendedplanning against −6%. The real result is −20%: the microbenchmark isolated the
right mechanism while understating its magnitude ~3×. Worth recording, because
it means the synthetic's ranking of levers was trustworthy even though its
arithmetic was not.
Not in this PR
The 16 B result-vector collect (
ef46356, onperf/store-scan-147) is heldback. It is real but ambiguous — −5.9% / −1.5% wall on ITS2,
syscollapsing93%, and
userrising 6–12% with no established mechanism — and the leadingaccount (glibc's 32 MB
mmapthreshold cap) predicts that the allocation,not the element width, is the thing to fix. Reusing one scratch buffer across
calls would remove it at either width; that gets prototyped and measured next.
Docs:
docs/findings/compare-store-scan.md, plus CHANGELOG.