Reduce allocations in template matching tries - #56185
Conversation
Store the common single-child case inline and materialize the public dictionary only when branching or requested.
|
Azure Pipelines: Successfully started running 1 pipeline(s). 2 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
🟡 Changes recommended
The newly added unit test likely fails to compile under repo-wide nullable+warnings-as-errors due to dereferencing nullable out variables without proving non-null to flow analysis.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR optimizes the TemplateEngine matching trie to reduce allocations by avoiding eager Dictionary creation for the common single-child case, while preserving matching behavior and providing a stability contract for NextNodes once materialized.
Changes:
- Add a compact single-child representation to
Trie<T>and lazily materialize theNextNodesdictionary only when needed. - Update
TrieEvaluator<T>to traverse using the new compactTryGetNextNodelookup to avoid forcing dictionary allocation. - Add a unit test validating
NextNodesmaterialization stability and that subsequent additions are reflected.
File summaries
| File | Description |
|---|---|
| test/TemplateEngine/Microsoft.TemplateEngine.Core.UnitTests/TokenTrieTests.cs | Adds coverage for NextNodes stability/materialization behavior. |
| src/TemplateEngine/Microsoft.TemplateEngine.Core/Matching/TrieEvaluator.cs | Switches evaluator traversal from NextNodes.TryGetValue to TryGetNextNode. |
| src/TemplateEngine/Microsoft.TemplateEngine.Core/Matching/Trie.cs | Implements compact single-child storage and lazy dictionary materialization with a new lookup API. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
|
The failing CI result is unrelated to this PR. The only failing test is |
|
The remaining CI failure is tracked by #56238. The Linux x64 |
|
Build
#56036 added |
Summary
NextNodesdictionary only when a node branches or callers request itNextNodesremains stable and reflects paths added after materializationPerformance
Measured with matched Windows x64 Release ReadyToRun SDK trees differing only in
Microsoft.TemplateEngine.Core.dll.All 80 semantic comparisons matched across 160 fresh process launches.
Validation
Microsoft.TemplateEngine.Core.UnitTests, Debug and Release,net11.0Microsoft.TemplateEngine.Core.UnitTests, Debug and Release,net481