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());