Repository navigation
perf(avro): one cached name index per schema, not a scan per case field (#103) - #107
Merged
Merged
Conversation
kryptt
force-pushed
the
perf/nominal-preindex-103
branch
2 times, most recently
from
September 18, 2026 13:07
cf65c60 to
855d48e
Compare
kryptt
added this pull request to stack #112
September 18, 2026 13:29
…dicts RED for issue #103. Nothing here changes behaviour; it makes both halves of the problem executable before the fix touches anything. `NominalResolutionOracle` is a VERBATIM copy of PR #98's nominal rung. It is the differential oracle: a cost rewrite of a resolution rule is only safe if it is verdict-identical, and "the examples still pass" cannot show that — an example pins one cell, while the rung's semantics are a function over (schema, case-field list, declaration index). `NominalResolutionParitySpec` diffs the live rung against that oracle over 259 synthetic schemas x 585 case-name lists (151,515 pairs, every declIdx in [-1..arity]) built from deliberately confusable names — exact, case and separator collisions — plus every codec behind the three #98 suites. It also names each semantic separately, so a failure says WHICH rule broke: exact-beats-normalised, two-normalised-hits-is-none, the `arity > fields.size` short-circuit, whole-list injectivity, the empty-caseNames (NamedTuple) abstention, and the out-of-range declIdx abstention. `NominalResolutionCostSpec` is the failing half. It asserts nothing in nanoseconds — this box is noisy and only within-run ratios are load-bearing — but on the SHAPE: double the field count and a quadratic rung costs ~4x, a linear one ~2x. Against the current implementation: snake_cased n=66 warm 472,474 ns per resolution 2n/n = 3.97 (quadratic) 4n/n = 15.44 (quadratic) n=66 snake / identity = 242x `totalNominalIndex` widens from `private` to `private[avro]` so the parity spec can read its verdicts directly rather than inferring them from a resolved field name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
…ld (#103) GREEN for issue #103. The nominal rung's doctrine is unchanged — total and injective or abstain entirely, exact before normalised, two normalised hits is no hit. Only its cost moves. `nominalIndex` tried Avro's exact-name hash and, on a miss, linear-scanned every schema field calling `sameFieldName`, deliberately without an early exit: it has to see a SECOND match before it can call a name ambiguous. Run once per case field, that is `arity x fields.size x nameLength` per drilled hop — and it is charged to exactly the codecs the rung exists for, because a name transform (`withSnakeCaseFieldNames`, a custom `transformFieldNames`, a `vulcan.Codec` rename map) GUARANTEES the exact-name miss. An identity-named codec answers from the hash and never scanned, so the population the rung was written for was the only one paying. Now: one O(n) pass over the record's fields builds a normalised-name -> position index, and each case field is one lookup. Collision detection is preserved and merely MOVED — a key already present while BUILDING is the same signal the scan produced on its second hit, seen once instead of per query. Normalisation is per-CHARACTER (`Character.toLowerCase`), not `String.toLowerCase`, which is locale-sensitive and can change a string's length; that keeps the key exactly as discriminating as the cursor walk it replaces. The exact-name path is untouched and still allocation-free, so the identity-named codec neither builds nor consults an index. Cache: a `ConcurrentHashMap` side table keyed on the schema's REFERENCE identity behind WEAK keys. - identity, not `Schema` itself: `Schema.equals` is deep-structural and `Schema` is mutable (`addProp` resets its cached hash), so a structural key would pay a deep compare per lookup and could lose an entry under annotation; - not Avro's own per-schema props, the obvious slot: a prop is part of `equals`, of `toString` and of the JSON and canonical forms, so a cache parked there would change what the schema SERIALISES AS; - weak, not bounded: schemas are usually module-level and immortal, where both are equivalent, but a registry client resolving writer schemas per message would leak under a strong map and thrash under a bound. The value holds no reference back to the schema, so the entry is genuinely collectable; `NominalNameIndexCacheSpec` proves it by dropping a schema and watching it be collected. Measured, ns per resolution, MIN of 25 trials with both implementations timed in the same JVM run (the frozen pre-index rung is kept in the test tree as `NominalResolutionOracle`); schema construction is outside the clock: identity n= 2 warm 34 -> 36 cold 108 -> 107 identity n=12 warm 194 -> 196 cold 289 -> 256 identity n=66 warm 2,128 -> 2,122 cold 5,902 -> 4,067 snake_cased n= 2 warm 490 -> 224 cold 605 -> 757 snake_cased n=12 warm 17,271 -> 1,361 cold 18,367 -> 3,963 snake_cased n=66 warm 496,939 -> 8,745 cold 500,700 -> 20,466 2n/n 3.97 -> 2.19 4n/n 15.44 -> 5.12 n=66 snake / identity 242x -> 3.9x warm = repeated resolution against one schema, which is what a `def`-shaped or parameterised-`given` optic pays per operation. cold = first resolution against each of 200 never-before-seen schemas, index build included. The cold column REGRESSES on small records, and says so: building the index costs more than the scan it replaces when there is almost nothing to scan. cold, snake_cased, med of 25 trials (two runs) n=1 147 -> 434 / 148 -> 395 +287 / +247 ns 2.9x SLOWER n=2 719 -> 831 / 723 -> 765 +113 / +41 ns 1.2x SLOWER n=3 1339 -> 1126 / 1320 -> 1169 -213 / -151 ns 0.85x n=4 2296 -> 1538 / 2294 -> 1518 -758 / -776 ns 0.67x Crossover at n=3; the penalty is a few hundred nanoseconds, once per schema, amortised over every later hop into it. And it trades bytes for time on the transformed path, which the scan did not: `normalisedName` materialises a String where `sameFieldName` walked two cursors, so a miss now allocates ~128 B per case field (deterministic, via `getThreadAllocatedBytes`): identity n= 2 24 -> 24 B n=12 64 -> 64 B identity n=66 280 -> 280 B snake_cased n= 2 24 -> 280 B n=12 64 -> 1,600 B snake_cased n=66 280 -> 8,728 B Per OPERATION, not per construction, for a `def`-shaped optic. ~490 us of CPU for ~8 KB of nursery at n=66 is the right way round, but "B/op gates, ns/op advises" is this repo's doctrine, so it is on the record rather than implied. Hoisting the index lookup out of the per-case-field loop does not close it: the normalised String is ~104 B of the 128 and would survive untouched, and that hoist was built and measured at byte-for-byte unchanged. Closing it means probing the index by a cursor-computed hash of the normalised name and confirming the candidate with the allocation-free pairwise comparison — a larger change than #103 asked for. The 151,515-cell verdict diff against the frozen implementation is EMPTY. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
…probe Three changes, all documentation. The nominal rung's known-residual note now says what CANNOT close the residual, not only that it is open. A 31-agent investigation settled this today and the idea keeps being re-proposed: neither the record's schema NAME nor its parsing FINGERPRINT can detect the residual pair (two renamed, swapped columns of the same physical type within one record), because both candidate readings carry the same fullname and the same fingerprint by construction, and a schema-derived value only ever sees ONE of the two operands a comparison would need. The only mechanism that reaches the class is the differential codec probe — issue #100. The pre-index's two COSTS are now on the record, in the CHANGELOG and beside the code that pays them, because the entry as written read as if nothing regressed: - `normalisedName` materialises a String, so a resolution that misses the exact-name hash allocates ~128 B per case field where the pairwise cursor walk it replaced allocated nothing: 24 -> 280 B at n=2, 280 -> 8,728 B at n=66 per resolution (deterministic, via getThreadAllocatedBytes). The identity-named path is byte-identical, as claimed. This is per OPERATION for a `def`-shaped optic, so it is nursery traffic on the hot path, not construction-time only — and "B/op gates, ns/op advises" is the repo's own doctrine, so a future gate will see it. - Building the index costs more than the scan it replaces on a one- or two-field record, so the FIRST resolution against one is slower. The crossover is at n=3. Both are the right trade — ~490 us of CPU per resolution traded for ~8 KB of nursery at n=66, and a few hundred nanoseconds once per small schema — but they belong in the record rather than in a test comment. Plus the CHANGELOG entry for #103. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
…field The multiplier issue #103 set out to remove survived one level down. `totalNominalIndex` asked `nominalIndex` once per case field, and every `nominalIndex` that missed the exact-name hash called `normalisedNameIndex`, which probes the cache with a freshly allocated `SchemaQuery` key. On the transformed path — where EVERY exact lookup misses by construction, which is the whole population the nominal rung exists for — that is `arity` ConcurrentHashMap probes and `arity` throwaway keys per resolution, all of them answering with the same map. At arity 66: 66 probes, 66 keys, one map. The index is now threaded through the `@tailrec` loop as a recursion parameter: `null` until some case field needs it, the resolved map afterwards. Threaded and not hoisted above the loop, because hoisting would make the identity-named codec build or fetch an index it never reads — that path must stay untouched, and it is: the exact-name hash rung is still FIRST, still `record.getField(scalaName)`, still allocation-free. `nominalIndex` disappears as a separate method (it was the per-field wrapper), its two rungs inlined into the loop where the index can be carried across iterations. Verdicts are unchanged by construction — same rungs, same order, same `Ambiguous` demotion — and `NominalResolutionParitySpec` re-confirms it: 151,515 synthetic (schema, case-name list, declIdx) cells plus every real #98 fixture, zero mismatches against the frozen pre-index oracle. Measured with `com.sun.management.ThreadMXBean.getThreadAllocatedBytes`, warm, bytes per resolution (the harness the PR's disclosed figures came from): shape n=2 n=12 n=33 n=66 per case field identity 24 64 152 280 4 B (the `out` array, unchanged) snake before 280 1,600 4,376 8,728 132 B snake after 256 1,336 3,608 7,168 108 B The saving is exactly `(arity - 1) x 24 B` — one `SchemaQuery` per case field beyond the first — confirming the key was NOT being scalar-replaced by escape analysis. Net: -17.9% on the transformed path at n=66. What remains is the normalised `String` itself, ~104 B per case field, which is the disclosed trade and needs the cursor-hash probe to close. The identity-named path is byte-identical before and after. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
`normalisedNameIndex` returned the live `java.util.HashMap` it had just built and cached. `private[avro]` is a Scala-only fence — the method is PUBLIC in bytecode — and the map it returns is a process-wide singleton per schema behind a `ConcurrentHashMap`, so a single in-module (or reflective) `put` on the returned reference silently and permanently corrupts nominal field resolution for that schema for the life of the JVM. Every later resolution would read the corrupted entry as if the index had been built that way: wrong slot, valid wire bytes, no error. Nothing in the type or the name warned against it. The map is now wrapped in `Collections.unmodifiableMap`, and the WRAPPER — not the backing map — is what goes into the cache, so the wrap is paid once per schema at build time rather than per lookup, and repeated lookups keep handing back the same instance. That instance stability is what `NominalNameIndexCacheSpec` pins with `beTheSameAs`, and it still holds: the spec's "built once per schema and handed back by reference" and "repeated resolution never rebuilds the index" examples pass unchanged against the stored wrapper, as does the ambiguity example, which reads a value out of it. No test needed weakening. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
The comment on `NominalResolutionParitySpec`'s injectivity example talked about
`a`, `_a` and `A` — an earlier fixture. None of those names appear in the
example: it builds `record{a_b, b}` and asks for case fields `ab` and `aB`.
Rewritten to say what that fixture actually shows: both case fields normalise to
the key `ab`, which names the single schema field `a_b`, so the map is
non-injective and the rung abstains for EVERY field rather than letting the first
case field keep slot 0. Comment only — the example was and is correct.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
…omething `NominalResolutionCostSpec` put three timing-ratio assertions — 2n/n < 3.0, 4n/n < 10.0, snake/identity < 15.0 — into the always-run root `sbt test`, which means they gate the temurin@17 / @21 / @25 matrix on every push and every PR, on shared GitHub runners. That contradicts the project's own standing rule: a nanosecond on a shared box is +/-15-50%, only B/op and within-run ratios are load-bearing, and timing claims belong in the JMH bench pipeline. The margins are healthy (2n/n measures 2.09-2.24, 4n/n 4.61-5.35, snake/identity 3.30-5.08) but 3.0 against 2.18 is a 38% excursion from turning the entire matrix red for a reason unrelated to the diff under test. The three assertions are now armed by `-Deo.costGate=true` and skip — visibly, with the reason printed — without it. `quality.yml` arms them, on release tags and on demand, in a job of its OWN rather than a step in the coverage+mutation job: a timing flake must not abort that sweep and cost the tag its QA artifacts, a runner doing nothing else is the only place a wall-clock ratio means anything, and a separate job never sees scoverage-instrumented bytecode, which would make any timing meaningless. Nothing is weakened: same sizes, same bounds, same interleaving, run where a clock means something. It is the one gate in a workflow that is otherwise all reports, and it is isolated accordingly. The table above them still prints on every lane. It gates nothing, costs ~4 s, and is how a human notices drift between release-tag runs. Also added, since the gate now runs in a fresh JVM rather than after the table: an explicit discarded warm-up pass over all three sizes before the first timed round. The spec is `sequential`, so the table example did warm those paths, but that was an ordering coincidence, not a contract. Why not the two alternatives: - WIDEN the bounds and keep them in the default lane. The widening would have to be large enough to survive an arbitrarily contended runner, at which point the assertion stops separating linear from quadratic — and it leaves a clock on the critical path of every diff in the repo. Note also that the tightest bound is 2n/n at 3.0, not snake/identity at 15.0; widening only the one with the noisy denominator would have addressed the smaller half of the problem. - REPLACE with a non-timing assertion. Preferred, and not available. The deterministic halves of the claim are already pinned: `NominalNameIndexCacheSpec` pins one index instance per schema and no rebuild across repeated resolutions, `NominalResolutionParitySpec` pins every verdict. What is left is "how much work per case field", which is not observable from outside the method: `Schema`'s constructors are package-private so it cannot be subclassed to count field accesses, and an internal probe counter would put mutable state on the hot path this issue exists to make cheap. Allocated bytes — the repo's other load-bearing metric, and deterministic here — cannot see the regression that matters either: the quadratic scan allocated LESS than the index does (280 B vs 7,168 B at n=66), so a return to it would read as an improvement. The reasoning is recorded in the spec's own scaladoc, so the next reader does not re-run the search. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
…dex view The #103 entry quoted ~128 B per case field and 8,728 B at n=66. Hoisting the cache probe out of the per-case-field loop removed one `SchemaQuery` per field beyond the first — exactly `(arity - 1) x 24 B`, so the key was not being scalar-replaced — and the figures are now ~104 B per case field, 24 -> 256 B at n=2 and 280 -> 7,168 B at n=66. Also states that the identity-named path is byte-identical before and after, which is the invariant the hoist had to keep, and adds the unmodifiable-view entry. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
kryptt
force-pushed
the
perf/nominal-preindex-103
branch
from
September 18, 2026 13:31
855d48e to
d9e4dda
Compare
Contributor
|
🚀 Cloudflare Pages preview for https://573af685.cats-eo-docs.pages.dev Branch alias: https://perf-nominal-preindex-103.cats-eo-docs.pages.dev Built from commit |
Contributor
Benchmark A/BAllocation (B/op) — authoritative
38 more benchmarks
Timing (ns/op) — directional only, same-VM but shared runner
base_sha: |
kryptt
added a commit
that referenced
this pull request
Sep 18, 2026
…Walk #107 landed while this branch was open and rewrote the code the spec targets: `sameFieldName` and the fuzzy per-field scan are gone, replaced by `normalisedName` plus a weak-keyed `normalisedNameIndex` cache. The spec itself needed no change — it was written against the public seam `AvroWalk.fieldNameAt` precisely so it would survive an internals rewrite, and it agrees with the new implementation cell for cell — but its `covers:` block named deleted symbols and pre-#107 line numbers. Re-aims it at the surviving constructs by name and drops the line numbers, and states the division of labour with #107's own `NominalResolutionOracle`: that one is the OLD ALGORITHM frozen verbatim (a refactor gate — "the new rung agrees with the one it replaced"), this one is written from the DOCTRINE without reading the implementation. Parity cannot detect a wrong doctrine, which is the #104 failure mode; the doctrine oracle still passing against wholly rewritten internals is the evidence that #107 preserved the contract and not just the code path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V
kryptt
added a commit
that referenced
this pull request
Sep 18, 2026
…led, 14 fewer examples (#114) * test(avro): nominal-resolution doctrine oracle — kills 12 mutants in 68 lines PR #98 added an all-or-nothing nominal rung to `AvroWalk.fieldNameAt`: resolve `.field(_.x)` by NAME when every case field maps to a distinct schema field, otherwise abstain to position. Four clauses; only TOTALITY was pinned. Injectivity, exact-beats-normalised, and ambiguity-implies-abstain had no fixture that could tell a correct implementation from a broken one — the sole ambiguity fixture (`ResolutionResidualSpec` "at3") passes for the wrong reason, because an unrelated unmapped field forces totality-driven abstention first and masks the ambiguity branch entirely. That is not cosmetic. Issue #104 recorded an experimental alias rung silently breaking a case the rung gets right while all 212 avro tests stayed green: a MIS-TARGETED optic satisfies get-put, put-get, put-put and modify fusion perfectly, so no optic law family can see this. `AvroNominalDoctrineSpec` states the doctrine as a declarative oracle and checks `fieldNameAt` against it EXHAUSTIVELY over ~66k cells: every injective field list of length 1..3 from a 6-name pool (each name both a legal Avro field name and a legal Scala identifier), crossed with every case-name list of length 0..3 WITH repetition, crossed with declIdx from -1 to arity+1. The oracle is written from the doctrine, not copied from the implementation — parity against a frozen copy of the code cannot detect a wrong doctrine, which is precisely the #104 failure mode. Enumeration, not `Gen`: the discriminating cells are threshold cells a default generator would essentially never produce — a duplicate resolution landing on slot 0, a fuzzy ambiguity whose FIRST hit is at index 0 versus at index >= 1, and `declIdx` exactly equal to the schema's field count. Verified by applying each mutant to `AvroWalk.scala` and confirming the property goes red: 291 (exact fast path), 301 / 304 (the injectivity scan), 169 / 171 / 173 (ambiguity => abstain), 213 (`sameFieldName`'s char inequality), 310 (`declIdx >= fields.size` at the exact boundary), and 242 / 244 / 245 / 249 (the four disjuncts of the all-or-nothing precondition). Deletes the subsumed `"fieldNameAt: non-record parent and out-of-range declIdx both throw loudly"` example: it probed `declIdx = 99` against a 2-field schema, a cell where `>=` and a `>` mutant agree, so it pinned neither boundary. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * test(avro): count-preserving JSON structure faults — kills 7 mutants in 13 lines `AvroJsoniter.readRecord`'s strict exact-cover parse has three independent refusals — unknown key, duplicate key, and the trailing arity check — plus end-or-comma refusals in the array, map and byte-array readers. The module had no fixture for any of the delimiter faults, and its one duplicate-key arm was MASKED: appending `"i":42` makes 14 keys against 13 schema fields, so the arity check refuses the document first and the `seen` scan is never exercised. Same masking shape as the ambiguity fixture this branch's first commit replaced. Every new arm is deliberately COUNT-PRESERVING: - the duplicate REPLACES a sibling key (`"l":...` becomes a second `"i":42`), leaving 13 keys with `l` unset, so `seen` is the only thing that can refuse; - the four delimiter faults close a nested record / array / map / bytes array with the wrong bracket, which the outer reader then resumes past, so the document parses end-to-end unless the inner reader refuses. Kills 54 (`seen(field.pos) = true`), 57 and 59 (the `(field eq null) || seen(...)` guard and its operator), 71 (the record's objectEndOrCommaError), and 39 / 55 / 62 (the array / map / byte-array end-or-comma refusals). Drops the now-dominated 14-key duplicate arm. The triage's proposed "unknown JSON key" arm is NOT added: under both surviving guard mutants an unknown key dereferences a null `field`, the NPE is caught by `parseSlice`'s `NonFatal` handler, and the result is byte-identical to the real refusal — an equivalent path, not a gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * test(avro): decoded-map keys and union diagnostic payloads — kills 6 mutants in 46 lines Three gaps in the in-memory walker, all of the same species: the existing block asserts the SHAPE of the answer and never its content. Map keys. The map-walk example used a hand-built `LinkedHashMap[String, _]`, so `asMap.get(name: String)` hits directly and the `direct == null` Utf8 retry is dead code under test. A decoded map is keyed by `Utf8` — the shape production code actually sees — and only the retry finds anything in it. The new arm puts the record through the binary codec and probes every key of a 6-name pool against map sizes 0 / 1 / 2 / 5, so present and absent probes both occur at every size. Kills 322 (the Utf8 retry) and 338 (the PathMissing refusal, whose removal returns `Right(null)`). Null union branch. No fixture walked `UnionBranch("null")` into a null payload, the one branch name for which a null focus is correct rather than a failure. Kills 191. Union diagnostics. `UnionResolutionFailed` carries the declared alternative list, and every existing assertion matched it with a wildcard. Recovering that list walks back to the nearest Field step, so it needs a union two records deep to separate the parents cursor's `pIdx >= 0` guard from a `pIdx == 0` one (kills 183), and a path whose recovered Field step names an ARRAY rather than a union to pin the empty-list degradation — asking a non-union for its alternatives throws an AvroRuntimeException straight out of the walk (kills 254 and 256). Folded into the existing per-shape composite blocks per this file's stated consolidation convention: no new registrations. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * test(avro): byte-cursor refusal identities and the strictness policy — kills 11 mutants in 83 lines Every guard in `AvroBinaryCursor`'s path walk, when removed, lets the cursor run on into the Avro runtime, which throws, which `locateFrom` catches and reports as `BinaryParseFailed`. A `Left` either way — so the module's `isLeft` assertions pin none of them, and the structured diagnostic the `.record` face publishes degrades silently to "parse failed". These rows assert WHICH `AvroFailure` comes back, never that one does. Called at the `AvroBinaryCursor` seam (`private[avro]`, same package) rather than through a prism: several rows need a path the drilling macros refuse to build, and the strictness flag is not reachable from the public surface at all. Kills 592 (Field step against a non-record), 500 and 597 (UnionBranch step against a non-union, and the diagnostic's own non-union guard), 601 and 605 (the ordinal scan running off the end of the alternative list), 534 (the `requested < 0` refusal under the LENIENT policy — the strict path refuses for a different reason and masks it), 575 (locateElements' non-array terminal) and 506 (locateElements resolving its prefix strictly). Two rows carry their own fixtures: - 542, the terminal-vs-interior union split. An interior union step must not anchor the returned span. Every downstream observable still round-trips under the mutant, because the mis-anchored span still opens on a decodable value; only `span.valueSchema` gives it away. - 440, `AvroPrism`'s byte-face read resolving its terminal union strictly. The existing union fixtures all mismatch INTO a decode error, which `=== None` cannot tell apart from a correct refusal. New `Twin` fixture: two branches with identical field shapes and different names, so `TwinA(100)` and `TwinB(100)` encode to the same bytes and a lenient read hands back a well-formed value of the wrong type. The lenient policy is correct for the graft/write path only. The triage listed 440's witness as a null-branch payload with a trailing long field. That does not discriminate: `decodeSlice` reads a LENGTH-BOUNDED slice, and a null branch skips zero bytes, so the mutant hits EOF and Misses exactly like the real path. Replaced with the `Twin` fixture above. Also pins AvroTraversal 359 — `.each`'s prefix must resolve to an array or throw loudly. The public macro rejects a non-array focus at compile time, so the guard is only reachable at the internal seam, which is what a future drilling entry point would use. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * test(avro): array framing survives a write — kills 4 mutants in 32 lines The traversal's write path has three guards nobody could see: the no-op guard (`active.isEmpty || arity mismatch`), the canonical-splice vs re-frame split, and the initial canonical flag the block walk threads. Mutating any of them still produces a payload that DECODES to the right value, which is why the Optional-law property block at the foot of this file — and every other write-side assertion in the module — leaves all four alive. What changes is the FRAMING: a payload that arrives blocked and leaves canonical, or arrives multi-block and leaves single-block, is a rewrite of bytes the caller never asked us to touch. Two cells, both asserted at byte level, and neither is reachable from the existing fixtures: - all-miss under NON-canonical framing (every element on the other union branch). The write must be the identity on the bytes. On canonical framing the mutants re-frame to something byte-identical, so only the blocked fixture separates them. Kills 364 and 361. - canonical MULTI-BLOCK framing plus a length-preserving edit. Spec-legal, and the only shape where an in-place splice and a whole-region re-frame differ: collapsing two positive-count blocks into one costs a count byte, so the payload shrinks while decoding identically. Every other fixture here is single-block, where a re-frame reproduces the input exactly. Kills 356 and 586. Hand-assembled, since neither avro encoder emits it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * test(avro): fold 27 examples into 9 — 18 fewer, 0 kills lost The mandatory consolidation phase. Seven folds, chosen by MEASURED redundancy rather than by reading: stryker4s records the killing test in `statusReason`, so the report itself says which examples carry kills. Of avro's 210 registered tests only 65 are ever the credited killer of a mutant; every example folded below is in the other 145, which makes zero kill regression a property of the report and not a hope — the credited killer of each of the 431 killed mutants survives every fold untouched. It was then measured anyway. 1. AvroCodecDecoderReuseSpec — SEVEN properties differing only in the (generator, writer schema, reader schema) triple and in which decode entry point they called, collapsed to ONE. The triple is now data (`reuseScenarios`), and each scenario is put through BOTH entry points — the thread-local cache and the `threadLocalStorage = false` opt-out — where before each was checked by one example each. Strictly more coverage per shape. 2. AvroFieldNamingSpec — nine examples to three. The four byte-face examples were one fixture on one code path (fixture guard, `.field` read, `.field` modify, `selectDynamic`); record face and nested descent were a second pair. The two `.fieldNamed` examples are duplicates of `AvroNominalResolutionSpec`'s, which asserts strictly more of the refusal message, so they go. `.fields` stays its own example: it is the one shape in the module where the FOCUS codec's names diverge from the parent's. 3. ConfluentReaderSpec — the two unframed-refusal examples (`reader` / `recordReader`) become one over a shared predicate, keeping both payload shapes; the MOVED-field and PROMOTED-field examples become one over a shared `readFramed` rig. 4. AvroWriteCorrectnessSpec — the three `.each.union[Branch]` examples (blocked framing, canonical framing, record face) were one fixture and one optic handed to three carriers: now one example, every assertion kept. 5. AvroWriteCorrectnessSpec — the two `.each.fields` examples likewise differed only in how the same basket was framed. 6. AvroJsonSpec — the enum/bytes/fixed RENDER example and the enum/bytes/fixed ROUND-TRIP example built two near-identical three-leaf schemas. One schema, one value, and the render half now asserts the WHOLE document (`avroToJson(record) === json`) instead of three field lookups — stronger than what it replaced. MEASURED, not assumed. `project avroIntegration; stryker` re-run and diffed against the pre-consolidation report by the full (file, line, column, mutator, replacement) key: 431 Killed / 38 Survived / 17 NoCoverage / 6 CompileError / 122 Ignored BEFORE, and exactly the same after, with zero keys added, zero removed, zero Killed -> anything. Mutation score unchanged at 88.68% total / 91.90% of covered. Ran twice during the phase (after folds 1-3 and after 4-6); both diffs clean. Against this run's original baseline the whole branch is +40 kills with zero regressions. No assertion was weakened and no fixture dropped: every fold keeps every `must`/`===` it merged, and three of them add one. avro suite: 177 -> 159 `>>` registrations, 5,201 -> 5,122 test LOC. Against the run's starting point (173 / 4,952) that is 14 FEWER examples for +40 mutant kills; the +170 lines are the net of 242 lines of new high-leverage tests minus 79 removed here, i.e. 4.25 net lines per additional kill. Gates on JDK 25: `avroIntegration/test` Passed 198, Failed 0, Errors 0; `scalafixAll; scalafmtAll` then `scalafmtCheckAll; scalafixAll --check` clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V * docs(avro): re-aim the doctrine spec's covers-block at post-#107 AvroWalk #107 landed while this branch was open and rewrote the code the spec targets: `sameFieldName` and the fuzzy per-field scan are gone, replaced by `normalisedName` plus a weak-keyed `normalisedNameIndex` cache. The spec itself needed no change — it was written against the public seam `AvroWalk.fieldNameAt` precisely so it would survive an internals rewrite, and it agrees with the new implementation cell for cell — but its `covers:` block named deleted symbols and pre-#107 line numbers. Re-aims it at the surviving constructs by name and drops the line numbers, and states the division of labour with #107's own `NominalResolutionOracle`: that one is the OLD ALGORITHM frozen verbatim (a refactor gate — "the new rung agrees with the one it replaced"), this one is written from the DOCTRINE without reading the implementation. Parity cannot detect a wrong doctrine, which is the #104 failure mode; the doctrine oracle still passing against wholly rewritten internals is the evidence that #107 preserved the contract and not just the code path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
PR #98's nominal rung is correct and stays correct. This PR changes only what it costs.
Closes #103. #105 has landed onmain(squash75c7eaa); this branch is rebased ontomainand its diff is its own work only.The mechanism
AvroWalk.totalNominalIndexcallsnominalIndexonce per case field.nominalIndextries Avro'sexact-name hash and, on a miss, linear-scans every schema field calling
sameFieldName— withno early exit, deliberately, because it has to see a second match before it can call a name
ambiguous.
So a drilled hop costs
arity x fields.size x nameLength. And the bill lands on exactly thepopulation the rung exists for: a name transform (
withSnakeCaseFieldNames, a customtransformFieldNames, avulcan.Codecrename map) guarantees the exact-name miss, while anidentity-named codec answers from the hash and never scans at all — and was already resolving
correctly by position before #98 existed.
Two multipliers on top: the work is redone per drilled hop (a k-hop chain into one record pays
k times) and a
def-shaped or parameterised-givenoptic re-resolves per operation.The fix. One O(n) pass over the record's fields builds a
normalisedName -> positionindex;each case field is then one lookup. The index is cached per record schema, so the k-hop chain and
the per-operation re-resolution both pay once.
Two details worth naming:
codec never builds or consults an index, so the control path is bit-for-bit what it was — and its
allocation count is measured byte-identical. The transformed path pays for the time it saves:
see It trades bytes for time.
Character.toLowerCase), notString.toLowerCase. The latteris locale-sensitive and can change a string's length (
U+0130lower-cases to two chars), so itwould not agree with the pairwise cursor walk it replaces. Per-char keeps the key exactly as
discriminating as
sameFieldNamewas.building the index is the same signal the scan produced on its second hit — detected once per
schema instead of once per query.
Before / after
Correction to an earlier revision of this section. It claimed the cold column improved at
n=2 (3,230 -> 2,690 ns). It does not, and the change's own cost spec says so: building the index
costs more than the scan it replaces when there is almost nothing to scan, so the first
resolution against a one- or two-field record is slower. Everything below is re-measured —
min of 25 trials, both implementations timed in the same JVM (the frozen pre-index rung is kept in
the test tree as
NominalResolutionOracle), schema construction outside the clock — and theregression is stated rather than averaged away.
ns per resolution:
warm = repeated resolution against one schema (what a
def-shaped or parameterised-givenopticpays per operation); cold = first resolution against each of 200 never-before-seen schemas,
index build included.
The cold path regresses on small records
Median of 25 trials, snake_cased, two independent runs of the same harness:
Crossover at n=3. The penalty is bounded and one-time: a few hundred nanoseconds per schema,
paid on the first hop and amortised over every hop after it, against a 24x cold and 57x warm win at
n=66. The trade is obviously right — it is here because the section previously asserted the
opposite.
It trades bytes for time, and only on the transformed path
The
sameFieldNamecursor walk it replaces was allocation-free by design;normalisedNamematerialises a
Stringper case field per resolution. Deterministic counts, viacom.sun.management.ThreadMXBean.getThreadAllocatedBytes, identical across repeat runs:The identity-named row is the whole of what that path allocates: the
outarray, 16 B of headerplus 4 B per case field, and nothing else. It is the control, and it did not move.
This is per operation for a
def-shaped or parameterised-givenoptic, not construction-timeonly, so it is nursery traffic on the hot path — and "B/op gates, ns/op advises" is this repo's own
benchmark doctrine, so a future gate will see it. ~490 us of CPU for ~8 KB of nursery at n=66 is
the right way round, but it belongs on the record, not in a test comment.
Correction. An earlier revision of this section said the index lookup had been hoisted out of
the per-case-field loop and measured byte-for-byte unchanged. Half of that was wrong, and the
numbers above are the post-hoist ones. The hoist is now in (
perf(avro): resolve the name index once per resolution): the index is threaded through the@tailrecloop as a recursion parameterrather than re-fetched per case field, which removes one
ConcurrentHashMapprobe and onethrowaway
SchemaQuerykey per case field beyond the first. Measured saving: exactly(arity - 1) x 24 Bat every size — 24 B at n=2, 1,560 B at n=66 — so the key was not beingscalar-replaced by escape analysis, contrary to what "byte-for-byte unchanged" implied. Net
-17.9% on the transformed path at n=66.
Threaded, not hoisted above the loop: hoisting would make the identity-named codec fetch an index
it never reads, and that path has to stay exactly as it is.
What the hoist does not do is close the regression, and that half was right: the normalised
Stringis the remaining ~104 B per case field. Closing it properly means probing the index by acursor-computed hash of the normalised name and confirming the candidate with the old
allocation-free pairwise comparison — a larger change than #103 asked for, and deliberately not
taken here.
Both costs are now stated in the CHANGELOG and in the scaladoc beside the code that pays them.
The shape changed, which is the actual claim:
Identity-named fast path: unchanged. Warm is flat to within a few nanoseconds at every size and
the allocation count is byte-identical; the code path is provably untouched, since the index is
reached only from the exact-miss branch.
Cache effectiveness. First hop into a schema (cold, n=66 snake) 20,466 ns; second and every hop
after (warm) 8,745 ns, with the index instance identical across hops —
NominalNameIndexCacheSpecpins that by reference (beTheSameAs), so "a k-hop chain rebuilds ktimes" is refuted deterministically rather than by timing.
The verdict diff — empty
This is the load-bearing correctness step: a cost rewrite of a resolution rule is only safe if it
is verdict-identical, and "the suite is still green" cannot show that — an example pins one cell,
while the rung's semantics are a function over (schema, case-field list, declaration index).
NominalResolutionOracleis a verbatim copy of #98's implementation, frozen in the test tree.NominalResolutionParitySpecdiffs the live rung against it over two corpora:deliberately confusable names (
a,A,_a,a_b,ab,aB,b— exact, case and separatorcollisions), crossed with every case-name list of length 0-3 over that pool plus one unmatched
name, crossed with every
declIdxin[-1..arity]. Duplicated case names included, because aduplicate is the sharpest injectivity probe.
DivergentCodecs,ResolutionFalsePositiveSpec,AvroNominalResolutionSpecandResolutionResidualSpec— 27 codecs, printed field by field.Each semantic that was easy to lose is also pinned as its own named example, so a failure says
which rule broke rather than printing a synthetic triple:
Doc/AcctResolutionResidualSpecat3 (Tw)arity > record.getFields.sizeshort-circuitsFpRow,FpClickFpVisit(partial: not-total, not not-injective)caseNames(NamedTuple parent)ThreeNtindirectlydeclIdx < 0or>= arityTwo were genuinely uncovered before: whole-list injectivity (the suite had "not total" fixtures
but no "not injective" one) and the out-of-range
declIdxguard.Cache design, and the leak story
A
ConcurrentHashMapside table keyed on the schema's reference identity behind weak keys.Schemaas the key.Schema.equalsis deep-structural — a record comparesevery field, recursively — and
Schemais mutable:addPropresets its cached hash. Astructural key would pay a deep compare per lookup and could lose an entry when a schema is
annotated after insertion. (Verified against the Avro 1.11/1.12 sources:
hashCode()memoisesinto a field that
addPropinvalidates;RecordSchema.equalswalks the field list.)issue asked. Refuted: a prop is part of
Schema.equals, oftoString, and of the JSON andcanonical forms — parking a cache there would change what the schema serialises as, and
therefore its fingerprint. Hence a side table.
equivalent; but a registry client resolving writer schemas per message would leak them forever
under a strong map, or thrash under a bound. Weak keys make the entry's lifetime the schema's own.
The cached value holds no reference back to the schema — only
Strings copied out of the fieldnames — so the entry is genuinely collectable, and cleared keys are drained from a
ReferenceQueueon the next cache miss (never on the hot path).NominalNameIndexCacheSpecproves the no-retention claim by resolving through a schema, dropping every strong reference, and
watching it be collected.
WeakReference), so a cache hit costs no GCregistration; the stored key is the weak one, and
equalschecks reference identity first so acleared key can still be removed by the purge.
Scaladoc
Three additions.
One sentence on the nominal rung's known-residual note, per the investigation that settled it:
neither the record's schema name nor its parsing fingerprint can detect the residual — the
residual pair is two renamed, swapped columns of the same physical type inside one record, so both
candidate readings carry the same fullname and the same fingerprint by construction, and a
schema-derived value only ever sees one of the two operands a comparison would need. The only
mechanism that reaches the class is the differential codec probe, #100. This is there to stop the
idea being re-proposed.
And the pre-index's two costs, stated beside the code that pays them: the ~104 B per case field the
normalised key materialises on a miss, and the small-
ncold regression with its crossover atn=3.
Review pass: three fixes on top of the green PR
Three independent audit lenses read the pre-index. All three findings were real; two were code, one
was a policy call.
1. The cache probe was still per case field
The multiplier #103 exists to remove had survived one level down.
totalNominalIndexasked aper-case-field helper for each name, and every helper call that missed the exact-name hash called
normalisedNameIndex, which probes the cache with a freshly allocatedSchemaQuery. On thetransformed path every exact lookup misses by construction, so at arity 66 that is 66 CHM probes
and 66 throwaway keys per resolution, all answering with the same map.
Fixed by threading the resolved index through the
@tailrecloop. The exact-name rung is untouchedand still first. Numbers in the allocation section above; the identity path is byte-identical.
2.
normalisedNameIndexhanded out the live mutable mapprivate[avro]is a Scala-only fence — the method is public in bytecode — and what it returned wasthe live
java.util.HashMapsitting in a process-wide, per-schema cache. Oneputon thatreference silently and permanently corrupts field resolution for that schema for the life of the
JVM: wrong slot, valid wire bytes, no error.
Now wrapped in
Collections.unmodifiableMap, with the wrapper stored in the cache so the wrapis one allocation per schema rather than one per lookup.
NominalNameIndexCacheSpecpins instanceidentity with
beTheSameAsand still passes unchanged against the stored wrapper — no test neededweakening.
3. Wall-clock assertions gated the whole CI matrix
NominalResolutionCostSpecput three timing-ratio assertions into the always-run rootsbt test,so they gated the temurin@17 / @21 / @25 matrix on shared GitHub runners. That contradicts this
repo's own standing rule — a nanosecond on a shared box is +/-15-50%, only B/op and within-run
ratios are load-bearing, which is why timing claims live in the JMH pipeline. The margins are
healthy, but 2n/n's bound of 3.0 against a measured 2.18 is a 38% excursion from turning the whole
matrix red for a reason unrelated to the diff under test. (Worth noting: the tightest bound is
2n/n, not the
snake/identityone with the noisy denominator.)Chosen: run them under the quality workflow. The three assertions are armed by
-Deo.costGate=trueand skip — visibly, with the reason printed — without it.quality.ymlarmsthem in a job of its own, not a step in the coverage+mutation job: a timing flake must not abort
that sweep and cost the tag its QA artifacts, a runner doing nothing else is the only place a
wall-clock ratio means anything, and a separate job never sees scoverage-instrumented bytecode,
which would make any timing meaningless. It is the one gate in a workflow that is otherwise all
reports, and it is isolated accordingly. Nothing is weakened: same sizes, same bounds, same
interleaving. The table above them still prints on every lane, gates nothing, and costs ~4 s. An
explicit discarded warm-up over all three sizes was added too, since the gate no longer runs right
after the table that used to warm it.
Rejected — widen and keep in the default lane. The widening would have to survive an arbitrarily
contended runner, at which point the assertion stops separating linear from quadratic, and it
leaves a clock on the critical path of every diff in the repo.
Rejected — replace with a non-timing assertion. Preferred, and searched for first; it does not
exist. The deterministic halves of the claim are already pinned (
NominalNameIndexCacheSpecpinsone index instance per schema and no rebuild across resolutions;
NominalResolutionParitySpecpinsevery verdict). What is left is "how much work per case field", which is not observable from
outside the method:
Schema's constructors are package-private so it cannot be subclassed to countfield accesses, and an internal probe counter would put mutable state on the hot path this issue
exists to make cheap. Allocated bytes — deterministic here, and this repo's other load-bearing
metric — cannot see the regression that matters either: the quadratic scan allocated less than
the index does (280 B vs 7,168 B at n=66), so a return to it would read as an improvement. The
reasoning is recorded in the spec's scaladoc so it is not re-searched.
Also
One stale comment: the parity spec's injectivity example described
a,_aandA, none of whichappear in it. Rewritten to describe the actual fixture — two case fields normalising onto one schema
field, so the map is non-injective and the rung abstains entirely rather than letting the first one
win. The example was and is correct.
Gates
JDK 25 (Gentoo Temurin 25.0.4.1), so the
kyomodule was in the root aggregate(
KyoOpticsSpecran).sbt compilesbt test(root aggregate, incl. kyo)sbt avroIntegration/testsbt benchmarks/compilesbt mimaReportBinaryIssuesmimaPreviousArtifactsis empty on every module in this branch state)sbt docs/mdoc; docs/laikaSitesbt scalafmtCheckAll; scalafixAll --check; scalafmtSbtCheck; benchmarks/scalafmtCheckThe 2 skipped are the cost spec's ratio examples, disarmed as described above. The example count is
unchanged at 878 — nothing was deleted, the two assertions moved lanes.
The verdict diff was re-run after the resolution-path change and is still empty: 151,515
synthetic cells plus every real fixture, zero mismatches.
Every gate above was re-run after the correction pass (which touched the CHANGELOG, two scaladoc
blocks and two commit messages) and the format gate ran last, clean, with no files rewritten.
One thing worth knowing:
scalafmtCheckAllreported success once from a stale cache while threetest files were in fact unformatted. It reproduced on the review pass — a check run straight after a
format run reads the cache that run just wrote and examines almost nothing. The final gate here was
therefore run with every
target/streams/*/scalafmt*and*/scalafix*cache deleted first, so all22 source groups (Compile + Test for every module, avro included) were genuinely re-read: clean.
Also worth knowing:
scalafixAllandscalafmtAlldisagree about a trailing comma in a multi-lineimport — scalafix removes what scalafmt accepts — so the two must be run in that order, not one or
the other.
Commits
test(avro)— the RED harness. Fails on the current implementation with 2n/n = 3.97, 4n/n =15.44 and 472,474 ns per resolution at 66 snake_cased fields.
perf(avro)— the pre-index and its cache.docs(avro)— the residual sentence, the cost disclosures (allocation and the small-ncoldregression), and the CHANGELOG entry.
Review pass, one commit per finding:
perf(avro)— resolve the name index once per resolution, not per case field.fix(avro)— hand out an unmodifiable view of the cached name index.docs(avro)— the injectivity comment describes the example below it.test(avro)— arm the wall-clock cost ratios only where a clock means something.docs(changelog)— the measured per-field cost, and the unmodifiable index view.Out of scope on purpose: no alias rung (#104), no new resolution rungs, no refactors the pre-index
did not need.
🤖 Generated with Claude Code
https://claude.ai/code/session_0194EHFR4NamCpTHiqy7B74V