Several changes targeted to improving perf of RazorSyntaxTree.Parse (dotnet/aspnetcore-tooling#1874)

* Several changes targeted to improving perf of RazorSyntaxTree.Parse

1) Modify ParserHelpers.IsNewLine to use a switch instead of Array.IndexOf
2) Modify Tokenizer.CreateToken to take in an array of RazorDiagnostics rather than an IReadOnlyList as that was causing a ToArray call on an empty diagnostics very often (during a SyntaxFactory.Token call)
3) Modify TokenizerBackedParser.Putback to allow an IReadOnlyList as a parameter to not require creation of a reverse enumerator.
4) Cut down allocations in HtmlMarkupParser.GetParserState by:
    a) Using an IReadOnlyList instead of IEnumerable to get rid of the allocations from the .any calls
    b) Don't allocate while reading initial spacing
    c) Inline the IsSpacingToken code so cut down on code executed and need to allocate a separate Func
5) Modify CSharpCodeParser.IsSpacingToken to now be a set of methods instead of a single method that allocates a Func. This is a very high traffic method.
6) Implement a fairly rudimentary Whitespace token cache, as they can be reused. This was based off Roslyn's SyntaxNodeCache, but simplified significantly. It's probably worth investigating whether you should more fully embrance token caching outside of whitespace.

* PR feedback and added one more optimization in LocateOwner that's been bugging me for years. Assuming all chidlren are contained within a nodes span, we can short-circuit the DFS this code was doing significantly cutting time in this method which is important as it's exercised on the main thread during typing.

* missed a space

* StringTextToSnapshot's switch to IsNewLine needed to use start as the index to begin the search, not zero.\n\nCommit migrated from 45411f7526
This commit is contained in:
Todd Grunke 2020-05-09 06:06:13 -07:00 committed by GitHub
parent ed7338c15a
commit b0d819f1e8
14 changed files with 155 additions and 57 deletions

View File

@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
});
private static readonly Func<SyntaxToken, bool> IsValidStatementSpacingToken =
IsSpacingToken(includeNewLines: true, includeComments: true);
IsSpacingTokenIncludingNewLinesAndComments;
internal static readonly DirectiveDescriptor AddTagHelperDirectiveDescriptor = DirectiveDescriptor.CreateDirective(
SyntaxConstants.CSharp.AddTagHelperKeyword,
@ -124,7 +124,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
NextToken();
var precedingWhitespace = ReadWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
var precedingWhitespace = ReadWhile(IsSpacingTokenIncludingNewLinesAndComments);
// We are usually called when the other parser sees a transition '@'. Look for it.
SyntaxToken transitionToken = null;
@ -1317,7 +1317,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
if (At(SyntaxKind.Whitespace))
{
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
if (tokenDescriptor.Kind == DirectiveTokenKind.Member ||
tokenDescriptor.Kind == DirectiveTokenKind.Namespace ||
@ -1443,7 +1443,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
directiveBuilder.Add(OutputTokensAsStatementLiteral());
}
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
SpanContext.ChunkGenerator = SpanChunkGenerator.Null;
switch (descriptor.Kind)
@ -1455,7 +1455,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
TryAccept(SyntaxKind.Semicolon);
directiveBuilder.Add(OutputAsMetaCode(Output(), AcceptedCharactersInternal.Whitespace));
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
if (At(SyntaxKind.NewLine))
{
@ -1478,7 +1478,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
directiveBuilder.Add(OutputAsMarkupEphemeralLiteral());
break;
case DirectiveKind.RazorBlock:
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.AllWhitespace;
directiveBuilder.Add(OutputTokensAsUnclassifiedLiteral());
@ -1502,7 +1502,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
});
break;
case DirectiveKind.CodeBlock:
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.AllWhitespace;
directiveBuilder.Add(OutputTokensAsUnclassifiedLiteral());
@ -1753,7 +1753,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
AcceptAndMoveNext();
// Accept 1 or more spaces between the await and the following code.
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
// Top level basically indicates if we're within an expression or statement.
// Ex: topLevel true = @await Foo() | topLevel false = @{ await Foo(); }
@ -1806,12 +1806,12 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
private void ParseConditionalBlock(in SyntaxListBuilder<RazorSyntaxNode> builder, Block block)
{
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
// Parse the condition, if present (if not present, we'll let the C# compiler complain)
if (TryParseCondition(builder))
{
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
ParseExpectedCodeBlock(builder, block);
}
@ -1874,7 +1874,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
Assert(SyntaxKind.Keyword);
var block = new Block(CurrentToken, CurrentStart);
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
ParseExpectedCodeBlock(builder, block);
}
@ -1937,7 +1937,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
var block = new Block(CurrentToken, CurrentStart);
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
if (At(CSharpKeyword.If))
{
// ElseIf
@ -2059,7 +2059,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
Accept(whitespace);
Assert(CSharpKeyword.While);
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
if (TryParseCondition(builder) && TryAccept(SyntaxKind.Semicolon))
{
SpanContext.EditHandler.AcceptedCharacters = AcceptedCharactersInternal.None;
@ -2078,7 +2078,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
var topLevel = transition != null;
var block = new Block(CurrentToken, CurrentStart);
var usingToken = EatCurrentToken();
var whitespaceOrComments = ReadWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
var whitespaceOrComments = ReadWhile(IsSpacingTokenIncludingComments);
var atLeftParen = At(SyntaxKind.LeftParenthesis);
var atIdentifier = At(SyntaxKind.Identifier);
var atStatic = At(CSharpKeyword.Static);
@ -2115,7 +2115,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
builder.Add(transition);
}
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
ParseStandardStatement(builder);
}
else
@ -2132,7 +2132,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
}
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
}
if (topLevel)
@ -2145,7 +2145,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
Assert(CSharpKeyword.Using);
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
Assert(SyntaxKind.LeftParenthesis);
if (transition != null)
@ -2156,7 +2156,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
// Parse condition
if (TryParseCondition(builder))
{
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
// Parse code block
ParseExpectedCodeBlock(builder, block);
@ -2174,14 +2174,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
AcceptAndMoveNext();
var isStatic = false;
var nonNamespaceTokenCount = TokenBuilder.Count;
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
var start = CurrentStart;
if (At(SyntaxKind.Identifier))
{
// non-static using
nonNamespaceTokenCount = TokenBuilder.Count;
TryParseNamespaceOrTypeName(directiveBuilder);
var whitespace = ReadWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
var whitespace = ReadWhile(IsSpacingTokenIncludingNewLinesAndComments);
if (At(SyntaxKind.Assign))
{
// Alias
@ -2189,7 +2189,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
Assert(SyntaxKind.Assign);
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingNewLinesAndComments);
// One more namespace or type name
TryParseNamespaceOrTypeName(directiveBuilder);
@ -2205,7 +2205,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
// static using
isStatic = true;
AcceptAndMoveNext();
AcceptWhile(IsSpacingToken(includeNewLines: false, includeComments: true));
AcceptWhile(IsSpacingTokenIncludingComments);
nonNamespaceTokenCount = TokenBuilder.Count;
TryParseNamespaceOrTypeName(directiveBuilder);
}
@ -2391,7 +2391,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
while (!EndOfFile)
{
var whitespace = ReadWhile(IsSpacingToken(includeNewLines: true, includeComments: true));
var whitespace = ReadWhile(IsSpacingTokenIncludingNewLinesAndComments);
if (At(SyntaxKind.RazorCommentTransition))
{
Accept(whitespace);
@ -2622,11 +2622,24 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
result.Value == keyword;
}
protected static Func<SyntaxToken, bool> IsSpacingToken(bool includeNewLines, bool includeComments)
{
return token => token.Kind == SyntaxKind.Whitespace ||
(includeNewLines && token.Kind == SyntaxKind.NewLine) ||
(includeComments && token.Kind == SyntaxKind.CSharpComment);
protected static bool IsSpacingToken(SyntaxToken token)
{
return token.Kind == SyntaxKind.Whitespace;
}
protected static bool IsSpacingTokenIncludingNewLines(SyntaxToken token)
{
return IsSpacingToken(token) || token.Kind == SyntaxKind.NewLine;
}
protected static bool IsSpacingTokenIncludingComments(SyntaxToken token)
{
return IsSpacingToken(token) || token.Kind == SyntaxKind.CSharpComment;
}
protected static bool IsSpacingTokenIncludingNewLinesAndComments(SyntaxToken token)
{
return IsSpacingTokenIncludingNewLines(token) || token.Kind == SyntaxKind.CSharpComment;
}
protected class Block

View File

@ -75,7 +75,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return new CSharpTokenizer(source);
}
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, IReadOnlyList<RazorDiagnostic> errors)
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, RazorDiagnostic[] errors)
{
return SyntaxFactory.Token(kind, content, errors);
}

View File

@ -344,7 +344,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return base.GetTokenContent(type);
}
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, IReadOnlyList<RazorDiagnostic> errors)
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, RazorDiagnostic [] errors)
{
return SyntaxFactory.Token(kind, content, errors);
}

View File

@ -120,7 +120,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
}
}
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, IReadOnlyList<RazorDiagnostic> errors)
protected override SyntaxToken CreateToken(string content, SyntaxKind kind, RazorDiagnostic [] errors)
{
return SyntaxFactory.Token(kind, content, errors);
}

View File

@ -1718,12 +1718,33 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return false;
}
private IReadOnlyList<SyntaxToken> FastReadWhitespaceAndNewLines()
{
if (EnsureCurrent() && (CurrentToken.Kind == SyntaxKind.Whitespace || CurrentToken.Kind == SyntaxKind.NewLine))
{
var whitespaceTokens = new List<SyntaxToken>();
whitespaceTokens.Add(CurrentToken);
NextToken();
while (EnsureCurrent() && (CurrentToken.Kind == SyntaxKind.Whitespace || CurrentToken.Kind == SyntaxKind.NewLine))
{
whitespaceTokens.Add(CurrentToken);
NextToken();
}
return whitespaceTokens;
}
return Array.Empty<SyntaxToken>();
}
private ParserState GetParserState(ParseMode mode)
{
var whitespace = ReadWhile(IsSpacingToken(includeNewLines: true));
var whitespace = FastReadWhitespaceAndNewLines();
try
{
if (!whitespace.Any() && EndOfFile)
if (whitespace.Count == 0 && EndOfFile)
{
return ParserState.EOF;
}
@ -1742,7 +1763,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
// Let the transition parser handle the preceding whitespace.
return ParserState.CodeTransition;
}
else if (whitespace.Any())
else if (whitespace.Count > 0)
{
// This whitespace isn't sensitive to what comes after it.
return ParserState.Misc;
@ -1792,7 +1813,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
}
finally
{
if (whitespace.Any())
if (whitespace.Count > 0)
{
PutCurrentBack();
PutBack(whitespace);

View File

@ -37,7 +37,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
get { return SyntaxKind.RazorCommentStar; }
}
protected override SyntaxToken CreateToken(string content, SyntaxKind type, IReadOnlyList<RazorDiagnostic> errors)
protected override SyntaxToken CreateToken(string content, SyntaxKind type, RazorDiagnostic[] errors)
{
return SyntaxFactory.Token(type, content, errors);
}

View File

@ -103,6 +103,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return type == KnownTokenType.Unknown || !Equals(GetKnownTokenType(type), GetKnownTokenType(KnownTokenType.Unknown));
}
protected abstract SyntaxToken CreateToken(string content, SyntaxKind type, IReadOnlyList<RazorDiagnostic> errors);
protected abstract SyntaxToken CreateToken(string content, SyntaxKind type, RazorDiagnostic[] errors);
}
}

View File

@ -95,14 +95,19 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
return null;
}
if (node.EndPosition < change.Span.AbsoluteIndex)
{
// no need to look into this node as it completely precedes the change
return null;
}
if (IsSpanKind(node))
{
var editHandler = node.GetSpanContext()?.EditHandler ?? SpanEditHandler.CreateDefault();
return editHandler.OwnsChange(node, change) ? node : null;
}
SyntaxNode owner = null;
IEnumerable<SyntaxNode> children;
IReadOnlyList<SyntaxNode> children;
if (node is MarkupStartTagSyntax startTag)
{
children = startTag.Children;
@ -124,8 +129,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
children = node.ChildNodes();
}
foreach (var child in children)
SyntaxNode owner = null;
for (int i = 0; i < children.Count; i++)
{
var child = children[i];
owner = LocateOwner(child, change);
if (owner != null)
{

View File

@ -10,16 +10,20 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
{
internal static class ParserHelpers
{
public static char[] NewLineCharacters = new[]
public static bool IsNewLine(char value)
{
'\r', // Carriage return
'\n', // Linefeed
'\u0085', // Next Line
'\u2028', // Line separator
'\u2029' // Paragraph separator
};
switch (value)
{
case '\r': // Carriage return
case '\n': // Linefeed
case '\u0085': // Next Line
case '\u2028': // Line separator
case '\u2029': // Paragraph separator
return true;
}
public static bool IsNewLine(char value) => Array.IndexOf<char>(NewLineCharacters, value) != -1;
return false;
}
public static bool IsNewLine(string value)
{

View File

@ -63,7 +63,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
public SourceLocation CurrentStart { get; private set; }
protected abstract SyntaxToken CreateToken(string content, SyntaxKind type, IReadOnlyList<RazorDiagnostic> errors);
protected abstract SyntaxToken CreateToken(string content, SyntaxKind type, RazorDiagnostic [] errors);
protected abstract StateResult Dispatch();

View File

@ -185,6 +185,14 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
}
}
protected internal void PutBack(IReadOnlyList<SyntaxToken> tokens)
{
for (int i = tokens.Count - 1; i >= 0; i--)
{
PutBack(tokens[i]);
}
}
protected internal void PutCurrentBack()
{
if (!EndOfFile && CurrentToken != null)

View File

@ -1,20 +1,19 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.Collections.Generic;
using System.Linq;
using System;
namespace Microsoft.AspNetCore.Razor.Language.Syntax.InternalSyntax
{
internal static partial class SyntaxFactory
{
internal static SyntaxToken Token(SyntaxKind kind, string content, IEnumerable<RazorDiagnostic> diagnostics)
{
return Token(kind, content, diagnostics.ToArray());
}
internal static SyntaxToken Token(SyntaxKind kind, string content, params RazorDiagnostic[] diagnostics)
{
if (kind == SyntaxKind.Whitespace && diagnostics.Length == 0)
{
return WhitespaceTokenCache.GetToken(content);
}
return new SyntaxToken(kind, content, diagnostics);
}

View File

@ -0,0 +1,46 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System;
namespace Microsoft.AspNetCore.Razor.Language.Syntax.InternalSyntax
{
// Simplified version of Roslyn's SyntaxNodeCache
internal static class WhitespaceTokenCache
{
private const int CacheSizeBits = 8;
private const int CacheSize = 1 << CacheSizeBits;
private const int CacheMask = CacheSize - 1;
private static readonly Entry[] s_cache = new Entry[CacheSize];
private struct Entry
{
public int Hash { get; }
public SyntaxToken Token { get; }
internal Entry(int hash, SyntaxToken token)
{
Hash = hash;
Token = token;
}
}
public static SyntaxToken GetToken(string content)
{
var hash = content.GetHashCode();
var idx = hash & CacheMask;
var e = s_cache[idx];
if (e.Hash == hash && e.Token?.Content == content)
{
return e.Token;
}
var token = new SyntaxToken(SyntaxKind.Whitespace, content, Array.Empty<RazorDiagnostic>());
s_cache[idx] = new Entry(hash, token);
return token;
}
}
}

View File

@ -191,7 +191,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy
protected override SyntaxToken CreateToken(
string content,
SyntaxKind type,
IReadOnlyList<RazorDiagnostic> errors)
RazorDiagnostic[] errors)
{
throw new NotImplementedException();
}