|
| 1 | +# RUSTIFY on top of memoization — does the array/COW rearchitecture help? |
| 2 | + |
| 3 | +**Branch:** `feature/memoization-plus-cow`, stacked on `feature/memoization` (PR #456), which |
| 4 | +is stacked on master. This PR's own diff is exactly one commit: the previously-reviewed, |
| 5 | +already-squashed `RUSTIFY: allocation- and CPU-optimized HermitCrab parsing` (originally |
| 6 | +`ea72cd79`, PR #446), cherry-picked onto `feature/memoization` and merge-resolved. |
| 7 | + |
| 8 | +**Question this answers:** `memoization.md` §5 flagged an open hypothesis — master lacks |
| 9 | +RUSTIFY's copy-on-write `Shape`/`ShapeNode` (flat, array-backed, COW-cloned) that the original |
| 10 | +prototype had, and `Word.ReplayOnto`'s plain deep-clone graft was suspected to cost more without |
| 11 | +it. One heavy word (H1) still hit the ≥3x target anyway; a second heavy word (H2) did not |
| 12 | +complete within a 400s memo-on budget at all, which was the first concrete data point the |
| 13 | +deep-clone tax might be real. This PR tests that directly: layer RUSTIFY's array/COW rearchitecture |
| 14 | +on top of memoization and re-measure the same two words, same methodology, same grammar. |
| 15 | + |
| 16 | +## What's actually in this PR |
| 17 | + |
| 18 | +RUSTIFY's own summary (from its commit message, unchanged by this port): |
| 19 | +- Flat/COW `Shape` and `ShapeNode` backing (parallel int-linked arrays instead of an |
| 20 | + `OrderedBidirList`; `ShapeNode` becomes an `(Owner, Index)` handle). |
| 21 | +- Copy-on-write `Shape` cloning: a clone of a frozen shape shares the source's backing until |
| 22 | + first mutation. |
| 23 | +- `FeatureStruct` bit-packed `ulong` flat-unify fast path for the common simple/no-variable |
| 24 | + case, falling back to the original engine otherwise. |
| 25 | +- int-offset FST traversal throughout (every HermitCrab rule-spec file migrated from |
| 26 | + `ShapeNode`-offset to `int`-offset pattern matching). |
| 27 | +- Cheap `GetHashCode` overrides on several hot dictionary/hashset key paths; `StringComparer.Ordinal` |
| 28 | + on hot sorts; a shared per-thread `Random`; a filtered-annotation-view cache on frozen |
| 29 | + `AnnotationList`s. |
| 30 | +- `SyntacticFeatureStruct` mutate-after-freeze correctness hardening. |
| 31 | + |
| 32 | +This is **general allocation/CPU work, not FST-specific** — nothing here compiles a grammar to |
| 33 | +an FST or changes analysis semantics. It was already independently reviewed as PR #446 (see |
| 34 | +memory: byte-identical to master on the full regression suite plus per-word signature diffs on |
| 35 | +Sena and Indonesian) before this port. |
| 36 | + |
| 37 | +**Deliberately NOT included:** the FST inverse-chain analyzer, `GrammarFstAdvisor`, and the |
| 38 | +~5,000 lines of FST planning/spike docs that a separate exploratory branch (`fst-advisor`) had |
| 39 | +bundled together with a rebased copy of this same RUSTIFY work. Those are a different |
| 40 | +optimization strategy (compile-to-FST vs. memoize-the-search) with their own, separately |
| 41 | +confirmed correctness gap on pathological words — out of scope here. This PR is the |
| 42 | +allocation/data-structure rearchitecture alone, isolated from that bundling, specifically so |
| 43 | +it can be measured as its own independent variable against memoization. |
| 44 | + |
| 45 | +## Merge notes (mechanical, not semantic) |
| 46 | + |
| 47 | +RUSTIFY (based on an older master commit) and memoization (based on current master) both touch |
| 48 | +`Word.cs`, `Morpher.cs`, `AnalysisStratumRule.cs`, and `MorpherTests.cs`. All four resolved |
| 49 | +cleanly by inspection — the conflicts were positional (both branches independently added new |
| 50 | +constructor logic / new tests near the same anchor lines), not competing implementations of the |
| 51 | +same behavior: |
| 52 | +- `Morpher`'s `maxDegreeOfParallelism` ctor: kept RUSTIFY's more general two-overload shape |
| 53 | + (defaults to `Environment.ProcessorCount`, throttles the parallel cascade to any requested |
| 54 | + degree) while preserving memoization's `== 1` → memo-eligible trigger. These compose cleanly: |
| 55 | + memoization only ever cared about the `1` case. |
| 56 | +- `AnalysisStratumRule`'s cascade selection: kept memoization's `MemoizedCombinationRuleCascade` |
| 57 | + for `== 1`, adopted RUSTIFY's `MaxDegreeOfParallelism` throttle on the parallel cascade for |
| 58 | + other values (a real improvement memoization-alone never had — it left the parallel path |
| 59 | + running at an unthrottled default regardless of the requested degree). |
| 60 | +- `ApplyTemplates`'s outer `.Distinct(...)` call: RUSTIFY had already removed this (both here and |
| 61 | + on the mrule-cascade call site) as a verified-redundant no-op — `CombinationRuleCascade`/`RuleBatch` |
| 62 | + already dedupe internally via their own `HashSet<TData>(comparer)`. Confirmed by reading |
| 63 | + `RuleCascade`/`RuleBatch`'s own `Apply` implementations, not just trusting the commit message. |
| 64 | + Adopted RUSTIFY's removal. |
| 65 | +- `Word`'s clone constructor: adopted RUSTIFY's lazy-null `_disjunctiveAllomorphIndices` allocation |
| 66 | + (only allocate when the source has entries) alongside memoization's `AnalysisScope = word.AnalysisScope` |
| 67 | + copy — independent fields, no interaction. |
| 68 | +- The engine-wide `ShapeNode` → `int` offset-type change (RUSTIFY) required updating memoization's |
| 69 | + own new files (`MemoizedCombinationRuleCascade : RuleCascade<Word, ShapeNode>` → |
| 70 | + `RuleCascade<Word, int>`, and the corresponding `IRule<Word, ShapeNode>` references in its test |
| 71 | + file) to match. `AnalysisStateKey.cs`, `AnalysisScope.cs`, and `Word.ReplayOnto` needed no |
| 72 | + changes — they operate on `Shape` (whose public `Freeze`/`GetFrozenHashCode`/`ValueEquals` API |
| 73 | + RUSTIFY preserves) and on `_mruleApps`/`_nonHeadApps` (untouched by the Shape rearchitecture), |
| 74 | + never on the offset type directly. |
| 75 | +- `MorpherTests.cs`: both branches added independent new tests at the same anchor point (RUSTIFY: |
| 76 | + `AnalyzeWord_SingleThreaded_MatchesParallel`, `AnalyzeWord_ConcurrentRepeatedParsing_IsDeterministic`; |
| 77 | + memoization: 4 tests + `WordAnalysisSignature`). Kept all of them; no actual overlap in behavior |
| 78 | + tested. |
| 79 | + |
| 80 | +Gate: 80/80 HermitCrab tests pass (78 from memoization + 2 from RUSTIFY), full `Machine.sln` |
| 81 | +test suite green. |
| 82 | + |
| 83 | +## Measured result: analysis-set identical, and faster on both sides |
| 84 | + |
| 85 | +Same methodology as `memoization.md` §5: canonical analysis-set signature comparison |
| 86 | +(never byte-identical object comparison), same two heavy words (H1, H2 — real corpus words, |
| 87 | +never named per the standing grammar-privacy constraint), same local uncommitted Sena grammar. |
| 88 | + |
| 89 | +| Word | memo-on (seq+memo+RUSTIFY) | memo-off (parallel+RUSTIFY) | Ratio | Divergences | vs. memoization-alone | |
| 90 | +|---|---|---|---|---|---| |
| 91 | +| H1 | **9.17s** | 90.0s | **9.82x** | 0 | was 22.1s / 136.1s / 6.2x — both sides faster, ratio improved | |
| 92 | +| H2 | **44.8s** | 263.8s | **5.89x** | 0 (both empty) | was >400s / >400s (never completed) — now completes cleanly | |
| 93 | + |
| 94 | +**H1** is the strong, unambiguous result: a real analysis (2 valid parses), confirmed |
| 95 | +analysis-set identical, and RUSTIFY makes *both* the memo path and the parallel-default path |
| 96 | +faster — the memo path more so (22.1s → 9.17s, 2.4x) than the parallel path (136.1s → 90.0s, |
| 97 | +1.51x), consistent with `ReplayOnto`'s clone being exactly the kind of operation COW should help |
| 98 | +most. |
| 99 | + |
| 100 | +**H2** is the headline: the word that memoization alone could not get through even a 400s |
| 101 | +budget now completes in 44.8s (memo-on) — this is the concrete answer to the open question |
| 102 | +`memoization.md` raised. One honest caveat: `words with no parse on both sides: 1` — H2 resolves |
| 103 | +to *zero* valid analyses on both memo-on and memo-off in this grammar (not a positive multi-analysis |
| 104 | +match like H1). The soundness confirmation here is "both sides agree on empty," not "both sides |
| 105 | +agree on the same non-trivial analysis set" — still a real, non-vacuous check (434,628 nogood |
| 106 | +hits recorded means the search space explored before concluding "no parse" is exactly the |
| 107 | +pathologically large one this word is known for), but weaker than H1's positive match. Worth |
| 108 | +re-confirming on a heavy word that *does* have a known-positive analysis if one is found. |
| 109 | + |
| 110 | +**A first anomalous run is worth recording, not hiding:** the first H1 attempt at a 240s budget |
| 111 | +timed out with no timing captured at all — alarming on its face, since 9.17s + 90.0s = 99.2s is |
| 112 | +comfortably under 240s. Re-run at a 600s budget completed cleanly with the numbers above, |
| 113 | +and the memo-hit counters from the *failed* run (29,736 / 88,426 / 37,512 / 0) exactly matched |
| 114 | +this run's completed tally — a deterministic property of the search space explored, independent |
| 115 | +of wall-clock time, and the tell that ruled out a correctness regression before the timing |
| 116 | +re-run confirmed it was transient machine load (this session had already run many consecutive |
| 117 | +heavy CPU benchmarks) rather than a real regression from the merge. |
| 118 | + |
| 119 | +## Typical (non-template) grammars: no change to the existing tradeoff |
| 120 | + |
| 121 | +Re-ran the Indonesian aggregate (121 words, same as `memoization.md`'s own measurement): |
| 122 | +0 divergences, memo-on total 2,425.7ms vs memo-off total 1,758.4ms (0.72x, ~38% slower) — |
| 123 | +within the same noise band memoization-alone showed on this grammar (0.68x-0.80x across 3 reps). |
| 124 | +RUSTIFY doesn't measurably change the typical-word tradeoff in either direction here; the |
| 125 | +existing honest caveat in `memoization.md` §6 (sequential+memo loses to parallel-default on |
| 126 | +typical words, mostly from the lost thread) stands unchanged. |
| 127 | + |
| 128 | +## Bottom line |
| 129 | + |
| 130 | +Stacking this rearchitecture on top of memoization is a real, verified improvement, not a |
| 131 | +"kind of, but not really": both measured heavy words get faster, and the one word memoization |
| 132 | +alone couldn't complete at all now does, at a **5.89x** ratio, with soundness holding |
| 133 | +(0 divergences on both). The cost is a much larger diff than memoization alone (110 files vs 11) |
| 134 | +touching the engine's core data representation — reviewed once already as PR #446, re-verified |
| 135 | +here against the current codebase plus memoization's new code paths. |
0 commit comments