From 8bbbcf41abde269747bc5fec838ba908f7cce89d Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 21 Oct 2026 20:41:03 -0400 Subject: [PATCH 1/4] perf(hermitcrab): share trailing affix-template slot batches --- .../AnalysisAffixTemplateRule.cs | 38 ++- .../AnalysisAffixTemplatesRule.cs | 145 ++++++++++ .../AnalysisStratumRule.cs | 7 +- .../AnalysisAffixTemplatesRuleTests.cs | 259 ++++++++++++++++++ 4 files changed, 432 insertions(+), 17 deletions(-) create mode 100644 src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs create mode 100644 tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplateRule.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplateRule.cs index 9044a8488..ba963745a 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplateRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplateRule.cs @@ -30,13 +30,34 @@ public AnalysisAffixTemplateRule(Morpher morpher, AffixTemplate template) public IEnumerable Apply(Word input) { - if (!_morpher.RuleSelector(_template)) + if (!CanApply(input)) return Enumerable.Empty(); + Word inWord = input.Clone(); + inWord.Freeze(); + + var output = new HashSet(FreezableEqualityComparer.Default); + if (_morpher.MaxDegreeOfParallelism == 1) + ApplySlots(inWord, _rules.Count - 1, output); + else + ParallelApplySlots(inWord, output); + + return output; + } + + internal AffixTemplate Template => _template; + + internal IReadOnlyList> SlotRules => _rules; + + internal bool CanApply(Word input) + { + if (!_morpher.RuleSelector(_template)) + return false; + // A check only: adding the template's required features here would make SyntacticFeatureStruct // depend on whether a template was un-applied, which Word.ValueEquals assumes cannot happen. if (!input.SyntacticFeatureStruct.IsUnifiable(_template.RequiredSyntacticFeatureStruct)) - return Enumerable.Empty(); + return false; if (_morpher.TraceManager.IsTracing) _morpher.TraceManager.BeginUnapplyTemplate(_template, input); @@ -58,19 +79,10 @@ public IEnumerable Apply(Word input) FailureReason.NonPartialRuleProhibitedAfterFinalTemplate ); } - return Enumerable.Empty(); + return false; } - Word inWord = input.Clone(); - inWord.Freeze(); - - var output = new HashSet(FreezableEqualityComparer.Default); - if (_morpher.MaxDegreeOfParallelism == 1) - ApplySlots(inWord, _rules.Count - 1, output); - else - ParallelApplySlots(inWord, output); - - return output; + return true; } private void ApplySlots(Word inWord, int index, HashSet output) diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs new file mode 100644 index 000000000..cbd08578f --- /dev/null +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs @@ -0,0 +1,145 @@ +using System.Collections.Generic; +using System.Linq; +using SIL.Machine.Annotations; +using SIL.Machine.Rules; +using SIL.ObjectModel; + +namespace SIL.Machine.Morphology.HermitCrab +{ + internal class AnalysisAffixTemplatesRule : RuleBatch + { + private readonly Morpher _morpher; + private readonly AnalysisAffixTemplateRule[] _templates; + private readonly bool[][] _slotOptionality; + private readonly SlotNode _root = new SlotNode(); + private readonly bool _hasSharedSlots; + + public AnalysisAffixTemplatesRule(Morpher morpher, IEnumerable> rules) + : base(rules, false, FreezableEqualityComparer.Default) + { + _morpher = morpher; + _templates = Rules.OfType().ToArray(); + if (_templates.Length != Rules.Count) + return; + + _slotOptionality = _templates.Select(t => t.Template.Slots.Select(s => s.Optional).ToArray()).ToArray(); + for (int t = 0; t < _templates.Length; t++) + { + AnalysisAffixTemplateRule template = _templates[t]; + SlotNode node = _root; + for (int s = template.Template.Slots.Count - 1; s >= 0; s--) + { + AffixTemplateSlot slot = template.Template.Slots[s]; + SlotNode child = node.Children.FirstOrDefault(n => n.Matches(slot)); + if (child == null) + { + child = new SlotNode + { + Slot = slot, + Optional = slot.Optional, + Rule = template.SlotRules[s], + }; + node.Children.Add(child); + } + else + { + _hasSharedSlots = true; + } + child.Templates.Add(t); + node = child; + } + node.Terminals.Add(t); + } + } + + public override IEnumerable Apply(Word input) + { + // A configured limit must stop before later templates are evaluated. + if ( + !_hasSharedSlots + || _morpher.TraceManager.IsTracing + || _morpher.MaxDegreeOfParallelism != 1 + || MaxAlternatives > 0 + || !SlotsUnchanged() + ) + return base.Apply(input); + + var outputs = new HashSet[_templates.Length]; + for (int t = 0; t < _templates.Length; t++) + { + if (_templates[t].CanApply(input)) + outputs[t] = new HashSet(Comparer); + } + + if (outputs.Any(o => o != null)) + { + Word inWord = input.Clone(); + inWord.Freeze(); + Walk(_root, inWord, outputs); + } + + // Template order determines which equal word survives and the order seen by analysis folding. + var output = new HashSet(Comparer); + foreach (HashSet templateOutput in outputs) + { + if (templateOutput != null) + output.UnionWith(templateOutput); + if (MaxAlternatives > 0 && output.Count > MaxAlternatives) + throw new MaxAlternativesExceededException("MaxAlternatives exceeded"); + } + return output; + } + + private bool SlotsUnchanged() + { + for (int t = 0; t < _templates.Length; t++) + { + IList slots = _templates[t].Template.Slots; + bool[] optionality = _slotOptionality[t]; + if (slots.Count != optionality.Length) + return false; + for (int s = 0; s < slots.Count; s++) + { + if (slots[s].Optional != optionality[s]) + return false; + } + } + return true; + } + + private static void Walk(SlotNode node, Word input, HashSet[] outputs) + { + foreach (int t in node.Terminals) + outputs[t]?.Add(input); + + foreach (SlotNode child in node.Children) + { + if (!child.Templates.Any(t => outputs[t] != null)) + continue; + + foreach (Word word in child.Rule.Apply(input)) + Walk(child, word, outputs); + if (child.Optional) + Walk(child, input, outputs); + } + } + + private sealed class SlotNode + { + public AffixTemplateSlot Slot { get; set; } + public bool Optional { get; set; } + public IRule Rule { get; set; } + public List Children { get; } = new List(); + public List Templates { get; } = new List(); + public List Terminals { get; } = new List(); + + public bool Matches(AffixTemplateSlot slot) + { + if (Optional != slot.Optional || Slot.Rules.Count != slot.Rules.Count) + return false; + + return Slot.Rules.Zip(slot.Rules, (a, b) => ReferenceEquals(a, b)).All(equal => equal); + } + } + } +} diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs index ef48637ac..811471d74 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisStratumRule.cs @@ -25,10 +25,9 @@ public AnalysisStratumRule(Morpher morpher, Stratum stratum) _prulesRule = new LinearRuleCascade( stratum.PhonologicalRules.Select(prule => CompilePhonologicalRule(prule, morpher)).Reverse() ); - _templatesRule = new RuleBatch( - stratum.AffixTemplates.Select(template => CompileAffixTemplate(template, morpher)), - false, - FreezableEqualityComparer.Default + _templatesRule = new AnalysisAffixTemplatesRule( + morpher, + stratum.AffixTemplates.Select(template => CompileAffixTemplate(template, morpher)) ); _mrulesRule = null; IEnumerable> mrules = stratum diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs new file mode 100644 index 000000000..9d91572ee --- /dev/null +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs @@ -0,0 +1,259 @@ +using NUnit.Framework; +using SIL.Machine.Annotations; +using SIL.Machine.FeatureModel; +using SIL.Machine.Matching; +using SIL.Machine.Morphology.HermitCrab.MorphologicalRules; +using SIL.Machine.Rules; +using SIL.ObjectModel; + +namespace SIL.Machine.Morphology.HermitCrab; + +[TestFixture] +public class AnalysisAffixTemplatesRuleTests : HermitCrabTestBase +{ + [TestCase(0)] + [TestCase(1)] + [TestCase(2)] + [TestCase(3)] + public void SharedWalkMatchesPerTemplateWalkInOrder(int variant) + { + var d = Suffix("d", "d"); + var dd = Suffix("dd", "dd"); + var s = Suffix("s", "s"); + AffixTemplateSlot Tail(bool optional = false) => new(d, dd) { Optional = optional }; + AffixTemplateSlot Inner() => new(s) { Optional = (variant & 1) == 0 }; + AffixTemplate[] templates = + [ + Template(Inner(), Tail()), + Template(Tail()), + Template(Inner(), Tail(true)), + Template(new AffixTemplateSlot(d) { Optional = true }, Inner(), Tail()), + Template(new AffixTemplateSlot(s), Tail()), + Template(), + Template(Inner(), Tail()), + Template(new AffixTemplateSlot(dd, d)), + Template(new AffixTemplateSlot()), + ]; + templates[4].RequiredSyntacticFeatureStruct = Pos("N"); + templates[3].IsFinal = false; + if ((variant & 2) != 0) + Array.Reverse(templates); + + var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1) + { + RuleSelector = rule => variant != 3 || !ReferenceEquals(rule, templates[1]), + }; + IRule[] rules = templates.Select(t => t.CompileAnalysisRule(morpher)).ToArray(); + var shared = new AnalysisAffixTemplatesRule(morpher, rules); + var own = new RuleBatch(rules, false, FreezableEqualityComparer.Default); + + int compared = 0; + foreach (string text in Inputs(4)) + { + foreach (FeatureStruct fs in new[] { FeatureStruct.New().Value, Pos("N"), Pos("V") }) + { + foreach ( + FinalTemplateState finalState in new[] { FinalTemplateState.None, FinalTemplateState.NonTemplate } + ) + { + Word input = Input(text); + input.SyntacticFeatureStruct = fs; + input.FinalTemplateState = finalState; + input.Freeze(); + + Word[] expected = own.Apply(input).ToArray(); + Word[] actual = shared.Apply(input).ToArray(); + Assert.That(actual, Is.EqualTo(expected).Using(FreezableEqualityComparer.Default), text); + Assert.That( + actual.Select(AnalysisStateKey.PinAndKey), + Is.EqualTo(expected.Select(AnalysisStateKey.PinAndKey)), + text + ); + compared += actual.Length; + } + } + } + Assert.That(compared, Is.GreaterThan(1000)); + } + + [Test] + public void UntracedBatteryRunsSharedSlotBatchOnce() + { + var d = Suffix("d", "d"); + var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1); + IRule[] rules = new[] + { + Template(new AffixTemplateSlot(d)), + Template(new AffixTemplateSlot(d)), + } + .Select(t => t.CompileAnalysisRule(morpher)) + .ToArray(); + var own = new RuleBatch(rules, false, FreezableEqualityComparer.Default); + var shared = new AnalysisAffixTemplatesRule(morpher, rules); + Word input = Input("sagd"); + input.Freeze(); + + Word[] expected = own.Apply(input).ToArray(); + Assert.That(d.ApplyCount, Is.EqualTo(2)); + d.ApplyCount = 0; + Assert.That(shared.Apply(input), Is.EqualTo(expected).Using(FreezableEqualityComparer.Default)); + Assert.That(d.ApplyCount, Is.EqualTo(1), "two identical tails must execute one slot batch"); + _ = shared.Apply(input).ToArray(); + Assert.That(d.ApplyCount, Is.EqualTo(2), "each battery call must execute its own batch"); + } + + [TestCase(false, 2)] + [TestCase(true, 1)] + public void TracedAndParallelBatteriesKeepPerTemplateWalk(bool tracing, int degree) + { + var d = Suffix("d", "d"); + var traceManager = new TraceManager { IsTracing = tracing }; + var morpher = new Morpher(traceManager, Language, maxDegreeOfParallelism: degree); + var shared = new AnalysisAffixTemplatesRule( + morpher, + new[] { Template(new AffixTemplateSlot(d)), Template(new AffixTemplateSlot(d)) }.Select(t => + t.CompileAnalysisRule(morpher) + ) + ); + Word input = Input("sagd"); + input.Freeze(); + if (tracing) + traceManager.AnalyzeWord(Language, input); + _ = shared.Apply(input).ToArray(); + Assert.That(d.ApplyCount, Is.EqualTo(2)); + } + + [Test] + public void SharedWalkPreservesBatteryAlternativeLimit() + { + var d = Suffix("d", "d"); + var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1); + IRule[] rules = new[] + { + Template(new AffixTemplateSlot(d) { Optional = true }), + Template(new AffixTemplateSlot(d) { Optional = true }), + } + .Select(t => t.CompileAnalysisRule(morpher)) + .ToArray(); + var shared = new AnalysisAffixTemplatesRule(morpher, rules) { MaxAlternatives = 1 }; + var own = new RuleBatch(rules, false, FreezableEqualityComparer.Default) + { + MaxAlternatives = 1, + }; + Word input = Input("sagd"); + input.Freeze(); + Assert.Throws(() => own.Apply(input).ToArray()); + Assert.Throws(() => shared.Apply(input).ToArray()); + } + + [Test] + public void AlternativeLimitStopsBeforeLaterTemplateAdmission() + { + var d = Suffix("d", "d"); + AffixTemplate[] templates = + [ + Template(new AffixTemplateSlot(d) { Optional = true }), + Template(new AffixTemplateSlot(d) { Optional = true }), + Template(new AffixTemplateSlot(Suffix("s", "s"))), + ]; + var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1) + { + RuleSelector = rule => + !ReferenceEquals(rule, templates[2]) + ? true + : throw new InvalidOperationException("later template must not be reached"), + }; + IRule[] rules = templates.Select(t => t.CompileAnalysisRule(morpher)).ToArray(); + var own = new RuleBatch(rules, false, FreezableEqualityComparer.Default) + { + MaxAlternatives = 1, + }; + var shared = new AnalysisAffixTemplatesRule(morpher, rules) { MaxAlternatives = 1 }; + Word input = Input("sagd"); + input.Freeze(); + + Assert.Throws(() => own.Apply(input).ToArray()); + Assert.Throws(() => shared.Apply(input).ToArray()); + } + + [TestCase(false, 1)] + [TestCase(true, 0)] + public void ChangedSlotOptionalityPreservesPerTemplateWalk(bool optional, int changedTemplate) + { + var d = Suffix("d", "d"); + AffixTemplate[] templates = + [ + Template(new AffixTemplateSlot(d) { Optional = optional }), + Template(new AffixTemplateSlot(d) { Optional = optional }), + ]; + var morpher = new Morpher(TraceManager, Language, maxDegreeOfParallelism: 1); + IRule[] rules = templates.Select(t => t.CompileAnalysisRule(morpher)).ToArray(); + var own = new RuleBatch(rules, false, FreezableEqualityComparer.Default); + var shared = new AnalysisAffixTemplatesRule(morpher, rules); + templates[changedTemplate].Slots[0].Optional = !optional; + Word input = Input("sag"); + input.Freeze(); + + Assert.That(shared.Apply(input), Is.EqualTo(own.Apply(input)).Using(FreezableEqualityComparer.Default)); + } + + private Word Input(string text) => new(Morphophonemic, Table3.Segment(text)); + + private FeatureStruct Pos(string symbol) => FeatureStruct.New(Language.SyntacticFeatureSystem).Symbol(symbol).Value; + + private static AffixTemplate Template(params AffixTemplateSlot[] slots) + { + var template = new AffixTemplate(); + foreach (AffixTemplateSlot slot in slots) + template.Slots.Add(slot); + return template; + } + + private CountingSuffix Suffix(string name, string suffix) + { + var rule = new CountingSuffix { Name = name, MaxApplicationCount = 3 }; + rule.Allomorphs.Add( + new AffixProcessAllomorph + { + Lhs = + { + Pattern + .New("stem") + .Annotation(FeatureStruct.New().Symbol(HCFeatureSystem.Segment).Value) + .OneOrMore.Value, + }, + Rhs = { new CopyFromInput("stem"), new InsertSegments(Table3, suffix) }, + } + ); + return rule; + } + + private static IEnumerable Inputs(int maxLength) + { + string[] frontier = [""]; + yield return ""; + for (int length = 1; length <= maxLength; length++) + { + frontier = frontier.SelectMany(w => new[] { "s", "a", "d" }.Select(c => w + c)).ToArray(); + foreach (string word in frontier) + yield return word; + } + } + + private sealed class CountingSuffix : AffixProcessRule + { + public int ApplyCount; + + public override IRule CompileAnalysisRule(Morpher morpher) => + new CountingRule(this, base.CompileAnalysisRule(morpher)); + + private sealed class CountingRule(CountingSuffix owner, IRule rule) : IRule + { + public IEnumerable Apply(Word input) + { + Interlocked.Increment(ref owner.ApplyCount); + return rule.Apply(input); + } + } + } +} From 3b59dc9be7da7f858d0bd11a6473a75c3d47244a Mon Sep 17 00:00:00 2001 From: John Lambert Date: Wed, 21 Oct 2026 20:56:32 -0400 Subject: [PATCH 2/4] Address review: drop the unreachable limit check, explain the fallback guard, avoid per-node closures The shared walk is only taken when MaxAlternatives is 0, so its limit check could never fire; the guard comment now names all five fallback conditions; Walk checks admitted templates with a loop instead of a capturing lambda. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../AnalysisAffixTemplatesRule.cs | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs index cbd08578f..ff92b8fa6 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs @@ -54,7 +54,9 @@ public AnalysisAffixTemplatesRule(Morpher morpher, IEnumerable Apply(Word input) { - // A configured limit must stop before later templates are evaluated. + // The original walk is required when: no tail is shared; tracing (ApplySlots pairs per-template + // Begin/End events); parallel slots (a separate breadth-first walk); a configured alternative limit + // (it must stop before later templates run); or slot optionality changed since the trie was built. if ( !_hasSharedSlots || _morpher.TraceManager.IsTracing @@ -84,8 +86,6 @@ public override IEnumerable Apply(Word input) { if (templateOutput != null) output.UnionWith(templateOutput); - if (MaxAlternatives > 0 && output.Count > MaxAlternatives) - throw new MaxAlternativesExceededException("MaxAlternatives exceeded"); } return output; } @@ -114,7 +114,7 @@ private static void Walk(SlotNode node, Word input, HashSet[] outputs) foreach (SlotNode child in node.Children) { - if (!child.Templates.Any(t => outputs[t] != null)) + if (!AnyAdmitted(child.Templates, outputs)) continue; foreach (Word word in child.Rule.Apply(input)) @@ -124,6 +124,16 @@ private static void Walk(SlotNode node, Word input, HashSet[] outputs) } } + private static bool AnyAdmitted(List templates, HashSet[] outputs) + { + for (int i = 0; i < templates.Count; i++) + { + if (outputs[templates[i]] != null) + return true; + } + return false; + } + private sealed class SlotNode { public AffixTemplateSlot Slot { get; set; } From cc75163b7b98313e73b8ec1a5d7e7d8bf91c79e8 Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 23 Oct 2026 07:05:16 -0400 Subject: [PATCH 3/4] Shorten the shared-slot fallback comment to the hygiene budget Co-Authored-By: Claude Opus 5.5 --- .../AnalysisAffixTemplatesRule.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs index ff92b8fa6..58b55e194 100644 --- a/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs +++ b/src/SIL.Machine.Morphology.HermitCrab/AnalysisAffixTemplatesRule.cs @@ -54,9 +54,8 @@ public AnalysisAffixTemplatesRule(Morpher morpher, IEnumerable Apply(Word input) { - // The original walk is required when: no tail is shared; tracing (ApplySlots pairs per-template - // Begin/End events); parallel slots (a separate breadth-first walk); a configured alternative limit - // (it must stop before later templates run); or slot optionality changed since the trie was built. + // Use the original walk when no tail is shared, when tracing (per-template Begin/End events), for + // parallel slots, under an alternative limit, or if slot optionality changed after the trie was built. if ( !_hasSharedSlots || _morpher.TraceManager.IsTracing From 5ed55e0a4a9ffab702edf5ec5b6900186b4e24ab Mon Sep 17 00:00:00 2001 From: John Lambert Date: Fri, 23 Oct 2026 07:09:21 -0400 Subject: [PATCH 4/4] Compare shared-walk outputs with AnalysisMergeKey #522 removed AnalysisStateKey along with the analysis-cascade memo and kept the same identity as AnalysisStratumRule.AnalysisMergeKey, so the shared-walk equivalence test now compares outputs under that key. Co-Authored-By: Claude Opus 5.5 --- .../AnalysisAffixTemplatesRuleTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs b/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs index 9d91572ee..275d27325 100644 --- a/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs +++ b/tests/SIL.Machine.Morphology.HermitCrab.Tests/AnalysisAffixTemplatesRuleTests.cs @@ -65,8 +65,8 @@ public void SharedWalkMatchesPerTemplateWalkInOrder(int variant) Word[] actual = shared.Apply(input).ToArray(); Assert.That(actual, Is.EqualTo(expected).Using(FreezableEqualityComparer.Default), text); Assert.That( - actual.Select(AnalysisStateKey.PinAndKey), - Is.EqualTo(expected.Select(AnalysisStateKey.PinAndKey)), + actual.Select(w => new AnalysisStratumRule.AnalysisMergeKey(w)), + Is.EqualTo(expected.Select(w => new AnalysisStratumRule.AnalysisMergeKey(w))), text ); compared += actual.Length;