diff --git a/CHANGELOG.md b/CHANGELOG.md index b8932a1..f4ffb45 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,150 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [0.5.0] - 2026-05-06 + +_Work in progress — incremental-native architectural rework. See `docs/incremental/ARCHITECTURE-0.5.0.md`._ + +### Phase 0 — spike GO verdict (2026-05-07) + +Sandbox prototype of Lever A (stable IDs + LongLongMap NodeIndex) lands additively in `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/`. Existing 897-test suite untouched. Three GO/NO-GO gates green: + +- **Identity-preservation invariant** — `IdTreeSplicer` preserves sibling subtree reference equality through splices (spec §8 Q3 gate). +- **Trivia-bearing edits** — calculator grammar with `%whitespace` + comments, three representative edits, incremental update equivalent to full rebuild (spec §8 Q4 gate). +- **Perf** — JMH bench: 38× speedup at 100 nodes, 47× at 1000, 67× at 10000. Well above the 5× gate threshold; consistent with spec §2's projected 300× per-edit reduction. See [`docs/bench-results/phase0-spike-results.md`](docs/bench-results/phase0-spike-results.md) and [`docs/archive/PHASE-0-RESULTS.md`](docs/archive/PHASE-0-RESULTS.md). + +### Phase 1 prove-out (2026-05-07) + +Path A (offset decoupling from records) — RED. 1.10-1.29× speedup on flat-tree mid-buffer edits; not worth the cross-cutting refactor. See [`docs/bench-results/phase1-spanindex-results.md`](docs/bench-results/phase1-spanindex-results.md). + +Path D (stable-id ancestor preservation) — GREEN. **96-604× speedup** on flat-tree mid-buffer edits with absolute time flat across N (~25-40 ns), confirming genuine O(δ) scaling. The fix: TreeSplicer reuses old ancestor IDs across splices; applyIncremental skips ancestor-rewiring as a result. See [`docs/bench-results/path-d-results.md`](docs/bench-results/path-d-results.md) and [`docs/archive/PHASE-1-PROVE-OUT.md`](docs/archive/PHASE-1-PROVE-OUT.md). + +### Phase 1 production migration (2026-05-07) + +**BREAKING.** `CstNode` records gain `long id` as the first record component on all four variants (`Terminal`, `NonTerminal`, `Token`, `Error`). `equals`/`hashCode` overridden to exclude `id` per spec §7 R1 — structural equality preserved (`RoundTripTest` baselines unaffected). New `org.pragmatica.peg.tree.IdGenerator` interface + `PerSessionCounter` impl; threaded through `ParsingContext`, `PegEngine`, `ParserGenerator` emission templates. Generated parsers now contain inline `IdGenerator` (preserves the v0.4.2 standalone-parser invariant — no FQCN dep on peglib runtime). + +`NodeIndex` internals switch from `IdentityHashMap` to `LongLongMap` (hand-rolled linear-probing, promoted from sandbox to `internal/`). Includes a tombstone-saturation fix: resize triggers on `size + tombstones > threshold`; same-capacity rehash drains tombstones without growing the table. + +`IncrementalSession.applyIncremental` adopts Path D's optimized algorithm (steps 1 and 3 from spec §2 deleted because ancestor IDs are stable). Step 6 added: refresh `nodesById` for right-of-edit subtrees that `TreeSplicer.shiftAll` deep-copies — those subtrees retain stable IDs but their records carry post-edit shifted spans, so the lookup map needs a refresh to avoid stale-span pivot errors. + +Bench numbers vs 0.4.3 baseline on the 1900-LOC Java fixture (Regime B, cursor-moved-to-edit, same RNG seed `0xBEEFCAFE`): + +| Metric | 0.4.3 | 0.5.0 (post-1.7) | Change | +|---|---:|---:|---:| +| Median | 10.8 ms | 5.6 ms | **-48% (1.9× faster)** | +| p95 | 22.4 ms | 14.6 ms | **-35%** | +| p99 | 53.3 ms | 138.8 ms | +160% (large-pivot tail) | +| Max | 98.6 ms | 390.8 ms | +297% (large-pivot tail) | +| % under 16 ms | 91.5% | 95.9% | **+4.4 pp** | + +Median + p95 + frame-budget hit rate clearly improved. p99/max regressed for large-pivot edits where `TreeSplicer.shiftAll` deep-copies thousands of right-of-edit records — an accepted trade vs Path A's much-larger-scope refactor. Pivot-selection improvements (spec Lever B) deferred to Phase 2. + +See [`docs/incremental/PHASE-1-RESULTS.md`](docs/incremental/PHASE-1-RESULTS.md) for full sub-phase summary and bench caveats. + +### Phase 2 attempted, rolled back; bench + JBCT cleanup; Lever D Cursor split (2026-05-07) + +**Lever B (top-down pivot)** — investigated and deferred. Two empirical iterations both fail `IncrementalTriviaParityTest`: +- "Strict literal-prefix safe-pivot" (per spec §3): correctness-sound but cost 4× perf — only ~30/133 Java25 rules admitted, forcing walk-up to Block/RecordBody pivots. +- "Boundary-touch walk-up": pivot's internal child boundaries still produce trivia divergence. + +Root cause: trivia attribution is context-sensitive — Lever B retry blocked on trivia attribution refactor. `SafePivotAnalyzer` + `NodeIndex.smallestEnclosing` preserved as dormant infrastructure. Phase 2 wiring rolled back; Phase 1.7 algorithm restored. + +**Bench harness** — `IncrementalSessionBench` now validates each edit post-application; if the resulting buffer is syntactically invalid, the bench rolls back the session and skips the edit. Eliminates the 746-exception buffer-corruption tail that polluted prior bench logs. 41% faster wallclock. + +**JBCT cleanup** — `SessionFactory.parseFull` now returns `Result` instead of throwing `IllegalStateException`. New `SessionError` sealed Cause type. Public `Session.edit` always returns a Session; parse failures surface via new `Session.parseSuccessful()` instead of exceptional control flow. Bench's dead try/catch removed. Aligns with project JBCT mandate (Result for failure-return, no exceptions in business logic). + +**Sandbox cleanup** — Phase 0/1 prove-out code (the `experimental/` package + 5 sandbox JMH benches) removed. Production has equivalents (`IdGenerator` in `peglib-core/tree`, `LongLongMap` in `peglib-incremental/internal`). -5463 LOC across 31 files. Git history preserves the journey. + +**Lever D — Cursor/Session split** (per spec §5). Cursor state (offset + enclosingNodeId) extracted from `IncrementalSession` record into a new public `Cursor` record. New `EditOutcome(Session, Cursor)` and `ReparseOutcome(Session, Cursor)` records returned by `edit` / `reparseAll`. `Cursor.moveTo(int newOffset, NodeIndex index)` is pure — no Session allocation. `InitialSession(Session, Cursor)` returned by `IncrementalParser.initialize`. Cursor uses Phase 1's stable `long enclosingNodeId` so it survives session churn. + +Bench post-Lever-D vs Phase 1.7 baseline (Regime B): + +| Metric | Phase 1.7 | Post-Lever-D | +|---|---:|---:| +| Median | 5.5 ms | **5.0 ms** (-9%) | +| p95 | 14.6 ms | **11.2 ms** (-23%) | +| p99 | 192.7 ms | **90.5 ms** (-53%) | +| % under 16 ms | 95.4% | **96.5%** (+1.1 pp) | + +Lever C (peglib-rt IR unification per spec §4) is the next-session entry point. + +### Throughput engine — Tier 1 perf arc (2026-05-07 → 2026-05-08) + +Branding: parser generator output is now the **throughput engine** (full-reparse speed) — distinct from the **incremental engine** (interactive editing). Different optimization targets, different code shapes, no shared code. See [`docs/incremental/THROUGHPUT-ENGINE-TIER1.md`](docs/incremental/THROUGHPUT-ENGINE-TIER1.md) for the spec; [`docs/bench-results/throughput-tier1-results.md`](docs/bench-results/throughput-tier1-results.md) and [`docs/bench-results/generator-profile-baseline.md`](docs/bench-results/generator-profile-baseline.md) for the data. + +**Bench fixtures:** Java25ParseBenchmark now has TWO fixtures — `reference` (1900-LOC FactoryClassGenerator.java) and `selfhost` (the Java25 generated parser parsing its OWN 37k-line generated source — pre-Tier-1 OOM'd; now 1 second). Self-host stress test caught E's silent regression that the small bench missed. + +**Cumulative bench (Regime: full reparse, JDK 25, JMH 1.37, variant `phase1_allStructural_mutableResult_autoSkipPackrat`):** + +| Fixture | Original | Now | Δ | +|---|---:|---:|---:| +| Reference (1900 LOC) | 76.2 ms / 150 MB | **25.1 ms / 82.9 MB** | **-67% wallclock, -45% bytes** | +| Self-host (37k LOC) | OOM | **1035 ms / 2.19 GB** | from impossible to 1 sec | +| Reference gc.count | 205 | 50 | -75% | +| Reference gc.time | 2,844 ms | 354 ms | -87% | + +**Moves shipped:** +- **A** — opt-in `mutableParseResult` ParserConfig flag. Emits a mutable `CstParseResult` class with raw nullable fields (no `Option value` / `Option expected` wrappers) + raw-nullable `furthestFailure` / `furthestExpected` / `pendingFailureRecoveryOverride`. Eliminates 6,088 Option$Some allocation samples per parse → 0. +- **D** — `inlineLocations` flag default-on. Emitted CST construction uses `new SourceSpan(line, col, off, endLine, endCol, endOff)` directly instead of `SourceSpan.sourceSpan(new SourceLocation(...), new SourceLocation(...))`. Two passes: production sweep (cleanup, -63 LOC) + emission templates (the win). +- **D follow-up** — eliminated remaining `location()` callers in emission. SourceLocation samples 3,318 → ~600. +- **F (first-set Choice dispatch)** — extended `ChoiceDispatchAnalyzer` from literal-only to full transitive FIRST-set computation. CharClass + Reference (recursive) + mixed dispatch. 19/64 → 62/64 of Java25's choices now dispatch via `switch (text.charAt(pos))` instead of speculative PEG-ordered evaluation. -20% wallclock on both fixtures. +- **G (JBCT-style method splitting)** — top-level rule Choices extract each alt to `parse__alt(Rule rule)` helper methods; new `Rule` record (static singletons, zero allocation) holds rule metadata. parse_Stmt 27,783 → <3,000 bytes. Most parse methods now JIT-inlinable. +- **G2 + H** — Sequence chunking + nested Choice extraction. Same pattern applied recursively. +- **Selective packrat auto-detection** — `selectivePackrat=true` is now default; `PackratAnalyzer.autoSkipPackratRules(grammar)` derives the skip-set automatically. Leaf-like and single-call-site rules bypass cache. Biggest single win: -38% reference / -14% self-host wallclock; -75% gc.count. + +**Move that did NOT ship:** +- **E (packrat as `int[]`-keyed open-addressing map)** — clean on small bench, but **+22% regression on self-host stress test**. Linear probing scales badly at the load factors the 37k-LOC fixture stresses. Reverted. Lesson informed adding self-host as the second JMH fixture. +- **H2 (recursive per-alt extraction in nested Choices)** — bytecode size dropped further but +4-7% wallclock regression. C2 was already inlining post-H; further splitting traded inline-friendly bytecode for call-overhead. Reverted. + +**+ DFA fast-path (token-shaped rules)** — `tokenFastPath` flag default-on. Detects rules whose body matches `< CharClass + ZeroOrMore >` (Identifier-shape) and emits a tight inline scanner using pre-computed ASCII bitmasks instead of going through the framework's `matchCharClass` per character. **-9.8% reference / -7.6% self-host wallclock**. On Java25 only `Identifier` matches the spike's pattern; generalizing to `OneOrMore` (whitespace) and mixed Literal+CharClass (NumLit) is the obvious next extension and would compound. + +**vs javac comparison:** at the time of writing, peglib's throughput engine parses the 1900-LOC reference fixture in 22.6 ms (vs javac 9 ms = 2.5× of javac) with strictly more output (lossless CST + trivia for formatter+linter use cases). javac produces AST without trivia. + +**Items deferred:** +- **Generalized DFA lexer** — extend the spike pattern to `OneOrMore` (whitespace) and mixed Literal+CharClass (NumLit). Each generalization compounds with the Identifier win. +- **ASCII whitespace fast path** — folded into generalized DFA above. +- **Char-class bit-packing** — pre-emit ASCII bitmaps; bitwise test instead of range comparisons. Tactical, ~5-15% on char-class-heavy paths. **Reassess with care after Move B finding (below) — same risk class.** + +### Move B attempted, abandoned + post-rollback profile-driven optimization arc (2026-05-08) + +Move B (mutable parse-state singleton) was attempted across 5 incremental commits, then rolled back when bench data contradicted the hypothesis. A profile-driven optimization arc on the rolled-back baseline shipped 2 wallclock wins. Full session post-mortem with quantitative data: [`docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md`](docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md) §11 + [`docs/HANDOVER.md`](docs/HANDOVER.md) §11. + +**Move B failure (5 commits rolled back):** + +Replacing per-call `CstParseResult` allocation with a heap-bound singleton + boolean returns regressed wallclock monotonically while allocation dropped: + +| Stage | Wallclock (ms) | Alloc (MB) | Δ wallclock vs original | Δ alloc vs original | +|---|---:|---:|---:|---:| +| Original baseline | 22.6 | 75.6 | — | — | +| Commit 3 (match helpers) | 23.97 | 72.1 | +6.0% | -4.6% | +| Commit 4 (combinators) | 24.71 | 66.3 | +9.3% | -12.3% | +| Commit 5 (predicates/capture/cut/TB) | 25.09 | 66.0 | +11.0% | -12.8% | + +**Why:** modern JIT escape analysis was already scalar-replacing the per-call records (raw-nullable fields + immediate consume = textbook EA fodder). The singleton replacement defeated that optimization — heap-bound field, can't be scalarized, source-level aliasing forces the compiler to assume mutation visibility. Net: GC sees fewer survivor objects (alloc-rate metric drops), but optimized hot path is slower per call. Definitively abandoned. + +**Post-rollback wins (2 commits past `v0.5.0-candidate`):** + +- **Trivia snapshot via int size** (`4763251`) — replaced `List.copyOf(pendingLeadingTrivia)` snapshot + `clear()+addAll()` restore with primitive `int size` snapshot + `subList(snap, size).clear()` restore. Profile evidence: 6.0% reference CPU + 15.4% reference alloc in `Arrays.copyOf` (top alloc caller). Pattern: structural alloc elimination on hot path; JIT cannot elide bulk array copies. **-12.1% reference / -8.2% selfhost wallclock; -6.4% / -7.5% alloc.** +- **ASCII char interning pool** (`38b6a8e`) — eliminated `String.valueOf(c)` per match in `matchCharClassCst` + `matchAnyCst` via pre-computed `ASCII_CHAR_STRINGS[128]` static field. Profile evidence: 5.8% of total reference allocs from per-match fresh 1-char Strings. Pattern: per-call fresh String alloc with no JIT scalar-replacement path. **-3.95% reference / -4.59% selfhost wallclock; -3.76% / -1.38% alloc.** + +**Cumulative session wins (from `v0.5.0-candidate` baseline):** + +| Fixture | v0.5.0-candidate | After 38b6a8e | Δ wallclock | Δ alloc | +|---|---:|---:|---:|---:| +| Reference (1900 LOC) | 22.66 ms / 75.55 MB | **19.12 ms / 68.02 MB** | **-15.6%** | **-10.0%** | +| Selfhost (37k LOC) | 937 ms / 2.04 GB | **832 ms / 1.85 GB** | **-11.2%** | **-9.3%** | + +**Reference fixture is now under 20 ms** — original Move B target (~2× of javac territory) achieved without singleton mutation. + +**Resets that did NOT ship (5 candidates evaluated under strict bench gate):** +- **trackFailure dedup via LinkedHashSet** — predicted -6.4% wallclock; reality flat. Lesson: JIT inlines `String.contains` efficiently when the scanned buffer is small; the profile share was call-overhead-dominated. +- **Primitive long-keyed packrat cache** (replace HashMap) — predicted -12.5% CPU; reality selfhost +24.5% wallclock regression. Lesson: JDK HashMap chaining is per-op faster than custom open-addressing + SplitMix64 finalizer; replacing it cost more in latency than was saved in alloc. +- **HashMap initial-capacity hint** from input size — reality reference +3.9% / selfhost +5.3% regression. Lesson: over-sizing hurts cache locality more than it saves resize cost. JDK HashMap's growth schedule is well-tuned. +- **Lazy Token text materialization** (record→class with cache) — reality selfhost +10% regression. Lesson: Java records are JIT-scalar-replaceable in a way mutable classes (even with single cache field) are not. Same family of lesson as Move B. +- **Lazy SourceLocation in trackFailure** — reality flat on both fixtures. Lesson: SourceLocation is a record; JIT readily stack-allocates / dead-code-eliminates around hot-path early returns. Allocation share in profile is NOT predictive of wallclock improvement when JIT/EA already handles the alloc cleanly. + +**Refined optimization principle:** target allocations the JIT cannot elide (bulk array copies, per-call fresh objects with no scalar-replacement path). Avoid optimizing patterns the JIT already handles (records, immediately-consumed objects, JDK collection internals, mutable shared state on `this`). + ## [0.4.3] - 2026-05-06 Performance — interactive editing focus. 19% faster median, 26% faster p95 on the IncrementalBenchmark editing-session suite. **One breaking change**: SourceSpan record components changed from two SourceLocations to six ints (see migration note). @@ -78,7 +222,7 @@ The interpreter (PegEngine) is now FASTER than the generator's phase1 path becau ### Documentation - **HANDOVER §6.2 retraction.** The lever-1 incremental-perf "1-2 day fix" framing was wrong. Two failed attempts (12/100 and 31/100 parity regressions). Real fix needs 5-10 days of correctness analysis. Documented two latent bugs: (a) fallback-rule bypass — `tryIncrementalReparse` only checks the chosen pivot, not its ancestors; (b) `reparseAt`'s acceptance check proves length parity but not structural parity. -- **`docs/incremental/V2.5-SPIKE.md` addendum.** Retracts the spike doc's "zero correctness risk" claim. Parity was never asserted in the spike's probe — only timing. +- **`docs/archive/V2.5-SPIKE.md` addendum.** Retracts the spike doc's "zero correctness risk" claim. Parity was never asserted in the spike's probe — only timing. - **HANDOVER §6.4 correction.** Tier-1 perf flags (`inlineLocations`, `markResetChildren`, `selectivePackrat`) are generator-only. They do NOT speed up the interpreter path that `IncrementalParser` uses. ### Reverted @@ -92,7 +236,7 @@ The interpreter (PegEngine) is now FASTER than the generator's phase1 path becau ## [0.4.0] - 2026-05-01 -API consolidation + test hygiene. **Breaking.** No incremental v2.5 cache remap (the original 0.4.0 plan item; superseded by `docs/incremental/V2.5-SPIKE.md`'s NO-GO recommendation — the actual lever is pivot-selection, not cache invalidation). +API consolidation + test hygiene. **Breaking.** No incremental v2.5 cache remap (the original 0.4.0 plan item; superseded by `docs/archive/V2.5-SPIKE.md`'s NO-GO recommendation — the actual lever is pivot-selection, not cache invalidation). ### Changed (BREAKING) @@ -162,7 +306,7 @@ Generator-side `%recover` per-rule overrides. Non-breaking. ### Known limitations -- **Incremental parser `singleCharEdit` perf** is still ~325 ms/op on the 1,900-LOC fixture. `docs/incremental/V2.5-SPIKE.md` documents the diagnosis (the dominant cost is pivot overshoot in `findBoundaryCandidate`, not cache invalidation as v2.5 assumed) and a proposed "lever 1" fix (edit-anchored pivot selection). A naive lever-1 swap was attempted in 0.3.6 development but produced correctness regressions on `IncrementalParityTest` due to subtle interaction with `NodeIndex.contains`'s inclusive-boundary semantics — the smallestContaining lookup can return a node ending exactly at `editStart`, yielding a different pivot than the warm-pointer walk would. A correct fix needs careful boundary semantics work; deferred. +- **Incremental parser `singleCharEdit` perf** is still ~325 ms/op on the 1,900-LOC fixture. `docs/archive/V2.5-SPIKE.md` documents the diagnosis (the dominant cost is pivot overshoot in `findBoundaryCandidate`, not cache invalidation as v2.5 assumed) and a proposed "lever 1" fix (edit-anchored pivot selection). A naive lever-1 swap was attempted in 0.3.6 development but produced correctness regressions on `IncrementalParityTest` due to subtle interaction with `NodeIndex.contains`'s inclusive-boundary semantics — the smallestContaining lookup can return a node ending exactly at `editStart`, yielding a different pivot than the warm-pointer walk would. A correct fix needs careful boundary semantics work; deferred. ## [0.3.5] - 2026-05-01 @@ -183,7 +327,7 @@ Trivia attribution correctness — full byte-equal round-trip on all 22 corpus f - **CST hash baseline regenerated for `large/FactoryClassGenerator.java.txt`.** The Bug C'' children-rollback fix removes a duplicate trailing comma child in enum-constant lists; that fixture's CST shape legitimately changes. Other 21 fixtures' baselines are unchanged. Committed as a separate baseline-shift commit alongside the Bug C'' fix. Anyone diffing 0.3.4 baselines against 0.3.5 will see the single-fixture shift; this is expected. - **`RoundTripTest` re-enabled.** All 22 corpus fixtures round-trip byte-equal via the generated parser. The `@Disabled` annotation and pointer comment in the test are removed. - `docs/TRIVIA-ATTRIBUTION.md` — updated to document the full Bug A/B/C/C'/C'' resolution. -- `docs/RELEASE-PLAN-0.3.5-0.4.0.md` — Phase 1 marked complete; Bug C originally deferred to 0.3.6 was solved in 0.3.5 along with Bug C'/C''. +- `docs/RELEASE-PLAN-0.3.5-0.4.0.md` — Phase 1 marked complete; Bug C originally deferred to 0.3.6 was solved in 0.3.5 along with Bug C'/C''. (File deleted in 0.5.0-candidate cleanup; recover from git history if needed.) ### Known limitations @@ -318,7 +462,7 @@ Next lever: **v2.5 span-rewriting cache remap** (SPEC §5.4). Not part of this r ### Added -- **`peglib-incremental` module — v1 implementation** per `docs/incremental/SPEC.md`. Cursor-anchored stateful parser that reparses only the subtree affected by an edit, falling back to full reparse when needed. Designed for editor-scale workflows (formatters on save, live diagnostics, LSP backends). +- **`peglib-incremental` module — v1 implementation** per `docs/archive/SPEC-incremental-original.md`. Cursor-anchored stateful parser that reparses only the subtree affected by an edit, falling back to full reparse when needed. Designed for editor-scale workflows (formatters on save, live diagnostics, LSP backends). - Public API in `org.pragmatica.peg.incremental`: - `IncrementalParser.create(grammar)` / `create(grammar, config)` — factory. - `Session initialize(String buffer)` / `initialize(String buffer, int cursorOffset)` — immutable session. @@ -378,7 +522,7 @@ Infrastructure-only minor-bump release. No new user-facing features beyond the ` ### Added -- `Parser#parseRuleAt(Class ruleId, String input, int offset)` — partial-parse entry point. Parses a specific rule against input starting at the given offset; returns `Result` wrapping the resulting CST subtree and its end offset. Implemented by `PegEngine` (interpreter) and by generated parsers via an identity map keyed on the `RuleId` marker classes the generator has been emitting since 0.2.6. This is the API `peglib-incremental` (0.3.1) depends on per `docs/incremental/SPEC.md` §5.6. +- `Parser#parseRuleAt(Class ruleId, String input, int offset)` — partial-parse entry point. Parses a specific rule against input starting at the given offset; returns `Result` wrapping the resulting CST subtree and its end offset. Implemented by `PegEngine` (interpreter) and by generated parsers via an identity map keyed on the `RuleId` marker classes the generator has been emitting since 0.2.6. This is the API `peglib-incremental` (0.3.1) depends on per `docs/archive/SPEC-incremental-original.md` §5.6. - `org.pragmatica.peg.parser.PartialParse` record `(CstNode node, int endOffset)`. - `peglib-incremental` and `peglib-formatter` shell modules — empty placeholders with just a `package-info.java` each. Reservations for 0.3.1 and 0.3.3. - `docs/PARTIAL-PARSE.md` — `parseRuleAt` API reference. @@ -638,7 +782,7 @@ Interpreter speedup is below the 1.5× plan target, reflecting the interpreter's - `bulkAdvanceLiteral` — on successful match of literal text with no `\n`, updates `pos` and `column` in bulk rather than looping `advance()` per char. - `skipWhitespaceFastPath` — emits a first-char precheck derived from the grammar's `%whitespace` rule (e.g. `' '`/`'\t'`/`'\r'`/`'\n'`/`'/'` for Java 25); returns `List.of()` immediately when current char can't start trivia. - `reuseEndLocation` — allocates end-position `SourceLocation` once per successful match instead of twice (span end + result endLocation). -- Phase-2 flag defaults (set per measured win per PERF-REWORK-SPEC §12.6): +- Phase-2 flag defaults (set per measured win per `docs/archive/PERF-REWORK-SPEC.md` §12.6): - `choiceDispatch` default **on** — measured 2.49× speedup over phase-1 baseline. - `markResetChildren`, `inlineLocations` default **off** — no statistically significant individual win on the reference JVM. - `selectivePackrat` default **off** — marginal combo win (~5%) sits inside measurement noise; callers opting in must also provide `packratSkipRules`. diff --git a/CLAUDE.md b/CLAUDE.md index 7721d11..0a41824 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -148,8 +148,9 @@ Sum <- Number '+' Number { return (Integer)$1 + (Integer)$2; } - [x] Advanced error recovery with Rust-style diagnostics - [x] Generated parser ErrorReporting (BASIC/ADVANCED) for optional Rust-style diagnostics - [x] Cut operator (^/↑) - commits to current choice, prevents backtracking -- [x] 565 passing tests, 1 skipped (RoundTripTest — pre-existing trivia gap) -- [x] Performance rework (0.2.2): 4.23× speedup on 1,900-LOC Java 25 fixture via generator-time perf flags; see `docs/PERF-REWORK-SPEC.md` and `docs/bench-results/`. +- [x] 705 passing peglib-core tests + supporting modules (peglib-formatter 66, peglib-maven-plugin 5, peglib-playground 27); RoundTripTest enabled and 22/22 since 0.3.5 +- [x] Performance rework (0.2.2): 4.23× speedup on 1,900-LOC Java 25 fixture via generator-time perf flags; see `docs/archive/PERF-REWORK-SPEC.md` (archived) and `docs/bench-results/`. +- [x] Throughput-engine Tier 1 arc + post-rollback wins (0.5.0-candidate): reference fixture **19.12 ms / 68.02 MB** (vs 76.2 ms / 150 MB pre-Tier-1); selfhost fixture 832 ms / 1.85 GB. See `docs/incremental/THROUGHPUT-ENGINE-TIER1.md` and `docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md` (Move B post-mortem). ### Remaining Work - [ ] Lambda action attachment (lowest priority) - attach actions programmatically using type-safe RuleId: @@ -289,7 +290,7 @@ error: unexpected input ### Recovery Points Parser recovers at: `,`, `;`, `}`, `)`, `]`, newline -## Test Coverage (565 tests) +## Test Coverage (705 peglib-core tests + supporting modules) ### Grammar Parser Tests (17 tests) - Simple rules, actions, sequences, choices @@ -419,7 +420,7 @@ The `Keyword` rule should only include hard keywords. Contextual keywords are ma ```bash mvn compile # Compile -mvn test # Run tests (565 passing, 1 skipped) +mvn test # Run tests (705+ peglib-core tests; 0 skipped) mvn verify # Full verification ``` diff --git a/README.md b/README.md index 74b0946..0077fce 100644 --- a/README.md +++ b/README.md @@ -36,7 +36,7 @@ Quick links: - [`peglib-incremental` README](peglib-incremental/README.md) — incremental reparsing - [`peglib-formatter` README](peglib-formatter/README.md) — pretty-printer framework - [`docs/PRETTY-PRINTING.md`](docs/PRETTY-PRINTING.md) — formatter design notes -- [`docs/incremental/SPEC.md`](docs/incremental/SPEC.md) — incremental parsing spec +- [`docs/incremental/ARCHITECTURE-0.5.0.md`](docs/incremental/ARCHITECTURE-0.5.0.md) — incremental parsing architecture (0.5.0) ## Quick Start @@ -46,7 +46,7 @@ Quick links: org.pragmatica-lite peglib - 0.4.3 + 0.5.0 ``` @@ -394,7 +394,7 @@ The `peglib-maven-plugin` module (separate artifact, sibling to `peglib`) wraps org.pragmatica-lite peglib-maven-plugin - 0.4.3 + 0.5.0 @@ -453,13 +453,31 @@ public sealed interface CstNode { ## Performance -### Interpreter (PegEngine) — 0.4.1 +### Throughput engine (generator) — 0.5.0-candidate + +The parser generator output is now branded the **throughput engine** (full-reparse speed) and is distinct from the **incremental engine** (PegEngine + IncrementalSession, optimized for interactive editing). Different optimization targets, different code shapes, no shared code. + +Cumulative arc on the 1,900-LOC reference fixture (`FactoryClassGenerator.java.txt`, JMH 1.37 avgT, JDK 25, Apple Silicon, variant `phase1_allStructural_mutableResult_autoSkipPackrat`): + +| Stage | Wallclock | Allocation | vs original | +|---|---:|---:|---:| +| Pre-Tier-1 baseline | 76.2 ms | 150 MB | — | +| Post Tier 1 (A + D + F + G + G2/H + selective packrat + DFA Identifier) | 22.6 ms | 75.6 MB | -70% wallclock, -50% bytes | +| Post-rollback wins (trivia int snapshot, ASCII char interning) | **19.12 ms** | **68.02 MB** | **-75% wallclock, -55% bytes** | + +A 37k-LOC self-host stress fixture (the Java25 generated parser parsing its own generated source) lands at **832 ms / 1.85 GB** — pre-Tier-1 it OOM'd. vs javac comparison: peglib parses the reference fixture in ~2× of javac wallclock with strictly more output (lossless CST + trivia for formatter and linter use cases). -On the same 1,900-LOC Java 25 fixture, the interpreter (`PegParser.fromGrammar(...).parseCst(...)`) is **3.88× faster in 0.4.1** than 0.4.0 (281 ms → 72.4 ms; JMH 1.37 avgT, JDK 25.0.2, Apple Silicon). Incremental cursor-far edits (`peglib-incremental`) drop from 322 ms to 107 ms (3.0×). Three flame-graph-driven changes: HashMap rule-lookup cache, singleton `ParseMode` constants, `LinkedHashSet` dedup for the furthest-failure expected-set. See [`docs/bench-results/post-0.4.0/`](docs/bench-results/post-0.4.0/) for raw JMH + JFR data. +See [`docs/incremental/THROUGHPUT-ENGINE-TIER1.md`](docs/incremental/THROUGHPUT-ENGINE-TIER1.md) and [`docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md`](docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md) for the full session post-mortem (including 5 reset attempts banked as lessons), and [`CHANGELOG.md`](CHANGELOG.md#050---2026-05-06) for the per-move detail. + +### Incremental engine (interactive editing) — 0.5.0-candidate + +`peglib-incremental`'s `IncrementalSession` provides cursor-anchored single-edit reparsing. Phase 1 of the 0.5.0 architectural rework (stable node IDs + `LongLongMap` NodeIndex + Path D ancestor-preservation algorithm) ships **1.9× faster median (10.8 ms → 5.6 ms)** and **96.5% of edits within the 16 ms frame budget** vs the 0.4.3 baseline. Lever D (Cursor/Session split) layers on a further -9% median / -53% p99. See [`docs/incremental/PHASE-1-RESULTS.md`](docs/incremental/PHASE-1-RESULTS.md). + +### Interpreter (PegEngine) — 0.4.1 -### Generated parsers — 0.2.2 +On the same 1,900-LOC Java 25 fixture, the interpreter (`PegParser.fromGrammar(...).parseCst(...)`) is **3.88× faster in 0.4.1** than 0.4.0 (281 ms → 72.4 ms). Three flame-graph-driven changes: HashMap rule-lookup cache, singleton `ParseMode` constants, `LinkedHashSet` dedup for the furthest-failure expected-set. See [`docs/bench-results/post-0.4.0/`](docs/bench-results/post-0.4.0/) for raw JMH + JFR data. -The generated CST parser emits tuned helper variants driven by generator-time flags in `ParserConfig`. On the same fixture, the all-flags `phase1` variant runs at ~76 ms (4.7× over the pre-0.2.2 baseline; raw data in [`docs/bench-results/`](docs/bench-results/)). +### Generator-time perf flags (0.2.2 origin, evolved through 0.5.0) Flags (all consumed at generation time — no runtime branching in the emitted parser): @@ -471,12 +489,14 @@ Flags (all consumed at generation time — no runtime branching in the emitted p | `bulkAdvanceLiteral` | 1 | on | Bulk `pos`/`column` update for no-newline literals | | `skipWhitespaceFastPath` | 1 | on | First-char precheck derived from `%whitespace` rule | | `reuseEndLocation` | 1 | on | Reuse end-position `SourceLocation` across span + result | -| `choiceDispatch` | 2 | on | `switch(input.charAt(pos))` dispatch for literal-prefixed Choice | +| `choiceDispatch` | 2 | on | First-set `switch(input.charAt(pos))` dispatch for Choice (extended in 0.5.0 to CharClass + Reference + mixed) | +| `inlineLocations` | 2 | on (since 0.5.0) | Inline int locals at rule entry instead of SourceLocation | +| `selectivePackrat` | 2 | on (since 0.5.0) | Skip packrat cache for rules in `packratSkipRules`; auto-derives skip-set when empty via `PackratAnalyzer.autoSkipPackratRules(grammar)` | +| `tokenFastPath` | 2 | on (since 0.5.0) | DFA fast-path scanner for token-shaped rules (`< CharClass + ZeroOrMore >`) | | `markResetChildren` | 2 | off | Replace children clone+clear+addAll with mark-and-trim | -| `inlineLocations` | 2 | off | Inline int locals at rule entry instead of SourceLocation | -| `selectivePackrat` | 2 | off | Skip packrat cache for rules in `packratSkipRules` | +| `mutableParseResult` | 2 | off (opt-in) | Emit mutable `CstParseResult` with raw nullable fields — eliminates Option boxing | -The three default-off flags can be flipped on per-project via a custom `ParserConfig`. See [`docs/PERF-FLAGS.md`](docs/PERF-FLAGS.md) for the per-flag reference and guidance on when to flip, [`docs/PERF-REWORK-SPEC.md`](docs/PERF-REWORK-SPEC.md) for the underlying design, and [`docs/bench-results/java25-parse.json`](docs/bench-results/java25-parse.json) for raw JMH data. +Default-off flags can be flipped on per-project via a custom `ParserConfig`. See [`docs/PERF-FLAGS.md`](docs/PERF-FLAGS.md) for the per-flag reference and guidance on when to flip, [`docs/archive/PERF-REWORK-SPEC.md`](docs/archive/PERF-REWORK-SPEC.md) for the underlying design (archived; superseded by [`docs/incremental/THROUGHPUT-ENGINE-TIER1.md`](docs/incremental/THROUGHPUT-ENGINE-TIER1.md)), and [`docs/bench-results/java25-parse.json`](docs/bench-results/java25-parse.json) for raw JMH data. To reproduce benchmarks: @@ -505,6 +525,8 @@ Full history in [`CHANGELOG.md`](CHANGELOG.md). Highlights since the 0.2.x line: | Version | Date | What | |---|---|---| +| **0.5.0** | 2026-05-08 (candidate) | **Throughput engine** rebrand + Tier 1 perf arc — reference fixture **76.2 ms → 19.12 ms** (-75%) and self-host (37k LOC) **OOM → 832 ms**. Incremental engine Phase 1: stable node IDs + LongLongMap NodeIndex + Path D ancestor-preservation — **1.9× faster median** for cursor-aware edits. **BREAKING:** `CstNode` records gain `long id` as the first record component (equals/hashCode exclude id). New `Cursor` value type split out from `Session`. | +| **0.4.3** | 2026-05-06 | Interactive editing perf: -19% median, -26% p95. **BREAKING:** `SourceSpan` now stores int triples instead of `SourceLocation` refs. | | **0.4.2** | 2026-05-05 | Generated parsers now truly standalone — emit zero peglib FQCN references in their source. Drop them into a project with no peglib runtime and they compile. | | **0.4.1** | 2026-05-04 | **3.88× interpreter speedup** + 3.0× incremental cursor-far edit. Three flame-graph-driven fixes; no API change. | | **0.4.0** | 2026-05-03 | **Breaking** — API consolidation. Multi-module split (`peglib`, `peglib-incremental`, `peglib-formatter`, `peglib-maven-plugin`, `peglib-playground`). Consistent factory naming. Result-typed pipelines at every boundary. Parse-don't-validate `Grammar`. Immutable `FormatterConfig` record. Migration notes in CHANGELOG. | diff --git a/docs/AUDIT-REPORTS/docs-backreference.md b/docs/AUDIT-REPORTS/docs-backreference.md index 117aaab..e0c489c 100644 --- a/docs/AUDIT-REPORTS/docs-backreference.md +++ b/docs/AUDIT-REPORTS/docs-backreference.md @@ -27,7 +27,7 @@ No missing implementation, no stubs (`TODO`/`FIXME`/`UnsupportedOperationExcepti | `InterpreterBaselineGenerator` util | ✓ | ✓ | ✓ | Same test dir. | | `ParsingContext#bulkAdvanceNoNewline` | ✓ | ✓ | ✓ | Used in `parseLiteral`/`parseDictionary`. | | JMH `interpreter` benchmark variant | ✓ | ✓ | ✓ | `docs/bench-results/java25-parse-interpreter.json` present. | -| PERF-REWORK-SPEC doc | ✓ | ✓ | n/a | `docs/PERF-REWORK-SPEC.md`. | +| PERF-REWORK-SPEC doc | ✓ | ✓ | n/a | `docs/archive/PERF-REWORK-SPEC.md` (archived 2026-05-08). | ## 0.2.4 — Trivia attribution + grammar DSL directives @@ -178,7 +178,7 @@ All three mojos have `@Mojo(name = ..., defaultPhase = ..., threadSafe = true)` All 10 docs under `docs/` referenced by CHANGELOG entries are present: -- `docs/PERF-REWORK-SPEC.md` (0.2.2/0.2.3) +- `docs/archive/PERF-REWORK-SPEC.md` (0.2.2/0.2.3) — archived 2026-05-08 - `docs/TRIVIA-ATTRIBUTION.md` (0.2.4) - `docs/GRAMMAR-DSL.md` (0.2.4 + 0.2.8) - `docs/PERF-FLAGS.md` (0.2.4) @@ -186,7 +186,7 @@ All 10 docs under `docs/` referenced by CHANGELOG entries are present: - `docs/ERROR_RECOVERY.md` (expanded 0.2.4) - `docs/PLAYGROUND.md` (0.2.7) - `docs/PARTIAL-PARSE.md` (0.3.0) -- `docs/incremental/SPEC.md` (0.3.1) +- `docs/archive/SPEC-incremental-original.md` (0.3.1) — archived 2026-05-08 - `docs/PRETTY-PRINTING.md` (0.3.3) Module READMEs: diff --git a/docs/BENCHMARKING.md b/docs/BENCHMARKING.md index 7711e9f..bb9788c 100644 --- a/docs/BENCHMARKING.md +++ b/docs/BENCHMARKING.md @@ -67,14 +67,25 @@ Listed in the harness's `@Param` annotation: | Variant | Configuration | |---|---| | `none` | All generator flags off — unflagged baseline | -| `phase1` | `ParserConfig.DEFAULT` (phase-1 on, `choiceDispatch` on, other phase-2 off) | +| `phase1` | Phase-1 flags on, all phase-2 flags off | | `phase1_choiceDispatch` | phase-1 + `choiceDispatch` only | | `phase1_markResetChildren` | phase-1 + `markResetChildren` only | | `phase1_inlineLocations` | phase-1 + `inlineLocations` only | | `phase1_allStructural` | phase-1 + all phase-2 (no `selectivePackrat`) | -| `phase1_allStructural_skipPackrat` | above + `selectivePackrat` with skip-set `{Identifier, QualifiedName, Type}` | +| `phase1_allStructural_skipPackrat` | above + `selectivePackrat` with explicit skip-set `{Identifier, QualifiedName, Type}` | +| `phase1_allStructural_mutableResult` | above + `mutableParseResult` (Move A: -8.5% wallclock vs `phase1_allStructural`) | +| `phase1_allStructural_mutableResult_autoSkipPackrat` | above with empty `packratSkipRules` → auto-derived skip-set via `PackratAnalyzer.autoSkipPackratRules`. **Currently the headline variant for cumulative throughput-engine numbers** (-67% wallclock vs pre-Tier-1 baseline). | | `interpreter` | `PegEngine` path, `ParserConfig.DEFAULT` | +The harness is also parametrized on a `fixture` param: + +| Fixture | Source | Purpose | +|---|---|---| +| `reference` | 1,900-LOC `FactoryClassGenerator.java.txt` | Stable, low-variance throughput bench | +| `selfhost` | The Java25 generated parser parsing its own ~37k-LOC generated source | Stress fixture; exposes scaling regressions the small fixture misses (e.g. Move E's IntCstParseResultMap regressed self-host by 22% while reference stayed in noise) | + +The `selfhost` fixture is incompatible with `variant=interpreter` (no generated source to feed back) — that combination is rejected at `@Setup`. + ## Adding a new variant Edit `Java25ParseBenchmark.configFor(...)` and add an entry to the @@ -169,6 +180,6 @@ raw files serve as machine-readable backup. ## Related - [`PERF-FLAGS.md`](PERF-FLAGS.md) — what each `ParserConfig` flag does -- [`PERF-REWORK-SPEC.md`](PERF-REWORK-SPEC.md) — design rationale and - phase-by-phase specification +- [`archive/PERF-REWORK-SPEC.md`](archive/PERF-REWORK-SPEC.md) — design rationale and + phase-by-phase specification (archived; superseded by [`incremental/THROUGHPUT-ENGINE-TIER1.md`](incremental/THROUGHPUT-ENGINE-TIER1.md)) - [`CHANGELOG.md`](../CHANGELOG.md) — per-release measured numbers diff --git a/docs/GRAMMAR-DSL.md b/docs/GRAMMAR-DSL.md index 63a712f..0e07d78 100644 --- a/docs/GRAMMAR-DSL.md +++ b/docs/GRAMMAR-DSL.md @@ -215,8 +215,8 @@ Input `clss` produces a diagnostic with note `help: did you mean 'class'?`. Multiple `%suggest` directives are permitted; their vocabularies combine. Vocabularies are precomputed once per `ParsingContext` and do not change -between parse attempts (this matters for the planned incremental parser -in 0.3.1, which forwards the dictionary across reparses without +between parse attempts (this matters for the incremental parser introduced +in 0.3.0–0.3.2, which forwards the dictionary across reparses without recomputation). If no `%suggest` directive is declared, no suggestion logic runs and @@ -251,7 +251,7 @@ Each finding has a stable tag for tooling integration. The full catalog: | `grammar.nullable-rule` | INFO / WARNING | Rule can match the empty string. Promoted to WARNING when the rule is on a direct left-recursive path (infinite-loop risk) | | `grammar.duplicate-literal` | ERROR | Literal repeated verbatim within the same `Choice` | | `grammar.whitespace-cycle` | ERROR | `%whitespace` expression transitively references itself | -| `grammar.has-backreference` | INFO | Rule uses `$name` back-reference — forward-compat note: incremental parsing (planned for 0.3.1) falls back to full reparse on such rules | +| `grammar.has-backreference` | INFO | Rule uses `$name` back-reference — forward-compat note: incremental parsing (since 0.3.2) falls back to full reparse on such rules | The ambiguous-choice check is conservative: it flags only choices where *every* alternative has a fixed literal prefix. Rule-reference-prefixed or diff --git a/docs/HANDOVER.md b/docs/HANDOVER.md index 6f3a59a..015aa4d 100644 --- a/docs/HANDOVER.md +++ b/docs/HANDOVER.md @@ -121,13 +121,13 @@ Tagged 2026-04-21 originally. Published to Central on demand because a downstrea Trigger ID `trig_01HhXqsGeHfRoWNNnqM7TLod`, fires **2026-05-08T14:00:00Z**. Runs JMH + async-profiler against the post-0.3.6 baseline, captures flame graphs, posts results as a comment on the open release-0.3.6 PR or as a new issue. View at https://claude.ai/code/routines/trig_01HhXqsGeHfRoWNNnqM7TLod -The agent will report which Tier (per `docs/incremental/V2.5-SPIKE.md` § "Alternative levers") the next bottleneck lives in. +The agent will report which Tier (per `docs/archive/V2.5-SPIKE.md` § "Alternative levers") the next bottleneck lives in. **Important context:** the agent assumes 0.3.6 contains the lever 1 fix, which it doesn't. The agent may report numbers worse than the spike's projected 5-15ms (because lever 1 is still not landed). The agent's logic still works as a baseline-capture — just interpret the results in light of "lever 1 not yet shipped." ### 6.2 Lever 1 — incremental perf (DEFERRED — deeper than the spike claimed) -**Status:** the spike doc's "zero correctness risk" claim is **retracted**. See `docs/incremental/V2.5-SPIKE.md` "Addendum (post-0.4.0)" for the retraction. Two failed attempts on this lever: +**Status:** the spike doc's "zero correctness risk" claim is **retracted**. See `docs/archive/V2.5-SPIKE.md` "Addendum (post-0.4.0)" for the retraction. Two failed attempts on this lever: | Attempt | Approach | Failures | Stash | |---|---|---|---| @@ -242,7 +242,7 @@ Regen via: ### Bench - `peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java` - `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/IncrementalBenchmark.java` -- `docs/incremental/V2.5-SPIKE.md` — **read this before any incremental perf work** +- `docs/archive/V2.5-SPIKE.md` — **read this before any incremental perf work** (archived; lever-1 superseded by 0.5.0 architecture) ## 9. Things that are easy to get wrong @@ -259,28 +259,167 @@ Regen via: ## 10. Where to find historical context -- `docs/RELEASE-PLAN-0.3.5-0.4.0.md` — the plan that drove the 0.3.5→0.4.0 arc, marked complete through Phase 8. +- *(deleted 0.5.0-candidate cleanup)* `docs/RELEASE-PLAN-0.3.5-0.4.0.md` was the plan that drove the 0.3.5→0.4.0 arc. Marked complete through Phase 8 before removal; recover from git history if needed. - `docs/AUDIT-REPORTS/CONSOLIDATED-BACKLOG.md` — the audit findings that drove 0.3.4 cleanup. Most P3 items shipped in 0.4.0. -- `docs/PERF-REWORK-SPEC.md` — the 0.2.2 perf rework spec; historical. -- `docs/incremental/SPEC.md` — the 0.3.0-0.3.2 incremental spec. v2 shipped, v2.5 NO-GO'd by spike. -- `docs/incremental/V2.5-SPIKE.md` — the v2.5 NO-GO + lever-1 design + post-0.4.0 retraction. Lever-1 superseded by 0.5.0 architecture. -- `docs/incremental/UNSAFE-GENERATOR-SPIKE.md` — the post-0.4.0 unsafe-generator design + status. Infrastructure landed (5 commits in `release-0.4.2` history); behavior conversion deferred to 0.5.0. +- `docs/archive/PERF-REWORK-SPEC.md` — the 0.2.2 perf rework spec; archived/historical. +- `docs/archive/SPEC-incremental-original.md` — the 0.3.0-0.3.2 incremental spec (archived). v2 shipped, v2.5 NO-GO'd by spike. +- `docs/archive/V2.5-SPIKE.md` — the v2.5 NO-GO + lever-1 design + post-0.4.0 retraction (archived). Lever-1 superseded by 0.5.0 architecture. +- `docs/archive/UNSAFE-GENERATOR-SPIKE.md` — the post-0.4.0 unsafe-generator design + status (archived). Infrastructure landed (5 commits in `release-0.4.2` history); behavior conversion deferred to 0.5.0. +- `docs/archive/PHASE-0-RESULTS.md`, `docs/archive/PHASE-1-PROVE-OUT.md` — interim 0.5.0 phase results (archived); final state in `docs/incremental/PHASE-1-RESULTS.md`. - `docs/incremental/ARCHITECTURE-0.5.0.md` — **forward-looking** architectural spec for the 0.5.0 incremental-native rework. Read this before touching incremental perf or correctness. - `docs/bench-results/` — committed JMH JSON from each perf-touching release. ## 11. Recommended next session -The 0.4.x perf arc is complete (281 ms → 10.8 ms median, 26× from baseline; 0.4.3 ships at p99 ≈ 53 ms with 91.5% of edits under the 16 ms frame budget). Further per-edit-cost reductions are gated on architectural change, not algorithmic tweaks. +**Branch pushed at `fd278fe`. Tag `v0.5.0-candidate` at HEAD.** 922 tests green. The full 0.5.0 arc shipped this session: incremental engine (Phase 1 Path D + Lever D Cursor split) + throughput engine (Tier 1 partial: A, D, F, G, G2+H, selective packrat, DFA fast-path). -1. **Read `docs/incremental/ARCHITECTURE-0.5.0.md` first** — the spec that supersedes the §6.2 lever-1 puzzle and §6.4 unsafe-generator work. Both items dissolve into the proposed 0.5.0 design. -2. **Phase 0 prototype** (per spec §6) — Lever A (stable node IDs + `LongLongMap` NodeIndex) on the calculator grammar end-to-end. ~1 week. GO/NO-GO gate on whether incremental NodeIndex update materially beats the 0.4.3 baseline on synthetic edits. -3. **If GO:** execute Phases 1–5 over 4–5 weeks. Levers B (top-down pivot), C (`peglib-rt` interpreter/generator unification), D (Cursor split). Single major-version bump at 0.5.0. -4. **If NO-GO:** revisit individual levers tactically; the spec stays as a future reference; ship 0.4.x patches as needed for downstream bugs. +### Headline cumulative numbers -Do NOT attempt lever 1 as a 0.4.x patch — both attempts proved the data structure forbids it. The 5-10 day correctness estimate in §6.2 is what's required to make it work *without* the architectural change; the spec proposes a path that makes lever-1 a 30-line method instead. +**Incremental engine** (`IncrementalSessionBench`, Regime B cursor-moved-to-edit, 1900-LOC fixture): -Do NOT pursue further allocation reduction in the 0.4.x interpreter without the architectural change — the SourceLocation interning probe and ParseResult.Failure singleton probe both confirmed bytes-allocated metrics don't translate to wall-time wins under the current data structure (HashMap.Node entries promote and dominate). The structural SourceSpan refactor that DID ship in 0.4.3 was the exception because it eliminated long-lived refs from CstNodes. +| Metric | 0.4.3 | 0.5.0 | Δ | +|---|---:|---:|---:| +| Median | 10.8 ms | **5.0 ms** | **-54%** | +| p95 | 22.4 ms | **11.2 ms** | **-50%** | +| p99 | 53.3 ms | 90.5 ms | +70% (large-pivot tail) | +| % under 16ms | 91.5% | **96.5%** | **+5pp** | + +**Throughput engine** (`Java25ParseBenchmark`, variant `phase1_allStructural_mutableResult_autoSkipPackrat`): + +| Fixture | Original | 0.5.0 | Δ vs original | vs javac | +|---|---:|---:|---:|---:| +| Reference (1900 LOC) | 76.2 ms / 150 MB | **22.6 ms / 75.6 MB** | **-70% wallclock, -50% bytes** | 2.5× of javac (9 ms) | +| Self-host (37k LOC) | OOM | **956 ms / 2.04 GB** | from impossible to 1 sec | n/a | +| GC time (reference) | 2,844 ms | 354 ms | **-87%** | — | + +### Branch state at `fd278fe` — 40 commits past `1619604` chore + +**Phase 0 — sandbox spike (later cleaned up):** `d00eaa1` … `a8c6efe` +**Phase 1 — incremental engine production:** `8f844eb` Path A RED → `8b27dd6` Path D GREEN → `2443779` CstNode long id → `39e11f9` NodeIndex LongLongMap + Path D applyIncremental → `65a719f` nodesById refresh → `43baaf8` results doc +**Phase 2 attempted, rolled back:** `e038e4f` Lever B literal-prefix cost 4× — reverted +**Bench/JBCT cleanup:** `0ea98af` bench post-edit validation → `4ad5824` parseFull→Result +**Sandbox cleanup + Lever D:** `5275d86` -5463 LOC → `4f06046` Cursor split (p99 -53%) +**Throughput engine Tier 1:** +- `0ed2dcd` A spike — Option boxing eliminated (`mutableParseResult` flag) +- `fedc389` A coverage extension — wallclock -12% vs original +- `478b89b` D production sweep (cleanup) +- `5b2b6a1` D emission templates — `inlineLocations` default-on +- `2ad2674` E packrat int-keyed → reverted at `8f844eb`-style sequence +- `8f844eb`-equivalent: `9e9414a` E revert (regressed self-host 22%) +- `2a8cefc` self-host bench fixture added +- `7fdd5e8` D2 — eliminate remaining location() callers +- `dd1f150` F — FIRST-set Choice dispatch (62/64 choices, -20% both fixtures) +- `eec8ba3` G — JBCT-style method splits (parse_Stmt 27k→3k) +- `59be764` G2+H — Sequence chunks + nested Choice extraction (-5/-8%) +- `ca2dcfe` Selective packrat auto-detect (**biggest single win: -38% reference, -14% self-host**) +- `0ea0765` DFA fast-path Identifier-shape rules (-10% reference) +- `fd278fe` Tier 1 results + DFA javac comparison + CHANGELOG + +### What's reverted (and why — pattern) + +E (int-keyed packrat): regressed self-host 22% — linear probing scales badly at large load factors. +H2 (recursive nested-Choice per-alt): +4-7% slower — call overhead exceeded JIT inline benefit. +DFA generalization (whitespace + NumLit): neutral — low-volume rules don't pay off. + +**Pattern:** high-volume single-target wins (A, F, selective packrat, Identifier fast-path) deliver big. Broad generalizations don't. + +### Move B — attempted and abandoned 2026-05-08 + +The mutable parse-state singleton arc was attempted in this session: 5 incremental commits landed (`88c15f3` foundation → `a86fa97` parse_→boolean → `23ba500` match helpers → `98f4c11` combinators → `ed95951` predicates/capture/cut/TB), then **policy-driven rollback to `v0.5.0-candidate`**. Branch state preserved. + +**Why it failed (well-supported by 5 bench data points):** modern JIT was already scalar-replacing the per-call `CstParseResult` records (raw-nullable fields + immediate-consume call sites are textbook escape-analysis fodder). Replacing them with a heap-bound singleton **defeated** that optimization. Allocation rate dropped (-12.8% cumulative — short of the §9 15% gate) **but wallclock regressed monotonically** (+11.0% cumulative). The trajectory was clear by commit 5: pushing further would have hurt more. + +| Stage | Wallclock (ms/op) | Alloc (MB/op) | +|---|---:|---:| +| Baseline (this session start) | 22.6 | 75.6 | +| Commit 3 | 23.97 | 72.1 | +| Commit 4 | 24.71 | 66.3 | +| Commit 5 | 25.09 | 65.96 | + +**Full post-mortem with hypothesis, ruled-out moves, and reoriented optimization directions: [`docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md`](incremental/THROUGHPUT-ENGINE-MOVE-B.md) §11.** + +The 5 commits are in reflog (`git reflog show release-0.5.0`) for ~30 days if forensic inspection is needed. Don't re-attempt; new info has retired the approach. + +### Reoriented next moves (post-Move-B reassessment) + +The Move B failure tells us **allocation rate is no longer a productive target** in the throughput engine. The engine is now allocation-optimized to the point where further alloc reduction has negative ROI on wallclock. Future wins live elsewhere: + +- **Profile-driven wallclock work** (NEW, highest-ROI suggestion) — `async-profiler` in CPU mode + flame graphs on the reference fixture. Identify the actual hot CPU work post-Tier-1, target that directly. The Move B failure proved alloc-rate metrics can be misleading; CPU samples are the trustworthy signal now. **EXECUTED THIS SESSION — see "Post-rollback profile-driven optimization arc" below for outcomes.** +- **Char-class bit-packing** — pre-emit ASCII bitmaps; bitwise test instead of range comparisons. **Reassess with care** — same risk class as Move B. Bench wallclock first to confirm range-comparison isn't already JIT-optimized via SIMD/code-motion. Potential ~5-15% on char-class-heavy paths IF measurably hot. +- **Lever B retry** (incremental engine) — gated on trivia attribution rework. SafePivotAnalyzer + NodeIndex.smallestEnclosing live as dormant infrastructure for the eventual retry. Independent of allocation patterns; unaffected by Move B finding. +- **Trivia attribution rework** — context-independent attachment. Unblocks Lever B; comparable scope to Lever C. Independent of allocation; unaffected by Move B finding. +- **Lever C — IR unification** (spec §4) — multi-week. Eliminates "every fix paid twice" pattern between PegEngine and ParserGenerator emission templates. Reduces 7,440 LOC to ~1,700. Maintainability + complexity reduction primary value, not raw perf. + +### Post-rollback profile-driven optimization arc (this session, 2026-05-08) + +After Move B was abandoned and rolled back to `v0.5.0-candidate` (`e849b63`), a profile-driven optimization arc executed in the same session. Two profile passes (CPU + alloc × reference + selfhost via async-profiler) identified specific candidates; each candidate landed under a strict bench gate (>3% wallclock OR >5% alloc → ship; >1% wallclock regression → reset). + +**Branch now at `38b6a8e`** — 2 commits past `v0.5.0-candidate`: + +| Commit | Direction | Reference Δ | Selfhost Δ | +|---|---|---|---| +| `4763251` | trivia snapshot via int size (eliminates `List.copyOf` per backtrack) | **-12.1% wallclock / -6.4% alloc** | **-8.2% wallclock / -7.5% alloc** | +| `38b6a8e` | matchCharClassCst ASCII char interning pool (eliminates `String.valueOf(c)` per match) | **-3.95% wallclock / -3.76% alloc** | **-4.59% wallclock / -1.38% alloc** | + +**Cumulative (from `v0.5.0-candidate` baseline, this machine, this session):** + +| Fixture | v0.5.0-candidate | After 38b6a8e | Δ wallclock | Δ alloc | +|---|---:|---:|---:|---:| +| Reference (1900 LOC) | 22.66 ms / 75.55 MB | **19.12 ms / 68.02 MB** | **-15.6%** | **-10.0%** | +| Selfhost (37k LOC) | 937 ms / 2.04 GB | **832 ms / 1.85 GB** | **-11.2%** | **-9.3%** | + +**Reference fixture is now under 20 ms — the original Move B wallclock target, achieved without singleton mutation.** + +### Lesson taxonomy from this session's optimization arc + +7 candidates attempted post-rollback. 2 ships, 5 resets. Pattern that emerged: + +| Pattern | Result | Reason | +|---|---|---| +| Replace `List.copyOf` (varargs / array copy) with primitive int snapshot | **WIN** | JIT cannot elide bulk array copies — real alloc cost eliminated | +| Replace per-call `String.valueOf(c)` with interned ASCII pool | **WIN** | JIT allocates fresh String per call; no escape-analysis path | +| Replace `String.contains` quadratic scan with `LinkedHashSet` dedup | RESET | JIT inlines String.contains efficiently; call-overhead-dominated | +| Replace `HashMap` with custom open-addressed long-keyed map | RESET | JDK HashMap per-op faster than custom; per-op latency tax exceeds alloc savings | +| Provide `HashMap` initial-capacity hint from input size | RESET | Over-sizing hurts cache locality more than it saves resize cost | +| Convert `Token` record → mutable class with text cache (lazy substring) | RESET | Records are JIT-scalar-replaceable; mutable class with cache field defeats EA | +| Defer `SourceLocation` construction in trackFailure to parse-end | RESET | SourceLocation is a record; JIT readily stack-allocates / dead-code-eliminates | + +**Refined principle:** allocation share in a profile is NOT predictive of wallclock improvement when JIT escape analysis already handles the alloc. Successful patterns target allocations the JIT cannot elide: +- Bulk array copies (`Arrays.copyOf`, `ArrayList.toArray`) +- Per-call freshly-allocated objects with no scalar replacement path (e.g. `String.valueOf(c)` materializes a fresh char[] backing) + +Failed patterns target allocations the JIT already optimizes: +- Records (compact, scalar-replaceable, value-class-like) +- Immediately-consumed objects in a single method's scope +- JDK collection internals (HashMap is heavily JIT-optimized) +- Mutable shared state on `this` (defeats EA — Move B's lesson) + +### What's now also ruled out (this session) + +- Per-call `CstParseResult` elimination via singleton (Move B — confirmed; rolled back) +- `String.contains` micro-optimization in trackFailure +- Custom long-keyed packrat cache map +- HashMap initial-capacity sizing hints +- Lazy Token text materialization (record→class transition) +- Lazy SourceLocation construction in trackFailure + +### What's still viable + +- **Char-class bit-packing** — REASSESS WITH CARE per same risk class. Bench wallclock first. +- **Lever B retry** (incremental engine) — independent of allocation patterns +- **Trivia attribution rework** — independent +- **Lever C IR unification** — multi-week, maintainability-first +- **Re-profile post-`38b6a8e`** — the profile has shifted again; new candidates may emerge or the profile may be flat (no clear next move). Run a third profile pass if pursuing further wallclock work. + +### Items superseded by this session's work + +- §6.2 lever-1 puzzle: dissolved by Path D's stable-id algorithm. +- §6.4 unsafe-generator work: out of scope; 0.5.0 design doesn't need it. + +Do NOT pursue further allocation reduction in the 0.4.x interpreter — old guidance still holds. + +**Updated post-Move-B (2026-05-08):** Do NOT pursue further allocation reduction in the **generated parser** either. The Move B failure proved alloc-rate is no longer a productive target — JIT escape analysis is already doing aggressive scalar replacement on per-call records. Future perf work should be CPU-profile-driven, not alloc-profile-driven. --- -**Last updated:** 2026-05-03, after the post-0.4.0 lever-1 retry failed (31/100 parity regressions). Handover from the 0.3.5 → 0.4.0 arc, plus the failed-lever-1 forensics. +**Last updated:** 2026-05-08, end of profile-driven optimization arc (post-Move-B rollback). Branch `release-0.5.0` at `38b6a8e` — 2 commits past `v0.5.0-candidate` (`e849b63`). peglib-core 705 tests green (includes 6 smoke tests from the 2 shipped commits — CandidateTwoSmokeTest 3, CandidateFourSmokeTest 3); full reactor green via `mvn install`. Cumulative wins this session: -15.6% reference wallclock / -11.2% selfhost wallclock. Reference now under 20 ms. Move B post-mortem: [`docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md`](incremental/THROUGHPUT-ENGINE-MOVE-B.md) §11. Next session: re-profile post-`38b6a8e` if pursuing further wallclock; otherwise pivot to trivia attribution rework / Lever B retry / Lever C IR unification. diff --git a/docs/PARTIAL-PARSE.md b/docs/PARTIAL-PARSE.md index 293b401..4608fca 100644 --- a/docs/PARTIAL-PARSE.md +++ b/docs/PARTIAL-PARSE.md @@ -6,7 +6,7 @@ parsing stopped. Unlike `parseCst`, the matched rule is not required to consume all remaining input — parsing stops when the rule itself finishes. The feature is primarily consumed by the `peglib-incremental` module for -cursor-anchored reparsing (see [`docs/incremental/SPEC.md`](incremental/SPEC.md) §5.6) +cursor-anchored reparsing (see [`docs/archive/SPEC-incremental-original.md`](archive/SPEC-incremental-original.md) §5.6) but is also useful for grammar debugging and testing. ## API @@ -104,7 +104,7 @@ lookup. ## See also -- [`docs/incremental/SPEC.md`](incremental/SPEC.md) — full incremental reparse spec +- [`docs/archive/SPEC-incremental-original.md`](archive/SPEC-incremental-original.md) — original incremental reparse spec (archived; current architecture in [`incremental/ARCHITECTURE-0.5.0.md`](incremental/ARCHITECTURE-0.5.0.md)) - [`GRAMMAR-DSL.md`](GRAMMAR-DSL.md) — grammar syntax reference - [`RuleId`](../peglib-core/src/main/java/org/pragmatica/peg/action/RuleId.java) and diff --git a/docs/PERF-FLAGS.md b/docs/PERF-FLAGS.md index 5069a00..640b476 100644 --- a/docs/PERF-FLAGS.md +++ b/docs/PERF-FLAGS.md @@ -1,15 +1,21 @@ # Performance Flags Reference -Peglib's `ParserConfig` carries ten boolean performance flags consumed at +Peglib's `ParserConfig` carries twelve boolean performance flags consumed at **generator time**. They select which helper variant `ParserGenerator` emits; they do **not** produce runtime `if (flag)` branches in the generated output. A given `(grammar, ParserConfig)` pair produces deterministic, byte-identical Java source. +> **0.5.0 update:** `inlineLocations`, `selectivePackrat`, and `tokenFastPath` +> are now default `true`. `mutableParseResult` was added as an opt-in flag +> (default `false`) — see [`THROUGHPUT-ENGINE-MOVE-B.md`](incremental/THROUGHPUT-ENGINE-MOVE-B.md) +> for why the singleton-mutation extension on top of it was abandoned. + For the design rationale behind each flag — including the patches originally prototyped and the measured wins — see -[`PERF-REWORK-SPEC.md`](PERF-REWORK-SPEC.md) §§6–7. This document is a -tabular quick-reference; the spec is the source of truth. +[`archive/PERF-REWORK-SPEC.md`](archive/PERF-REWORK-SPEC.md) §§6–7 +(archived). This document is a tabular quick-reference; the spec is +the source of truth. ## Scope @@ -46,10 +52,12 @@ any regression can be bisected to a single flag. | Flag | Default | Optimization | |---|---|---| -| `choiceDispatch` | `true` | For `Choice` whose alternatives are all literal-prefixed, emit a `switch (input.charAt(pos))` dispatch that narrows to the subset of alternatives starting with the current char. Falls through to the general backtracking chain for alternatives without a fixed prefix. **Default flipped on in 0.2.2 based on a 2.49× measured speedup over the phase-1 baseline.** | +| `choiceDispatch` | `true` | First-set `switch (input.charAt(pos))` dispatch for Choice. Originally literal-prefix only (0.2.2); **extended in 0.5.0** to full transitive FIRST-set computation covering CharClass + Reference (recursive) + mixed dispatch. On Java25, 62/64 of choices now dispatch via switch. **-20% wallclock on both fixtures vs phase-1 baseline.** | | `markResetChildren` | `false` | Replace `children` clone + `clear` + `addAll` in `Choice` with mark-and-trim (`children.subList(mark, size()).clear()`). O(1) mark; O(delta) reset. No statistically significant individual win on the reference JVM. | -| `inlineLocations` | `false` | Introduce per-rule `int startOffset` / `startLine` / `startColumn` locals and materialize `SourceLocation` only at span boundaries. No statistically significant individual win on the reference JVM. | -| `selectivePackrat` | `false` | Skip packrat cache for rules listed in `packratSkipRules`. Marginal combo win (~5%) sits inside measurement noise on the reference workload. Callers must supply `packratSkipRules` or the flag is a no-op. | +| `inlineLocations` | `true` (since 0.5.0) | Per-rule `int startOffset` / `startLine` / `startColumn` locals; materialize `SourceLocation` only at span boundaries. Default flipped on in 0.5.0 after the emission-template sweep that eliminated SourceLocation/SourceSpan allocations on the rule-entry path. | +| `selectivePackrat` | `true` (since 0.5.0) | Skip packrat cache for rules listed in `packratSkipRules`. **0.5.0 semantics:** when `packratSkipRules` is empty, the generator auto-derives the skip-set via `PackratAnalyzer.autoSkipPackratRules(grammar)` — leaf-like rules and single-call-site rules without quantifiers bypass the cache; left-recursive rules are excluded. Pass a non-empty explicit set to override. **Biggest single win in the 0.5.0 throughput-engine arc: -38% reference / -14% self-host wallclock; -75% gc.count.** | +| `tokenFastPath` | `true` (since 0.5.0) | DFA fast-path scanner for token-shaped rules: detects `< CharClass + ZeroOrMore >` (Identifier-shape) and emits a tight inline scanner with pre-computed ASCII bitmasks instead of going through `matchCharClass` per character. **-9.8% reference / -7.6% self-host wallclock.** | +| `mutableParseResult` | `false` (opt-in) | Emits a mutable `CstParseResult` class with raw nullable fields (no `Option value` / `Option expected` wrappers) plus raw-nullable `furthestFailure` / `furthestExpected` / `pendingFailureRecoveryOverride`. Eliminates Option boxing on every result construction. The Move B extension that replaced per-call records with a heap-bound singleton was attempted and abandoned; see [`THROUGHPUT-ENGINE-MOVE-B.md`](incremental/THROUGHPUT-ENGINE-MOVE-B.md) for the post-mortem. | ### `packratSkipRules` @@ -62,7 +70,9 @@ output. Default: `Set.of()`. ## When to flip defaults -All three default-off flags live behind a custom `ParserConfig`: +As of 0.5.0 most optimizations default on. The two remaining default-off / +opt-in flags (`markResetChildren`, `mutableParseResult`) live behind a custom +`ParserConfig`: ```java var config = new ParserConfig( @@ -71,10 +81,12 @@ var config = new ParserConfig( true, // captureTrivia true, true, true, true, true, true, // phase-1 (keep on) true, // choiceDispatch (keep on) - true, // markResetChildren (flip on) - true, // inlineLocations (flip on) - true, // selectivePackrat (flip on) - Set.of("Identifier", "QualifiedName", "Type")); // skip set + true, // markResetChildren (flip on if measured) + true, // inlineLocations (default on since 0.5.0) + true, // selectivePackrat (default on since 0.5.0) + Set.of(), // packratSkipRules — leave empty to auto-derive + true, // mutableParseResult (opt-in; -8.5% wallclock per Move A spike) + true); // tokenFastPath (default on since 0.5.0) Result source = PegParser.generateCstParser( grammarText, "com.example", "MyParser", @@ -83,13 +95,20 @@ Result source = PegParser.generateCstParser( Guidance: -- **`markResetChildren` / `inlineLocations`** — try on grammars with many - alternatives per choice or with span-heavy rules. Re-benchmark against - your workload; the reference fixture shows no individual win. -- **`selectivePackrat`** — gather per-rule cache hit ratios first - (`PackratStatsProbe` at `src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java`). - Rules with `hits/puts < 0.1` are candidates. Only sound for rules - that are not left-recursive. +- **`markResetChildren`** — try on grammars with many alternatives per choice + or with span-heavy rules. Re-benchmark against your workload; the reference + fixture shows no individual win. Still default off. +- **`selectivePackrat`** — defaults on with auto-derivation. To override the + auto-derived skip-set, pass an explicit non-empty `packratSkipRules`. + Gather per-rule cache hit ratios with `PackratStatsProbe` at + `peglib-core/src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java` + if you want to compare. Rules with `hits/puts < 0.1` are candidates. Only + sound for rules that are not left-recursive. +- **`mutableParseResult`** — opt-in for users targeting the throughput engine's + full perf envelope. Eliminates Option boxing on every parse-method result. + The Move B singleton extension on top of this flag was attempted and + abandoned (modern JIT escape analysis already scalar-replaces the per-call + records); see [`THROUGHPUT-ENGINE-MOVE-B.md`](incremental/THROUGHPUT-ENGINE-MOVE-B.md). ## Measured reference numbers diff --git a/docs/RELEASE-PLAN-0.3.5-0.4.0.md b/docs/RELEASE-PLAN-0.3.5-0.4.0.md deleted file mode 100644 index 5fc2aff..0000000 --- a/docs/RELEASE-PLAN-0.3.5-0.4.0.md +++ /dev/null @@ -1,269 +0,0 @@ -# Release Plan — 0.3.5 + 0.4.0 - -**Status:** locked 2026-04-25. Starts from `main` at v0.3.4. - -Two releases close out the audit backlog and ready the library for IDE-plugin consumption (IntelliJ + VS Code first; Eclipse later). - -- **0.3.5** — trivia round-trip + `%recover` wiring. Non-breaking. -- **0.4.0** — API consolidation + test hygiene + incremental v2.5 perf. Breaking. - -Subagent convention per repo: `jbct-coder` for all code; `build-runner` for `mvn` invocations; `jbct-reviewer` for parallel focused review. Never invoke other coders. - ---- - -## Release 0.3.5 — "trivia round-trip" — ✅ SHIPPED 2026-05-01 - -**Final status:** all 22 perf-corpus fixtures round-trip byte-equal via the -generated parser. `RoundTripTest` re-enabled. Five distinct bugs (Bug A -through Bug C'') were required, three discovered after the initial plan was -written. `%recover` directive wired end-to-end on the interpreter side -(generator-side per-rule overrides remain deferred to 0.3.6). - -**Goal:** un-`@Disable` `RoundTripTest`, prove `%recover` works end-to-end. Revised estimate: ~5-7 working days (originally 4 — diagnostic on 2026-04-26 revealed three distinct bugs, not one — and a follow-up diagnostic on 2026-04-30 revealed two more, all fixed in 0.3.5). - -### Diagnostic findings (2026-04-26) - -Empirical round-trip diagnostic on `DeepGenerics.java` (smallest failing corpus file) showed two failure shapes, not one. Plus the originally-planned third. All three need fixing for byte-equal round-trip: - -- **Bug A — pending-trivia snapshot is size-only.** `ParsingContext.savePendingLeadingTrivia()` returns just the list size; `restorePendingLeadingTrivia(int)` only truncates if the buffer grew. Items *consumed* inside a backtracked branch are permanently lost. Fix: change snapshot to a full `List` and restore by replacing buffer contents. -- **Bug B — packrat cache bakes in dynamic leading trivia.** `cacheAt` stores the wrapped node *including* its `leadingTrivia`. Cache hits at the same `(ruleName, position)` return that stale leading regardless of current pending. Result: trivia gets attributed to both an outer wrapper rule AND the cached inner rule (the duplication seen as `rec > src` in the diag). Fix: cache nodes with empty leadingTrivia; reattach current pending on cache hit. -- **Bug C — rule-exit pos-rewind missing.** The original plan item. After a rule body completes, any pending trivia not drained by a child gets dropped. Fix: at rule-exit, if pending is non-empty, rewind `pos` past it and attach to the last child's `trailingTrivia`. Manifests as `rec < src` in the diag (-16, -30, -301 byte cases). - -### Phase 1A — Bug A: list-snapshot pending trivia (~0.5 day) - -- `ParsingContext.savePendingLeadingTrivia()` → returns `List` snapshot -- `ParsingContext.restorePendingLeadingTrivia(List)` → replaces buffer with snapshot contents -- All call sites in `PegEngine` (Sequence ~1490, Choice ~1538, ZoM ~1587, OoM ~1652, Repetition, Optional, And, Not) updated to pass/receive `List` -- Mirror in `ParserGenerator` emission templates (~4406-4414) -- All 874 existing tests must stay green; baselines unchanged at this phase - -### Phase 1B — Bug B: cache-safe leading trivia (~1 day) - -- `parseRule` wraps with empty leading before caching; reattaches `ruleLeading` only on the path that returns -- Cache hit path: take cached node (no leading), reattach current `takePendingLeadingTrivia()` as leading -- Same logic in `parseRuleWithLeftRecursion` and `parseRuleWithActions` -- Mirror in generator's emitted rule wrappers (lines ~2454-2570 and LR variant ~2581-2700) -- Existing parity baselines should hold (this phase doesn't change spans, just trivia attribution invariants) - -### Phase 1C — Bug C: cache-hit leading-trivia ambiguity (✅ shipped) - -Empirical diagnostic isolated the duplication to the **generator** (interpreter -was already correct). The generator's cache stored the wrapped-with-leading -body, so subsequent cache hits with empty pending preserved stale leading -through `attachLeadingTrivia`'s short-circuit, attaching trivia at multiple -nesting levels. - -Fix: cache an empty-leading wrap, return the actual-leading wrap. Cache -hits now apply current pending without inheriting stale state. Interpreter -unchanged — already cached the body, not the wrap. - -After this fix, generator dropped from 12/22 → 5/22 round-trip pass -because pre-existing trivia gaps (previously masked by Bug C duplication -accidentally compensating for them) became visible. - -### Phase 1C' — Bug C': rule-exit trailing-trivia attribution (✅ shipped) - -The visible loss cases revealed by Bug C: trivia consumed by the body's -last inter-element `skipWhitespace` (e.g. before a zero-width tail like -empty ZoM/Optional) ends up in `pendingLeadingTrivia` with no child to -claim it. Originally planned as rule-exit pos-rewind, but rewinding pos -broke predicate combinators (`!isPredicate(element)` skip-no-whitespace -semantics). - -Fix: at rule-exit success path, attach pending trivia to the last child's -`trailingTrivia` (or to the rule node's trailing if children empty). -**Pos is not rewound.** Applied symmetrically in `PegEngine` and -`ParserGenerator`. - -After this fix, **interpreter reached 22/22**. Generator reached 21/22 — -the remaining `FactoryClassGenerator.java.txt` fixture had a duplicate -trailing comma exposed by Bug C''. - -### Phase 1C'' — Bug C'': generator Sequence children rollback (✅ shipped) - -Generator emitted Sequences using the rule-method's outer `children` list -directly. On element failure, location and pending were restored — but -children were not, so partial child additions from earlier elements of -the failed Sequence stayed in the parent's tree. Symptom: the trailing -comma in enum-constant lists appeared as a child of both the inner -ZoM-NT and the outer Sequence. - -Fix: snapshot `children` at Sequence start; restore on element failure -(both cut-failure and regular-failure branches). Interpreter uses a -local `children` list per `parseSequenceWithMode` call so was already -correct. - -After this fix, **generator reached 22/22**. - -See `docs/TRIVIA-ATTRIBUTION.md` for the full attribution model. - -### Phase 3 — baseline regeneration (✅ shipped) - -Only `large/FactoryClassGenerator.java.txt` shifted (the Bug C'' children-rollback fix removes a duplicate trailing comma in enum lists). Other 21 corpus baselines unchanged. Regenerated via `BaselineGeneratorRunner` (`-Dperf.regen=1`); committed as `9ac3307` with explicit "baseline-shift" CHANGELOG callout. - -### Phase 4 — `RoundTripTest` un-disable + verify 22/22 (✅ shipped) - -`@Disabled` removed; 22/22 corpus files round-trip byte-equal. `docs/TRIVIA-ATTRIBUTION.md` updated. - -### Phase 5 — `%recover` wiring debug (✅ shipped) - -Root cause identified: rule-level recovery override was popped in `parseRule`'s `finally` block before `parseWithRecovery` consulted it. Fix: capture the failed rule's override into a per-context `pendingFailureRecoveryOverride` field BEFORE the pop, with deepest-wins semantics. Committed as `ca2ac9f`. - -### Phase 6 — `%recover` proof test (✅ shipped) - -`RecoverDirectiveProofTest` (interpreter-side) committed alongside Phase 5 fix. Uses `:` as override terminator (outside default char-set) so override-vs-default discriminator is unambiguous. Pre-fix the test fails; post-fix passes. - -### Phase 0.3.5 release - -Per established 15-step pattern (handover §5). Tag `v0.3.5`. Narrative release notes emphasizing baseline shift + trivia round-trip resolution. - ---- - -## Release 0.4.0 — "API consolidation + perf" - -**Goal:** absorb all P3 architectural breaks, finish test-assertion hygiene, ship incremental v2.5 perf for IDE-plugin consumption. Estimated 3 working weeks. **Breaking.** - -### Phase 0 — v2.5 spike (1 day, GO/NO-GO gate) - -**Before any 0.4.0 implementation work.** Validate `docs/incremental/SPEC.md` §5.4 v2.5 mental model holds against measurements. - -Probe scope: -- Instrument cache invalidation cost on the 1,900-LOC fixture under current 0.3.4 incremental -- Quantify how much of the 325 ms/op is wholesale-invalidation vs. back-reference fallback vs. parse work -- Validate that span-rewriting remap can preserve enough cache entries to hit a credible target (proposing **< 10 ms/op**, relaxing SPEC's `< 1 ms`) - -**Gate:** -- **GO** → schedule v2.5 in Phase 9. 0.4.0 ships full bundle. -- **NO-GO** → defer v2.5 to 0.4.1 with redesigned approach. 0.4.0 ships breaks + hygiene only. Re-plan v2.5 once probe data is in hand. - -Write spike findings to `docs/incremental/V2.5-SPIKE.md` regardless of outcome. - -### Phase 1 — parse-don't-validate `Grammar` (~1 day) - -- Collapse `Grammar#validate()` into a `Result`-returning factory -- All `Grammar` construction goes through the factory; remove the post-construction validate step -- Update all call sites (lexer/parser, GrammarResolver, Maven plugin, tests) - -### Phase 2 — factory rename sweep + test-assertion rewrite (~5 days, bundled) - -The big mechanical sweep. Per-package commits within the release branch to keep diffs reviewable. - -**Renames** (~20 production sites + every test): -- `of()` → `typeName()` (e.g., `Grammar.of(...)` → `Grammar.grammar(...)`) -- `create()` → `typeName()` -- `at()` → `typeName()` -- Catalogue every site in `docs/AUDIT-REPORTS/CONSOLIDATED-BACKLOG.md` factory-rename entries before starting - -**Test-assertion rewrite** (~1000 sites): replace classic `assertTrue(result.isSuccess())` / `.unwrap()` with JBCT idiom that surfaces failure causes. Target pattern (confirm with first PR): - -```java -result.onFailureDo(cause -> fail(cause.message())) - .onSuccessDo(value -> { /* assertions on value */ }); -``` - -Or whichever pattern the first reviewer green-lights as canonical. Bundle per-file with the rename sweep — touching the file once. - -**Watch:** parity tests must stay green throughout. After each per-package commit, `mvn test` via `build-runner`. - -### Phase 3 — `SessionImpl` → record (~0.5 day) - -- File: `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/internal/SessionImpl.java` -- Convert to record. Move state-mutation methods to return new instances. -- Drop `Impl` suffix per JBCT naming. - -### Phase 4 — Mojo `execute()` → Result pipelines (~1 day) - -- Files: `peglib-maven-plugin/src/main/java/org/pragmatica/peg/maven/{Generate,Lint,Check}Mojo.java` -- Rewrite `execute()` bodies as Result pipelines -- Add `@Contract` annotations on Mojo boundary methods (Maven calls into untyped land) - -### Phase 5 — `Formatter` immutable builder (✅ shipped) - -- Replaced mutable builder with immutable `FormatterConfig` record + immutable nested `Builder` record -- New entry points: `FormatterConfig.builder()` / `Formatter.builder()` → `FormatterConfig.Builder`; terminal `.build()` → `FormatterConfig`; `Formatter.formatter(config)` → `Formatter` -- `Formatter` is now stateless / thread-safe post-construction; the public `new Formatter()` constructor is removed -- Files touched: - - `peglib-formatter/src/main/java/org/pragmatica/peg/formatter/FormatterConfig.java` (new) - - `peglib-formatter/src/main/java/org/pragmatica/peg/formatter/Formatter.java` - - `peglib-formatter/src/main/java/org/pragmatica/peg/formatter/package-info.java` - - `peglib-formatter/README.md` - - `peglib-formatter/src/test/java/.../examples/{Json,Sql,Arithmetic}Formatter.java` - -### Phase 6 — `Result.lift` boundaries (~1 day) - -Three sites flagged in handover §8.10: -- `PegEngine.createWithoutActions` → `Result` symmetric with `create(...)` -- Action-dispatch try/catch → `Result.lift` -- `parseRequestBody` / HttpHandler in playground → `Result.lift` - -### Phase 7 — nullable helpers → `Option` (~0.5 day) - -- `tryIncrementalReparse` (peglib-incremental) -- `findBoundaryCandidate` (peglib-incremental) -- Sweep for any other internal nullable returns; convert to `Option` - -### Phase 8 — `@Contract` annotations on remaining boundaries (~0.5 day) - -- All CLI `main` entry points (`AnalyzerMain`, `PlaygroundRepl`, `PlaygroundServer`) -- Any remaining Mojo boundary methods -- Generator entry points exposed through Maven plugin - -### Phase 9 — incremental v2.5 cache remap (~5 days, conditional on Phase 0 GO) - -If Phase 0 said GO: -- Implement span-rewriting cache remap per the (validated) `docs/incremental/SPEC.md` §5.4 v2.5 design -- Target: < 10 ms/op single-char edit on 1,900-LOC fixture -- Update `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/internal/SessionImpl.java` (now `Session` record) + cache layer -- Hold off on back-reference rule path — that's a separate, longer effort - -### Phase 10 — bench-results commit (~0.5 day) - -- Re-run JMH on Phase 9 result -- Commit raw JSON + summary to `peglib-incremental/target/bench-results/` then promote to `docs/bench-results/` -- Update `docs/PERF-FLAGS.md` and `docs/incremental/SPEC.md` with measured numbers - -### Phase 0.4.0 release - -Per established 15-step pattern. Tag `v0.4.0`. Narrative release notes lead with **breaking changes** + migration guide. Reference the `docs/AUDIT-REPORTS/CONSOLIDATED-BACKLOG.md` cleared P3 items. - ---- - -## Risks + mitigations - -| Risk | Likelihood | Impact | Mitigation | -|---|---|---|---| -| Phase 0 spike says NO-GO | Medium | High — defers v2.5 perf | Ship 0.4.0 without v2.5; schedule 0.4.1 with redesigned approach. IDE plugin can integrate against 0.4.0 API and pick up 0.4.1 perf later. | -| Phase 1 ZoM infinite loops resurface | Medium | Medium — blocks 0.3.5 | Carry over the partial debugging notes from 0.2.4 attempt; budget extra day; if still stuck, ship `RoundTripTest` un-disabled with a smaller fixture set and document scope reduction. | -| Bundled rename + assertion sweep diff is unreviewable | Low | Medium | Per-package commits within release branch (not per-rename). Reviewer reads each commit isolated. | -| JBCT idiom for assertion rewrite isn't settled | Medium | Low | First PR proposes one canonical pattern; reviewer confirms; rest follows. Don't start the sweep until pattern is locked. | -| API break churn for downstream IDE plugin | High | Low | Plugin isn't built yet — absorb the breaks now while there's no consumer cost. | - ---- - -## Out of scope - -- §8.3 actions on left-recursive rules — documented limitation, no real demand -- §8.4 indirect left-recursion — out of scope per 0.2.9 SPEC -- §8.5 back-reference incremental fallback — substantial, separate effort, no consumer asks for it -- §8.7 in-parse playground tracer — current post-parse tracer is good enough for web UI - -These remain documented in `docs/AUDIT-REPORTS/CONSOLIDATED-BACKLOG.md` as P3 / deferred. Revisit if a consumer surfaces. - ---- - -## Execution checklist (per release) - -- [ ] Branch `release-X.Y.Z` from `main` -- [ ] Bump 6 poms + README dependency snippet -- [ ] CHANGELOG entry added -- [ ] Implementation phases (this doc) -- [ ] `mvn test` green, then `mvn install` for downstream -- [ ] `mvn jbct:format` before push (CI will reject otherwise) -- [ ] PR opened against `main`, CI green -- [ ] Merge, tag, push tag -- [ ] `gh release create` with narrative notes -- [ ] Branch deleted local + remote - ---- - -**Owner:** whoever picks this up. Update phase status as work progresses; mark phases ✓ complete in-place. diff --git a/docs/PERF-REWORK-SPEC.md b/docs/archive/PERF-REWORK-SPEC.md similarity index 99% rename from docs/PERF-REWORK-SPEC.md rename to docs/archive/PERF-REWORK-SPEC.md index e5c9bab..b8817d5 100644 --- a/docs/PERF-REWORK-SPEC.md +++ b/docs/archive/PERF-REWORK-SPEC.md @@ -1,5 +1,8 @@ # java-peglib performance rework — detailed spec +> **Status:** historical / archived 2026-05-08. Executed in 0.2.2; superseded by THROUGHPUT-ENGINE-TIER1.md. + + **Target repo:** `java-peglib` (self-contained; no dependency on any other repo). **Status:** proposal / spec for an implementing agent. Not yet executed. **Scope:** optimizations to the PEG code generator (`ParserGenerator`) only. diff --git a/docs/archive/PHASE-0-RESULTS.md b/docs/archive/PHASE-0-RESULTS.md new file mode 100644 index 0000000..88c75de --- /dev/null +++ b/docs/archive/PHASE-0-RESULTS.md @@ -0,0 +1,86 @@ +# Phase 0 — GO/NO-GO Results + +> **Status:** historical / archived 2026-05-08. Interim phase results; final state in PHASE-1-RESULTS.md. + + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 +**Spec:** [`ARCHITECTURE-0.5.0.md`](../incremental/ARCHITECTURE-0.5.0.md) + +## Verdict: **GO** + +All three GO/NO-GO gates green. Recommend proceeding with Phases 1–5 per spec §6. + +| Gate | Criterion | Result | +|------|-----------|:------:| +| Q3 — identity-preservation invariant | Sibling subtrees of the splice path satisfy reference equality (`==`) with their pre-edit counterparts | **GREEN** | +| Q4 — trivia-bearing edits | Calculator grammar with `%whitespace`/comments; 3 representative edits each show incremental update equivalent to full rebuild + invariant preserved | **GREEN** | +| Perf — incremental NodeIndex update materially beats full rebuild | ≥5× speedup at representative tree sizes (≥1000 nodes) | **GREEN — 47× at 1k nodes, 67× at 10k nodes** | + +## Work delivered + +Phase 0 on the `release-0.5.0` branch, sandboxed in `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/`. No production code touched. Existing 897-test suite unaffected throughout. + +| Phase | Commit | Deliverable | New tests | +|-------|--------|-------------|----------:| +| 0a | `d00eaa1` | `IdGenerator` + `PerSessionCounter`; `LongLongMap` + `LinearProbingLongLongMap` | 17 | +| 0b | `f0696a1` | `IdCstNode` (sealed, ID-bearing variant of production CstNode); `IdCstNodeBuilder` (production-tree → IdCstNode converter) | 19 | +| 0c | `849b4ba` | `IdNodeIndex` with `build` (O(N) full) + `applyIncremental` (O(splicedSize + depth × branching) per spec §2) | 8 | +| 0d.1 | `9b55253` | `IdTreeSplicer` (record-identity-preserving splicer); identity-invariant test (Q3 gate); calculator + trivia regression test (Q4 gate) | 10 | +| 0d.2 | `a2dd8ac` | JMH bench `Phase0SpikeBench` + results note `docs/bench-results/phase0-spike-results.md` | — | + +**Test totals:** 154 incremental tests (was 100; +54 from Phase 0). 699 core tests unchanged. Full suite green at every checkpoint. + +## Gate details + +### Q3 — identity-preservation invariant + +**Test:** `IdTreeSplicerTest` (7 tests). Constructs a 4-deep, 4-wide tree, splices at one location, asserts that for every node on the splice path, all siblings at indexes other than the spliced one are reference-equal (`==`) in old and new trees. Implementation: `IdTreeSplicer.splice(...)` builds new `NonTerminal` records by `ArrayList(old.children())` + `set(spliceIndex, current)` — every other child slot is preserved by reference. + +**Microcount evidence:** Phase 0c's depth-3 splice on a 28-node tree triggered exactly 8 `parents.put` calls (vs 28 for full rebuild) — confirms the algorithm walks only the splice path + spliced subtree + ancestor children, not the whole tree. + +### Q4 — trivia-bearing edits + +**Test:** `CalculatorTriviaIncrementalTest` (3 tests, one per edit kind). Calculator grammar with `Comment <- '/*' (!'*/' .)* '*/'` and `%whitespace <- ([ \t\r\n]+ / Comment)+`. For each of: + +- Edit A — insert blank line before operand (`"1+2"` → `"1+\n 2"`) +- Edit B — delete comment between operands (`"1 /*hi*/ + 2"` → `"1 + 2"`) +- Edit C — insert comment inside expression (`"1+2"` → `"1+/*x*/2"`) + +The test parses both inputs, converts to `IdCstNode`, identifies the splice point pragmatically (the `Number` whose token text is "2"), splices via `IdTreeSplicer`, applies `applyIncremental`, and asserts the resulting parents map is structurally equivalent to a fresh `IdNodeIndex.build(newRoot)`. Identity invariant verified concurrently — the test instrumentation logs `siblingsChecked` for each edit. + +All three edits pass both assertions. The Q4 gate is satisfied **for the algorithm given identity-shared trees from a splicer**. Production parser's lack of cross-parse record sharing is a separate concern flagged for Phase 1 (see §"Notes for Phase 1" below). + +### Perf gate — incremental update beats full rebuild ≥5× + +**Bench:** `Phase0SpikeBench` (synthetic perfect 4-ary tree, depth-3 splice, JDK 25, JMH 1.37). Full numbers in [`docs/bench-results/phase0-spike-results.md`](../bench-results/phase0-spike-results.md). + +| Tree size | fullRebuild (μs) | incrementalUpdate (μs) | Speedup | +|-----------|-----------------:|-----------------------:|--------:| +| 100 | 2.678 ± 0.389 | 0.069 ± 0.001 | 38.8× | +| 1000 | 9.611 ± 0.312 | 0.203 ± 0.011 | 47.3× | +| 10000 | 176.468 ± 16.561 | 2.627 ± 0.237 | 67.2× | + +Speedup grows with tree size — incremental cost is dominated by `O(depth × branching)` (≈ `log_4(N) × 4`), full-rebuild cost is `O(N)`. The 0.4.3 fixture (1900 LOC, ~10k nodes) is in the regime where 0.5.0 can deliver order-of-magnitude per-edit gains. + +The spec's projected 0.5.0 floor (p99 ≤ 16ms on the 1900-LOC fixture) is comfortably attainable: at 10k nodes the bench shows applyIncremental at 2.6μs — five orders of magnitude under the 16ms target. Even with the realistic costs of `TreeSplicer` work, parser invocation, and trivia attribution layered on top, the budget is large. + +## Notes for Phase 1 + +Three concrete items surfaced during Phase 0 that Phase 1 should address explicitly: + +1. **Cross-parse record sharing.** `IdCstNodeBuilder` re-assigns fresh IDs on every conversion — the production parser produces a fresh tree per parse with no record sharing. The 0.5.0 algorithm's perf depends on the editing flow producing identity-shared trees, which is `TreeSplicer.spliceAndShift`'s job, not the parser's. Phase 1's first task: confirm `TreeSplicer.spliceAndShift` preserves sibling identity (per spec §8 Q3 second sentence). If it doesn't, fix it; spec calls this part of Phase 0/1 scope. + +2. **`applyIncremental` mutate-in-place semantics.** Spike API invalidates the receiver — caller must use the returned instance. Phase 1 needs to choose: (a) copy-on-write via `LongLongMap.copy()` (O(N) per edit, partially defeats the point — though still much cheaper than a full Index rebuild because the copy is primitive arrays vs IdentityHashMap entries), (b) persistent map (e.g., HAMT-based long-long), (c) keep mutate-in-place + document loudly. Decision affects the API shape of `IncrementalSession`. + +3. **Worst-case splices not benched.** The spike measures depth-3, small-pivot splices. Class-body-scale rewrites (large pivots) need a separate bench in Phase 1 to confirm the algorithm scales gracefully there too. + +## Recommendation + +Proceed with Phases 1–5 per spec §6 schedule (4–5 weeks elapsed, 2–3 weeks of focused engineering). The architectural premise is empirically validated: + +- Algorithm is correct under both invariants the spec calls out. +- Perf gain is order-of-magnitude — well above the 5× threshold and consistent with the spec's 300× projection. +- Sandbox approach proved viable: Phase 0 landed without disturbing the 897-test production surface. + +Open the next session with **Phase 1 — Lever A on full Java grammar**: migrate production `CstNode` to ID-bearing variants, switch `NodeIndex` to `LongLongMap`, verify all 897 tests + 22×100 IncrementalParityTest stay green, re-bench against the 0.4.3 baseline on the 1900-LOC fixture. diff --git a/docs/archive/PHASE-1-PROVE-OUT.md b/docs/archive/PHASE-1-PROVE-OUT.md new file mode 100644 index 0000000..73d4c3b --- /dev/null +++ b/docs/archive/PHASE-1-PROVE-OUT.md @@ -0,0 +1,110 @@ +# Phase 1 Prove-Out — Path A → Path D + +> **Status:** historical / archived 2026-05-08. Interim phase results; final state in PHASE-1-RESULTS.md. + + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (commits `8f844eb` through `8b27dd6`) +**Spec:** [`ARCHITECTURE-0.5.0.md`](../incremental/ARCHITECTURE-0.5.0.md), [`PHASE-0-RESULTS.md`](PHASE-0-RESULTS.md) + +## Verdict + +**Path D is Phase 1's architecture.** Path A (offset decoupling from records) was prove-out RED; Path D (stable-id ancestor preservation) is GREEN with **96× at 1000 nodes, 604× at 10000 nodes** — and a flat absolute cost (~25-40 ns) that confirms genuine O(δ) scaling independent of tree size. + +Phase 0's spike measured 67× speedup on balanced trees. Real CSTs aren't balanced — Java method bodies have flat fanout (1 NonTerminal "Block" with N "Stmt" children). Phase 1's prove-out exposed that the spec §2 algorithm degrades to O(N) on flat trees because step 3 rewires every sibling under a freshly-IDed ancestor. Path D fixes this by reusing old ancestor IDs across splices — the parents-map entries for unchanged siblings stay valid, step 3 disappears. + +## Why Path A was prove-out RED + +**Hypothesis:** removing `SourceSpan` from `CstNode` records eliminates the deep-copy of right-of-edit subtrees in `TreeSplicer.shiftAll` (lines 113-142 in `TreeSplicer.java`). Sibling subtrees retain identity → incremental NodeIndex update stays at O(δ). + +**Bench result (path A — `MidBufferBench`, flat tree, mid-buffer single-Terminal splice):** + +| Tree size | productionStyle (μs) | offsetDecoupled (μs) | Speedup | +|----------:|---------------------:|---------------------:|--------:| +| 100 | 1.272 | 1.153 | 1.10× | +| 1000 | 12.524 | 9.710 | 1.29× | +| 10000 | 81.656 | 102.620 | 0.80× | + +**RED.** The hypothesis was wrong about the bottleneck. Path A *did* preserve right-of-edit sibling identity (correctness test green), but the dominant cost on flat trees isn't the deep-copy — it's `applyIncremental` step 3 rewiring every direct child of the freshly-IDed ancestor. + +The Path A migration (eliminate `span()` from CstNode interface across all 5 modules; introduce SpanIndex; refactor every span-reading site) would have been weeks of cross-cutting refactor for a 1.10-1.29× best-case speedup. Honest dead end. + +## Why Path D is GREEN + +**Hypothesis:** reuse `oldAncestor.id()` when building new ancestor records during splice. Then: +- Old `parents` map entries for unchanged sibling subtrees remain valid (parent's *ID* didn't change, only its record instance did) +- step 3's "rewire children to new ancestor IDs" becomes redundant work +- step 1's "remove oldPath ancestors' up-pointers" becomes wrong (those entries are still valid) + +Optimized `applyIncremental`: +- Skip step 1 oldPath ancestor removal +- Just remove oldPivot + descendants (those ARE dead — newPivot has fresh IDs) +- Step 2: insert newPivot subtree + set newPivot's parent to `oldPath[size-2].id()` +- Skip step 3 entirely + +Total cost: O(oldPivotSize + newPivotSize). Independent of N AND of tree shape. + +**Bench result (`PathDBench`, same flat-tree shape):** + +| Tree size | productionStyle (μs) | stableIdNoOpt (μs) | pathD (μs) | pathD speedup vs production | +|----------:|---------------------:|-------------------:|-----------:|----------------------------:| +| 100 | 0.304 | 0.286 | 0.023 | 13.2× | +| 1000 | 2.697 | 2.843 | 0.028 | **96.3×** | +| 10000 | 24.762 | 23.298 | 0.041 | **604×** | + +**GREEN.** And the absolute time is **flat** across tree sizes (~25-40 ns) — proving the algorithm's cost is purely a function of splice size, not tree size. + +Microcount evidence: 1 `put` + 1 `remove` per `applyIncremental` for a single-node splice on a 1000-node flat tree, vs ~1002 ops for the original algorithm. The 500× microcount ratio tracks the wallclock speedup at the appropriate scale. + +## Critical insight — splicer and index migrate as a unit + +The bench's middle column (`stableIdNoOpt` — stable IDs from splicer, but original spec §2 `applyIncremental`) matches `productionStyle` within noise. Stable ancestor IDs alone don't help. The optimized `applyIncremental` that **trusts** the stability is what realizes the gain. + +This means Phase 1 must migrate splicer + index together, not piecemeal. A staging branch that flips the splicer to stable IDs but keeps the old `applyIncremental` would show no perf change — Phase 1 must commit to both at once. + +## Phase 1 production migration plan (Path D) + +**Total scope: ~3-5 days focused engineering.** Substantially smaller than original spec §6 estimate of 1 week, because: +- No `SpanIndex` needed (Path A is shelved) +- No CstNode interface change beyond adding `id()` accessor +- TreeSplicer change is **one line** (replace fresh ancestor record allocation with reused-id construction) +- `applyIncremental` simplifies (steps 1 and 3 deleted) + +### Sub-phases + +1. **1.2 — production `CstNode` ID field.** Add `long id` as the leading record component to all four variants (`Terminal`, `NonTerminal`, `Token`, `Error`). Override `equals`/`hashCode` to exclude id (per spec §7 R1). Migrate all pattern-match callers across `peglib-core`, `peglib-incremental`, `peglib-formatter`, `peglib-maven-plugin`, `peglib-playground` to include the new component. Hundreds of sites; mostly mechanical sweep. + +2. **1.3 — production `IdGenerator` plumbing.** Thread `IdGenerator` through `PegEngine` and `ParserGenerator`'s emission templates. Per-Session counter is the v0.5.0 default per spec §8 Q1. + +3. **1.4 — `TreeSplicer.spliceAndShift` reuses ancestor IDs.** One-line behavioral change. Add a parity test asserting `oldPath[i].id() == newPath[i].id()` for ancestors. + +4. **1.5 — `NodeIndex` switches from `IdentityHashMap` to `LongLongMap`.** Port the experimental package's `LinearProbingLongLongMap` to production. NodeIndex's API stays put externally. + +5. **1.6 — `IncrementalSession.applyIncremental` adopts Path D's optimized algorithm.** Steps 1 and 3 deleted (per Path D's analysis). Microcount instrumentation in tests as a hard regression net. + +6. **1.7 — verify parity + bench against 0.4.3 baseline on the 1900-LOC fixture.** All 897 tests + IncrementalParityTest 22×100 must stay green. JMH bench replicates the IncrementalSessionBench suite to confirm the speedup translates to real workloads (mid-buffer realistic edits, not just synthetic). + +### Risks specific to Path D + +- **TreeSplicer's `shiftAll` for right-of-edit subtrees still deep-copies.** Path D fixes the *NodeIndex* perf problem but doesn't address record allocation pressure for right-of-edit sibling subtrees. Phase 1 ships with this remaining; if profiling shows it's a tail-latency contributor, address as 0.5.x patch via path-A-style offset decoupling, but only if the bench shows it's worth it. The MidBufferBench Path A run (`phase1-spanindex-results.md`) is the existing data point — at 10k nodes path A was *slower* due to its own overheads. So decoupling for allocation-pressure reasons alone isn't a clear win. + +- **ID semantics shift.** Pre-0.5.0: nodes have JVM identity. Post-0.5.0: nodes have explicit `long id`, which is preserved across edits *unless the splicer says otherwise*. This is the right abstraction for incremental editing (LSP, IDE plugins want to track "is this still the same Block as before this keystroke?"), but downstream callers must understand the new semantic. Document loudly in the migration guide. + +- **Pattern-match call site sweep.** Adding `long id` to record variants breaks every `case Terminal(SourceSpan span, ...)` style match across the codebase. Mechanical but high-volume. Phase 1.2 is the bulk of the work; recommend doing it via a single `jbct-coder` pass with a clean baseline. + +## Files added during prove-out + +In `peglib-incremental/src/main/java/.../experimental/` (sandbox, all additive): + +- Path A (commit `8f844eb`, RED): `SpanIndex`, `OffsetDecoupledNode`, `OffsetDecoupledNodeIndex`, `OffsetDecoupledSplicer`, plus `LongLongMap.forEachEntry` extension. +- Path D (commit `8b27dd6`, GREEN): `StableIdSplicer`, `StableIdNodeIndex`. + +In test sources: corresponding test classes for both paths. + +In `peglib-incremental/src/jmh/java/.../bench/`: `MidBufferBench`, `MidBufferTreeBuilder` (Path A), `PathDBench` (Path D). + +Bench result MDs: +- `docs/bench-results/phase1-spanindex-results.md` (Path A) +- `docs/bench-results/path-d-results.md` (Path D) + +**Sandbox stays sandbox.** No production source touched. Existing 897-test suite unchanged. Sandbox tests: 190 (was 100 pre-Phase-0; +90 from Phase 0 + Path A + Path D combined). diff --git a/docs/incremental/SPEC.md b/docs/archive/SPEC-incremental-original.md similarity index 99% rename from docs/incremental/SPEC.md rename to docs/archive/SPEC-incremental-original.md index 81c4e59..95988f3 100644 --- a/docs/incremental/SPEC.md +++ b/docs/archive/SPEC-incremental-original.md @@ -1,5 +1,8 @@ # peglib-incremental — detailed spec +> **Status:** historical / archived 2026-05-08. Original 0.2.x-era proposal; shipped through 0.4.x; current architecture in ARCHITECTURE-0.5.0.md. + + **Target module:** `peglib-incremental` (new; sibling to `peglib`). **Depends on:** `peglib:0.2.x+` (compile-scope; read-only use of `Grammar`, `CstNode`, `Trivia`, `SourceSpan`, `PegParser`). **Status:** proposal / spec for an implementing agent. Not yet executed. diff --git a/docs/incremental/UNSAFE-GENERATOR-SPIKE.md b/docs/archive/UNSAFE-GENERATOR-SPIKE.md similarity index 99% rename from docs/incremental/UNSAFE-GENERATOR-SPIKE.md rename to docs/archive/UNSAFE-GENERATOR-SPIKE.md index aa22c2b..e9ccd3b 100644 --- a/docs/incremental/UNSAFE-GENERATOR-SPIKE.md +++ b/docs/archive/UNSAFE-GENERATOR-SPIKE.md @@ -1,5 +1,8 @@ # UNSAFE-GENERATOR-SPIKE — boolean-returning + parser-state generator emission +> **Status:** historical / archived 2026-05-08. Out-of-scope investigation. + + Branch: `release-0.4.2` · Status: design (Phase 1) · Author: investigation pass --- diff --git a/docs/incremental/V2.5-SPIKE.md b/docs/archive/V2.5-SPIKE.md similarity index 99% rename from docs/incremental/V2.5-SPIKE.md rename to docs/archive/V2.5-SPIKE.md index 2eec86e..4f60261 100644 --- a/docs/incremental/V2.5-SPIKE.md +++ b/docs/archive/V2.5-SPIKE.md @@ -1,5 +1,8 @@ # V2.5 Spike — GO/NO-GO +> **Status:** historical / archived 2026-05-08. Spike with NO-GO finding. + + **Decision: NO-GO.** v2.5 attacks a cost that does not dominate. The real lever is pivot-selection in `SessionImpl.findBoundaryCandidate`. Defer v2.5 indefinitely. ## Premise summary (SPEC §5.4) diff --git a/docs/bench-results/generator-profile-baseline.md b/docs/bench-results/generator-profile-baseline.md new file mode 100644 index 0000000..f0307dd --- /dev/null +++ b/docs/bench-results/generator-profile-baseline.md @@ -0,0 +1,135 @@ +# Generator (Throughput Engine) — Profiling Baseline + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (post-Lever-D, commit `1e524e2`) +**Bench:** `Java25ParseBenchmark.parse`, variant `phase1_allStructural` (current best gen-time config) +**Fixture:** `large/FactoryClassGenerator.java.txt` (1900 LOC, 101178 chars) +**JVM:** OpenJDK 25.0.2 (Apple Silicon, Homebrew) +**Tools:** JMH 1.37 + async-profiler 4.4 (`/opt/homebrew/lib/libasyncProfiler.dylib`) + +## Headline numbers + +| Metric | Value | +|---|---:| +| Mean parse time (avgt, 5 iter / 1 fork) | **76.2 ms** ± 20.2 (CI 99.9%) | +| Throughput | 13 ops/sec | +| **Allocation rate** | **1,884 MB/sec** | +| **Allocated per parse (norm)** | **150 MB/op** | +| GC count over bench | 164 across ~10 seconds → ~16/sec | +| GC time as fraction of bench | **26.6%** | + +**Allocation is the dominant lever.** 150 MB allocated to parse a 1900-LOC file = ~80 KB per source line = ~800 bytes per character. 26.6% of bench time spent in GC. Algorithmic / micro-optimization wins are a second-order concern compared to reducing allocation pressure. + +## Allocation profile (top sites by sample count, async-profiler `-e alloc`) + +| Samples | Class | +|---:|---| +| **3,190** | `pragmatica.lang.Option$Some` — Option-boxing (every `Option.option(x)` call) | +| 3,008 | `Object[]` — generic backing arrays (ArrayList, varargs, etc.) | +| **2,574** | `CstParseResult` — per-parse-attempt result record (allocated even when backtracked) | +| **2,367** | `SourceLocation` — position records (line, column, offset) | +| 2,191 | `byte[]` — string backing | +| 1,096 | `SourceSpan` — span records (lazily reconstructed from int triples since 0.4.3) | +| 1,027 | `Long` — boxing | +| 893 | `ArrayList` — children lists, scratch buffers | +| 868 | `String` — various | +| 622 | `HashMap.Node` — packrat cache entries | +| 587 | `CstNode.NonTerminal` — RETAINED CST nodes | +| 553 | `CstNode.Terminal` — RETAINED CST nodes | +| 174 | `HashMap.Node[]` — packrat cache backing | +| 111 | `Trivia.Whitespace` — trivia records | +| 75 | `CstNode.Token` — RETAINED CST nodes | + +**Reading this:** +- Top 4 allocators (Option$Some, Object[], CstParseResult, SourceLocation) account for >50% of allocation samples. None of these are user-visible CST output. +- Retained CST = 1,215 records (NonTerminal + Terminal + Token). Allocation profile suggests roughly 3-5× more CST-shaped intermediate records are allocated and discarded during speculative parsing. +- Packrat cache (HashMap.Node + HashMap.Node[]) = 796 samples — meaningful but not dominant. + +## CPU profile (top leaf frames, async-profiler `-e cpu`, application frames only) + +JIT compilation activity dominates raw samples (the parser is large and the JIT works hard during the 3-iter warmup). After filtering JVM/JIT internals: + +| Samples | Frame | +|---:|---| +| 41 | `java.util.HashMap.resize` — packrat cache resize during fill | +| 36 | `java.lang.String.indexOf` — terminator scanning (already an intrinsic) | +| 27 | `java.util.Arrays.copyOf` — backing array growth | +| 25 | `generated.parse_Stmt` — the most-sampled rule body | +| 11 | `generated.parse_Class.skipWhitespace` | +| 11 | `CstParseResult.success` — result construction | +| 11 | `StringUTF16.compress` — string construction inside the parser | +| 10 | `CstParseResult.` — result-record construction | +| 9 | `Option.some` — Option boxing | +| 9 | `HashMap.putVal` — packrat cache writes | + +The CPU picture is consistent with the allocation picture: hot code = result-record construction + Option boxing + packrat HashMap operations. Pure parsing CPU (parse_Stmt, skipWhitespace) is only a small fraction of the sampled time. + +## Architectural moves, ranked by data-driven payoff + +### Tier 1 — design-level, addresses dominant allocation + +**A. Eliminate `Option` boxing on the parse hot path** (~25-30% allocation reduction) +- Top allocator (3,190 Option$Some samples). Every per-attempt success/failure currently wraps in Option. +- Replace with sentinel-based primitive return: parse method returns `int` (matched-end-offset) or `-1` (no-match), with mutable thread-local scratch state for the actual result data. +- Compatible with CST output — only changes how parse-result-flow is plumbed internally. + +**B. Mutable parse state replacing per-attempt `CstParseResult` records** (~20-25% allocation reduction) +- 2nd-largest allocator (2,574 CstParseResult samples). Every parse attempt — successful or backtracked — allocates one of these. +- Generator emits a parse method that returns primitive end-offset; on success, mutates a thread-local state struct (current rule's children, trivia accumulator, etc.). On backtrack, the state is rolled back via a checkpoint mechanism. +- This is essentially the "unsafe-generator" idea from HANDOVER §6.4 — but resurrected with a clear single-purpose justification (throughput) and the data to back it. + +**C. Lazy CST construction** (~15-20% allocation reduction; CST output identical) +- Currently CstNode records are allocated as we parse; speculative-branch nodes get GC'd on backtrack. +- Change: parser emits a compact **trace** during parsing (16-byte records: rule-id, start, end, child-count). CST materialization is a post-parse pass over the committed trace. +- Backtracking cost shifts from "allocate-and-discard CstNode" to "rewind trace position pointer" — much cheaper. +- Trace-walking pass also opens a clean home for trivia attribution rework (currently embedded in PegEngine + emission templates). + +**D. Pack SourceLocation/SourceSpan into primitives** (~15-20% allocation reduction) +- 3,463 combined samples (SourceLocation + SourceSpan). Every `node.span().start()` accessor currently allocates a fresh SourceLocation. +- Store spans as `long[]` aligned by node-id (or as packed-long fields on CstNode records), eliminating the lazy reconstruction path. +- Already partially started in 0.4.3 (SourceSpan as int triples) — finish the job by removing SourceLocation as a record entirely; expose primitive-returning accessors. + +### Tier 2 — algorithmic, packrat-focused + +**E. Replace HashMap packrat with `long[][]` indexed by `(ruleId, pos)`** (~5-10% CPU + reduced HashMap allocation) +- 41 + 9 CPU samples on HashMap operations; 622 + 174 allocation samples on HashMap nodes/buckets. +- Direct array access; no hashing, no resize. Combined with selective memoization (only memoize rules with profile-confirmed re-entry rate > 1). +- Pre-emit packrat array sizes from grammar analysis: `(numRules × textLen)` long entries. + +### Tier 3 — algorithmic, dispatch-focused + +**F. First-set Choice dispatch** (10-30% CPU on Java-shaped grammars) +- Existing `choiceDispatch` flag in ParserConfig — investigate what it does already; may be a partial implementation. +- Static FIRST-set analysis at gen-time. For `a / b / c`, emit `switch (text.charAt(pos))` to skip alternatives whose FIRST excludes the current char. + +**G. DFA-based lexer for token rules** (2-3× on lex-heavy paths) +- Detect token rules at gen-time; compile to DFA; emit DFA scanner alongside parser. +- Bigger architectural lift (~2-3 weeks); evaluate after Tier 1 lands. + +## Recommended sequence + +The data points clearly to **allocation-reduction first**, not algorithmic micro-opts. Suggested order: + +1. **Investigate the existing `choiceDispatch` flag** — quick read of ParserConfig + ParserGenerator emission. If it's already first-set dispatch, F is partially done; if not, that's a near-term tactical win. +2. **Tier 1 combined**: A + B + C + D land as a coordinated pass. Together they target the top 4 allocators (Option$Some, CstParseResult, SourceLocation, Object[]). Goal: reduce 150 MB/op to <30 MB/op (5× reduction). +3. **Re-profile** after Tier 1 to confirm gains and identify next bottleneck. +4. **Tier 2 (E)** if profile shows packrat still dominant. +5. **Tier 3 (F, G)** based on remaining hot paths. + +## Caveats + +- **JIT warmup**: 3 warmup iterations may be insufficient; CPU profile shows JIT compilation activity dominating sample count. Production runs (not bench) will see fewer compile samples and proportionally more parse samples. +- **150 MB/op figure includes all allocations** — JIT compilation, JMH harness, GC structures. Application allocation rate is somewhat lower; the relative breakdown remains directionally accurate. +- **Sample-based profiling**: async-profiler samples; not exhaustive accounting. Top-N rankings are reliable; absolute byte counts in the table are sample counts, not bytes. +- **`phase1_allStructural` is already the best of 7 gen-time variants.** The "none" baseline (all gen-time flags off) would show worse numbers. Worth running for a comparison if we want to verify Phase 1 flags' contribution. + +## Files + +- Flame graphs (HTML, viewable in browser): + - `/tmp/peglib-profile/.../flame-cpu-{forward,reverse}.html` + - `/tmp/peglib-profile/.../flame-alloc-{forward,reverse}.html` +- Collapsed text profiles for analysis: + - `/tmp/peglib-profile-collapsed/.../collapsed-alloc.csv` (~67 MB) + - `/tmp/peglib-profile-cpu/.../collapsed-cpu.csv` + +These artifacts are **not committed** — `/tmp` only. Re-run via the commands at the top of this doc. diff --git a/docs/bench-results/path-d-results.md b/docs/bench-results/path-d-results.md new file mode 100644 index 0000000..83cdf79 --- /dev/null +++ b/docs/bench-results/path-d-results.md @@ -0,0 +1,89 @@ +# Path D — Stable-ID Ancestor Preservation Bench Results + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (post-Phase-1.0/1.1, sandbox additions only) +**JMH version:** 1.37 +**JVM:** OpenJDK 25.0.2 (Apple Silicon, Homebrew) + +## Configuration + +- **Bench class:** `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/PathDBench.java` +- **Tree shape:** identical to `MidBufferBench` — flat single-level `Root[child_0 … child_{N-1}]` (long Java method body, N statement nodes). +- **Edit:** replace child at index N/2 with a fresh single-Terminal pivot. (Span-decoupled; this bench measures *applyIncremental* cost only — splice work is pre-computed in `@Setup(Level.Trial)`.) +- **Mode:** `AverageTime`, microseconds/op +- **Run flags (reduced for spike):** `-i 3 -wi 2 -f 1` — class declares `(iterations=5, warmup=3, fork=2)` for full runs; the spike uses reduced flags to keep total bench time under ~80 seconds. +- **Per-invocation setup:** all three arms' indices are rebuilt via `IdNodeIndex.build(oldRoot)` / `StableIdNodeIndex.build(oldRoot)` on every `Level.Invocation` because `applyIncremental` mutates the receiver's parents map. Build cost is identical across arms; the bench measures the differential cost of `applyIncremental`. + +## Three arms compared + +| Arm | Splicer | applyIncremental | +|-----|---------|-------------------| +| **productionStyle** | `IdTreeSplicer` (fresh ancestor IDs) | `IdNodeIndex.applyIncremental` (full algorithm: oldPath removal + sibling rewire) | +| **stableIdNoOpt** | `StableIdSplicer` (stable ancestor IDs) | `IdNodeIndex.applyIncremental` (unchanged — measures whether splicer alone helps) | +| **pathD** | `StableIdSplicer` (stable ancestor IDs) | `StableIdNodeIndex.applyIncremental` (skips ancestor removal + sibling rewire) | + +The three-way comparison isolates which change matters: the splicer's ID strategy alone, or the index's optimized walk, or the combination. + +## Measurements + +| Tree size | productionStyle (μs) | stableIdNoOpt (μs) | pathD (μs) | pathD speedup vs production | +|----------:|---------------------:|-------------------:|-----------:|----------------------------:| +| 100 | 0.304 ± 0.019 | 0.286 ± 0.025 | 0.023 ± 0.004 | **13.2×** | +| 1000 | 2.697 ± 0.277 | 2.843 ± 1.112 | 0.028 ± 0.009 | **96.3×** | +| 10000 | 24.762 ± 3.278 | 23.298 ± 6.686 | 0.041 ± 0.176 | **604×** | + +Total bench wallclock: 73 seconds. + +## Perf gate + +**Criterion:** pathD ≥ 5× faster than productionStyle at tree size ≥ 1000. + +**Result: GREEN.** At 1000 nodes the speedup is 96.3× (~19× above the 5× gate). At 10000 nodes the speedup is 604× — and `pathD`'s absolute time (~0.04 μs) is *flat* relative to tree size, exactly as the O(δ) algorithm predicts. `productionStyle` scales linearly with N (0.304 → 2.697 → 24.762 μs at N = 100/1000/10000, ~9× per decade), confirming the original O(N) bottleneck identified in the Phase 1.0/1.1 RED report. + +## Microcount evidence + +From `StableIdNodeIndexTest.microcount_o_delta` on a flat 1000-node tree, single-Terminal-pivot splice: + +``` +[Path D] flat-tree(N=1000) incremental microcount: puts=1 removes=1 (vs full-rebuild N=1001) +``` + +- `parents.put` calls: **1** (wire newPivot → stable root id) +- `parents.remove` calls: **1** (kill oldPivot's up-pointer) +- Total map operations: **2** +- For comparison, `IdNodeIndex.applyIncremental` (Phase 0c) on the same shape would perform `~N` puts (one per direct child of the rebuilt root in step 3) plus `~oldPath.size()` + `oldPivotSize` removes. For N=1000 that's roughly `1000 + 2 = 1002` map operations — **501× more work** for the same logical edit. + +The 96-604× wall-clock speedup is consistent with that microcount ratio; the small headroom factor is HotSpot/TLAB overhead amortizing the constant-time work into measurement noise. + +## Why `stableIdNoOpt` matches `productionStyle` + +The middle arm (stable IDs but unoptimized `applyIncremental`) clocks identical numbers to `productionStyle` (within noise: 0.286/2.843/23.298 vs 0.304/2.697/24.762 μs). This is the key isolation result: **the splicer change alone doesn't pay off**. `IdNodeIndex.applyIncremental`'s step 3 walks every direct child of the new root regardless of whether ancestor IDs are stable. The win comes from *combining* stable IDs with an `applyIncremental` that knows it can skip step 3 entirely — that is, from the `StableIdNodeIndex` algorithm. + +The corollary: a real Path D promotion must ship both pieces together. Shipping just the splicer (without `StableIdNodeIndex`) is no improvement; shipping just `StableIdNodeIndex` against an `IdTreeSplicer`-built tree is incorrect (siblings would point to the dead old ancestor IDs). The two are coupled and must migrate as a unit. + +## Caveats + +- **Bench measures `applyIncremental` only.** Splice cost (`StableIdSplicer.splice`) is pre-computed in `@Setup(Level.Trial)` and *not* part of the per-invocation timing. This is appropriate for the perf-gate question (`applyIncremental` was the identified bottleneck), but the end-to-end edit cost will be `splice + applyIncremental`. On the flat-tree shape the splicer cost is `O(depth × branching)` ≈ a few records, dwarfed by even the optimized `applyIncremental`. +- **Bench tree is single-level flat.** Real source trees are nested. In a deeper shape, the original `IdNodeIndex.applyIncremental` step 3 cost is `O(depth × branching)` — much smaller than the flat-tree O(N). Path D's win shrinks correspondingly: on a balanced 4-ary depth-7 tree (≈16k nodes, depth ≈ 7), step 3 walks ~28 children, so the absolute speedup is bounded by ~28×, not 600×. The flat shape is the worst case for the original algorithm and the best case for Path D. Real-world wins will land between the flat-shape and balanced-tree extremes. +- **`pathD@10000` error band is wide** (±0.176 μs on 0.041 μs mean). At ~40 ns/op the measurement is at the edge of JMH's resolution; the bench is dominated by setup/teardown noise. The *qualitative* result — flat scaling, ≥100× faster than `productionStyle` — is robust. +- **Mutate-in-place semantics still in effect.** `StableIdNodeIndex.applyIncremental` mutates the receiver's parents map in place; the receiver is invalid after the call. Production v0.5.0 will need a persistent map for snapshot/rollback (out of scope for this spike). +- **Single edit measured.** A long edit session would amortize `StableIdNodeIndex.build` cost over many `applyIncremental` calls — the per-edit advantage of Path D compounds across an editing session. + +## Verdict + +**GO** — Path D is the architecture for Phase 1.2+. The flat-tree perf gate is met by ~19× headroom at N=1000 and ~120× at N=10000, the microcount confirms genuine O(δ) behaviour (2 map ops on a 1001-node tree), and the migration scope is small: two new sandbox classes (`StableIdSplicer`, `StableIdNodeIndex`), each a near-clone of an existing class with one localized change. + +### ID-semantics judgement + +The semantic shift is real and worth naming: under `IdTreeSplicer` an `id` was equivalent to JVM record identity (one record, one id); under `StableIdSplicer` an `id` is logical identity preserved across structural rebuilds at the splicer's discretion. Two distinct `IdCstNode` records may share an id across edit generations. + +For the use case this design serves — IDE-style incremental reparse, where the question "is this still the same node as before?" is *exactly* the question we want a stable id to answer — this is a clean architectural fit, not an awkward concession. The stable id IS the logical-identity primitive; treating it as JVM identity was the accidental simplification, and Phase 0c's `IdNodeIndex` was paying linear cost per edit specifically because it didn't trust the id beyond JVM identity. The honest framing for the v0.5.0 contract is: **id = logical node identity, preserved across splices unless the splicer says otherwise** — which is what every downstream caller (LSP server, IDE refactoring engine, language server diagnostics) actually wants. + +## Files + +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/StableIdSplicer.java` +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/StableIdNodeIndex.java` +- `peglib-incremental/src/test/java/org/pragmatica/peg/incremental/experimental/StableIdSplicerTest.java` +- `peglib-incremental/src/test/java/org/pragmatica/peg/incremental/experimental/StableIdNodeIndexTest.java` +- `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/PathDBench.java` +- `peglib-incremental/target/path-d.json` — raw JMH output (not committed) diff --git a/docs/bench-results/phase0-spike-results.md b/docs/bench-results/phase0-spike-results.md new file mode 100644 index 0000000..b300ae9 --- /dev/null +++ b/docs/bench-results/phase0-spike-results.md @@ -0,0 +1,47 @@ +# Phase 0 Spike Bench Results + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (post-Phase-0d.1, commit 9b55253) +**JMH version:** 1.37 +**JVM:** OpenJDK 25.0.2 (Apple Silicon, Homebrew) + +## Configuration + +- **Bench class:** `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/Phase0SpikeBench.java` +- **Tree shape:** synthetic perfect 4-ary tree built by `SyntheticTreeBuilder` +- **Splice point:** depth 3 (representative interior pivot) +- **Mode:** `AverageTime`, microseconds/op +- **Run flags (reduced for spike):** `-i 3 -wi 2 -f 1` — class declares `(iterations=5, warmup=3, fork=2)` for full runs; the spike uses reduced flags to keep total bench time under one minute. Run signal is unambiguous so reduced iterations suffice for GO/NO-GO. +- **Per-invocation setup:** `oldIndex` rebuilt via `IdNodeIndex.build(oldRoot)` per `Level.Invocation` because `applyIncremental` mutates the receiver. JMH `Level.Invocation` overhead (~tens of ns) is negligible vs μs-range measurements. + +## Measurements + +| Tree size | fullRebuild (μs) | incrementalUpdate (μs) | Speedup | +|-----------|-----------------:|-----------------------:|--------:| +| 100 | 2.678 ± 0.389 | 0.069 ± 0.001 | 38.8× | +| 1000 | 9.611 ± 0.312 | 0.203 ± 0.011 | 47.3× | +| 10000 | 176.468 ± 16.561 | 2.627 ± 0.237 | 67.2× | + +Speedup grows with tree size, exactly as the O(δ) algorithm predicts: incremental cost grows only logarithmically (with depth), full-rebuild cost grows linearly with N. + +Total bench wallclock: 49 seconds. + +## Perf gate + +**Criterion:** incrementalUpdate ≥5× faster than fullRebuild at representative tree sizes (≥1000 nodes). + +**Result: GREEN.** 47× at 1000 nodes, 67× at 10000 nodes — both well above the 5× threshold and consistent with the spec §2 algorithmic projection ("typically 100-300 operations vs the 91,000 we walk today, ~300× per-edit reduction"). + +## Caveats + +- **Synthetic balanced tree, branching factor 4.** Real CST topologies are irregular (e.g., Java method bodies have unbalanced fanout). Phase 1 will re-run on the production 1900-LOC fixture for the apples-to-apples comparison against 0.4.3's `NodeIndex.build`. +- **`applyIncremental` mutates the receiver.** Bench accounts for this via `Level.Invocation` setup that rebuilds `oldIndex` from `oldRoot`. Production 0.5.0 will need either copy-on-write (with `LongLongMap.copy()`, ~O(N) cost) or a persistent map (out of scope for spike). The choice does not affect this gate — even if we paid full-rebuild cost on every applyIncremental call, we'd be at break-even, not a regression. +- **`LongLongMap` is the hand-rolled linear-probing impl from Phase 0a.** Future Funnel-hashing swap (per spec §8 Q2) untested. +- **Numbers reflect a depth-3 splice with a small pivot subtree (single Terminal).** Worst-case (deep splice with large pivot, e.g., a class body rewrite) not in this bench. Spec §2 establishes the cost is `O(splicedSize + depth × branching)` so scaling is predictable. +- **JMH Compiler Blackholes are in use** (auto-detected on JDK 25). Numbers are stable across the 3 measurement iterations and within tight CI bands, so this is informational rather than a confidence concern. + +## Files + +- `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/Phase0SpikeBench.java` — bench class +- `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/SyntheticTreeBuilder.java` — tree synthesizer +- `peglib-incremental/target/phase0-spike.json` — raw JMH output (not committed) diff --git a/docs/bench-results/phase1-spanindex-results.md b/docs/bench-results/phase1-spanindex-results.md new file mode 100644 index 0000000..11f3250 --- /dev/null +++ b/docs/bench-results/phase1-spanindex-results.md @@ -0,0 +1,106 @@ +# Phase 1.0/1.1 SpanIndex (Path A) Bench Results + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (post-Phase-0d.2, sandbox additions only) +**JMH version:** 1.37 +**JVM:** OpenJDK 25.0.2 (Apple Silicon, Homebrew) + +## Configuration + +- **Bench class:** `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/MidBufferBench.java` +- **Tree shape:** flat single-level — `Root[child_0 … child_{N-1}]` — mimicking a long Java method body of N statement nodes (vs Phase 0's balanced 4-ary tree). +- **Edit:** replace child at index N/2 with a fresh terminal of width 13, original width 10 → delta = +3, editEnd = (N/2)·10 + 10. About half the children sit right of the edit and need their offsets shifted. +- **Mode:** `AverageTime`, microseconds/op +- **Run flags (reduced for spike):** `-i 3 -wi 2 -f 1` — class declares `(iterations=5, warmup=3, fork=2)` for full runs; the spike uses reduced flags to keep total bench time under one minute. +- **Per-invocation setup:** `idOldIndex` rebuilt via `IdNodeIndex.build(idRoot)` and `odOldIndex` via `OffsetDecoupledNodeIndex.build(odRoot)` on every `Level.Invocation`, because `applyIncremental` mutates the receiver's parent map. + +## Two arms compared + +| Arm | What it does | +|-----|-------------| +| **productionStyle** | For every right-of-edit sibling, deep-copy the record via `shiftAll`-equivalent (rebuilds `IdCstNode.Terminal` with new `SourceSpan`). Then `IdNodeIndex.applyIncremental`. Mirrors the production `TreeSplicer.spliceAndShift` behaviour for siblings whose offsets must move. | +| **offsetDecoupled** | `OffsetDecoupledSplicer.splice` — copies `SpanIndex` and shifts all entries with `start ≥ editEnd` via a single `LongLongMap.forEachEntry` walk over a primitive `long[]`. Sibling records are reference-shared. Then `OffsetDecoupledNodeIndex.applyIncremental`. | + +## Measurements + +| Tree size | productionStyle (μs) | offsetDecoupled (μs) | Speedup | +|-----------|---------------------:|---------------------:|--------:| +| 100 | 1.272 ± 0.413 | 1.153 ± 0.219 | 1.10× | +| 1000 | 12.524 ± 9.498 | 9.710 ± 0.877 | 1.29× | +| 10000 | 81.656 ± 6.085 | 102.620 ± 31.160 | 0.80× | + +Total bench wallclock: 49 seconds. + +## Perf gate + +**Criterion:** offsetDecoupled ≥ 5× faster than productionStyle at representative tree sizes (≥ 1000 nodes). + +**Result: RED.** At 1000 nodes the speedup is 1.29× (well below 5×). At 10000 nodes path A is *slower* than productionStyle (0.80×). Path A does not pay off on this shape. + +## Why path A under-delivered + +The bench measures `splice + applyIncremental` end-to-end. The decomposition is: + +| Component | productionStyle | offsetDecoupled | +|-----------|-----------------|-----------------| +| Splice work | `O(N/2)` record allocations + `SourceSpan` rewrites for right-of-edit siblings | `O(N)` `SpanIndex.copy` + `O(N)` shift walk over a primitive `long[]` | +| `applyIncremental` step 3 | `O(N)` parent-link rewires for ALL N direct children of the rebuilt root | `O(N)` parent-link rewires for ALL N direct children | + +For the flat single-level shape, `applyIncremental` step 3 walks every direct child of the new root regardless of arm — that's `O(N)` `LongLongMap.put` operations on both sides. This common-cost term dominates at N = 10000: + +- offsetDecoupled total ≈ `SpanIndex.copy` (10000 entries) + `SpanIndex.shift` walk (10000 entries) + `applyIncremental` step 3 (10000 puts) ≈ `3 × O(N)` work. +- productionStyle total ≈ `N/2` record rebuilds + `applyIncremental` step 3 (10000 puts) ≈ `O(N)` heavyweight + `O(N)` lightweight. + +Path A swaps "rebuild N/2 records" for "two extra full walks over N primitive entries". On the flat shape, the constant factor of three light walks ends up *equal to or larger than* one heavy half-walk. Record allocation in HotSpot's TLAB is fast for short-lived objects; the L1-resident primitive arrays in `LongLongMap` end up similarly cheap per-element. The differential is in the noise. + +The Phase 0 bench saw 67× because the depth-3 splice in the balanced 4-ary tree had only ~16 right-of-edit nodes (1/4 fan-out at each level above the splice), so `applyIncremental` step 3 did `~depth × branching` ≈ 12 puts, not N. The splice cost was a tiny fraction of total. Phase 1's flat shape inverts that — `applyIncremental` itself is `O(N)` and dominates. + +## Architectural insight (affects Phase 1.2+ migration if pursued) + +Two compounding facts sink the path-A claim on the realistic shape: + +1. **`SpanIndex.copy` cost.** The eager-copy snapshot semantics (mirroring what production v0.5.0 will need) is itself `O(N)`. Even before any shift, we've spent the time it would take to rebuild N/2 records. +2. **`applyIncremental` step 3 is `O(direct_children_of_rebuilt_root)`.** On a flat method body the rebuilt root *is* the body, so step 3 alone is `O(N)`. Path A doesn't help with that step at all — sibling records preserved or not, the parent-link rewires happen for every direct child of every newly-allocated ancestor. + +To make path A pay off, both would need to change: + +- Snapshot via path-copying / persistent map instead of full copy. Out of scope (spec §8 future work). +- `applyIncremental` step 3 must avoid rewiring siblings whose parent-id is *unchanged*. Today the rebuilt ancestor has a fresh id, forcing all-children rewire. A "stable-id ancestor rebuild" — keep ancestor id fixed when only one child changed and the ancestor's identity is otherwise structural — would limit step 3 to `O(depth)`. That's a separate architectural change; path A alone doesn't unlock it. + +## Right-of-edit identity invariant — confirmed? + +**Yes** — `OffsetDecoupledSplicerTest.rightOfEditSiblingsPreserveIdentity` passes. After splice, `newRoot.children().get(2) == C` (the right-of-edit sibling) holds by reference equality, AND `newSpans.startOffset(C.id()) == oldStart + delta`. This is the central correctness claim of path A and it is met. + +That the perf gate is RED *despite* the invariant being met is the architectural insight: identity preservation on the record side does not translate to per-edit speedup when other O(N) costs (SpanIndex copy, applyIncremental step 3 on the rebuilt root) dominate. + +## Caveats + +- **Numbers are noisy at 10000.** stdev on `productionStyle@10000` is 6 μs on an 81 μs mean (acceptable); on `offsetDecoupled@10000` it's 31 μs on a 102 μs mean (high — likely TLAB/GC effects from the `SpanIndex.copy` allocation pattern). A full `-i 5 -wi 3 -f 2` run would tighten bands but not change the verdict — even taking the lower bound of `offsetDecoupled` (~71 μs) and upper bound of `productionStyle` (~88 μs), the speedup is ≤ 1.24× at best, far below the 5× gate. +- **Bench tree is single-level flat.** Real source isn't this extreme; method bodies are nested. A future "moderately deep" shape (say, `class > method > 50 statements > expression-tree-of-depth-5`) would see less of the step-3 dominance and might tilt closer to favourable. But such a shape would *also* trigger the production `TreeSplicer.spliceAndShift` ancestor-rebuild path, which is `O(depth)` and small. The flat shape was chosen specifically to expose the right-of-edit deep-copy cost; if path A doesn't win there, it doesn't win anywhere meaningful. +- **`SpanIndex` uses packed-long encoding via `LinearProbingLongLongMap`** (resolution of the choice in the brief). One additional `forEachEntry` API was added to `LongLongMap` — additive, no test regression. +- **Trivia spans untouched.** Production `Trivia` still carries `SourceSpan`; for the prove-out, all trivia lists are empty. A real path-A migration would need a parallel `TriviaIndex`, doubling the snapshot-copy cost. + +## Verdict + +**NO-GO** for path A as a complete strategy. The bench validates the user's concern in the brief: the Phase 0 67× was specific to the balanced-tree depth-3 splice; on the realistic flat shape, the right-of-edit identity preservation that path A buys is not worth the `SpanIndex.copy` snapshot cost it pays. + +Recommended next steps (in order of attractiveness): + +1. **Path C (accept partial win).** Keep the production `TreeSplicer` as-is. The Phase 0c `IdNodeIndex` already gives O(δ) on the tree shapes Phase 0's bench targeted (depth-3 splices in deep trees); ship that without committing to span decoupling. Real fixtures (1900-LOC Java) are deeper than the Phase 1 flat shape, so the Phase 0 win partially translates. +2. **Path B (lazy shift).** Track shift deltas as offset annotations on the splice path and resolve lazily on read. Avoids the `SpanIndex.copy` cost. More complex; the design is a separate spike. +3. **Stable-id ancestor preservation.** Independent of path-A vs B. If the ancestor on the splice path can keep its old id (only its `children` changed; rule/trivia identical), `applyIncremental` step 3 reduces to `O(depth)`. This is the highest-leverage change visible from this bench. + +## Files + +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/SpanIndex.java` +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/OffsetDecoupledNode.java` +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/OffsetDecoupledSplicer.java` +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/OffsetDecoupledNodeIndex.java` +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/LongLongMap.java` (additive: `forEachEntry`) +- `peglib-incremental/src/main/java/org/pragmatica/peg/incremental/experimental/LinearProbingLongLongMap.java` (impl of `forEachEntry`) +- `peglib-incremental/src/test/java/org/pragmatica/peg/incremental/experimental/SpanIndexTest.java` +- `peglib-incremental/src/test/java/org/pragmatica/peg/incremental/experimental/OffsetDecoupledNodeTest.java` +- `peglib-incremental/src/test/java/org/pragmatica/peg/incremental/experimental/OffsetDecoupledSplicerTest.java` +- `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/MidBufferTreeBuilder.java` +- `peglib-incremental/src/jmh/java/org/pragmatica/peg/incremental/bench/MidBufferBench.java` +- `peglib-incremental/target/phase1-spanindex.json` — raw JMH output (not committed) diff --git a/docs/bench-results/throughput-tier1-results.md b/docs/bench-results/throughput-tier1-results.md new file mode 100644 index 0000000..97d89f4 --- /dev/null +++ b/docs/bench-results/throughput-tier1-results.md @@ -0,0 +1,99 @@ +# Throughput Engine — Tier 1 Results + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (commits `0ed2dcd` through `9e9414a`) +**Baseline:** [`docs/bench-results/generator-profile-baseline.md`](generator-profile-baseline.md) +**Spec:** [`docs/incremental/THROUGHPUT-ENGINE-TIER1.md`](../incremental/THROUGHPUT-ENGINE-TIER1.md) +**Bench:** `Java25ParseBenchmark.parse`, 5 iter / 3 warmup / 1 fork, JDK 25, async-profiler 4.4 + +## Headline numbers (1900-LOC Java fixture, Regime: full reparse) + +| Pass | Wallclock (ms/op) | Bytes/op | vs Original | +|---|---:|---:|:---:| +| Original baseline (post-Lever-D) | 76.2 | 150 MB | — | +| **+A** spike + coverage extension | **66.4** | **122.9 MB** | **-12.9% / -18%** | +| +D production sweep | 65.2 | 123.1 MB | -14.4% / -18% | +| +D emission templates (`inlineLocations` default-on) | 64.1 | 118.3 MB | -15.9% / -21% | +| ~~+E packrat int-keyed~~ — **REVERTED** (regressed self-host stress test by +22%) | — | — | — | +| **Current state** (post-revert = post-D) | **~64** | **~118 MB** | **-16% / -21%** | + +The headline win is **A**: -90.5% Option$Some allocation samples, -12.9% wallclock alone. D added an incremental allocation reduction but the wallclock signal stayed within CI noise. **E was reverted** after self-host stress testing exposed a 22% regression masked by the small reference fixture (see "E reverted" below). + +## What landed + +### A — Sentinel-based ParseResult flow (mutable, no Option boxing) + +Two passes: +1. **A spike** (`0ed2dcd`): added opt-in `mutableParseResult` ParserConfig flag. Emits a mutable `CstParseResult` class with raw nullable fields instead of `Option value` / `Option expected`. Hot-path emission templates use `result.value != null` instead of `.isPresent()`. +2. **A coverage extension** (`fedc389`): applied the same pattern to the remaining residual emission paths (Capture, BackReference, Cut, And, Not, error-recovery infrastructure: `furthestExpected`, `pendingFailureRecoveryOverride`). + +Result: Option$Some samples 6,088 → 0 (-100%). Bytes/op 150 MB → 123 MB (-18%). Wallclock 76.2 → 66.4 ms (-12.9%). + +### D — Packed SourceLocation/SourceSpan emission + +Two passes: +1. **Production sweep** (`478b89b`): replaced `.span().start().X()` patterns across 7 files (PegEngine, Diagnostic, FormatContext, Formatter, TreeSplicer, TriviaRedistribution + ParserGenerator error paths) with primitive accessors. Bench-neutral (production code paths aren't on the throughput bench's hot loop), but valuable cleanup (-63 net LOC). +2. **Emission templates** (`5b2b6a1`): completed the `inlineLocations` flag implementation in ParserGenerator and flipped its default to `true`. Emitted CST construction now uses `new SourceSpan(line, col, off, endLine, endCol, endOff)` directly instead of `SourceSpan.sourceSpan(new SourceLocation(...), new SourceLocation(...))`. + +Result: SourceLocation samples 2,367 → 1,962 (-17%); SourceSpan samples 1,096 → 1,391 (sample noise — bytes/op confirmed -2-3%). Wallclock essentially unchanged. + +## E reverted (E regressed self-host stress test) + +**Status:** E (`2ad2674`) was reverted in `9e9414a` after the self-host stress test exposed a regression that the small reference fixture missed. + +**What E was:** an emitted `IntCstParseResultMap` class (linear-probing, `int[]` keys + `CstParseResult[]` values, no Integer autoboxing). Replaced `HashMap` per-rule packrat caches. + +**A/B numbers** (variant `phase1_allStructural_mutableResult`, self-host fixture — parser parsing its own generated source, ~37k lines / ~1.8 MB): + +| Metric | pre-E (post-D) | post-E (with IntCstParseResultMap) | Delta | +|---|---:|---:|---:| +| Wallclock | 1700 ms | 2074 ms | **+22%** | +| Allocated | 2915 MB | 2741 MB | -6% | + +**Diagnosis:** `IntCstParseResultMap` likely has worse cache behavior than `HashMap` at the load factors the self-host fixture stresses. Linear probing with millions of insertions and high collision density turns into linear scans in `get`/`put`; `HashMap`'s tree-bin promotion bounds worst-case probe length. The small reference fixture's per-rule cache populations stay below the threshold where this matters, so the regression sat inside CI noise on the small bench while costing 22% on the large one. + +**Lesson:** the small bench was incomplete coverage. Pre-Tier-1 the parser couldn't even complete files this size ("previously such files caused OOM"); now it parses them in 1.7 sec / 2.9 GB allocated. That capability headroom let us add **self-host as a second JMH fixture** (`Java25ParseBenchmark` `fixture=selfhost`) — the parser parses its own freshly generated source code (~25× the work of `FactoryClassGenerator.java.txt`). Future structural changes are now A/B-tested on both fixtures before landing. + +## What didn't land + +### B — Mutable parse-state singleton + +The agent flagged real complexity (left-recursive seed-and-grow snapshotting, packrat cache aliasing, ~80 emission sites, cross-call read discipline) that can't be safely landed in a single autonomous pass. Per the agent's recommendation, B would need 4-6 incremental commits, each gated on parity tests. Deferred for incremental delivery in a future session. + +Target if landed: -50% CstParseResult allocations (3,123 → ~200), expected -10-15% additional wallclock. + +### C — Lazy CST construction (parse → trace → materialize) + +Multi-week refactor per spec. Targets the 3-5× ratio of speculative-and-discarded CstNode allocations. May incidentally unblock Lever B's trivia attribution problem. + +Target if landed: -50% CST allocations, -15-20% wallclock, opens trivia attribution rework. + +### F — First-set Choice dispatch + +The existing `choiceDispatch` flag was investigated and found to be already partially implemented per the bench's variant naming (`phase1_choiceDispatch`). The flag is on by DEFAULT in ParserConfig per the v0.4.x perf rework. Not a separate move — it's already shipping. + +### G — DFA-based lexer for token rules + +Biggest individual potential gain (2-3× on lex-heavy paths) but biggest architectural lift (~2-3 weeks). Not attempted. + +## Honest assessment + +We hit ~40% of the Tier 1 spec target on wallclock and ~26% on bytes/op (post-revert): + +| Metric | Baseline | Spec target | Actual (post-D, post-E-revert) | % of target | +|---|---:|---:|---:|---:| +| Wallclock | 76.2 ms | ≤ 50 ms (-34%) | 64.1 ms (-16%) | 47% of target | +| Bytes/op | 150 MB | ≤ 30 MB (-80%) | 118.3 MB (-21%) | 26% of target | + +The remaining gain requires either: +- B's mutable parse-state (incremental delivery, ~4-6 days) +- C's lazy CST construction (single architectural lift, ~1-2 weeks) +- DFA lexer (architectural, ~2-3 weeks) + +A's gain held; D was modest because most of D's targets are inside the emitted parser, not production code; E was reverted (self-host regression) — a re-attempt would need a packrat structure that doesn't degrade at large cache populations. + +## Recommendation + +Ship the current state as 0.5.0-alpha (it's a real ~16% throughput improvement + ~21% allocation reduction with byte-equal CST output, A+D landed, E reverted). Tackle B/C/DFA in dedicated multi-day sessions when the time is available — they require careful incremental work that doesn't fit single-pass autonomous delivery. Any re-attempt of E (or a different packrat structure) must A/B against both `fixture=reference` and `fixture=selfhost`. + +Cursor + parser optimization arc summary: -54% median + -50% p95 + 96.5% frame budget on the incremental engine (Lever A+D shipped); -16% throughput + -21% allocation on the throughput engine (Tier 1 partial, A+D shipped, E reverted). Both engines now have measurable improvements over 0.4.3 baselines with clear forward paths documented. diff --git a/docs/incremental/ARCHITECTURE-0.5.0.md b/docs/incremental/ARCHITECTURE-0.5.0.md index 5d2ad26..a2cb600 100644 --- a/docs/incremental/ARCHITECTURE-0.5.0.md +++ b/docs/incremental/ARCHITECTURE-0.5.0.md @@ -6,7 +6,7 @@ ## 200-word summary -Peglib's 0.4.x incremental parser is grafted on top of an architecture optimized for parse-once correctness. Every persistent perf challenge documented across `docs/incremental/V2.5-SPIKE.md`, the lever-1 puzzle in `HANDOVER.md` §6.2, the failed unsafe-generator spike, and the failed `NodeIndex.evolve` attempt is a manifestation of the same mismatch — the data structures fight incremental work. +Peglib's 0.4.x incremental parser is grafted on top of an architecture optimized for parse-once correctness. Every persistent perf challenge documented across `docs/archive/V2.5-SPIKE.md`, the lever-1 puzzle in `HANDOVER.md` §6.2, the failed unsafe-generator spike, and the failed `NodeIndex.evolve` attempt is a manifestation of the same mismatch — the data structures fight incremental work. This spec proposes an **incremental-native** redesign for v0.5.0, gathered into four levers: diff --git a/docs/incremental/PHASE-1-RESULTS.md b/docs/incremental/PHASE-1-RESULTS.md new file mode 100644 index 0000000..991df3f --- /dev/null +++ b/docs/incremental/PHASE-1-RESULTS.md @@ -0,0 +1,90 @@ +# Phase 1 — Production Migration Results + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (commits `2443779` through `65a719f`) +**Spec:** [`ARCHITECTURE-0.5.0.md`](ARCHITECTURE-0.5.0.md), [`PHASE-1-PROVE-OUT.md`](../archive/PHASE-1-PROVE-OUT.md), [`PHASE-0-RESULTS.md`](../archive/PHASE-0-RESULTS.md) + +## Verdict + +**Phase 1 production migration of Path D is complete.** All 993 tests green across 5 modules (699 core + 196 incremental + 66 formatter + 5 maven-plugin + 27 playground). IncrementalParityTest 22×100 stays green throughout. `IncrementalSessionBench` shows **median 1.9× faster than 0.4.3, p95 35% better, frame-budget hit rate +4.4pp**, validating the Path D architecture on the real 1900-LOC fixture. + +The win is in the typical case. Large-pivot edits (Block/RecordBody ≥ 2k nodes) regressed at p99/max — an acceptable trade we entered Phase 1 with eyes open, having shelved Path A's much-larger-scope offset decoupling at the prove-out gate. + +## Sub-phase summary + +| Sub-phase | Commit | Deliverable | Status | +|-----------|--------|-------------|:------:| +| 1.2 | `2443779` | Production `CstNode` gains `long id`; equals/hashCode override (spec §7 R1); 91 call sites swept across all 5 modules; new `IdGenerator` in `peglib-core/tree`; threaded through `ParsingContext`, `PegEngine`, `ParserGenerator` emission templates | ✅ | +| 1.4 | `2443779` (side-effect) | `TreeSplicer.rebuildNonTerminal` reuses `oldAncestor.id()` (line 102) — Path D's stable-id ancestor preservation lands as a side-effect of 1.2's mechanical sweep | ✅ | +| 1.5/1.6 | `39e11f9` | `LinearProbingLongLongMap` + `LongLongMap` promoted from sandbox to `internal/`; production `NodeIndex` switches to LongLongMap-keyed parents map; `IncrementalSession.applyIncremental` adopts Path D's optimized algorithm; `LongLongMap` tombstone-saturation fix (resize trigger now `size + tombstones > threshold`) | ✅ | +| 1.7 | `65a719f` | `NodeIndex.applyIncremental` Step 6: refresh `nodesById` for right-of-edit subtrees that `TreeSplicer.shiftAll` deep-copied (preserves stable IDs but replaces records with shifted spans); bench exception diagnostics added to `IncrementalSessionBench` | ✅ | + +## Bench results — IncrementalSessionBench, Regime B (cursor-moved-to-edit) + +The realistic editor regime: cursor moved to the edit offset before each edit. Same RNG seed (`0xBEEFCAFE`) and same 1000-edit sequence as 0.4.3. + +| Metric | 0.4.3 baseline (HANDOVER §5) | 0.5.0 post-1.7 | Change | +|---|---:|---:|---:| +| Median | 10.8 ms | **5.6 ms** | **-48% (1.9× faster)** ✅ | +| p95 | 22.4 ms | **14.6 ms** | **-35%** ✅ | +| p99 | 53.3 ms | 138.8 ms | **+160% regression** ⚠️ | +| Max | 98.6 ms | 390.8 ms | **+297% regression** ⚠️ | +| % under 16 ms (frame budget) | 91.5% | **95.9%** | **+4.4 percentage points** ✅ | +| Successful incremental edits (Regime B) | ~915 (implied) | 565 | see "Bench caveats" | + +### Per-class medians (Regime B) + +| Class | Count | Median | p95 | p99 | Max | +|---|---:|---:|---:|---:|---:| +| single-char | 392 | 5.6 | 11.3 | 138.8 | 390.8 | +| word | 150 | 5.5 | 14.6 | 266.0 | 280.5 | +| line | 18 | 5.8 | 136.2 | 136.2 | 136.2 | +| block | 5 | 5.7 | 87.4 | 87.4 | 87.4 | +| ALL | 565 | 5.6 | 14.6 | 138.8 | 390.8 | + +### Top outliers (Regime B) + +The p99/max regression localizes to large-pivot edits where the boundary algorithm chose a high-fanout interior pivot. `TreeSplicer.shiftAll` deep-copies all right-of-edit descendants (with fresh records but stable IDs); Step 6's `nodesById` refresh then walks them. Total cost is bounded by the right-of-edit subtree size: + +| Latency | Class | pivotRule | pivotNodes | +|---:|---|---|---:| +| 390.8 ms | single-char | Block | 7065 | +| 303.9 ms | single-char | Block | 2211 | +| 280.5 ms | word | RecordBody | 7652 | +| 266.0 ms | word | RecordBody | 7652 | +| 240.2 ms | single-char | RecordBody | 7652 | +| 138.8 ms | single-char | Block | 2185 | + +Pivot selection itself was not changed in Phase 1 — that's Lever B (Phase 2 of the spec). The same edits in Regime A (cursor-pinned) hit `CompilationUnit` (90k+ nodes) but cap at ~120 ms because the algorithm degenerates more cleanly there. The Regime B outliers above 100 ms are all cases where the boundary algorithm picked a non-trivially-sized interior pivot AND the right-of-edit deep-copy + nodesById refresh dominated. + +## Bench caveats + +1. **Edit-plan corruption tail.** 398 of 435 Regime B exceptions are the same `IllegalStateException: full parse failed: Unexpected 'D' at 119:110` from `SessionFactory.parseFull`. Once an early edit breaks the buffer at line 119, every subsequent edit's full-reparse fallback hits the same error. This is a pre-existing bench-edit-plan issue (the random-edit generator can produce sequences that corrupt Java syntax over time); not a Phase 1 regression. The 0.4.3 baseline numbers in HANDOVER §5 don't include comparable exception-count data, so the absolute success count of 565 vs 0.4.3's implied ~915 may be a Phase-1-introduced shift OR the same bench flaw with a different seed/path. Phase 2's bench harness should gate edits on parse validity before committing them. + +2. **Cursor-aware vs cursor-pinned asymmetry, 57 edits.** Regime A (cursor-pinned) reports 622 successes; Regime B reports 565. The 57-edit gap means a small set of edits succeed when the cursor stays at offset 0 but fail when the cursor is moved to the edit point. Most likely cause: the warm-pointer pivot search in `IncrementalSession.tryIncrementalReparse` selects a slightly different pivot in Regime B that occasionally falls outside the safe-pivot set. Defer to Phase 2's pivot-algorithm rework (Lever B). + +3. **JMH not used here.** `IncrementalSessionBench` is a single-pass standalone main, not a JMH-warmup-controlled bench. Numbers are JIT-stable but lack the multi-fork variance bound that JMH provides. `IncrementalParityTest` 22×100 is the correctness regression net. + +## Items that didn't ship in Phase 1 + +- **Lever B (top-down pivot search).** Per spec §3, this dissolves the lever-1 puzzle into a 30-line method. Phase 2 entry point. The 57-edit Regime-asymmetry above and the large-pivot p99 regression are the strongest current motivators — both suggest pivot selection is the next bottleneck. +- **Lever C (peglib-rt unification).** Per spec §4. Multi-day refactor extracting per-Expression parse helpers into a new `peglib-rt` module. Defer until Phase 2 lands and a new perf bench can baseline against the resulting cleaner architecture. +- **Lever D (Cursor split).** Per spec §5. Smaller change; can land alongside Phase 2 or as a 0.5.x patch. +- **Bench harness fixes.** Edit-plan validity gating + JMH-style multi-fork wrapper. Should land in Phase 2. + +## Test state + +| Module | Tests | Failures | Errors | Skipped | +|---|---:|---:|---:|---:| +| peglib-core | 699 | 0 | 0 | 0 | +| peglib-incremental | 196 | 0 | 0 | 0 | +| peglib-formatter | 66 | 0 | 0 | 0 | +| peglib-maven-plugin | 5 | 0 | 0 | 0 | +| peglib-playground | 27 | 0 | 0 | 0 | +| **Aggregate** | **993** | **0** | **0** | **0** | + +`IncrementalParityTest` 22×100 (re-enabled in 0.3.5, still passing): all green. `IncrementalTriviaParityTest` 22 cases: all green. + +## Recommendation + +Push `release-0.5.0` to remote as a public Phase 1 marker before any Phase 2 work begins. The Phase 1 commits represent a substantial architectural change (CstNode shape, NodeIndex internals, applyIncremental algorithm) that downstream consumers — and any future `git bisect` for regressions — should be able to reference. Tagging `v0.5.0-alpha.1` (or similar) is also reasonable; canonical release per HANDOVER §7.3 happens at the end of Phase 5 with the full 0.5.0 surface. diff --git a/docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md b/docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md new file mode 100644 index 0000000..7950e0e --- /dev/null +++ b/docs/incremental/THROUGHPUT-ENGINE-MOVE-B.md @@ -0,0 +1,337 @@ +# Throughput Engine — Move B Detailed Spec (Mutable Parse-State Singleton) + +**Date:** 2026-05-08 +**Branch starting point:** `release-0.5.0` at `fd278fe` (pushed; tag `v0.5.0-candidate`) +**Spec ancestor:** [`THROUGHPUT-ENGINE-TIER1.md`](THROUGHPUT-ENGINE-TIER1.md) §2 B +**Bench baseline (post all current optimizations):** +- Reference fixture (1900-LOC Java): **22.6 ms / 75.6 MB** per parse +- Self-host fixture (37k-LOC Java25 generated parser): **956 ms / 2.04 GB** per parse +- vs javac (parse-only): **2.5× of javac** wallclock with strictly more output + +--- + +## 1. What's left and why B is the last big lever + +Throughput engine arc to date (cumulative -70% reference wallclock, -50% bytes vs original 76.2 ms / 150 MB baseline): + +``` +A Option boxing eliminated (-13% wallclock, -18% bytes) +D SourceLocation/SourceSpan packed (-2% bytes) +D2 location() callers swept (-6% wallclock, -4% bytes) +F FIRST-set choice dispatch (-20% wallclock both fixtures) +G JBCT-style method splits (-10% both fixtures, parse_Stmt 27k → 3k bytes) +G2+H Sequence chunks + nested choices (-5% reference, -8% selfhost) +SP Selective packrat auto-detect (-38% reference, -14% selfhost) +DFA Identifier fast-path scanner (-10% reference, -8% selfhost) +``` + +**Reverted (bench-driven discipline):** +- E (int-keyed packrat) — regressed self-host 22% +- H2 (recursive nested-Choice per-alt) — added call overhead, regressed 4-7% +- DFA generalization beyond Identifier — neutral (low-volume rules) + +**Pattern that emerged:** high-volume single targets win big; broad generalizations don't pay back the call/dispatch overhead. **Move B targets the highest-remaining single allocator on every parse path** — the per-call `CstParseResult` record allocation. + +### B's target: 5,264 CstParseResult allocations per self-host parse + +Allocation profile of the current state shows `CstParseResult` is still the #2 allocator after `Object[]`. Every parse method's success or backtrack allocates one. After A made each record cheaper (raw nullable fields, no Option boxing), the per-allocation cost dropped — but the COUNT didn't. Each call-site still allocates. + +**B replaces per-call allocation with a single shared mutable instance.** Each `parse_` method mutates `this.result` and returns `boolean`. Callers consume the result fields immediately before making the next parse call. + +**Projected gains** (per Tier 1 spec §2 B): +- -50% to -90% reduction in CstParseResult allocations (target: 5,264 → ~200, where the residual is packrat cache snapshots only) +- -10-15% additional wallclock on top of current baseline +- Brings reference fixture into the **20 ms / 60 MB** range, **~2× of javac** territory + +--- + +## 2. Why the first attempt declined + +Agent who attempted B in a single autonomous pass surfaced four concrete blockers: + +1. **Left-recursive seed-and-grow** — the LR algorithm holds a `lastSeed: CstParseResult` across iterations of "grow." With a single shared mutable instance, the seed gets clobbered on the next inner parse call. Need an explicit per-LR-rule snapshot record. + +2. **Packrat cache aliasing** — cache stores `CstParseResult` instances by value. With a shared mutable singleton, a cache entry would alias the next parse's mutation. Need a `CacheEntry` record (immutable copy of singleton's fields) at cache-put time; reload on cache-hit. + +3. **~80 emission sites** — every `return CstParseResult.success(...)`, every `r.value.isPresent()`, every `r.expected.isPresent()` becomes singleton mutation + caller reads `this.result.X`. High-volume mechanical sweep with subtle aliasing risk per site. + +4. **Cross-call read discipline** — caller MUST consume result fields before any subsequent parse call (otherwise next call's mutation overwrites). Existing emission already follows this pattern in practice (parse + immediate read), but the discipline must be enforced and verified. + +The agent's quote: *"I'd rather report this honestly than produce broken code claiming green tests."* Smart pushback. The right delivery shape is multi-commit, parity-gated. + +--- + +## 3. The delivery plan: 4-6 incremental commits + +Each commit is independently green (`mvn -pl peglib-core test` passes Phase1ParityTest 22/22 + RoundTripTest). Bench A/B at end of each major commit, but the gate is parity, not perf — perf evaluation comes after the full sequence lands. + +### Commit 1 — Foundation (no behavior change) + +**Scope:** Emit infrastructure, leave existing static factory call sites untouched. + +- Add `private final CstParseResult result = new CstParseResult();` field to emitted parser class (under flag ON only — gate via existing `mutableParseResult` flag) +- Emit a new `CstParseResult` shape with mutator methods returning `boolean`: + ```java + boolean setSuccess(Object v, int endPos, int endLine, int endColumn) { ... return true; } + boolean setFailure(String expected) { ... return false; } + boolean setCutFailure(String expected) { ... this.cutFailed = true; return false; } + ``` + KEEP the existing static factories `CstParseResult.success(...)`, `.failure(...)`, etc. as `return new CstParseResult(...)` for backward compat during the migration. They'll be removed in commit 4-5 once all emission sites are converted. +- Emit `private record CacheEntry(...)` for packrat snapshots (matching CstParseResult's field set). +- Emit packrat cache put/hit converted to use CacheEntry: on success return AND cache miss, snapshot fields into a new `CacheEntry` and store; on cache hit, load CacheEntry's fields into `this.result.success/value/expected/...` and return `success`. + +**Gate:** all 922 tests green. Bench: no perf change expected (packrat path slightly different but most parse calls still allocate via static factories). + +### Commit 2 — Migrate `parse_` rule methods + +**Scope:** Convert top-level rule methods (the entry points produced from each grammar rule) to use the singleton. + +Each `parse_` method: +- Old signature: `private CstParseResult parse_() { ... return CstParseResult.success(...); }` +- New signature: `private boolean parse_() { ... return setSuccess(...); }` +- Body: replaces inner `var r = parse_();` pattern with `parse_();` (boolean return ignored; reads happen via `this.result.X`) + +Caller emission for inner-rule references at this stage: the helper `parse_()` returns boolean; caller reads `this.result.success` and `this.result.value`/`.expected`/etc. immediately before the next inner call. + +Match helpers (`matchLiteralCst`, `matchCharClassCst`, `matchAnyCst`, `matchDictionaryCst`) STAY allocating CstParseResult via static factory at this commit. They're converted in commit 3. + +**Gate:** all 922 tests green. Phase1ParityTest 22/22 (CST output byte-identical). Bench: starting to see allocation reduction; wallclock similar. + +### Commit 3 — Migrate match helpers + +**Scope:** Convert `matchLiteralCst`, `matchCharClassCst`, `matchAnyCst`, `matchDictionaryCst` to mutate the singleton. + +These are the hot inner parse helpers; converting them removes a substantial fraction of CstParseResult allocations. + +Pattern: `return CstParseResult.success(text, endPos, endLine, endColumn);` becomes `return setSuccess(text, endPos, endLine, endColumn);` (mutates `this.result` and returns true). + +**Gate:** parity tests + RoundTripTest. Bench: meaningful allocation drop; wallclock starts improving. + +### Commit 4 — Migrate Expression emission + +**Scope:** Convert emitted parsers for Sequence, Choice, ZeroOrMore, OneOrMore, Optional, Repetition expressions. + +These are emission templates that compose the inner-parse calls into the rule body. They construct `CstParseResult` for intermediate states. Convert to singleton mutation. + +This is the bulk of the ~80 emission sites count. High mechanical density but each individual change is small. + +**Gate:** parity + RoundTripTest. Bench: largest single drop in CstParseResult allocations (back to packrat-cache-snapshot count only). + +### Commit 5 — Migrate predicate + capture + cut + +**Scope:** Convert And, Not, Capture, BackReference, Cut, TokenBoundary emission. + +These are smaller volume but add up. Predicates (And/Not) need careful state restoration since they don't consume input on success. + +**Gate:** parity + RoundTripTest. Bench: should be close to final allocation profile. + +### Commit 6 — LR seed-and-grow snapshot + +**Scope:** Convert the left-recursive seed-and-grow wrapper to use a per-rule snapshot record. + +Currently `lastSeed: CstParseResult` is preserved across grow iterations. With singleton mutation, the seed gets clobbered. Solution: emit a small `LRSeed` record per LR rule that captures the singleton's fields between iterations: + +```java +private record LRSeed(boolean success, Object value, String expected, int endPos, int endLine, int endColumn, boolean cutFailed) {} + +// In LR rule: +LRSeed seed = LRSeed.from(this.result); // snapshot +while (...) { + parse_(); // mutates this.result + if (improved) seed = LRSeed.from(this.result); + else break; +} +LRSeed.restoreTo(this.result, seed); // restore for caller +``` + +Then remove the static factories `CstParseResult.success/failure/cutFailure` from the emitted class — all callers are converted. + +**Gate:** parity + RoundTripTest + `LeftRecursionTest` (specific LR coverage in peglib-core). + +--- + +## 4. Risks & mitigations + +### R1 — Aliasing across cross-call reads (HIGH likelihood, HIGH impact) + +The cross-call discipline ("consume `this.result.X` before next call") is correct in current emission BY ACCIDENT — emission templates happen to read result-fields immediately. After B, this becomes load-bearing. + +**Mitigation:** +- Phase1ParityTest is the regression net (CST output byte-equal to OFF-flag baseline). If aliasing breaks, parity diverges. 22/22 must stay green at every commit. +- Audit each emission site for "consume before next call" — annotate any non-trivial pattern. +- The `mutableParseResult` flag stays — comparison against OFF baseline is the safety net. + +### R2 — Packrat snapshot allocates per cache miss (MEDIUM likelihood, MEDIUM impact) + +Each cache miss followed by parse success allocates one `CacheEntry`. If cache miss rate is high, this re-introduces allocation we tried to eliminate. Selective packrat (already shipped) significantly reduces cache traffic, mitigating this. + +**Mitigation:** measure cache miss rate after commit 1; if high, may need to pool CacheEntry instances or pack into long-keyed primitive map. Monitor via post-B allocation profile. + +### R3 — LR seed-snapshot adds allocation (LOW likelihood, MEDIUM impact) + +LR rules typically iterate 2-3 times per call. Each iteration may snapshot the seed (one LRSeed allocation per snapshot). Negligible volume on typical grammars. + +**Mitigation:** measure on bench. If LR rules dominate allocation, restructure snapshot to use parser-instance fields rather than a record. + +### R4 — Multi-commit delivery breaks under concurrent edits (LOW likelihood, LOW impact) + +Each commit is on the release branch. If a parallel session lands changes during the multi-commit work, conflicts. + +**Mitigation:** branch is local-controlled; no parallel sessions expected. Each commit lands sequentially. + +--- + +## 5. Validation procedure per commit + +After each commit: + +1. `mvn install` — full reactor green (922 tests). +2. Special attention: `Phase1ParityTest`, `RoundTripTest`, `LeftRecursionTest`. +3. Bench (after commits 2, 4, 5, 6) BOTH fixtures: + ```bash + mvn -pl peglib-core -am -Pbench -DskipTests package + java -jar peglib-core/target/benchmarks.jar Java25ParseBenchmark.parse \ + -p variant=phase1_allStructural_mutableResult_autoSkipPackrat \ + -p fixture=reference,selfhost \ + -i 5 -wi 3 -f 1 -t 1 -prof gc 2>&1 | tail -30 + ``` + Compare to current baseline (reference 22.6 ms / 75.6 MB; selfhost 956 ms / 2.04 GB). Expect monotonic improvement after commits 3-4-5. +4. Allocation profile (after commit 5 or 6): + ```bash + java -jar peglib-core/target/benchmarks.jar Java25ParseBenchmark.parse \ + -p variant=phase1_allStructural_mutableResult_autoSkipPackrat -p fixture=selfhost \ + -i 3 -wi 2 -f 1 -t 1 \ + -prof "async:libPath=/opt/homebrew/lib/libasyncProfiler.dylib;output=collapsed;event=alloc;dir=/tmp/post-B-alloc" + ``` + Verify CstParseResult allocations dropped to expected level (packrat-CacheEntry residue only). +5. `mvn jbct:format` — keep CI happy. + +--- + +## 6. Reference numbers — current state + +These are the targets to beat after B lands cleanly: + +| Metric | Current (post-DFA-spike) | B target | +|---|---:|---:| +| Reference parse time | 22.6 ms | ≤ 20 ms | +| Reference allocation | 75.6 MB | ≤ 60 MB | +| Self-host parse time | 956 ms | ≤ 850 ms | +| Self-host allocation | 2.04 GB | ≤ 1.7 GB | +| CstParseResult samples (selfhost) | 5,264 | < 500 | +| vs javac (parse) | 2.5× | ~2× | + +If post-B metrics fall significantly short (e.g. wallclock improves by < 5%), the multi-pass investment may not be worth shipping. Honest evaluation gate at end of commit 6. + +--- + +## 7. Files involved + +**Primary:** `peglib-core/src/main/java/org/pragmatica/peg/generator/ParserGenerator.java` (5500+ lines, will gain ~300-500 net under flag-ON branch). + +**Secondary:** `peglib-core/src/main/java/org/pragmatica/peg/parser/ParserConfig.java` (no changes expected — flag already exists from A spike). + +**Bench:** `peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java` — variant `phase1_allStructural_mutableResult_autoSkipPackrat` is the standard A/B variant; no change needed. + +**Tests:** Phase1ParityTest, RoundTripTest, LeftRecursionTest, all parity tests are gates. Don't add B-specific tests; existing parity coverage IS the regression net. + +--- + +## 8. Resource constraints + practical session shape + +Multi-commit work in a single autonomous session: + +- Commits 1, 2, 3, 4, 5, 6 ideally land in this order over 2-3 days of focused work. +- Each agent pass: scope ONE commit; deliver and validate before moving to next. +- Use `jbct-coder` per project mandate. Keep each agent prompt tight (under ~400 words). +- Use `build-runner` for `mvn` invocations to keep main context clean. + +If a commit fails parity, **revert that single commit** and re-attempt with refined scope. Do NOT ship partial. + +--- + +## 9. Decision gates + +- **After commit 1:** if foundation breaks parity, abandon B entirely and write up findings. +- **After commit 3:** if no measurable allocation reduction by this point, the per-call CstParseResult was not the load-bearing alloc — abandon B, document the surprise. +- **After commit 6:** if total wallclock improvement < 5%, ship the work but clearly document that B was less impactful than projected. Could still be valuable for self-host scenarios where allocation pressure dominates. + +--- + +## 10. After B + +If B succeeds: +- Reference parse time at ~20 ms (~2.2× of javac with strictly more output). +- Self-host stress test in the ~850 ms range. +- Tier 1 spec target (≤ 30 MB/op, ≤ 50 ms/op on reference) substantially met. +- 0.5.0 release candidate. + +If B falls short: +- Document; ship 0.5.0 with current state (already excellent). +- Future moves: char-class bit-packing, generalized DFA (if better-targeted), Lever C IR unification (multi-week). + +--- + +## 11. Post-mortem — Move B attempted, abandoned 2026-05-08 + +Five commits landed (`88c15f3` … `ed95951`) before policy-driven rollback. Hard-reset to `v0.5.0-candidate` (`e849b63`) — branch state preserved as if Move B never happened. The 5 commits remain in reflog for ~30 days if forensic detail is needed. + +### Bench trajectory (reference fixture, `-i 5 -wi 3 -prof gc`) + +| Stage | Wallclock (ms/op) | Alloc (MB/op) | Δ wallclock vs orig | Δ alloc vs orig | +|---|---:|---:|---:|---:| +| Original baseline (`fd278fe`) | 22.6 | 75.6 | — | — | +| Commit 1 (foundation, no behavior change) | not benched | not benched | (expected ≈ 0) | (expected ≈ 0) | +| Commit 2 (parse_ → boolean) | not benched | not benched | unknown | unknown | +| Commit 3 (match helpers) | 23.97 | 72.1 | **+6.0%** | -4.6% | +| Commit 4 (combinators) | 24.71 | 66.3 | **+9.3%** | -12.3% | +| Commit 5 (predicates/capture/cut/TB) | 25.09 | 65.96 | **+11.0%** | -12.8% | + +Wallclock regressed **monotonically** with each migration; allocation dropped **monotonically but with diminishing returns**. By commit 5 the Δ-per-commit had flatlined: +0.38 ms wallclock, -0.34 MB alloc. The §9 gate (≥ 15% allocation drop) was not reached and the trajectory said it would not reach it. + +### Hypothesis (well-supported by the data) + +JIT escape analysis was already scalar-replacing the per-call `CstParseResult` records on the hot path. The records had: +- Raw nullable fields (Spike A removed Option boxing, leaving primitives + nullable refs) +- Local-scope construction in each parse method +- Immediate consume at call site (no cross-method handoff via stable references) +- No escape into fields/threads/exceptions + +These are textbook scalar-replacement targets. The "allocation" the JMH `gc.alloc.rate.norm` profiler reported was overstated relative to actual heap pressure — the JVM was decomposing each record into stack-allocated primitives. + +The singleton replacement: +- Field on `this` (heap-bound, cannot be scalarized) +- Field stores/loads cross method boundaries (defeats some inlining and code-motion) +- Source-level aliasing forces the compiler to assume mutation visibility (defeats further reordering) + +Net effect: GC sees fewer survivor objects (alloc-rate metric drops) **but** wallclock regresses because the optimized hot path is now slower per call. + +### What this means for future optimization (peglib and elsewhere) + +1. **Allocation rate ≠ optimization opportunity.** Short-lived records on hot paths may already be near-zero-cost via scalar replacement. JMH's `-prof gc` alloc-rate is a useful diagnostic, not a target. +2. **Bench wallclock first, alloc second.** A move that drops alloc but regresses wallclock is a loss. +3. **Profile via CPU sampling, not alloc sampling, when looking for actual hot work.** `async-profiler` in `cpu` mode + flame graphs. +4. **Singleton-mutable-state patterns from C/C++ codebases do NOT translate.** They beat C++'s allocator overhead because C++ has no escape analysis. Java's JIT does that work for you on short-lived records. + +### What's now ruled out + +- **Move B itself** (per-call `CstParseResult` elimination via singleton). Definitively abandoned. +- By extension: any "share a mutable state object on `this`" pattern targeting short-lived per-call records in the generated parser. Apply skepticism. + +### What's still viable (reassessed) + +- **Char-class bit-packing** (spec §10 future moves) — REASSESS WITH CARE. Same risk class: char-by-char comparison may already be JIT-optimized. Profile wallclock first; verify there's a measurable hot-path win before attempting. +- **Lever B retry (incremental engine)** — gated on trivia attribution rework. Independent of allocation patterns; still viable. +- **Trivia attribution rework** — context-independent attachment. Independent; still viable. Comparable scope to Lever C. +- **Lever C — IR unification** (spec §4) — multi-week. Maintainability + complexity reduction primary value, not raw perf. Still viable. +- **Profile-driven wallclock optimization** — `async-profiler` CPU mode, identify actual hot CPU work, target THAT. Probably the highest-ROI next direction. + +### What got preserved + +- The HANDOVER + this spec at `e849b63` document the experiment for posterity. +- `v0.5.0-candidate` tag at `e849b63` is the shippable 0.5.0 state. +- The 5 Move B commits remain in reflog (`git reflog show release-0.5.0`) for ~30 days. + +--- + +**Last updated:** 2026-05-08, end of Move B (abandoned). Successor work: see HANDOVER §11. diff --git a/docs/incremental/THROUGHPUT-ENGINE-TIER1.md b/docs/incremental/THROUGHPUT-ENGINE-TIER1.md new file mode 100644 index 0000000..4235fa2 --- /dev/null +++ b/docs/incremental/THROUGHPUT-ENGINE-TIER1.md @@ -0,0 +1,236 @@ +# Throughput Engine — Tier 1 allocation reduction spec + +**Date:** 2026-05-07 +**Branch:** release-0.5.0 (post A-spike GO at commit `0ed2dcd`) +**Profile baseline:** [`docs/bench-results/generator-profile-baseline.md`](../bench-results/generator-profile-baseline.md) +**Naming:** the parser generator is now branded the **throughput engine** — distinct from the **incremental engine** (PegEngine + IncrementalSession). They have different optimization targets, different code shapes, and no shared code. + +--- + +## 1. Motivation + +The throughput engine's profiling baseline shows it allocates **150 MB per parse** of 1900 LOC and spends **26.6% of bench wallclock in GC**. Top allocators are internal plumbing, not user-visible CST: + +| Samples | Class | Tier 1 move addressing it | +|---:|---|---| +| 3,190 | `Option$Some` | **A** (mutable ParseResult — Option boxing eliminated) | +| 3,008 | `Object[]` | C, D (fewer ArrayLists; primitive packing) | +| 2,574 | `CstParseResult` | **B** (mutable scratch — fewer record allocs) | +| 2,367 | `SourceLocation` | **D** (pack into primitive long fields) | +| 1,096 | `SourceSpan` | **D** | +| 587 | `CstNode.NonTerminal` (retained) | **C** (lazy materialization) — 3-5× more allocated than retained | +| 553 | `CstNode.Terminal` (retained) | **C** | + +The use case constraint: **CST output is mandatory** (formatter + linter both consume the lossless tree). Pure-AST mode is off the table. Wins must come from changing *how* the CST is constructed, not whether. + +The A spike already proved the approach is sound: 90.5% reduction in Option$Some samples + 16% reduction in bytes/op, with ZERO algorithmic change. Tier 1 stacks four coordinated moves to compound the win. + +--- + +## 2. Tier 1 moves + +### A — Sentinel-based ParseResult flow (PROVEN) + +**Status:** spike landed at commit `0ed2dcd`. Opt-in via `mutableParseResult` ParserConfig flag. + +**What changed:** the generator's emitted `ParseResult` class became a mutable record-like with raw nullable fields (no `Option value` / `Option expected` wrappers). Emission templates use `result.value != null` instead of `result.value.isPresent()`, etc. + +**Result (A alone vs phase1_allStructural baseline):** +- Option$Some allocations: 6,088 → 577 (**-90.5%**) +- Bytes/op: 150.4M → 126.3M (**-16%**) +- Wallclock: 81.97 → 75.01 ms/op (**-8.5%**, marginal due to CI variance) + +**Open work in A:** the spike covered hot-path emission templates only. Capture, BackReference, Cut, And, Not still emit Option-style code. Coverage extension is a small follow-up — apply the same patterns. Estimated 2-3 hours. + +### B — Mutable parse-state replacing per-attempt CstParseResult records + +**Target allocation:** 2,574 CstParseResult samples per parse. Each successful or backtracked parse-method call currently allocates a fresh CstParseResult record holding (success, value, expected, endPos, endLine, endColumn, cutFailed). + +**Proposal:** replace per-call allocation with a SINGLE shared mutable instance per parser invocation. Every parse method mutates this instance in place; callers read the fields immediately (no aliasing across calls). + +```java +// Before +CstParseResult result = parseRule_X(pos, line, column); +if (result.success) { ... use result.endPos, result.value ... } + +// After (B applied; the parser-instance field `state` is the singleton) +boolean ok = parseRule_X(pos, line, column); // returns boolean (or int end-pos) +if (ok) { ... use state.endPos, state.value ... } +``` + +**Tradeoffs:** +- The singleton is non-aliasable: callers must consume the result fields BEFORE making any subsequent parse call. Emission templates already follow this pattern (parse + immediate read), so the constraint is already satisfied in practice. +- Packrat cache currently stores CstParseResult instances by value. Cache entries CANNOT use the shared mutable singleton — packrat needs immutable snapshots. Fix: when storing a cache entry, copy the singleton's fields into a small CacheEntry record (or directly into a primitive-packed long[]). +- Cut-failure short-circuits across rule calls work because the singleton's `cutFailed` field is checked immediately on return (existing pattern preserves). + +**Expected gain:** -2,574 CstParseResult allocations (down to ~packrat-cache-entry count, which is bounded by rule × position memoization rate). Together with A's reduction in result-record-related allocations, this should knock another ~10-15% off bytes/op. + +**Implementation effort:** ~3-4 days. Modifications to ParserGenerator emission templates: change `return CstParseResult.success(...)` to `state.setSuccess(...); return true;` patterns. Caller emission changes correspondingly. Packrat cache emission changes to copy fields into CacheEntry on put. + +**Gate:** -50% CstParseResult allocations OR -10% wallclock vs (A applied) baseline. + +### C — Lazy CST construction (parse → trace → materialize) + +**Target allocation:** 587 + 553 + 75 = 1,215 retained CstNode records, but profile suggests **3-5× more are allocated and discarded during speculative parsing**. The 1140 retained vs. allocated-and-discarded ratio is what makes this the second-largest design move under the CST-mandatory constraint. + +**Proposal:** decouple parse-time work from CST construction. + +``` +Parse phase (hot loop): + - Each rule that succeeds appends a TraceEntry to a pre-sized growing buffer. + - TraceEntry is a 16-byte primitive record: { ruleId: int, startPos: int, endPos: int, childTraceCount: int }. + - Children are pre-order: a node's TraceEntry is followed by its children's TraceEntries. + - Backtracking = rewind the trace's write-pointer; no allocation rollback needed. + +Materialization phase (post-parse): + - Walk the committed trace top-down; for each TraceEntry, allocate the corresponding CstNode. + - Trivia attribution happens during materialization, with full tree context — not during parse. +``` + +**Why this is high-leverage:** +- Backtracked branches no longer allocate CstNode records — they only advance the trace write pointer, which then gets rewound for free. +- TraceEntry is 16 bytes; the average CstNode (with its children list, span, trivia lists) is 80+ bytes. Even retained nodes pay less during the parse phase (just the TraceEntry). +- CST is materialized exactly once with full structural context — opens a clean home for **trivia attribution rework**, which Lever B is currently waiting on. The "context-sensitive trivia attribution" problem may dissolve here because materialization is single-pass with full tree knowledge. + +**Tradeoffs:** +- Memory profile shifts: the trace buffer needs to grow to ~10× the retained CST size (because of speculative entries). Pre-size based on grammar + input length; growth via amortized doubling. Worst case: a few MB of trace, vs. the current 150 MB allocation pressure. +- Trivia attribution semantics may need careful design. Currently embedded in PegEngine + emission templates; concentrating it into one materialization pass is cleaner BUT changes the timing — trivia decisions are now made AFTER all parsing is done, with full context. **This may be where Lever B's blocker dissolves.** +- More invasive than A or B: emission templates change substantially. ~1 week of focused work. + +**Gate:** -50% CST-related allocations (NonTerminal + Terminal + Token + ArrayList for children) OR -15% wallclock vs (A+B applied) baseline. + +**Bonus payoff:** trace-walking pass becomes a natural place to fix Lever B. Worth investigating during C's implementation. + +### D — Pack SourceLocation / SourceSpan into primitive long fields + +**Target allocation:** 2,367 SourceLocation + 1,096 SourceSpan = 3,463 combined samples per parse. Already partially packed in 0.4.3 (SourceSpan as int triples). Remaining work: eliminate SourceLocation as a record entirely. + +**Current state (per HANDOVER §5 v0.4.3 entry):** +```java +record SourceSpan(int startOffset, int startLine, int startColumn, + int endOffset, int endLine, int endColumn) { + public SourceLocation start() { return new SourceLocation(startLine, startColumn, startOffset); } // ALLOC + public SourceLocation end() { return new SourceLocation(endLine, endColumn, endOffset); } // ALLOC +} +``` + +Every `node.span().start()` accessor currently allocates a SourceLocation. The 2,367 samples are these on-demand reconstructions. + +**Proposal:** eliminate SourceLocation as a record. Replace with primitive accessors on SourceSpan: +```java +record SourceSpan(int startOffset, int startLine, int startColumn, + int endOffset, int endLine, int endColumn) { + public int startOffset() { return startOffset; } + public int startLine() { return startLine; } + public int startColumn() { return startColumn; } + public int endOffset() { return endOffset; } + public int endLine() { return endLine; } + public int endColumn() { return endColumn; } + // start() / end() removed +} +``` + +Callers of `span.start().offset()` / `span.start().line()` change to `span.startOffset()` / `span.startLine()`. Pure mechanical sweep. + +**Tradeoffs:** +- Public API change: SourceLocation may be referenced by user code (the maven-plugin or playground emits diagnostics). Likely small surface; maintain a `SourceLocation` record class but stop *constructing* it from SourceSpan accessors. +- Test sweep: many tests pattern-match on SourceLocation — those need updating. Estimated ~50-100 sites across 5 modules. + +**Implementation effort:** ~3-4 days. Mostly mechanical sweep + test updates. + +**Gate:** -90% SourceLocation allocations + -50% SourceSpan allocations OR -5% wallclock vs (A+B+C applied) baseline. + +--- + +## 3. Combined Tier 1 target + +| Metric | Baseline (current) | Tier 1 target | Reduction | +|---|---:|---:|---:| +| Bytes/op | 150 MB | ≤ 30 MB | **-80%** | +| Wallclock | 76.2 ms | ≤ 50 ms | **-34%** | +| GC time fraction | 26.6% | ≤ 10% | **-62%** | +| Option$Some allocs | 3,190 | < 500 | -85% (A confirmed) | +| CstParseResult allocs | 2,574 | < 200 | -92% (B target) | +| CST allocs (NonTerminal+Terminal+Token) | 1,215 retained, 3-5× discarded | 1,215 retained, 0 discarded | -75% (C target) | +| SourceLocation allocs | 2,367 | < 100 | -95% (D target) | + +**Aggregate target: 5× allocation reduction, 1.5× throughput improvement.** + +--- + +## 4. Implementation sequence + +Each move is GATED by re-profiling. If a move misses its gate, pause and reassess before continuing. + +1. **A coverage extension** (3 hours) — apply the spike's pattern to the remaining emission paths (Capture, BackReference, Cut, And, Not). Re-bench. Close the A gate at the wallclock margin. + +2. **B implementation + parity tests** (3-4 days) — mutable parse-state singleton; emission template rewrite; packrat cache CacheEntry copy. Bench gate: -50% CstParseResult allocs, -10% wallclock. + +3. **Re-profile after A+B** — fresh profile. Confirms Tier 1 is on track. Identifies which move (C or D) to prioritize next based on residual allocator weights. + +4. **C implementation** (1 week) — trace + materialization. **Investigate Lever B retry during C** — if trivia attribution moves into materialization, the literal-prefix gate may become unnecessary. Bench gate: -50% CST allocs, -15% wallclock. + +5. **D implementation + sweep** (3-4 days) — eliminate SourceLocation construction; mechanical caller sweep. Bench gate: -90% SourceLocation allocs. + +6. **Final Tier 1 bench + writeup** (1 day) — JFR + async-profiler runs against the final state; comparison table vs baseline; results doc at `docs/bench-results/throughput-tier1-results.md`. + +**Total elapsed:** 2-3 weeks. Each gate provides a stop-loss; if A+B alone clear the user's actual perf needs, C and D can be deferred. + +--- + +## 5. Risks + +### R1 — Mutable shared state in B introduces aliasing bugs (LOW likelihood, MEDIUM impact) + +The mutable singleton requires "consume immediately, then make next call" discipline. Existing emission already follows this pattern; the risk is in subtle paths where a result is held across calls. + +**Mitigation:** the parity tests (`Phase1ParityTest`, `Phase2*ParityTest`) compare CST output between flag-on and flag-off variants. If aliasing breaks something, parity diverges immediately. + +### R2 — C's trace-buffer architecture is invasive (MEDIUM likelihood, HIGH impact) + +Lazy CST is the largest emission-template change in Tier 1. Subtle bugs in trace materialization (missing children, wrong rule-id mapping, trivia mis-attribution) could break the CST output. + +**Mitigation:** parity tests + RoundTripTest. C's implementation gates on RoundTripTest stays green; if it breaks, fix before merging. + +### R3 — D's SourceLocation removal is a public API change (HIGH likelihood, LOW impact) + +Downstream consumers may pattern-match on SourceLocation records (maven-plugin diagnostics, playground UI rendering). + +**Mitigation:** keep `SourceLocation` as a record class; just stop *constructing* it from SourceSpan accessors. External code that explicitly creates SourceLocation continues to work. + +### R4 — The 5× allocation target is overshoot; may settle at 3-4× (MEDIUM likelihood, MEDIUM impact) + +The 80% reduction target assumes A+B+C+D compound multiplicatively. They likely overlap (same allocations targeted from different angles). Real combined reduction may be 60-70% (still excellent). + +**Mitigation:** measure each gate empirically; gates are sized for individual-move reduction, not the combined target. + +### R5 — Wallclock improvement gates may continue to be CI-variance-bound (MEDIUM likelihood, LOW impact) + +The A spike showed 8.5% wallclock improvement but with ±37 ms CI bands. JIT warmup and GC pauses dominate variance. + +**Mitigation:** when measuring Tier 1, use longer iteration counts (`-i 10 -wi 5 -f 2`) to tighten CI. The bytes/op metric is much less variable; treat it as the primary gate; wallclock as confirmation. + +--- + +## 6. Compatibility commitments + +- **Generator output remains a single self-contained Java file** — depending only on `pragmatica-lite:core`. Tier 1 doesn't introduce a runtime dependency. +- **CST output is byte-identical to current generator** — byte-equality verified by RoundTripTest 22/22. +- **Public Session API unchanged** — Tier 1 is generator-internal. +- **`mutableParseResult` flag** (and any new gates we add) — opt-in for users who want the new emission. Once Tier 1 is fully proven, can flip the default; old behavior preserved as a flag. + +--- + +## 7. Out of scope for Tier 1 + +Deferred items, ranked roughly by next-pass priority: + +- **Packrat as `long[][]` array** (was Tier 2 E in the profile doc). 622 + 174 HashMap allocations + 41 CPU samples. Worth ~5-10% additional after Tier 1. +- **First-set Choice dispatch** (was Tier 3 F). Existing `choiceDispatch` flag — investigate first; may be partial. ~10-30% on Java-shaped grammars. +- **DFA-based lexer for token rules** (was Tier 3 G). Biggest individual gain but biggest architectural lift. 2-3 weeks. Evaluate after Tier 1's residual profile. +- **Vectorized terminator scanning, ASCII whitespace fast path** — small tactical wins; defer. +- **Lever B retry** — gated on C's trivia-attribution rework. May dissolve the literal-prefix gate. + +--- + +**Last updated:** 2026-05-07 by the post-A-spike review. diff --git a/peglib-core/pom.xml b/peglib-core/pom.xml index 7c063b4..61c47a9 100644 --- a/peglib-core/pom.xml +++ b/peglib-core/pom.xml @@ -7,7 +7,7 @@ org.pragmatica-lite peglib-parent - 0.4.3 + 0.5.0 ../pom.xml diff --git a/peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java b/peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java index d997aca..8e97afa 100644 --- a/peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java +++ b/peglib-core/src/jmh/java/org/pragmatica/peg/bench/Java25ParseBenchmark.java @@ -33,10 +33,17 @@ /** * JMH harness for commit #10's default-flipping decision. * - *

Parametrized across seven {@link ParserConfig} variants (spec §8, PERF-REWORK-SPEC.md). - * Each variant compiles a freshly generated parser class at {@link Level#Trial} and benchmarks - * parsing the 1,900-LOC {@code FactoryClassGenerator.java.txt} fixture with a fresh parser - * instance per invocation (generated parsers are single-use). + *

Parametrized across seven {@link ParserConfig} variants (spec §8, PERF-REWORK-SPEC.md) and + * two {@code fixture} sources: {@code reference} (1,900-LOC {@code + * FactoryClassGenerator.java.txt}, ~100 KB) and {@code selfhost} (the generated parser source + * itself, ~25× larger — exposes scaling problems the small bench misses). + * + *

Each variant compiles a freshly generated parser class at {@link Level#Trial} and + * benchmarks parsing the chosen fixture with a fresh parser instance per invocation (generated + * parsers are single-use). + * + *

The {@code interpreter} variant is incompatible with {@code fixture=selfhost} (no generated + * source exists to feed back); that combination is rejected explicitly in {@link #setup()}. * *

Run with: {@code mvn -Pbench clean compile exec:java}. */ @@ -61,10 +68,28 @@ public class Java25ParseBenchmark { "phase1_inlineLocations", "phase1_allStructural", "phase1_allStructural_skipPackrat", + "phase1_allStructural_mutableResult", + "phase1_allStructural_mutableResult_autoSkipPackrat", "interpreter" }) public String variant; + /** + * Source under test: + *

    + *
  • {@code reference} — the canonical 1,900-LOC {@code FactoryClassGenerator.java.txt} + * fixture (~100 KB). Stable, low variance, fits L2; primary throughput bench.
    • + *
  • {@code selfhost} — the generated parser source itself (~25× the work, + * millions of CST nodes). Exposes scaling/cache-behavior regressions the small fixture + * can miss (e.g. E's IntCstParseResultMap regressed self-host by 22% while the small + * fixture stayed within noise).
    • + *
+ * The {@code selfhost} fixture is incompatible with {@code variant=interpreter} — there's no + * generated source to feed back. That combination is rejected at {@link #setup()}. + */ + @Param({"reference", "selfhost"}) + public String fixture; + private Class parserClass; private Method parseMethod; private String fixtureSource; @@ -76,9 +101,16 @@ public class Java25ParseBenchmark { @Setup(Level.Trial) public void setup() throws Exception { var grammarText = loadResource(GRAMMAR_RESOURCE); - fixtureSource = loadResource(FIXTURE_RESOURCE); if ("interpreter".equals(variant)) { + if ("selfhost".equals(fixture)) { + throw new IllegalArgumentException( + "fixture=selfhost is not supported with variant=interpreter " + + "(interpreter does not produce a generated parser source). " + + "Use variant=interpreter with fixture=reference for " + + "interpreter-vs-generator comparison on the small fixture."); + } + fixtureSource = loadResource(FIXTURE_RESOURCE); var grammar = GrammarParser.parse(grammarText).unwrap(); interpreterParser = PegParser.fromGrammarWithoutActions(grammar, ParserConfig.DEFAULT).unwrap(); return; @@ -92,8 +124,26 @@ public void setup() throws Exception { if (sourceResult.isFailure()) { throw new IllegalStateException("Failed to generate parser for variant " + variant + ": " + sourceResult); } - parserClass = compileAndLoad(sourceResult.unwrap(), fqcn); + var generatedSource = sourceResult.unwrap(); + parserClass = compileAndLoad(generatedSource, fqcn); parseMethod = parserClass.getMethod("parse", String.class); + fixtureSource = fixtureSourceFor(fixture, generatedSource); + } + + /** + * Resolves the input string the bench will parse. + *
    + *
  • {@code reference} → the canonical {@code FactoryClassGenerator.java.txt} fixture.
    • + *
  • {@code selfhost} → the freshly generated parser's own source code (the parser parses + * itself; ~25× the work of the reference fixture).
    • + *
+ */ + private static String fixtureSourceFor(String fixture, String generatedSource) throws Exception { + return switch (fixture) { + case "reference" -> loadResource(FIXTURE_RESOURCE); + case "selfhost" -> generatedSource; + default -> throw new IllegalArgumentException("Unknown fixture: " + fixture); + }; } @Benchmark @@ -115,6 +165,14 @@ private static ParserConfig configFor(String variant) { case "phase1_allStructural" -> withStructural(true, true, true, false, Set.of()); case "phase1_allStructural_skipPackrat" -> withStructural(true, true, true, true, Set.of("Identifier", "QualifiedName", "Type")); + case "phase1_allStructural_mutableResult" -> withStructural(true, true, true, false, Set.of(), true); + // Phase 1.8: structural+mutableResult variant with selectivePackrat=ON and empty + // packratSkipRules — triggers auto-detection in PackratAnalyzer (LR rules excluded). + case "phase1_allStructural_mutableResult_autoSkipPackrat" -> withStructural(true, true, true, true, Set.of(), true); + // Phase 1.9 (DFA spike): structural + autoSkipPackrat WITHOUT tokenFastPath — A/B baseline. + case "phase1_allStructural_mutableResult_autoSkipPackrat_noFastPath" -> withStructural(true, true, true, true, Set.of(), true, false); + // Phase 1.9 (DFA spike): structural + autoSkipPackrat WITH tokenFastPath — A/B variant. + case "phase1_allStructural_mutableResult_autoSkipPackrat_fastPath" -> withStructural(true, true, true, true, Set.of(), true, true); default -> throw new IllegalArgumentException("Unknown variant: " + variant); }; } @@ -135,7 +193,9 @@ private static ParserConfig allOff() { false, // markResetChildren false, // inlineLocations false, // selectivePackrat - Set.of()); // packratSkipRules + Set.of(), // packratSkipRules + false, // mutableParseResult + false); // tokenFastPath } /** @@ -147,6 +207,25 @@ private static ParserConfig withStructural(boolean choiceDispatch, boolean inlineLocations, boolean selectivePackrat, Set packratSkipRules) { + return withStructural(choiceDispatch, markResetChildren, inlineLocations, selectivePackrat, packratSkipRules, false); + } + + private static ParserConfig withStructural(boolean choiceDispatch, + boolean markResetChildren, + boolean inlineLocations, + boolean selectivePackrat, + Set packratSkipRules, + boolean mutableParseResult) { + return withStructural(choiceDispatch, markResetChildren, inlineLocations, selectivePackrat, packratSkipRules, mutableParseResult, true); + } + + private static ParserConfig withStructural(boolean choiceDispatch, + boolean markResetChildren, + boolean inlineLocations, + boolean selectivePackrat, + Set packratSkipRules, + boolean mutableParseResult, + boolean tokenFastPath) { return new ParserConfig( true, // packratEnabled RecoveryStrategy.BASIC, @@ -161,7 +240,9 @@ private static ParserConfig withStructural(boolean choiceDispatch, markResetChildren, inlineLocations, selectivePackrat, - Set.copyOf(packratSkipRules)); + Set.copyOf(packratSkipRules), + mutableParseResult, + tokenFastPath); // phase 1.9 (DFA spike) } static String loadResource(String resourcePath) throws Exception { diff --git a/peglib-core/src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java b/peglib-core/src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java index 8bf7f98..32a25f3 100644 --- a/peglib-core/src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java +++ b/peglib-core/src/jmh/java/org/pragmatica/peg/bench/PackratStatsProbe.java @@ -60,7 +60,9 @@ public static void main(String[] args) throws Exception { true, true, true, true, true, true, // phase-1 flags on false, false, false, // structural flags off false, // selectivePackrat - Set.of()); + Set.of(), // packratSkipRules + false, // mutableParseResult + true); // tokenFastPath var sourceResult = PegParser.generateCstParser( grammarText, PACKAGE_NAME, CLASS_NAME, ErrorReporting.BASIC, config); diff --git a/peglib-core/src/main/java/org/pragmatica/peg/error/Diagnostic.java b/peglib-core/src/main/java/org/pragmatica/peg/error/Diagnostic.java index c39543f..52e8e36 100644 --- a/peglib-core/src/main/java/org/pragmatica/peg/error/Diagnostic.java +++ b/peglib-core/src/main/java/org/pragmatica/peg/error/Diagnostic.java @@ -172,30 +172,25 @@ public String format(String source, String filename) { .append(message) .append("\n"); // Location: --> filename:line:column - var loc = span.start(); sb.append(" --> "); if (filename != null) { sb.append(filename) .append(":"); } - sb.append(loc.line()) + sb.append(span.startLine()) .append(":") - .append(loc.column()) + .append(span.startColumn()) .append("\n"); // Find all lines we need to display - int minLine = span.start() - .line(); - int maxLine = span.end() - .line(); + int minLine = span.startLine(); + int maxLine = span.endLine(); for (var label : labels) { minLine = Math.min(minLine, label.span() - .start() - .line()); + .startLine()); maxLine = Math.max(maxLine, label.span() - .end() - .line()); + .endLine()); } // Calculate gutter width int gutterWidth = String.valueOf(maxLine) @@ -238,9 +233,7 @@ public String format(String source, String filename) { private List