Skip to content

Add TraceManager.RetainWords to keep trace text, not Words - #571

Open
johnml1135 wants to merge 2 commits into
masterfrom
perf/hc-lean-trace
Open

johnml1135 wants to merge 2 commits into
masterfrom
perf/hc-lean-trace

Conversation

@johnml1135

@johnml1135 johnml1135 commented Oct 27, 2026 •

Copy link
Copy Markdown
Collaborator

Start here: src/SIL.Machine.Morphology.HermitCrab/TraceManager.cs -- RetainWords and the Node(...) helper that every trace callback now goes through.

Status: open; stands alone on master (#534 was closed). Review fixes F1–F4 are in 3f45fbe.

TraceManager gets an opt-in RetainWords switch. Set it to false and each trace node keeps shape text (InputText/OutputText) instead of the Word. On Mbugwe yɔɔna the retained trace falls from 1,035.0 MiB to 22.6 MiB with the same 120,147 nodes. The default stays true, the search is untouched, and the HermitCrab CLI (HCContext) sets it false. Closes #563.

What a reviewer arrives wondering: does lean mode change what is printed or parsed? Node-by-node tests say no; the CLI output was byte-identical on Indonesian (timing lines aside, one processor).

Where to look:

  • Lean and full trees match node by node, text included -- LeanTrace_MatchesFullTraceNodeForNode.
  • Lean nodes hold no Word; parses are equal with tracing off, full and lean -- LeanTrace_KeepsNoWords, ParseWord_IsIdenticalWithTracingOffFullAndLean.
  • The frozen-word text cache lives in two private fields on Word and is not copied to clones -- FrozenWord_CachesItsTraceTextAndCopiesDoNotInherit.
  • Trace.Input/Output are null in lean mode; callers read InputText/OutputText, which work in both modes.

Not here:

  • Flipping the default to lean. Trace.Input/Output are public, so that is a breaking choice for the owner.
  • Bounding or streaming the trace; it still holds one node per callback.

Verification: run on this branch: dotnet test tests/SIL.Machine.Morphology.HermitCrab.Tests -c Release (0 failed), dotnet csharpier check . and pwsh -NoProfile -File ./scripts/comment-hygiene.ps1 -BaseRef origin/master (both clean). The full ./local_check.sh --agent-strict also passes at 3f45fbe; no coverage run.

Next: approve the opt-in switch.


Reading this a year from now -- start here

Every trace callback used to store the Word it was handed, or a deep Clone() of it (six places), and the tree is never trimmed. A Word owns its shape, feature structures and dictionaries, so the trace held about 17 KB per node. Both real consumers read only strings: the CLI prints the shape text, and FieldWorks' production code does not use TraceManager (it implements ITraceManager in its own trace class; not checkable from this repository).

With RetainWords = false, Trace.Input and Trace.Output are always null and InputText/OutputText are filled when the node is made. The text follows the old CLI rule: regular-expression form for the analysis node types, plain form otherwise. A frozen Word caches each form once in _traceTextRegex and _traceTextPlain, which the copy constructor does not copy, so many not-unapplied nodes that share one input format it once. Unfrozen words are formatted fresh. In full mode the two properties format lazily from Input/Output.

What remains in lean mode is the node objects and two short strings each, about 190 bytes a node on yɔɔna (estimate from the totals, not a heap count).

Decisions, and why
  • A switch on the manager, not a new class, so callers opt in with one line and the default is unchanged.
  • CompoundingRuleAnalysis keeps the plain form, because the old CLI switch did not list it as an analysis type. Byte-identical output wins over tidiness.
  • No dictionary keyed on words. The cache is one slot per frozen Word, freed with it.
  • Mutation check: three callback paths receive unfrozen words (phonological rule input, a private pre-rule clone; the CompoundingRuleAnalysis temp clone; phonological rule outputs, which the manager already cloned). None is mutated after its callback, and the full-vs-lean test covers all three, so lean text equals full text.
Evidence

Retained trace MiB, full to lean, measured with the census context harness; same parse counts and node counts in every word compared:

Word or grammar Full Lean Nodes
Mbugwe yɔɔna 1,035.0 22.6 120,147
Mbugwe kojeema 1,010.6 9.7 51,075
Mbugwe woocha 673.6 52.9 373,091
Mbugwe chaanecha (largest) 5,485.6 331.8 2,138,994
Aweti tsãnekozokwawut 656.2 29.4 214,989
Mbugwe, 40 words, mean 407.2 25.1 6,111,033 total
Aweti, 99 words, mean 35.6 2.3 1,689,825 total

Steps are unchanged: the census (its own trace manager) gives identical rows with and without this change on Indonesian and on yɔɔna (21,200 analysis events, 41,130 synthesis steps).

Coverage gap: compounding-rule synthesis failures and the GenerateWords root, which ParseWord never makes, are not covered by the new tests.

🤖 Generated with Claude Code


This change is Reviewable

With RetainWords false, trace nodes store the shape text of their input and
output (InputText, OutputText) instead of Word references and clones. The
default is unchanged and the CLI turns lean mode on by setting it false.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 27, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.82759% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.18%. Comparing base (c245476) to head (3f45fbe).
⚠️ Report is 13 commits behind head on master.

Files with missing lines Patch % Lines
.../SIL.Machine.Morphology.HermitCrab/TraceManager.cs 93.02% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #571      +/-   ##
==========================================
+ Coverage   74.57%   75.18%   +0.60%     
==========================================
  Files         453      453              
  Lines       38342    38361      +19     
  Branches     5284     5297      +13     
==========================================
+ Hits        28593    28840     +247     
+ Misses       8583     8379     -204     
+ Partials     1166     1142      -24     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@claude

This comment has been minimized.

@claude

claude Bot commented Nov 7, 2026

Copy link
Copy Markdown
Contributor
  1. Verdict: approve. The only change since 9f7828e is documentation and a test comment (3f45fbe), and it addresses all four earlier findings.
  2. The one that mattered most, F1 (src/SIL.Machine.Morphology.HermitCrab/Trace.cs:197), is fixed: the InputText doc now names CompoundingRuleAnalysis as the plain-form exception.
  3. Counts: Critical 0, Important 0, Low 0 open. All four earlier Low findings are addressed.
  4. Ran git diff 9f7828e8 3f45fbe2 (three comment edits, no code change) and a fresh /code-review high over the whole PR. Nothing new is on changed lines or Critical, and its GenerateWords tracing concern is on a SynthesizeWord line that is unchanged from master. Checks: Build on ubuntu-22.04 and Build on windows-latest pass, both advisory Report comment hygiene checks pass, Create NuGet package pending.
  5. Not verified: the strict comment-hygiene scan (--agent-strict), which I did not run (the PR body reports it passes at 3f45fbe), and the memory figures in the PR body.

Public API: adds TraceManager.RetainWords (default true) and Trace.InputText/OutputText in SIL.Machine.Morphology.HermitCrab. Existing semantics are unchanged. No target framework, package, or sillsdev/machine.py parity change.

Findings: F1 addressed, F2 addressed, F3 addressed, F4 addressed.

Reviewed at 3f45fbe

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

HermitCrab analysis: lean trace keeps shape text instead of Word objects

3 participants