[DO NOT MERGE AS-IS] hop as AND over a memoised per-facet mask — measured, conditional - #41
Merged
Merged
Conversation
…MMENDED AS-IS The operator's framing was right and sharper than mine: "a masking sweep is an AND", "and over the masks". The hop decomposes four ways and only the last resists mask algebra -- classid(row, facet) == needle -> a mask payload_hi32 == 0 -> a mask intersect src -> THE AND dst[decode(row, facet)] = 1 -> a data-dependent scatter, not a set op -- and the gather that #40 shipped had traded the algebra away: the classid predicate stopped being a mask and became an inline byte compare. The contract held (no java/, no abi.md, no minor bump, GraphHopTest's allowlist + G2 + G3 all green at the identical 384-byte floor) but the STRUCTURE did not. WHAT THIS COMMIT ADDS. `RowStore::facet_bits(classid) -> Arc<[u32]>`: one 32-bit MASK per row of which facets carry that class, built once through the sanctioned `simd_rowstore_facet_match` kernel and shared by refcount after. `lgj_hop` becomes `facet_bits[row] & effective` -- an AND -- plus the irreducible scatter. No invalidation needed: `RowStore` exposes no `&mut self` method, so the buffer is immutable for the store's life. Bounded at 4 classids (n_rows*4 bytes each). MEASURED, AND THE ANSWER IS CONDITIONAL. Component G, 65_536 rows: density gather memo_cold memo_warm 0.01% 2.7 6901.7 2.8 1.00% 46.8 6984.0 10.1 25.00% 3673.3 7647.6 271.5 100.00% 6070.2 9828.7 1952.3 us/op WARM, the AND wins everywhere -- up to 13.5x. The mask is the right shape once it is paid for, exactly as the operator reasoned. COLD, it is flat at ~6900 us at EVERY density, because the O(n) build dominates. At a 0.01% frontier that is 2500x WORSE than the gather. Break-even from these numbers: ~190 hops at a 1% frontier, ~2 hops at 25%+. SO THIS IS NOT RECOMMENDED FOR MERGE AS-IS. Unconditional memoisation ships a 2500x regression on one-shot sparse hops, and this repo's rule is that a gate needs evidence at the point it is turned on rather than a plausible story. The branch exists so the work and its numbers are not stranded, and so the choice is made on data: 1. this, unconditional -- best for deep/dense traversal, worst case 2500x. 2. keep #40's gather (what main has) -- no regression anywhere, leaves 13.5x on the table for repeated dense hops. 3. LAZILY-FILLED mask -- fill each row's u32 during the gather that first touches it, with a per-row filled bit. Cold ~= gather, warm = the AND, no O(n) build ever, no tuning knob. Bounded downside; upside depends on rows being re-hopped, which is NOT measured. Recommended, pending that measurement. A SEPARATE FINDING, from isolating one variable at a time. The first run of this change was on JDK 27 and looked like a further win; re-running the SAME binary on JDK 26 gave 0.984 / 10.856 / 7.367 / 250.0 vs 27's 1.002 / 10.713 / 7.694 / 262.5 -- within ~5%, JDK 26 marginally faster. The JDK contributes NOTHING here, which confirms by measurement what the source already said: `grep` finds zero `value record`/`value class` in java/src/main/java, so JDK 27 gives a different JIT and no Valhalla flattening. Every Component G number in this repo, past and present, includes ordinary heap allocation for the per-hop Mask and WideFieldMask wrappers. Panama is real and measured; Valhalla has never run outside valhalla-lab. Also fixes a doc comment this change displaced: inserting `facet_bits` above `bytes_arc` orphaned the latter's doc onto the former. Caught by clippy's missing-docs gate, not by reading. Gates: lgj-abi 134/134; AllTests 304, GraphHopTest 66 (G3 unchanged), TradesParity 12, TradesAllocation 3, BricksAuth 62 = 447 Java checks; clippy -D warnings + fmt clean. No ABI change, no minor bump. Evidence: .claude/board/hop-memoisation-cold-vs-warm.txt (five populations x twelve densities, all four shapes, equivalence asserted at every point).
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_7ab3d8b0-e173-4423-902e-8ef028e78dc1) |
AdaWorldAPI
marked this pull request as ready for review
August 27, 2026 13:55
AdaWorldAPI
pushed a commit
that referenced
this pull request
Aug 27, 2026
PR_ARC_INVENTORY had entries for #1-#12, #14, #16, #18, #20 and #32, and nothing else. Missing: #13, #22-#31, #33-#41 — twenty PRs. (#15/#17/#19/#21 are also absent and correctly so: each is itself an arc-entry-only PR, exempt under the termination clause.) #32's own entry still read "(draft, opened …)" with no merge sha. Corrects this branch's first count, which said nineteen and read the gap as starting at #21. Both were wrong — it is twenty, and it starts at #13. The first count was a range subtraction over a file with holes; the enumeration is what found the difference. Method, which is the point rather than an aside: each entry drafted from that PR's own body and diff, five parallel agents over four PRs each, none permitted to work from a later session's recall. Every backfilled entry's Confidence bullet ends "Backfilled 2026-08-27 from the PR body and diff, not written at merge time", so reconstructed entries are distinguishable from ones written at merge time; several state which claims are the PR body's own and were not re-verified. Every cited sha, date and merge-vs- squash label machine-checked against git. Four things the backfill turned up, each recorded in the entry it belongs to: #25's body asserts "no code, no reproducer changes" and its own diff contradicts it; #39 left its lgj_hop doc comment describing the pre-change design; #34's banked evidence file did not identify its own JDK; and #41 is on main while its own title reads [DO NOT MERGE AS-IS], recorded as unresolved disposition rather than an endorsement. ISS-LGJ-ARC-INVENTORY-STOPPED-AT-32 moves to RESOLVED with the corrected count and the standing rule restated: the entry goes in at open, in the PR's own commit. The backfill is the repair, not the process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DCfrD5y19cvFc4AoyydXYv
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.
The framing was right, and sharper than mine
"A masking sweep is an AND." The hop decomposes four ways and only the last resists mask algebra:
classid(row, facet) == needlepayload_hi32 == 0srcdst[decode(row, facet)] = 1The gather #40 shipped had traded the algebra away: the classid predicate stopped being a mask and became an inline byte compare. The contract held — no
java/, noabi.md, no minor bump, GraphHopTest's allowlist + G2 + G3 green at the identical 384-byte floor — but the structure didn't.What this adds
RowStore::facet_bits(classid) -> Arc<[u32]>— one 32-bit mask per row of which facets carry that class, built once through the sanctionedsimd_rowstore_facet_matchkernel, shared by refcount after.lgj_hopbecomesfacet_bits[row] & effective— an AND — plus the irreducible scatter.No invalidation needed:
RowStoreexposes no&mut selfmethod, so the buffer is immutable for the store's life. Bounded at 4 classids (n_rows × 4bytes each).Measured — and the answer is conditional
Component G, 65 536 rows:
Warm, the AND wins everywhere — up to 13.5×. The mask is the right shape once it's paid for.
Cold, it's flat at ~6 900 µs at every density, because the O(n) build dominates. At 0.01 % that's 2 500× worse than the gather. Break-even: ~190 hops at a 1 % frontier, ~2 hops at 25 %+.
The three options
mainhas) — no regression anywhere, leaves 13.5× on the table for repeated dense hops.u32during the gather that first touches it, with a per-row filled bit. Cold ≈ gather, warm = the AND, no O(n) build ever, no tuning knob. Bounded downside; upside depends on rows being re-hopped, which is not measured. ← recommended, pending that measurement.A separate finding — one variable at a time
The first run of this change was on JDK 27 and looked like a further win. Re-running the same binary on JDK 26:
Within ~5 %, JDK 26 marginally faster. The JDK contributes nothing, which confirms by measurement what the source already said:
grepfinds zerovalue record/value classinjava/src/main/java, so JDK 27 is a different JIT and no Valhalla flattening.Every Component G number in this repo — past and present — includes ordinary heap allocation for the per-hop
MaskandWideFieldMaskwrappers. Panama is real and measured; Valhalla has never run outsidevalhalla-lab.Also fixes a doc comment this change displaced: inserting
facet_bitsabovebytes_arcorphaned the latter's doc onto the former. Caught by clippy's missing-docs gate, not by reading.Gates
lgj-abi 134/134; AllTests 304, GraphHopTest 66 (G3 unchanged), TradesParity 12, TradesAllocation 3, BricksAuth 62 = 447 Java checks;
clippy -D warnings+fmtclean. No ABI change, no minor bump.Evidence:
.claude/board/hop-memoisation-cold-vs-warm.txt— five populations × twelve densities, all four shapes, equivalence asserted at every point.Generated by Claude Code