diff --git a/.github/workflows/quality.yml b/.github/workflows/quality.yml index a9ca36da..068186fa 100644 --- a/.github/workflows/quality.yml +++ b/.github/workflows/quality.yml @@ -29,6 +29,42 @@ concurrency: cancel-in-progress: true jobs: + # The one thing in this workflow that IS a gate. `NominalResolutionCostSpec`'s + # three ratio assertions — 2n/n and 4n/n over the schema's field count, and + # snake/identity — are armed only by `-Deo.costGate=true`, and this is the lane + # that arms them (issue #103). They are a clock, and the push/PR test matrix is + # three JDK jobs on shared runners where project policy says a nanosecond is + # +/-15-50%, so leaving them there would gate every diff in the repo on someone + # else's noisy neighbour. The spec's cost table prints on every lane either way. + # + # A SEPARATE JOB, not a step in `report`, for two reasons: a timing flake must + # not abort the coverage and mutation sweep and cost this tag its QA artifacts, + # and a runner doing nothing else is the only place a ratio of wall-clock + # numbers means anything. It also never sees scoverage-instrumented bytecode, + # which would make any timing meaningless. + cost: + name: Nominal resolution cost gate + runs-on: ubuntu-22.04 + timeout-minutes: 30 + env: + SBT_OPTS: "-Xmx4g -Xss8m -Dsbt.color=false" + steps: + - name: Checkout + uses: actions/checkout@v7 + + - name: Setup sbt + uses: sbt/setup-sbt@v1 + + - name: Setup Java (temurin@25) + uses: actions/setup-java@v6 + with: + distribution: temurin + java-version: 25 + cache: sbt + + - name: Cost ratios (armed) + run: sbt -batch -Deo.costGate=true "avroIntegration/testOnly *NominalResolutionCostSpec" + report: name: Coverage + mutation runs-on: ubuntu-22.04 diff --git a/CHANGELOG.md b/CHANGELOG.md index f0e2ace3..f9c234bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,39 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 name that misses at runtime. `AvroPrism` / `AvroTraversal`, `JsonPrism` / `JsonTraversal` (eo-circe) and `JsoniterPrism` / `JsoniterTraversal` (eo-jsoniter) all share the parser, so all six surfaces are covered. +- **Schema-field name resolution is linear in the record's field count, not quadratic** (#103): the + nominal rung above called a per-case-field lookup that, on an exact-name MISS, linear-scanned + every schema field with no early exit — it has to see a second match to call a name ambiguous. So + a drilled hop cost `arity x fields.size x nameLength`, and it was charged to exactly the codecs + the rung exists for, since a name transform (`withSnakeCaseFieldNames`, a custom + `transformFieldNames`, a `vulcan.Codec` rename map) guarantees the miss; an identity-named codec + answers from Avro's own name hash and never scanned. Measured on a 66-field snake_cased record: + **486 us -> 8.8 us per resolution**, with the n -> 2n cost ratio falling from 3.97 (quadratic) to + 2.2 (linear). Resolution now builds one normalised-name index per record schema in a single pass + and caches it under the schema's REFERENCE identity behind weak keys, so a k-hop chain into one + record builds it once and a `def`-shaped optic — which re-resolves per operation — stops paying + per operation. The doctrine is untouched: `NominalResolutionParitySpec` diffs every verdict of the + new rung against the frozen pre-index implementation over 151,515 synthetic + (schema x case-name-list) pairs plus every fixture behind the #95 suites, and the diff is empty. + The exact-name fast path is unchanged and still allocation-free — an identity-named codec + neither builds nor consults an index, and its per-resolution allocation is byte-identical before + and after (24 / 64 / 152 / 280 B at n=2 / 12 / 33 / 66, all of it the `out` array). The index is + resolved ONCE per resolution and threaded through the case fields, not re-fetched per field: + per-field it would be a `ConcurrentHashMap` probe and a throwaway lookup key each, which is the + same multiplier this issue removes, merely one level down. Two costs are traded for the time, + both on the transformed path only and both measured with + `com.sun.management.ThreadMXBean.getThreadAllocatedBytes`: the normalised key is a materialised + `String`, so a resolution that misses the exact-name hash now allocates **~104 B per case field** + where the pairwise cursor walk it replaces allocated nothing (24 -> 256 B at n=2, 280 -> + 7,168 B at n=66 per resolution); and building the index makes the FIRST resolution against a 1-2 + field record **slower** (n=1 ~+250 ns, n=2 ~+100 ns, once per schema), with the crossover at n=3 + and a 1.5x / 4.6x / 24x cold win at n=4 / 12 / 66. +- **The cached name index is handed out as an unmodifiable view** (#103): `private[avro]` is a + Scala-only fence and the cached index is a process-wide singleton per schema, so returning the + live `HashMap` meant one in-module `put` could silently corrupt field resolution for that schema + for the life of the JVM. The wrapper, not the backing map, is what the cache stores, so the wrap + costs one allocation per schema rather than one per lookup and repeated lookups still hand back + the same instance. ### Changed diff --git a/avro/src/main/scala/dev/constructive/eo/avro/AvroWalk.scala b/avro/src/main/scala/dev/constructive/eo/avro/AvroWalk.scala index ba9ee609..f261b8d4 100644 --- a/avro/src/main/scala/dev/constructive/eo/avro/AvroWalk.scala +++ b/avro/src/main/scala/dev/constructive/eo/avro/AvroWalk.scala @@ -4,7 +4,16 @@ import scala.annotation.tailrec import scala.jdk.CollectionConverters.* import dev.constructive.eo.widenRight -import java.util.{ArrayList, LinkedHashMap, List as JList, Map as JMap} +import java.lang.ref.{ReferenceQueue, WeakReference} +import java.util.concurrent.ConcurrentHashMap +import java.util.{ + ArrayList, + Collections as JCollections, + HashMap as JHashMap, + LinkedHashMap, + List as JList, + Map as JMap +} import org.apache.avro.Schema import org.apache.avro.generic.{GenericData, GenericEnumSymbol, GenericFixed, IndexedRecord} @@ -433,7 +442,13 @@ private[avro] object AvroWalk: * * '''Known residual''': a codec that both renames beyond recognition AND reorders (equal arity, * no name hit) still resolves by position, and is still wrong — nothing about the names or the - * shape can see it. Use [[AvroPrism.fieldNamed]] there. + * shape can see it. Use [[AvroPrism.fieldNamed]] there. Nothing SCHEMA-DERIVED can close it + * either — not the record's name, not its parsing fingerprint: the residual pair is two renamed, + * swapped columns of the same physical type within ONE record, so both candidate readings carry + * the same fullname and the same fingerprint by construction, and a schema-derived value sees + * only one of the two operands the comparison would need. The only mechanism that reaches this + * class is the differential codec probe (issue #100) — encoding a discriminating value through + * the codec and reading which slot moved. * * '''Known behaviour change''': a codec that PERMUTES the Scala names (writes case field `a` * into a schema field literally named `b`, and vice versa) resolved correctly by position and @@ -511,8 +526,38 @@ private[avro] object AvroWalk: * matches nothing, the map is not total, the rung abstains, position stays right. * * An empty `caseNames` (a NamedTuple parent, which has no case fields) abstains too. + * + * '''Per case field, two rungs, and the order matters.''' The EXACT name first, through Avro's + * own per-record name hash: O(1), allocation-free, and the only rung an identity-named codec + * ever reaches — it neither builds nor consults an index. Only a miss consults the normalised + * index (issue #103), which is built once per record schema and cached, rather than re-scanning + * every schema field per case field. + * + * The index is resolved ONCE per resolution and threaded through the remaining case fields. Per + * case field it would be a `ConcurrentHashMap` probe and a throwaway lookup key EACH — at arity + * 66, 66 probes and 66 keys that all answer with the same map — which is the very multiplier + * issue #103 set out to remove, merely moved one level down. Threaded as a recursion parameter + * and not hoisted above the loop, because that would make the identity-named codec pay for an + * index it never reads. + * + * What the miss path pays INSTEAD of the scan, stated because it is a real regression and the + * repo's benchmark doctrine gates on bytes: [[normalisedName]] materialises a `String`, so a + * resolution that misses the exact-name hash allocates ~104 B per case field where the pairwise + * cursor walk it replaced allocated nothing (measured, snake_cased: 24 -> 256 B at n=2, 280 -> + * 7,168 B at n=66, the residue being the `out` array both shapes allocate). That is the + * deliberate trade — ~490 us of CPU per resolution for ~7 KB of nursery at n=66 — and it is per + * OPERATION, not per construction, for a `def`-shaped optic. Closing it would mean probing the + * index by a cursor-computed hash of the normalised name and confirming the candidate with an + * allocation-free pairwise comparison: a larger change than issue #103. + * + * `private[avro]` only so `NominalResolutionParitySpec` can diff every verdict of this function + * against the frozen pre-index implementation it was rewritten from (issue #103). */ - private def totalNominalIndex(record: Schema, caseNames: List[String], declIdx: Int): Int = + private[avro] def totalNominalIndex( + record: Schema, + caseNames: List[String], + declIdx: Int, + ): Int = val arity = caseNames.size // A total, injective map needs at least as many schema fields as case fields — a free // precondition that skips the whole scan for the codec that drops a field. @@ -524,54 +569,174 @@ private[avro] object AvroWalk: // Set here costs a boxed Integer and a new Set node per field. @tailrec def seen(j: Int, idx: Int): Boolean = j < 0 || (out(j) != idx && seen(j - 1, idx)) - @tailrec def loop(i: Int, rest: List[String]): Boolean = + // `index` is the normalised index once some case field has needed it, `null` until then. + @tailrec def loop(i: Int, rest: List[String], index: JMap[String, Integer] | Null): Boolean = rest match case Nil => true case n :: t => - val idx = nominalIndex(record, n) - if idx < 0 || !seen(i - 1, idx) then false + val exact = record.getField(n) + if exact != null then + val idx = exact.pos + if !seen(i - 1, idx) then false + else + out(i) = idx + loop(i + 1, t, index) else - out(i) = idx - loop(i + 1, t) - if loop(0, caseNames) then out(declIdx) else -1 + val here = if index != null then index else normalisedNameIndex(record) + val hit = here.get(normalisedName(n)) + // A key present with `Ambiguous` is the same verdict the scan produced on its SECOND + // hit: more than one schema field normalises to this name, so the signal is no signal. + val idx = if hit == null then -1 else hit.intValue + if idx < 0 || !seen(i - 1, idx) then false + else + out(i) = idx + loop(i + 1, t, here) + if loop(0, caseNames, null) then out(declIdx) else -1 - /** Position of the schema field naming `scalaName` — exactly, else uniquely up to separators and - * case. `-1` when no field matches or when more than one does: an ambiguous signal is no signal. - */ - private def nominalIndex(record: Schema, scalaName: String): Int = - val exact = record.getField(scalaName) - if exact != null then exact.pos - else - val fields = record.getFields - @tailrec def loop(i: Int, found: Int): Int = - if i >= fields.size then found - else if sameFieldName(fields.get(i).name, scalaName) then - if found >= 0 then -1 else loop(i + 1, i) - else loop(i + 1, found) - loop(0, -1) - - /** `a` and `b` name the same field up to `_` / `-` / `.` and case: `landingPageId`, - * `landing_page_id`, `LANDING_PAGE_ID` and `landing-page-id` all match. Two cursors rather than - * two normalised copies — this runs once per candidate schema field per drilled hop, at - * construction time, and has no business allocating. + /** `_` / `-` / `.` stripped and everything lower-cased: the canonical form under which + * `landingPageId`, `landing_page_id`, `LANDING_PAGE_ID` and `landing-page-id` are one name. + * + * Lower-casing CHARACTER BY CHARACTER (`Character.toLowerCase`), not `String.toLowerCase`: the + * latter is locale-sensitive and can change a string's LENGTH (`U+0130` maps to two chars), so + * it would not agree with the pairwise cursor comparison this replaces. Per-char keeps the + * normalised key exactly as discriminating as the old `sameFieldName` walk. * * NOT a transform inverter: it recognises only the letter-preserving transform family, which is * exactly why an unrecognised transform falls through to the positional rung (issue #35) instead * of being guessed at. */ - private def sameFieldName(a: String, b: String): Boolean = - @tailrec def skip(s: String, i: Int): Int = - if i >= s.length then i - else - val c = s.charAt(i) - if c == '_' || c == '-' || c == '.' then skip(s, i + 1) else i - @tailrec def loop(i: Int, j: Int): Boolean = - val x = skip(a, i) - val y = skip(b, j) - if x >= a.length || y >= b.length then x >= a.length && y >= b.length - else if Character.toLowerCase(a.charAt(x)) != Character.toLowerCase(b.charAt(y)) then false - else loop(x + 1, y + 1) - loop(0, 0) + private def normalisedName(name: String): String = + val out = new java.lang.StringBuilder(name.length) + @tailrec def go(i: Int): Unit = + if i < name.length then + val c = name.charAt(i) + if c != '_' && c != '-' && c != '.' then out.append(Character.toLowerCase(c)): Unit + go(i + 1) + go(0) + out.toString + + /** The ambiguity sentinel stored in a [[normalisedNameIndex]] — two or more schema fields share + * one normalised name, so no case field may resolve through it. + */ + private val Ambiguous: Integer = Integer.valueOf(-1) + + /** `normalisedName(field) -> field position`, or [[Ambiguous]] where two schema fields collide — + * ONE O(n) pass over the record's fields, cached per schema (issue #103). + * + * What this replaces: a linear scan of every schema field, per case field, per drilled hop, with + * no early exit (the scan had to see a second match to call it ambiguous). That is + * `arity x fields.size x nameLength` for every codec under a name transform — which is exactly + * the population the nominal rung exists for, since an identity-named codec was already right + * positionally. Collision detection is preserved and merely MOVED: a key already present while + * BUILDING is the same signal the scan's second hit produced, seen once instead of per query. + * + * Not free at every size: building the index costs more than the scan it replaces on a ONE- or + * TWO-field record, so the FIRST resolution against such a schema is measurably slower (~+250 ns + * at n=1, ~+100 ns at n=2, once per schema, amortised over every later hop). The crossover is at + * n=3; by n=66 the first resolution is ~24x faster and every later one ~57x. + * + * What is handed back is an UNMODIFIABLE view, and the view — not the backing map — is what the + * cache stores, so the wrap is paid once per schema rather than per lookup. `private[avro]` is a + * Scala-only fence: the method is PUBLIC in bytecode, and the value is a process-wide singleton + * per schema, so handing out the live `HashMap` would let one careless in-module (or reflective) + * `put` corrupt field resolution for that schema for the life of the JVM — silently, since every + * later caller would read the corrupted entry as if the index had been built that way. + * + * `private[avro]` only so the differential harness can compare cache instances directly. + */ + private[avro] def normalisedNameIndex(record: Schema): JMap[String, Integer] = + val cached = nameIndexCache.get(new SchemaQuery(record)) + if cached != null then cached + else + val fields = record.getFields + val built = new JHashMap[String, Integer](Math.max(4, fields.size * 2)) + @tailrec def fill(i: Int): Unit = + if i < fields.size then + val key = normalisedName(fields.get(i).name) + // `put`, then demote to Ambiguous on a collision: the FIRST writer's position is + // irrelevant once a second field claims the key — both are unreachable through it. + val prior = built.put(key, Integer.valueOf(i)) + if prior != null then built.put(key, Ambiguous): Unit + fill(i + 1) + fill(0) + val view = JCollections.unmodifiableMap(built) + // Drain first: a purge costs nothing on the hot path because it only runs on a cache MISS, + // which is once per schema for the lifetime of that schema. + purgeStaleIndexKeys() + val raced = nameIndexCache.putIfAbsent(new SchemaWeakKey(record, staleIndexKeys), view) + if raced == null then view else raced + + /** Weak, IDENTITY-keyed side table behind [[normalisedNameIndex]]. + * + * Identity and not `Schema` itself as the key: `Schema.equals` is deep-structural (a record + * compares every field, recursively) and `Schema` is MUTABLE — `addProp` resets its cached hash + * — so a structural key pays a deep compare per lookup and can lose an entry when a schema is + * annotated after insertion. Reference identity is O(1), stable, and is the right granularity + * anyway: the index depends on nothing but this object's field names. + * + * Not Avro's own per-schema property storage, which was the obvious slot to reach for first: a + * prop is part of `Schema.equals`, of `toString`, and of the JSON and canonical forms, so + * parking a cache there would change what the schema SERIALISES AS. Refuted, hence the side + * table. + * + * WEAK keys and not a bounded cache: schemas are typically module-level and live forever, in + * which case both are equivalent — but a caller that builds schemas dynamically (a registry + * client resolving writer schemas per message) would either leak them forever under a strong map + * or thrash under a bound. Weak keys make the entry's lifetime the schema's own. The VALUE holds + * no reference back to the schema (only `String` keys copied out of the field names), so an + * entry is genuinely collectable. + */ + private val nameIndexCache: ConcurrentHashMap[SchemaId, JMap[String, Integer]] = + new ConcurrentHashMap[SchemaId, JMap[String, Integer]]() + + /** Cleared [[SchemaWeakKey]]s awaiting removal from [[nameIndexCache]]. */ + private val staleIndexKeys: ReferenceQueue[Schema] = new ReferenceQueue[Schema]() + + /** Evict the entries whose schema has been collected. Runs on a cache MISS only. */ + private def purgeStaleIndexKeys(): Unit = + @tailrec def drain(): Unit = + val stale = staleIndexKeys.poll() + if stale != null then + nameIndexCache.remove(stale.asInstanceOf[SchemaId]): Unit + drain() + drain() + + /** Identity-hashing key face: a live query key and a stored weak key must hash and compare alike. + */ + sealed private trait SchemaId: + + /** The schema this key stands for, or `null` once a weak key's referent is collected. */ + def schemaRef: Schema | Null + + def idHash: Int + + final override def hashCode(): Int = idHash + + final override def equals(other: Any): Boolean = + other match + case that: SchemaId => + // Identity FIRST so a cleared weak key can still be removed by the purge — its referent + // is gone, so the reference comparison below would refuse to match even itself. + (this eq that) || { + val mine = schemaRef + mine != null && (mine eq that.schemaRef) + } + case _ => false + + end SchemaId + + /** Throwaway lookup key — a plain object, so a cache HIT costs no `WeakReference` registration. + */ + final private class SchemaQuery(record: Schema) extends SchemaId: + val schemaRef: Schema = record + val idHash: Int = System.identityHashCode(record) + + /** The stored key: weak, so the entry dies with the schema. */ + final private class SchemaWeakKey(record: Schema, queue: ReferenceQueue[Schema]) + extends WeakReference[Schema](record, queue), + SchemaId: + val idHash: Int = System.identityHashCode(record) + def schemaRef: Schema | Null = get() /** Union-branch name for a runtime value. Schema-driven where possible — the named leaves * (records, enums, fixed) answer with their schema's FULL name, matching how union branches are diff --git a/avro/src/test/scala/dev/constructive/eo/avro/NominalCostFixtures.scala b/avro/src/test/scala/dev/constructive/eo/avro/NominalCostFixtures.scala new file mode 100644 index 00000000..a0109171 --- /dev/null +++ b/avro/src/test/scala/dev/constructive/eo/avro/NominalCostFixtures.scala @@ -0,0 +1,119 @@ +package dev.constructive.eo.avro + +import scala.annotation.tailrec + +import java.util.ArrayList +import java.util.concurrent.atomic.{AtomicInteger, AtomicLong} +import org.apache.avro.Schema + +/** Schema generators and a min-of-reps timer for the issue-#103 cost specs. + * + * Two codec shapes, because the nominal rung's two rungs have wildly different costs and only one + * of them was ever exercised by the #98 benchmark: + * + * - IDENTITY-named — the schema field is spelled exactly like the case field, so Avro's own + * per-record name hash answers in O(1) and nothing else runs. This is the control: it must not + * move. + * - SNAKE_CASED — every exact lookup misses, which is the population the nominal rung exists for + * (an identity-named codec was already right positionally). This is where the cost lives. + */ +object NominalCostFixtures: + + /** Fixed-width names so the per-comparison character count does not drift with the field count — + * otherwise a cost ratio would conflate `fields.size` with `nameLength`. + */ + def fieldName(i: Int, snake: Boolean): String = + val n = f"$i%03d" + if snake then s"alpha_beta_gamma_$n" else s"alphaBetaGamma$n" + + /** The Scala case-field names: always camelCase, whatever the schema spells. */ + def caseNames(n: Int): List[String] = List.tabulate(n)(i => fieldName(i, snake = false)) + + private val serial = new AtomicInteger(0) + + /** A fresh record schema of `n` string fields. Distinct record names, so no two generated schemas + * are structurally equal by accident. + */ + def schema(n: Int, snake: Boolean): Schema = + named(s"Rec${serial.incrementAndGet()}", n, snake) + + /** As [[schema]], with the record name pinned — two calls with one name give STRUCTURALLY EQUAL + * but non-identical schemas, which is how the cache's identity keying is pinned. + */ + def named(recordName: String, n: Int, snake: Boolean): Schema = + val fields = new ArrayList[Schema.Field](n) + (0 until n).foreach: i => + fields.add( + new Schema.Field(fieldName(i, snake), Schema.create(Schema.Type.STRING), null, null) + ) + Schema.createRecord(recordName, null, "eo.perf", false, fields) + + /** Keeps the measured results reachable so nothing under test folds away. */ + val sink: AtomicLong = new AtomicLong(0L) + + /** Is the WALL-CLOCK gate armed? `-Deo.costGate=true` arms it; it is disarmed by default. + * + * Disarmed means disarmed for the `Test (temurin@17 / @21 / @25)` matrix that gates every push + * and every PR, and that is the point. Those are shared GitHub runners, and this project's + * standing rule is that 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 bench pipeline and not in + * `sbt test`. A ratio of two timings taken in one interleaved run is the least noisy thing a + * clock can give, and it is still a clock: the 2n/n bound of 3.0 sits only 39% above the + * measured 2.16, so one contended window on one of the three JDK lanes reds the whole matrix for + * a reason that has nothing to do with the diff under test. + * + * Armed in + * [[https://github.com/Constructive-Programming/eo/blob/main/.github/workflows/quality.yml quality.yml]], + * which runs on release tags and on demand — the same lane, and for the same reason, as the + * coverage and mutation reports: expensive, noise-sensitive checks worth having at release time + * and not worth a flaky red on every push. The gate is not weakened there; it is the same + * assertion, run where a clock means something. + */ + val costGateArmed: Boolean = + java.lang.Boolean.parseBoolean(sys.props.getOrElse("eo.costGate", "false")) + + /** Why a gated example skipped, printed in the specs2 output so the exclusion is VISIBLE rather + * than a quietly absent assertion. + */ + val costGateDisarmed: String = + "wall-clock gate disarmed: re-run with -Deo.costGate=true (quality.yml arms it on release tags)" + + /** Nanoseconds per resolution, MIN over `reps` passes of `iters` resolutions. + * + * Min and not mean: this box is noisy (project policy — ns/op here is +/-15-50%, only within-run + * ratios are load-bearing), and the minimum is the sample least contaminated by whatever else + * the machine was doing. Every committed assertion is on a RATIO of two such numbers taken in + * the same run, never on an absolute. + */ + def costNs(reps: Int, iters: Int)(op: Int => Int): Double = + @tailrec def pass(r: Int, acc: Double): Double = + if r <= 0 then acc + else + @tailrec def run(i: Int, total: Int): Int = + if i >= iters then total else run(i + 1, total + op(i)) + val t0 = System.nanoTime() + val out = run(0, 0) + val t1 = System.nanoTime() + sink.addAndGet(out.toLong): Unit + val ns = (t1 - t0).toDouble / iters.toDouble + pass(r - 1, if ns < acc then ns else acc) + pass(reps, Double.MaxValue) + + /** Cost of the FIRST resolution against each of `k` never-before-seen schemas — one single pass, + * because a second pass would find them all cached and would be measuring something else. Schema + * construction happens outside the clock. + */ + def coldCostNs(n: Int, snake: Boolean, k: Int)(resolve: (Schema, List[String]) => Int): Double = + val names = caseNames(n) + val warmup = Array.fill(k)(schema(n, snake)) + warmup.foreach(s => sink.addAndGet(resolve(s, names).toLong)) + val fresh = Array.fill(k)(schema(n, snake)) + val t0 = System.nanoTime() + @tailrec def run(i: Int, total: Int): Int = + if i >= k then total else run(i + 1, total + resolve(fresh(i), names)) + val out = run(0, 0) + val t1 = System.nanoTime() + sink.addAndGet(out.toLong): Unit + (t1 - t0).toDouble / k.toDouble + +end NominalCostFixtures diff --git a/avro/src/test/scala/dev/constructive/eo/avro/NominalNameIndexCacheSpec.scala b/avro/src/test/scala/dev/constructive/eo/avro/NominalNameIndexCacheSpec.scala new file mode 100644 index 00000000..396e105c --- /dev/null +++ b/avro/src/test/scala/dev/constructive/eo/avro/NominalNameIndexCacheSpec.scala @@ -0,0 +1,77 @@ +package dev.constructive.eo.avro + +import scala.annotation.tailrec + +import java.lang.ref.WeakReference +import org.apache.avro.Schema +import org.specs2.mutable.Specification + +/** The cache contract behind the nominal rung's normalised-name index (issue #103). + * + * The index itself only makes the rung linear in the field count; it is this cache that makes a + * k-hop chain into one record pay for it ONCE instead of k times, and a `def`-shaped optic — which + * re-resolves on every operation — pay for it once instead of once per operation. + * + * Three properties, all of them consequences of the two design choices (identity keys, weak keys), + * and all pinned here because a future "simplification" to a plain `HashMap[Schema, _]` would + * silently reintroduce both a deep-structural compare per lookup and an unbounded leak. + */ +class NominalNameIndexCacheSpec extends Specification: + + import NominalCostFixtures.* + + "the index is built once per schema and handed back by reference" >> { + val sch = schema(24, snake = true) + AvroWalk.normalisedNameIndex(sch) must beTheSameAs(AvroWalk.normalisedNameIndex(sch)) + } + + "repeated resolution against one schema never rebuilds the index" >> { + // The k-hop claim: drilling k times into the same record consults one index instance. + val sch = schema(24, snake = true) + val names = caseNames(24) + val first = AvroWalk.normalisedNameIndex(sch) + (1 to 64).foreach(_ => sink.addAndGet(AvroWalk.totalNominalIndex(sch, names, 0).toLong)) + AvroWalk.normalisedNameIndex(sch) must beTheSameAs(first) + } + + "the cache keys on REFERENCE identity, not on structural equality" >> { + // `Schema.equals` is a deep, recursive compare 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. Two structurally EQUAL schemas therefore get one entry each. + val a = named("TwinRec", 8, snake = true) + val b = named("TwinRec", 8, snake = true) + (a === b) + .and(a must not(beTheSameAs(b))) + .and(AvroWalk.normalisedNameIndex(a) must not(beTheSameAs(AvroWalk.normalisedNameIndex(b)))) + } + + "the cache does not keep a schema alive — the keys are weak" >> { + // The leak story, as a fact rather than a claim: resolve against a schema, drop every strong + // reference to it, and it must still become collectable. A strong-keyed side table would pin + // every schema ever resolved through, for the life of the process. + def resolveAndForget(): WeakReference[Schema] = + val sch = named("Ephemeral", 8, snake = true) + sink.addAndGet(AvroWalk.totalNominalIndex(sch, caseNames(8), 0).toLong) + new WeakReference(sch) + + val ref = resolveAndForget() + @tailrec def awaitCollection(tries: Int): Boolean = + if ref.get() == null then true + else if tries <= 0 then false + else + System.gc() + Thread.sleep(20L) + awaitCollection(tries - 1) + awaitCollection(50) must beTrue + } + + "two schema fields normalising alike are recorded as ambiguous, not as first-wins" >> { + // The collision signal the linear scan produced on its SECOND hit, moved to index-build time. + val fields = new java.util.ArrayList[Schema.Field]() + fields.add(new Schema.Field("user_id", Schema.create(Schema.Type.STRING), null, null)) + fields.add(new Schema.Field("userId", Schema.create(Schema.Type.STRING), null, null)) + val sch = Schema.createRecord("Collide", null, "eo.perf", false, fields) + AvroWalk.normalisedNameIndex(sch).get("userid").intValue === -1 + } + +end NominalNameIndexCacheSpec diff --git a/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionCostSpec.scala b/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionCostSpec.scala new file mode 100644 index 00000000..fa49d03b --- /dev/null +++ b/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionCostSpec.scala @@ -0,0 +1,138 @@ +package dev.constructive.eo.avro + +import org.apache.avro.Schema +import org.specs2.mutable.Specification + +/** The COST gate for the nominal rung (issue #103). + * + * PR #98's rung is correct and stays correct; what it was not is affordable. It asked a per-case- + * field helper for each name, and that helper linear-scanned EVERY schema field on an exact-name + * miss, deliberately without an early exit (it had to see a second match to call the name + * ambiguous). So the cost was `arity x fields.size x nameLength` — quadratic in the field count — + * and it was paid by exactly the codecs the rung exists for, since a miss is what a name transform + * guarantees. An identity-named codec answers from Avro's hash and never scanned. That rung is + * still reachable as [[NominalResolutionOracle]], which this spec times side by side with the live + * one. + * + * '''What is asserted, and what merely prints.''' This box is noisy — project policy is that ns/op + * here is +/-15-50% and only within-run ratios are load-bearing — so nothing here asserts a + * nanosecond count. The assertion is on the SHAPE: double the field count and a quadratic + * implementation costs ~4x while a linear one costs ~2x. The absolute table is printed for the + * record, never gated. + * + * '''Where the assertions run.''' A ratio of two same-run timings is the least noisy thing a clock + * can give, but it is still a clock, and the default `sbt test` lane is three JDK jobs on shared + * GitHub runners — the very place the ns/op policy says not to trust one. So the two ratio + * examples are ARMED ONLY under `-Deo.costGate=true`, which `quality.yml` passes on release tags + * and on demand, alongside the coverage and mutation reports. They skip, visibly and with a + * reason, everywhere else. See `NominalCostFixtures.costGateArmed`. + * + * The table above them keeps printing on every lane. It costs seconds, gates nothing, and is how a + * human notices drift between release-tag runs of the gate. + * + * A non-timing gate was looked for first and does not exist. The deterministic halves of the claim + * are already pinned elsewhere — `NominalNameIndexCacheSpec` pins one index instance per schema + * and no rebuild across repeated resolutions, `NominalResolutionParitySpec` pins every verdict — + * and the part those cannot see is precisely "how much work per case field", which is not + * observable from outside: `Schema` cannot be subclassed to count field accesses (its constructors + * are package-private), and counting probes would mean a mutable counter on the hot path of the + * method this issue exists to make cheap. Allocated bytes, the repo's other load-bearing metric, + * is deterministic here but cannot see the regression that matters: the scan this replaced + * allocated LESS than the index does, so a return to it would read as an improvement. + */ +class NominalResolutionCostSpec extends Specification: + + import NominalCostFixtures.* + + sequential + + private val Reps = 7 + private val Iters = 400 + + /** Repeated resolution against ONE schema — a `def`-shaped or parameterised-`given` optic + * re-resolves per operation, so this is the shape a call site actually pays. + */ + private def warmNs(n: Int, snake: Boolean)(resolve: (Schema, List[String]) => Int): Double = + val sch = schema(n, snake) + val names = caseNames(n) + costNs(Reps, Iters)(_ => resolve(sch, names)) + + private val live: (Schema, List[String]) => Int = + (s, ns) => AvroWalk.totalNominalIndex(s, ns, 0) + + private val frozen: (Schema, List[String]) => Int = + (s, ns) => NominalResolutionOracle.totalNominalIndex(s, ns, 0) + + // ---- the table (printed, never gated) ------------------------------------- + + "the resolution cost table is printed for the record" >> { + val rows = + for + snake <- List(false, true) + n <- List(2, 12, 66) + yield + val shape = if snake then "snake_cased" else "identity " + val w = warmNs(n, snake)(live) + val c = coldCostNs(n, snake, 200)(live) + val wo = warmNs(n, snake)(frozen) + val co = coldCostNs(n, snake, 200)(frozen) + f" $shape%s n=$n%3d warm ${wo}%10.1f -> ${w}%10.1f ns cold ${co}%10.1f -> ${c}%10.1f ns" + println("=== nominal resolution, ns per resolution (frozen pre-index -> live) ===") + rows.foreach(println) + ok + } + + // ---- the gate ------------------------------------------------------------ + + "resolution cost is sub-quadratic in the schema's field count" >> { + if !costGateArmed then skipped(costGateDisarmed) + else + // 33 / 66 / 132: the middle row is the field count issue #103 measured in the wild. The three + // sizes are measured INTERLEAVED — one pass of each, `Reps` rounds, per-size minimum — so a + // burst of load on this (noisy) box lands on all three rather than inflating one ratio. + val sizes = List(33, 66, 132) + val fixed = sizes.map(n => (schema(n, snake = true), caseNames(n))) + // Warm every size before the first timed round. The table example above happens to have run + // these paths already (the spec is `sequential`), but that is a coincidence of ordering, not + // a contract — this example must stand on its own if the table is ever moved or removed. + fixed.foreach((s, ns) => sink.addAndGet(costNs(1, Iters)(_ => live(s, ns)).toLong)) + val best = + (1 to Reps).foldLeft(sizes.map(_ => Double.MaxValue)): (acc, _) => + acc + .zip(fixed) + .map: (prev, sc) => + val ns = costNs(1, Iters)(_ => live(sc._1, sc._2)) + if ns < prev then ns else prev + val List(t33, t66, t132) = best: @unchecked + val double = t66 / t33 + val quadruple = t132 / t33 + println( + f" shape: n=33 ${t33}%.1f ns, n=66 ${t66}%.1f ns, n=132 ${t132}%.1f ns" + + f" | 2n/n = ${double}%.2f (linear ~2, quadratic ~4)" + + f" 4n/n = ${quadruple}%.2f (linear ~4, quadratic ~16)" + ) + // Linear gives 2 and 4; quadratic gives 4 and 16. The thresholds sit between. The 4n/n bound + // is the looser of the two because a linear rung still allocates one normalised key per case + // field per resolution, so its 4x carries a GC tail (measured ~4.9-6.5); the pre-index rung + // measured 15.4 there, so 10 still separates the two shapes with room to spare. + (double must beLessThan(3.0)).and(quadruple must beLessThan(10.0)) + } + + "the snake_cased shape is not a different ORDER of cost from the identity-named one" >> { + if !costGateArmed then skipped(costGateDisarmed) + else + // The identity-named codec answers from Avro's name hash: O(arity), no scan, no index. Under + // the scan the transformed codec paid `fields.size` times that; a linear rung brings it back + // within a small constant factor. + val ident = warmNs(66, snake = false)(live) + val snake = warmNs(66, snake = true)(live) + println( + f" n=66: identity ${ident}%.1f ns, snake ${snake}%.1f ns, ratio ${snake / ident}%.2f" + ) + // Measured 242x under the scan and 3.6-5.1x under the index, across repeated runs on this + // box. The bound is loose on purpose: the DENOMINATOR is the identity-named path, which this + // work does not touch, so every bit of its noise lands in the ratio. + snake / ident must beLessThan(15.0) + } + +end NominalResolutionCostSpec diff --git a/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionOracle.scala b/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionOracle.scala new file mode 100644 index 00000000..ee77434c --- /dev/null +++ b/avro/src/test/scala/dev/constructive/eo/avro/NominalResolutionOracle.scala @@ -0,0 +1,69 @@ +package dev.constructive.eo.avro + +import scala.annotation.tailrec + +import org.apache.avro.Schema + +/** A VERBATIM copy of the scan-shaped nominal rung as PR #98 shipped it, frozen here as a + * differential oracle for issue #103. + * + * Issue #103 replaces the per-case-field linear scan with a per-schema normalised-name index. The + * doctrine that rung encodes — total and injective or abstain entirely, exact before normalised, + * two normalised hits is no hit — is NOT up for renegotiation, so the pre-index implementation is + * kept as executable ground truth and [[NominalResolutionParitySpec]] diffs every verdict of the + * live [[AvroWalk.totalNominalIndex]] against it. + * + * Do not "fix" this object. If it disagrees with `AvroWalk`, the live code changed the doctrine. + */ +private[avro] object NominalResolutionOracle: + + /** Frozen `AvroWalk.totalNominalIndex`. */ + def totalNominalIndex(record: Schema, caseNames: List[String], declIdx: Int): Int = + val arity = caseNames.size + if arity == 0 || declIdx < 0 || declIdx >= arity || arity > record.getFields.size then -1 + else + val out = new Array[Int](arity) + @tailrec def seen(j: Int, idx: Int): Boolean = + j < 0 || (out(j) != idx && seen(j - 1, idx)) + @tailrec def loop(i: Int, rest: List[String]): Boolean = + rest match + case Nil => true + case n :: t => + val idx = nominalIndex(record, n) + if idx < 0 || !seen(i - 1, idx) then false + else + out(i) = idx + loop(i + 1, t) + if loop(0, caseNames) then out(declIdx) else -1 + + /** Frozen `AvroWalk.nominalIndex` — exact hash hit, else a full linear scan with no early exit + * (the scan is what issue #103 is about: it is `arity x fields.size x nameLength` per hop). + */ + private def nominalIndex(record: Schema, scalaName: String): Int = + val exact = record.getField(scalaName) + if exact != null then exact.pos + else + val fields = record.getFields + @tailrec def loop(i: Int, found: Int): Int = + if i >= fields.size then found + else if sameFieldName(fields.get(i).name, scalaName) then + if found >= 0 then -1 else loop(i + 1, i) + else loop(i + 1, found) + loop(0, -1) + + /** Frozen `AvroWalk.sameFieldName`. */ + private def sameFieldName(a: String, b: String): Boolean = + @tailrec def skip(s: String, i: Int): Int = + if i >= s.length then i + else + val c = s.charAt(i) + if c == '_' || c == '-' || c == '.' then skip(s, i + 1) else i + @tailrec def loop(i: Int, j: Int): Boolean = + val x = skip(a, i) + val y = skip(b, j) + if x >= a.length || y >= b.length then x >= a.length && y >= b.length + else if Character.toLowerCase(a.charAt(x)) != Character.toLowerCase(b.charAt(y)) then false + else loop(x + 1, y + 1) + loop(0, 0) + +end NominalResolutionOracle diff --git a/avro/src/test/scala/dev/constructive/eo/avro/vulcan/NominalResolutionParitySpec.scala b/avro/src/test/scala/dev/constructive/eo/avro/vulcan/NominalResolutionParitySpec.scala new file mode 100644 index 00000000..119c7649 --- /dev/null +++ b/avro/src/test/scala/dev/constructive/eo/avro/vulcan/NominalResolutionParitySpec.scala @@ -0,0 +1,198 @@ +package dev.constructive.eo.avro.vulcan + +import scala.jdk.CollectionConverters.* +import scala.language.implicitConversions + +import dev.constructive.eo.avro.{AvroCodec, AvroWalk, NominalResolutionOracle} +import java.util.ArrayList +import org.apache.avro.Schema +import org.specs2.mutable.Specification + +/** The DOCTRINE gate for issue #103: the pre-index rung must return the SAME verdict as the scan it + * replaces, for every input — not merely "the suite is still green". + * + * A performance 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). So this spec diffs the LIVE + * `AvroWalk.totalNominalIndex` against [[NominalResolutionOracle]] — the frozen pre-#103 + * implementation, kept verbatim — over two corpora: + * + * 1. EXHAUSTIVE SYNTHETIC. Every ordered schema-field list of length 1-3 drawn from a pool of + * deliberately confusable names (`a`, `A`, `_a`, `a_b`, `ab`, `aB`, `b` — exact collisions, + * case collisions, separator collisions), crossed with every case-name list of length 0-3 + * over that pool plus an unmatched name, crossed with every declaration index from -1 to + * arity. That covers each semantic the rewrite could lose, by construction rather than by + * hoping an example happens to hit it: 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. + * 1. REAL FIXTURES. Every codec behind the three #98 specs, so the diff is anchored to the + * shapes the doctrine was actually written for, not only to synthetic ones. + * + * The printed table is the artifact: verdict per (fixture, case field), plus the mismatch count, + * which must be zero. + */ +class NominalResolutionParitySpec extends Specification: + + // ---- corpus 1: exhaustive synthetic ------------------------------------------------------- + + /** Exact (`a` vs `a`), case (`a` vs `A`), separator (`a_b` vs `ab`) and cross (`aB` vs `a_b`) + * collisions all live in this pool, so the corpus contains records whose fields normalise alike + * and records where an exact hit and a normalised hit point at different positions. + */ + private val pool = List("a", "A", "_a", "a_b", "ab", "aB", "b") + + /** `c` matches nothing in the pool — the "not total" leg. */ + private val caseNamePool = pool :+ "c" + + private def record(names: List[String]): Schema = + val fields = new ArrayList[Schema.Field](names.size) + names.foreach(n => + fields.add(new Schema.Field(n, Schema.create(Schema.Type.STRING), null, null)) + ) + Schema.createRecord( + "P" + names.mkString("_").replace("-", "").replace(".", ""), + null, + "eo.p", + false, + fields + ) + + /** Ordered selections without repetition — a record cannot declare one name twice. */ + private val schemas: List[(String, Schema)] = + (1 to 3) + .toList + .flatMap(k => pool.combinations(k).flatMap(_.permutations).toList) + .map(ns => (ns.mkString("{", ",", "}"), record(ns))) + + /** Sequences WITH repetition — a duplicated case name is the sharpest injectivity probe. */ + private def sequences(k: Int): List[List[String]] = + if k == 0 then List(Nil) + else caseNamePool.flatMap(h => sequences(k - 1).map(h :: _)) + + private val caseLists: List[List[String]] = (0 to 3).toList.flatMap(sequences) + + // ---- corpus 2: the real #98 fixtures ------------------------------------------------------ + + private def entry[A](label: String, names: String*)(using + c: AvroCodec[A] + ): (String, Schema, List[String]) = + (label, c.schema, names.toList) + + private val fixtures: List[(String, Schema, List[String])] = + List( + entry[Three]("Three", "a", "b", "c"), + entry[Pair]("Pair", "alpha", "beta"), + entry[SnakeComputed]("SnakeComputed", "clickId", "landingPageId"), + entry[Inner]("Inner", "x", "y"), + entry[Outer]("Outer", "id", "inner"), + entry[Basket]("Basket", "items"), + entry[NPair]("NPair", "alpha", "beta"), + entry[PermPair]("PermPair", "alpha", "beta"), + entry[RPair]("RPair", "alpha", "beta"), + entry[HostileTransform.Hostile]("Hostile", "alpha", "beta"), + entry[ThreeNt.AB]("ThreeNt(NamedTuple)"), + entry[FpClick]("FpClick", "clickId", "landingPageId"), + entry[FpVisit]("FpVisit", "userId", "user"), + entry[FpRow]("FpRow", "id", "name", "cachedHash"), + entry[FpKeep]("FpKeep", "alpha", "beta"), + entry[FpItem]("FpItem", "sku", "qty"), + entry[FpCart]("FpCart", "owner", "items"), + entry[FpInner]("FpInner", "x", "y"), + entry[FpOuter]("FpOuter", "id", "inner"), + entry[FpSnake.SnakeClick]("SnakeClick", "clickId", "landingPageId"), + entry[FpHostile.HostileClick]("HostileClick", "clickId", "landingPageId"), + entry[FpPlain.Bag]("Bag", "tags", "total"), + entry[Ev]("Ev", "occurredAt", "seqNo"), + entry[Comp]("Comp", "a", "b", "c"), + entry[Doc]("Doc", "id", "payload"), + entry[Acct]("Acct", "userId", "balance"), + entry[Tw]("Tw", "userId", "tag"), + ) + + private def verdictName(schema: Schema, idx: Int): String = + if idx < 0 then "ABSTAIN" else schema.getFields.get(idx).name + + // ---- the diff ----------------------------------------------------------------------------- + + "every SYNTHETIC verdict is identical to the frozen pre-index rung" >> { + val mismatches = + for + (label, schema) <- schemas + names <- caseLists + i <- -1 to names.size + live = AvroWalk.totalNominalIndex(schema, names, i) + was = NominalResolutionOracle.totalNominalIndex(schema, names, i) + if live != was + yield s"$label case=${names.mkString("[", ",", "]")} declIdx=$i: was $was, now $live" + val cells = schemas.size * caseLists.size + println( + s"=== nominal verdict diff, synthetic: ${schemas.size} schemas x ${caseLists.size}" + + s" case-name lists ($cells pairs, every declIdx in [-1..arity]) ===" + ) + println(s" mismatches: ${mismatches.size}") + mismatches.take(20).foreach(m => println(s" DIFF $m")) + mismatches must beEmpty + } + + "every REAL-FIXTURE verdict is identical to the frozen pre-index rung" >> { + println("=== nominal verdict diff, real fixtures (case field -> schema field, or ABSTAIN) ===") + val mismatches = + fixtures.flatMap: (label, schema, names) => + val schemaFields = schema.getFields.asScala.map(_.name).mkString(",") + println(f" $label%-20s schema{$schemaFields}") + names + .zipWithIndex + .flatMap: (n, i) => + val live = AvroWalk.totalNominalIndex(schema, names, i) + val was = NominalResolutionOracle.totalNominalIndex(schema, names, i) + println( + f" $n%-16s was ${verdictName(schema, was)}%-16s now ${verdictName(schema, live)}%-16s" + + (if live == was then "same" else "*** DIFF ***") + ) + if live == was then Nil + else List(s"$label.$n: was $was, now $live") + println(s" mismatches: ${mismatches.size}") + mismatches must beEmpty + } + + // ---- the semantics, pinned directly ------------------------------------------------------- + // Each of these is covered by the exhaustive sweep above; they are spelled out separately so a + // failure NAMES the rule that broke instead of printing a synthetic triple. + + private val tricky = record(List("a", "A", "b")) + + "an EXACT name wins over a normalised match elsewhere" >> { + // `a` and `A` normalise alike; an exact hit must still answer, and answer with its OWN slot. + (AvroWalk.totalNominalIndex(tricky, List("a", "b"), 0) === 0) + .and(AvroWalk.totalNominalIndex(tricky, List("A", "b"), 0) === 1) + } + + "MORE than one normalised match is ambiguity, never first-wins" >> { + // `_a` is exact for neither `a` nor `A`, and normalises to both: no signal, so the whole + // mapping fails and every field abstains. + AvroWalk.totalNominalIndex(tricky, List("_a", "b"), 1) === -1 + } + + "arity > fields.size short-circuits before any lookup" >> { + AvroWalk.totalNominalIndex(record(List("a")), List("a", "b"), 0) === -1 + } + + "injectivity is checked across the WHOLE case-field list" >> { + // Case fields `ab` and `aB` normalise to the same key, and that key names ONE schema field, + // `a_b`: two case fields aimed at one slot, so the map is not injective. The rung then abstains + // for EVERY field — the second case field does not merely lose slot 0 to the first. + val r = record(List("a_b", "b")) + (AvroWalk.totalNominalIndex(r, List("ab", "aB"), 0) === -1) + .and(AvroWalk.totalNominalIndex(r, List("ab", "aB"), 1) === -1) + } + + "an empty caseNames (a NamedTuple parent) abstains" >> { + AvroWalk.totalNominalIndex(tricky, Nil, 0) === -1 + } + + "a declIdx outside [0, arity) abstains" >> { + (AvroWalk.totalNominalIndex(tricky, List("a", "b"), -1) === -1) + .and(AvroWalk.totalNominalIndex(tricky, List("a", "b"), 2) === -1) + } + +end NominalResolutionParitySpec