From 2bc13920d4dab5ecde3d745d102e741f121970fb Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 27 Aug 2026 13:14:31 +0000 Subject: [PATCH] =?UTF-8?q?lgj-abi:=20hop=20sweeps=20the=20store=20ONCE,?= =?UTF-8?q?=20not=2032=20times=20=E2=80=94=203.5x=20at=20scale?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Acts on ISS-LGJ-HOP-SWEEPS-FULL-POPULATION, which bench Component G opened hours earlier. The measurement authorised this; nothing here is taste. THE ARITHMETIC IS WHY, and it is not the obvious "too many rows". 32 x 65_536 strided u32 compares is ~2M operations -- nowhere near the 24 ms observed. But 32 passes over a 33 MB store, each re-reading every 512-byte row to look at 4 bytes of it, is ~1 GB of memory traffic. THE LOOP ORDER WAS THE COST, NOT THE ROW COUNT. That distinction is what makes the fix a re-ordering rather than a new kernel. WHAT CHANGED. `lgj_hop` no longer loops `for facet in 0..32 { sweep(all n rows) }`. It calls `simd_rowstore_facet_match` ONCE -- all 32 facets per row, one `U32x16::eq_bitmask` per 64-byte chunk via `MultiLaneColumn` -- and then walks SRC'S SET ROWS rather than every row x facet, taking only the facets that both matched and participate (`facet_bits[row] & effective`). NO NEW KERNEL. `simd_rowstore_facet_match` already existed and is the same sanctioned `ndarray::simd` surface (abi.md §8). It was simply being consumed the wrong way round. No ABI change, no signature change, no minor bump: this is strictly an internal re-ordering behind an unchanged contract. MEASURED, same instrument, same command: native_hop 1%/4096 1%/65536 25%/4096 25%/65536 before 479.0 24798.3 521.2 23633.9 us/op after 374.7 7120.3 375.8 8076.9 speed-up 1.28x 3.48x 1.39x 2.93x THE CONTROL MATTERS HERE. The two scalar arms are untouched code. Had the second run merely landed on a faster host they would have moved too -- measured, `classidScan` 150.6 -> 164.2 and `facetMatches` 7142.3 -> 7698.8 at 1%/65536, within ~9% and slightly SLOWER. So the native gain is real and if anything understated. (JMH's banner does report a different CPU string between the runs, which is exactly why the control arms are quoted rather than the box being assumed identical.) BEHAVIOUR UNCHANGED, verified rather than assumed: lgj-abi 134/134 (the hop's own aliasing and semantics tests among them), AllTests 304, GraphHopTest 66 INCLUDING G3 at the identical 384-byte allocation floor, TradesParity 12, TradesAllocation 3, BricksAuth 62 = 447 Java checks. clippy -D warnings and fmt clean. STILL SLOWER THAN THE BEST SCALAR ARM, and the issue is regraded RESOLVED IN PART rather than closed. At 1%/65536 native is 7120 us against classidScan's 164 us -- 43x. The catastrophic term is gone; a structural one is not: `simd_rowstore_facet_match` still sweeps the WHOLE population, so the decode half is now frontier-bounded and the compare half is not. The next rung is a genuinely different shape -- gather per src row, O(frontier) instead of O(population) -- and it is NOT obviously better, because a dense frontier should favour the sweep's sequential access. That is a real density crossover, so it is a measurement rather than a judgement call, and Component G is now the instrument that can find it. WHY NOT BOTH AT ONCE. The one-pass change is bounded, needs no new kernel, has no crossover, and is strictly less work at every point in the measured space. The gather rewrite is none of those. Landing them together would have made a regression in either impossible to attribute. Board artifacts in the same commit per the hygiene rule: ISSUES regraded with what remains open and why the split, RESULTS.md § G carries the before/after table, the control-arm reasoning and the named remaining gap. --- .claude/board/ISSUES.md | 25 ++++++++++- bench/RESULTS.md | 48 ++++++++++++++++++++ bench/results/jmh-results-G.csv | 24 +++++----- native/lgj-abi/src/exports.rs | 78 +++++++++++++++++++-------------- 4 files changed, 129 insertions(+), 46 deletions(-) diff --git a/.claude/board/ISSUES.md b/.claude/board/ISSUES.md index a8bbe25..75c39d1 100644 --- a/.claude/board/ISSUES.md +++ b/.claude/board/ISSUES.md @@ -1,6 +1,6 @@ # Issues Log — Open + Resolved (double-entry, append-only) -## ISS-LGJ-HOP-SWEEPS-FULL-POPULATION (2026-08-27) — OPEN +## ISS-LGJ-HOP-SWEEPS-FULL-POPULATION (2026-08-27) — RESOLVED IN PART, same day **Found.** By running bench Component G — the F-PARITY harness — for the first time. `lgj_hop` is the slowest of three arms at every configuration in @@ -45,6 +45,29 @@ no scalar analogue and was NOT isolated (implausible as the story at 470 µs on are noisy — ±12 844 on 24 798 µs — on a shared 4-vCPU container; the ordering is robust, the absolutes are not. One machine, one run. +**Resolved in part, same day.** `lgj_hop` now calls +`simd_rowstore_facet_match` ONCE — all 32 facets per row in a single +`MultiLaneColumn` pass — and walks src's set rows instead of every row × +facet. No new kernel; that function already existed and is the same +sanctioned `ndarray::simd` surface, it was just being consumed the wrong way +round. Measured **1.28×–3.48×**, largest at scale (24 798 → 7 120 µs at +1 %/65 536). The untouched scalar arms are the control and moved <9 %, so the +gain is not a faster host. + +**What remains OPEN, and it is structural.** `simd_rowstore_facet_match` +still sweeps the whole population, so at a 1 % frontier native is still 43× +the best scalar arm (7 120 µs vs 164 µs): the DECODE half is now +frontier-bounded, the COMPARE half is not. The next rung — gather per src row +— is O(frontier) but is NOT obviously better, because a dense frontier should +favour the sweep's sequential access. That crossover is a measurement, not a +judgement call, and Component G is the instrument for it. + +**Why this was not one bigger change.** The one-pass fix is bounded, needs no +new kernel, has no density crossover, and is strictly less work at every +point in the measured space. The gather rewrite is none of those things. +Landing them together would have made a regression in either impossible to +attribute. + Data: `bench/results/jmh-results-G.csv`; narrative: `bench/RESULTS.md` § G. diff --git a/bench/RESULTS.md b/bench/RESULTS.md index 8ef031a..81059a6 100644 --- a/bench/RESULTS.md +++ b/bench/RESULTS.md @@ -308,6 +308,54 @@ strided classid reads per call before a single edge is decoded. the placement follows that measurement — never taste"* — now has its measurement, and it points at a specific, local property of the kernel rather than at the architecture. +### FIXED — one pass instead of 32, re-measured + +The remedy the section above deferred landed immediately after, once the +measurement justified it. `lgj_hop` no longer loops `for facet in 0..32 { +sweep(all n rows) }`; it calls `simd_rowstore_facet_match` **once**, which +answers all 32 facets per row in a single `MultiLaneColumn` pass, and then +walks **src's set rows** rather than every row × facet. + +No new kernel: `simd_rowstore_facet_match` already existed and is the same +sanctioned `ndarray::simd` surface (abi.md §8) — it was simply being consumed +the wrong way round. + +| arm | 1 % / 4096 | 1 % / 65536 | 25 % / 4096 | 25 % / 65536 | +|---|---|---|---|---| +| `native_hop` **before** | 479.0 µs | 24 798.3 µs | 521.2 µs | 23 633.9 µs | +| `native_hop` **after** | **374.7 µs** | **7 120.3 µs** | **375.8 µs** | **8 076.9 µs** | +| speed-up | 1.28× | **3.48×** | 1.39× | **2.93×** | + +**The two scalar arms are the control, and they matter here.** They are +untouched code, so if the second run had merely landed on a faster host they +would have moved too. Measured: `classidScan` 150.6 → 164.2 µs and +`facetMatches` 7 142.3 → 7 698.8 µs at 1 %/65 536 — within ~9 %, and slightly +*slower*, so the native gain is real and if anything understated. (The JMH +banner does report a different CPU string between runs — 2.10 GHz vs 2.80 GHz +— which is exactly why the control arms are quoted rather than trusted to be +the same box.) + +Behaviour is unchanged, not merely believed to be: lgj-abi 134/134, AllTests +304, GraphHopTest 66 **including G3 at the identical 384-byte floor**, +TradesParity 12, TradesAllocation 3, BricksAuth 62. + +### Still slower than the best scalar arm — the honest remaining gap + +At 1 % / 65 536, native is 7 120 µs against `classidScan`'s 164 µs: **43×**. +The catastrophic term is gone; a structural one is not. + +`simd_rowstore_facet_match` still sweeps the **whole population**. At a 1 % +frontier the scalar arm touches 655 rows × 32 facets ≈ 21 k reads, while the +native arm answers all 65 536 rows before intersecting with `src`. The decode +half is now frontier-bounded; the compare half is not. + +The next rung is a genuinely different shape — gather per src row (read that +row's 512 bytes, answer its 32 facets, move on) — which is O(frontier) rather +than O(population). It is NOT obviously better: at a dense frontier the full +sweep's sequential access should beat a scattered gather, so there is a real +density crossover and it should be **measured, not assumed**. Component G is +now the instrument that can find it, which is the point of having built it. + ### What this does NOT say - **It is not a verdict on mask-native execution.** The architectural claims are pinned elsewhere diff --git a/bench/results/jmh-results-G.csv b/bench/results/jmh-results-G.csv index 5db5039..0ef16ae 100644 --- a/bench/results/jmh-results-G.csv +++ b/bench/results/jmh-results-G.csv @@ -1,13 +1,13 @@ "Benchmark","Mode","Threads","Samples","Score","Score Error (99.9%)","Unit","Param: frontierPct","Param: rows" -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,8.490547,2.478089,"us/op",1,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,150.625854,14.789930,"us/op",1,65536 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,204.042087,34.162658,"us/op",25,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,7173.594263,1892.301457,"us/op",25,65536 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,394.914736,11.441462,"us/op",1,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,7142.319800,1206.299281,"us/op",1,65536 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,433.789340,15.156296,"us/op",25,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,10071.452227,2928.788223,"us/op",25,65536 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,479.021148,62.305711,"us/op",1,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,24798.259478,12844.206716,"us/op",1,65536 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,521.225218,97.134679,"us/op",25,4096 -"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,23633.937499,9008.444839,"us/op",25,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,10.902422,18.771611,"us/op",1,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,164.242494,12.784405,"us/op",1,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,238.702506,41.554658,"us/op",25,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_classidScan","avgt",1,5,7042.946461,2272.034075,"us/op",25,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,392.556226,9.559705,"us/op",1,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,7698.823135,456.583748,"us/op",1,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,487.562356,57.601186,"us/op",25,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.java_scalar_facetMatches","avgt",1,5,10864.031411,2497.132997,"us/op",25,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,374.663354,21.906457,"us/op",1,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,7120.259949,1037.574620,"us/op",1,65536 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,375.800408,25.683170,"us/op",25,4096 +"com.adaworldapi.lancegraph.bench.G_HopExecutionBoundary.native_hop","avgt",1,5,8076.916011,1556.374964,"us/op",25,65536 diff --git a/native/lgj-abi/src/exports.rs b/native/lgj-abi/src/exports.rs index bbedb56..2238b6d 100644 --- a/native/lgj-abi/src/exports.rs +++ b/native/lgj-abi/src/exports.rs @@ -1581,42 +1581,54 @@ pub extern "C" fn lgj_hop( } let mut out = vec![0u64; n_words]; - // One scratch buffer for the classid-match sub-step, reused across - // every participating facet — never allocated per facet. - let mut classid_scratch = vec![0u64; n_words]; let bytes = rowstore.as_bytes(); - for facet in 0..crate::rowstore::ROW_FACETS { - if (effective >> facet) & 1 == 0 { - continue; - } - kernels::simd_rowstore_classid_mask( - bytes, - facet as usize * crate::rowstore::FACET_BYTES as usize, - n, - edge_classid, - &mut classid_scratch, - ); - // Walk the set bits of (src ∩ classid-mask) — decode + scatter - // has no ndarray primitive (spec §3.4 / council S2-2), so this - // half stays scalar; the compare above already ran through the - // sanctioned SIMD kernel. - for (w, (&sw, &cw)) in src_snapshot.iter().zip(classid_scratch.iter()).enumerate() { - let mut bits = sw & cw; - while bits != 0 { - let bit = bits.trailing_zeros(); - bits &= bits - 1; - let row = (w as u64) * ROWS_PER_WORD + bit as u64; - // Defensive: a conformant mask's tail is always zero, so - // this is unreachable for a well-formed src_mask — but - // guards against a deliberately corrupted snapshot - // rather than letting the byte-offset math below run - // past the buffer. - if row >= n_rows { - continue; - } + // ONE pass over the store answering ALL 32 facets, instead of one + // full-width sweep per facet. + // + // The previous shape looped `for facet in 0..32 { sweep(all n rows) }`, + // which re-read the entire store 32 times to look at 4 bytes of each + // 512-byte row per pass. Measured by bench Component G + // (ISS-LGJ-HOP-SWEEPS-FULL-POPULATION): cost FLAT in frontier density + // and ~linear in population — a hop paying for the population it + // ignores rather than the frontier it starts from. The arithmetic says + // why: 32 × 65_536 strided compares is ~2M operations, nowhere near the + // 24 ms observed, but 32 passes over a 33 MB store is ~1 GB of memory + // traffic. The loop ORDER was the cost, not the row count. + // + // `simd_rowstore_facet_match` already answers "which facets of this row + // carry class X" for every row in a single MultiLaneColumn pass, one + // `U32x16::eq_bitmask` per 64-byte chunk. Using it here is strictly + // less work and introduces no new kernel — it is the same sanctioned + // ndarray::simd surface (abi.md §8), just consumed the right way round. + let mut facet_bits = vec![0u32; n]; + kernels::simd_rowstore_facet_match(&rowstore.bytes_arc(), n, edge_classid, &mut facet_bits); + + // Decode + scatter, now driven by the FRONTIER rather than by the + // population: walk src's set rows, and for each take only the facets + // that both matched and participate. This half has no ndarray + // primitive (spec §3.4 / council S2-2) and stays scalar; the compare + // above already ran through the sanctioned SIMD kernel. + let effective_facets = effective as u32; + for (w, &sw) in src_snapshot.iter().enumerate() { + let mut bits = sw; + while bits != 0 { + let bit = bits.trailing_zeros(); + bits &= bits - 1; + let row = (w as u64) * ROWS_PER_WORD + bit as u64; + // Defensive: a conformant mask's tail is always zero, so this + // is unreachable for a well-formed src_mask — but guards + // against a deliberately corrupted snapshot rather than + // letting the byte-offset math below run past the buffer. + if row >= n_rows { + continue; + } + let mut fb = facet_bits[row as usize] & effective_facets; + while fb != 0 { + let facet = fb.trailing_zeros(); + fb &= fb - 1; let base = (row * crate::rowstore::ROW_BYTES - + facet as u64 * crate::rowstore::FACET_BYTES) + + u64::from(facet) * crate::rowstore::FACET_BYTES) as usize; let payload_hi32 = u32::from_le_bytes(bytes[base + 12..base + 16].try_into().unwrap());