Skip to content

Commit 77042ab

Browse files
Reduce allocations related to SyntaxListBuilder (#9612)
A couple days ago, @ToddGrun pointed me to some [Prism data](https://prism.vsdata.io/failure/?query=ch%3Drelease%20r%3D17.8&eventType=allocation&failureType=dualdirection&failureHash=30151e5e-e8aa-2e4c-a22f-106e4d7f8e6c) indicating that there's a lot of allocation due to `SyntaxListBuilder.Grow(...)`. I dug in and made several changes that should improve things. 1. `SyntaxListBuilder` is now pooled everywhere it's used. The builders contain single arrays that can grow to a maximum of 512 entries. That should eliminate the allocations that we're seeing in `SyntaxListBuilder.Grow(...)`. 2. I introduced a `ChildNodesHelper` struct to avoid boxing `ChildSyntaxList` to an `IReadOnlyList<SyntaxNode>` just to call `Skip(1).ToList()` in a special case. 3. Updated code to avoid boxing `ImmutableArray<SyntaxTree>` to `IReadOnlyList<SyntaxTree>` for imports. 4. Added helpers to avoid calling params arrays. There are some unrelated clean up changes, so it might be useful to review a commit at a time if those changes get in the way: 1. I've annotated `SyntaxNode`, `SyntaxList`, `SyntaxList<T>`, `SyntaxListBuilder` and `SyntaxListBuilder<T>` for nullability. 2. I renamed the misspelled API, `RazorEnginePhaseBase.OnIntialized()` to `RazorEnginePhaseBase.OnInitialized()`. 5. I refactored similar code in `FormattingVisitor` and `ClassifiedSpanVisitor` to be shared. These algorithms were pretty much identical, and I decided to refactor rather than update the code in both places.
2 parents f3d9a0b + af45a17 commit 77042ab

32 files changed

+732
-729
lines changed

src/Compiler/Microsoft.AspNetCore.Razor.Language/src/ClassifiedSpanVisitor.cs

Lines changed: 2 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System;
5-
using System.Collections.Generic;
65
using System.Collections.Immutable;
7-
using System.Diagnostics;
8-
using System.Linq;
96
using Microsoft.AspNetCore.Razor.Language.Legacy;
107
using Microsoft.AspNetCore.Razor.Language.Syntax;
118
using Microsoft.AspNetCore.Razor.PooledObjects;
@@ -148,7 +145,7 @@ public override void VisitMarkupStartTag(MarkupStartTagSyntax node)
148145
{
149146
WriteBlock(node, BlockKindInternal.Tag, n =>
150147
{
151-
var children = GetRewrittenMarkupStartTagChildren(node);
148+
var children = SyntaxUtilities.GetRewrittenMarkupStartTagChildren(node, includeEditHandler: true);
152149
foreach (var child in children)
153150
{
154151
Visit(child);
@@ -160,7 +157,7 @@ public override void VisitMarkupEndTag(MarkupEndTagSyntax node)
160157
{
161158
WriteBlock(node, BlockKindInternal.Tag, n =>
162159
{
163-
var children = GetRewrittenMarkupEndTagChildren(node);
160+
var children = SyntaxUtilities.GetRewrittenMarkupEndTagChildren(node, includeEditHandler: true);
164161
foreach (var child in children)
165162
{
166163
Visit(child);
@@ -342,100 +339,4 @@ private void WriteSpan(SyntaxNode node, SpanKindInternal kind, AcceptedCharacter
342339
var span = new ClassifiedSpanInternal(spanSource, blockSource, kind, _currentBlockKind, acceptedCharacters.Value);
343340
_spans.Add(span);
344341
}
345-
346-
private static SyntaxList<RazorSyntaxNode> GetRewrittenMarkupStartTagChildren(MarkupStartTagSyntax node)
347-
{
348-
// Rewrites the children of the start tag to look like the legacy syntax tree.
349-
if (node.IsMarkupTransition)
350-
{
351-
var tokens = node.DescendantNodes().Where(n => n is SyntaxToken token && !token.IsMissing).Cast<SyntaxToken>().ToArray();
352-
var tokenBuilder = SyntaxListBuilder<SyntaxToken>.Create();
353-
tokenBuilder.AddRange(tokens, 0, tokens.Length);
354-
var markupTransition = SyntaxFactory.MarkupTransition(tokenBuilder.ToList(), node.ChunkGenerator).Green.CreateRed(node, node.Position);
355-
var editHandler = node.GetEditHandler();
356-
if (editHandler != null)
357-
{
358-
markupTransition = markupTransition.WithEditHandler(editHandler);
359-
}
360-
361-
var builder = new SyntaxListBuilder(1);
362-
builder.Add(markupTransition);
363-
return new SyntaxList<RazorSyntaxNode>(builder.ToListNode().CreateRed(node, node.Position));
364-
}
365-
366-
SpanEditHandler? latestEditHandler = null;
367-
var children = node.LegacyChildren;
368-
var newChildren = new SyntaxListBuilder(children.Count);
369-
var literals = new List<MarkupTextLiteralSyntax>();
370-
foreach (var child in children)
371-
{
372-
if (child is MarkupTextLiteralSyntax literal)
373-
{
374-
literals.Add(literal);
375-
latestEditHandler = literal.GetEditHandler() ?? latestEditHandler;
376-
}
377-
else if (child is MarkupMiscAttributeContentSyntax miscContent)
378-
{
379-
foreach (var contentChild in miscContent.Children)
380-
{
381-
if (contentChild is MarkupTextLiteralSyntax contentLiteral)
382-
{
383-
literals.Add(contentLiteral);
384-
latestEditHandler = contentLiteral.GetEditHandler() ?? latestEditHandler;
385-
}
386-
else
387-
{
388-
// Pop stack
389-
AddLiteralIfExists();
390-
newChildren.Add(contentChild);
391-
}
392-
}
393-
}
394-
else
395-
{
396-
AddLiteralIfExists();
397-
newChildren.Add(child);
398-
}
399-
}
400-
401-
AddLiteralIfExists();
402-
403-
return new SyntaxList<RazorSyntaxNode>(newChildren.ToListNode().CreateRed(node, node.Position));
404-
405-
void AddLiteralIfExists()
406-
{
407-
if (literals.Count > 0)
408-
{
409-
var mergedLiteral = SyntaxUtilities.MergeTextLiterals(literals.ToArray());
410-
Debug.Assert(mergedLiteral != null);
411-
mergedLiteral = mergedLiteral!.WithEditHandler(latestEditHandler);
412-
literals.Clear();
413-
latestEditHandler = null;
414-
newChildren.Add(mergedLiteral);
415-
}
416-
}
417-
}
418-
419-
private static SyntaxList<RazorSyntaxNode> GetRewrittenMarkupEndTagChildren(MarkupEndTagSyntax node)
420-
{
421-
// Rewrites the children of the end tag to look like the legacy syntax tree.
422-
if (node.IsMarkupTransition)
423-
{
424-
var tokens = node.DescendantNodes().Where(n => n is SyntaxToken token && !token.IsMissing).Cast<SyntaxToken>().ToArray();
425-
var tokenBuilder = SyntaxListBuilder<SyntaxToken>.Create();
426-
tokenBuilder.AddRange(tokens, 0, tokens.Length);
427-
var markupTransition = SyntaxFactory.MarkupTransition(tokenBuilder.ToList(), node.ChunkGenerator).Green.CreateRed(node, node.Position);
428-
var editHandler = node.GetEditHandler();
429-
if (editHandler != null)
430-
{
431-
markupTransition = markupTransition.WithEditHandler(editHandler);
432-
}
433-
434-
var builder = new SyntaxListBuilder(1);
435-
builder.Add(markupTransition);
436-
return new SyntaxList<RazorSyntaxNode>(builder.ToListNode().CreateRed(node, node.Position));
437-
}
438-
439-
return node.LegacyChildren;
440-
}
441342
}

src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorDirectiveClassifierPhase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ internal class DefaultRazorDirectiveClassifierPhase : RazorEnginePhaseBase, IRaz
1111
{
1212
public IRazorDirectiveClassifierPass[] Passes { get; private set; }
1313

14-
protected override void OnIntialized()
14+
protected override void OnInitialized()
1515
{
1616
Passes = Engine.Features.OfType<IRazorDirectiveClassifierPass>().OrderBy(p => p.Order).ToArray();
1717
}

src/Compiler/Microsoft.AspNetCore.Razor.Language/src/DefaultRazorDocumentClassifierPhase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ internal class DefaultRazorDocumentClassifierPhase : RazorEnginePhaseBase, IRazo
1111
{
1212
public IRazorDocumentClassifierPass[] Passes { get; private set; }
1313

14-
protected override void OnIntialized()
14+
protected override void OnInitialized()
1515
{
1616
Passes = Engine.Features.OfType<IRazorDocumentClassifierPass>().OrderBy(p => p.Order).ToArray();
1717
}

0 commit comments

Comments
 (0)