Skip to content

Reduce allocations in template matching tries - #56185

Merged
JeremyKuhne merged 3 commits into
dotnet:mainfrom
JeremyKuhne:optimize-template-command-startup
Sep 11, 2026
Merged

Reduce allocations in template matching tries#56185
JeremyKuhne merged 3 commits into
dotnet:mainfrom
JeremyKuhne:optimize-template-command-startup

Conversation

@JeremyKuhne

Copy link
Copy Markdown
Member

Summary

  • store the common single-child trie case inline
  • materialize the shipped NextNodes dictionary only when a node branches or callers request it
  • keep evaluator lookups on the compact representation without changing matching behavior
  • test that NextNodes remains stable and reflects paths added after materialization

Performance

Measured with matched Windows x64 Release ReadyToRun SDK trees differing only in Microsoft.TemplateEngine.Core.dll.

  • reported allocation: 49,401,248 B to 40,047,344 B (18.93% reduction)
  • allocation events: 293 to 205
  • peak heap: 33.87 MB to 25.05 MB
  • initialized console creation, 30 alternating pairs: effectively neutral (-0.17% paired median, 15/30 pairs faster)
  • initialized template list: 4.08% faster
  • template help: 1.75% faster
  • initialized class library creation: 3.35% faster

All 80 semantic comparisons matched across 160 fresh process launches.

Validation

  • Microsoft.TemplateEngine.Core.UnitTests, Debug and Release, net11.0
  • Microsoft.TemplateEngine.Core.UnitTests, Debug and Release, net481
  • 363 passed, 1 existing skip, 0 failed in each captured full run
  • full Release ReadyToRun SDK build: 0 warnings, 0 errors

Store the common single-child case inline and materialize the public dictionary only when branching or requested.
Copilot AI lite review requested due to automatic review settings September 8, 2026 23:21
@JeremyKuhne
JeremyKuhne requested a review from a team as a code owner September 8, 2026 23:21
@azure-pipelines

Copy link
Copy Markdown
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.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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 the NextNodes dictionary only when needed.
  • Update TrieEvaluator<T> to traverse using the new compact TryGetNextNode lookup to avoid forcing dictionary allocation.
  • Add a unit test validating NextNodes materialization 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

@JeremyKuhne

Copy link
Copy Markdown
Member Author

The failing CI result is unrelated to this PR. The only failing test is ScopedCss_PublishIsBackwardsCompatible_WithPreviousVersions, where Razor exits with This writer does not support components. This exact regression is tracked by dotnet/roslyn#85132, and the corresponding SDK test quarantine is dotnet/sdk#56134. All platform builds, AOT builds, and the Windows Full Framework leg passed.

@JeremyKuhne

Copy link
Copy Markdown
Member Author

The remaining CI failure is tracked by #56238. The Linux x64 -mt row failed with MSB3883: Cannot release a lock that is not owned by the current thread; the serial row and recent PR/main executions passed.

Comment thread src/TemplateEngine/Microsoft.TemplateEngine.Core/Matching/Trie.cs
@JeremyKuhne

Copy link
Copy Markdown
Member Author

Build 20260910.31 failed because of two environment-variable races exposed by the class-level test parallelization added in #56036. Neither intersects this PR's Template Engine trie changes.

#56036 added ResourceLock(EnvironmentVariables) to the mutating tests, but the lock only coordinates with readers declaring the same resource. Both failed readers remain unannotated. All individual platform build legs passed; the four red checks are summaries of these two Helix failures.

@JeremyKuhne
JeremyKuhne enabled auto-merge (squash) September 11, 2026 15:38
@JeremyKuhne
JeremyKuhne merged commit 5bca416 into dotnet:main Sep 11, 2026
23 of 26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants