abi: minor 7 — whole-row layout probe, and the classid table made global - #30
Conversation
|
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_3ea088c4-7a64-41d6-b569-00dfda1798a2) |
… global
Three corrections and one addition, all from the same observation: a classid is
a GLOBAL address that captures LAYOUT.
CLASSIDS ARE NOT PER-SoA. The classid -> grouping table was on RowStore, one
64 KiB copy per dataset. The same classid means the same class in every SoA, so
the resolution is dataset-independent -- provably here, since FixtureClassView
is a unit struct with no per-store state. Hoisted to a process-global LazyLock
in class_view_provider, where it belongs: it is a property of the ClassView, not
of any store. Wrong in shape rather than in output -- the answers were right,
but the placement implied two datasets could disagree, which the address space
does not permit.
THE TABLE CAPTURES LAYOUT ONLY. Meaning, RBAC, ontology category and render
template are separate resolutions off the same address; none belong in it and
none can be inferred from it. Stated in the doc so the table does not accrete.
lgj_row_layout_probe: for EVERY facet, the set of groupings its selected rows
carry, in ONE crossing. Asking per facet would be 32 crossings and is how a
consumer drifts into the per-element loop §6 forbids.
Alignment falls out as arithmetic rather than a scan: per facet, OR-accumulate a
3-bit set (plus one bit for an unanswerable classid), then
aligned <=> popcount(byte) == 1 && no unanswerable bit
One `or` per (row, facet), no comparison, no early exit, cost independent of the
data. An OR-accumulated SET is exact where cheaper accumulators are not: a sum
of wire values cannot tell {0,2} from {1,1}, and an XOR cannot tell {1,1} from
{}. The set forgets multiplicity, which is exactly what the question does not
need. 0 means the EMPTY set and is deliberately distinguishable from
disagreement.
It paid immediately. A test asserting that a maskOfFacetClass(facet 3, …)
selection is fully aligned FAILED -- and the expectation was wrong, not the
code: that mask constrains facet 3 only, so the other 31 facets carry whatever
classids the generator gave them. Measured 1 of 32 facets aligned. That is the
confusion a whole-row probe exists to remove.
R10 (valhalla-lab) bolts the same three schemas into Valhalla and Panama and
proves all three descriptions agree: raw storage bytes, a Panama MemoryLayout,
and a Valhalla value class decode every register identically, each layout
describes exactly 12 bytes, and the schemas genuinely read differently so the
agreement is not trivial. The schema bolts on at the GROUP, not the register:
12 = 6x2 = 4x3 = 3x4 means the largest group is 4 B (half the budget) while the
register is 12 and the facet 16, neither of which Java can flatten or needs to.
Gates: 132 Rust, 288 Java, clippy -D warnings clean, fmt clean, abi 0.7.
9b2990a to
c7f853c
Compare
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_25dd9f09-4b55-478e-b6f1-acf1182f2f57) |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9b2990acab
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| v |= (long) (seg.get(ValueLayout.JAVA_BYTE, (long) g * s.groupBytes + k) & 0xFF) | ||
| << (8 * k); |
There was a problem hiding this comment.
Derive Panama reads from the declared layout
If Schema.layout() has incorrect nesting or offsets but still totals 12 bytes, this parity check continues to pass because fromPanama ignores that MemoryLayout and reads ValueLayout.JAVA_BYTE at a manually recomputed offset. Consequently, the recorded claim that Panama's layout agrees with storage and Valhalla is not actually tested; derive the access path or byte offset from s.layout() instead.
Useful? React with 👍 / 👎.
Three corrections and one addition, all from the same observation: a classid is a global address that captures layout.
Classids are not per-SoA
The
classid → groupingtable was onRowStore— one 64 KiB copy per dataset. The same classid means the same class in every SoA, so the resolution is dataset-independent; provably so here, sinceFixtureClassViewis a unit struct with no per-store state. Hoisted to a process-globalLazyLockinclass_view_provider, where it belongs: it is a property of the ClassView, not of any store.Wrong in shape rather than output — the answers were right, but the placement implied two datasets could disagree about what a classid carves into, which the address space does not permit.
The table captures layout only
Meaning, RBAC, ontology category and render template are separate resolutions off the same address. None belong in the table, none can be inferred from it — stated in the doc so it does not accrete.
lgj_row_layout_probe— alignment as arithmeticFor every facet, the set of groupings its selected rows carry, in one crossing. Per facet, OR-accumulate a 3-bit set (plus one bit for an unanswerable classid), then:
One
orper (row, facet) — no comparison, no early exit, cost independent of the data. An OR-accumulated set is exact where cheaper accumulators are not: a sum of wire values cannot tell{0,2}from{1,1}; an XOR cannot tell{1,1}from{}. The set forgets multiplicity, which is exactly what the question does not need.0is the empty set and stays distinguishable from disagreement.It paid immediately. A test asserting a
maskOfFacetClass(facet 3, …)selection is fully aligned failed — and the expectation was wrong, not the code: that mask constrains facet 3 only, so the other 31 facets carry whatever classids the generator gave them. Measured 1 of 32 facets aligned. That is precisely the confusion a whole-row probe exists to remove.R10 — the same schema in Valhalla and Panama
Proves all three descriptions agree: raw storage bytes, a Panama
MemoryLayout, and a Valhalla value class decode every register identically; each layout describes exactly 12 bytes; and the schemas genuinely read differently, so the agreement is not trivial.false— and cannot betrue, all threeThe schema bolts on at the GROUP, not the register.
12 = 6×2 = 4×3 = 3×4means the largest group in any carving is 4 bytes — half the flattening budget — while the register is 12 and the facet 16, neither of which Java can flatten or needs to.Gates
132 Rust · 288 Java · clippy
-D warningsclean · fmt clean ·abi 0.7.Generated by Claude Code