From 904fd19605ea9fcb395cdab90ba2b69d6844775f Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 11 Jun 2020 18:11:29 -0700 Subject: [PATCH] A couple more perf optimizations focused around memory allocations. This is the last of the easy wins that I could find for typing in large razor files (minus the logged bug to move the divergence checker code off the UI thread). In the profile for the large document editing, these changes reduce allocated memory during RazorSyntaxTree.Parse by about 25%. CPU wise, the win isn't quite as dramatic, only a couple percent improvement under RazorSyntaxTree.Parse. --- .../src/Legacy/CSharpTokenizer.cs | 11 ++- .../src/Legacy/LineTrackingStringBuffer.cs | 82 ++++++++++--------- .../src/Legacy/TokenizerBackedParser.cs | 78 ++++++++++-------- 3 files changed, 93 insertions(+), 78 deletions(-) diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/CSharpTokenizer.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/CSharpTokenizer.cs index 625ccb1741..6f61367580 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/CSharpTokenizer.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/CSharpTokenizer.cs @@ -554,13 +554,16 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return Transition(CSharpTokenizerState.Data, EndToken(SyntaxKind.StringLiteral)); } - private StateResult QuotedCharacterLiteral() => QuotedLiteral('\'', SyntaxKind.CharacterLiteral); + private StateResult QuotedCharacterLiteral() => QuotedLiteral('\'', IsEndQuotedCharacterLiteral, SyntaxKind.CharacterLiteral); - private StateResult QuotedStringLiteral() => QuotedLiteral('\"', SyntaxKind.StringLiteral); + private StateResult QuotedStringLiteral() => QuotedLiteral('\"', IsEndQuotedStringLiteral, SyntaxKind.StringLiteral); - private StateResult QuotedLiteral(char quote, SyntaxKind literalType) + private Func IsEndQuotedCharacterLiteral = (c) => c == '\\' || c == '\'' || ParserHelpers.IsNewLine(c); + private Func IsEndQuotedStringLiteral = (c) => c == '\\' || c == '\"' || ParserHelpers.IsNewLine(c); + + private StateResult QuotedLiteral(char quote, Func isEndQuotedLiteral, SyntaxKind literalType) { - TakeUntil(c => c == '\\' || c == quote || ParserHelpers.IsNewLine(c)); + TakeUntil(isEndQuotedLiteral); if (CurrentCharacter == '\\') { TakeCurrent(); // Take the '\' diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs index eb3c20c8c2..2af16c0481 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/LineTrackingStringBuffer.cs @@ -13,7 +13,6 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private readonly IList _lines; private readonly string _filePath; private TextLine _currentLine; - private TextLine _endLine; public LineTrackingStringBuffer(string content, string filePath) : this(content.ToCharArray(), filePath) @@ -22,28 +21,27 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy public LineTrackingStringBuffer(char[] content, string filePath) { - _endLine = new TextLine(0, 0); - _lines = new List() { _endLine }; + _lines = new List(); - Append(content); + BuildTextLines(content); _filePath = filePath; } public int Length { - get { return _endLine.End; } + get { return _lines[_lines.Count - 1].End; } } public SourceLocation EndLocation { - get { return new SourceLocation(_filePath, Length, _lines.Count - 1, _lines[_lines.Count - 1].Length); } + get { return new SourceLocation(_filePath, Length, _lines.Count - 1, Length); } } public CharacterReference CharAt(int absoluteIndex) { var line = FindLine(absoluteIndex); - if (line == null) + if (line.IsDefault) { throw new ArgumentOutOfRangeException(nameof(absoluteIndex)); } @@ -51,38 +49,38 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy return new CharacterReference(line.Content[idx], new SourceLocation(_filePath, absoluteIndex, line.Index, idx)); } - private void Append(char[] content) + private void BuildTextLines(char[] content) { + string lineText; + var lineStart = 0; + for (int i = 0; i < content.Length; i++) { - AppendCore(content[i]); - - // \r on it's own: Start a new line, otherwise wait for \n - // Other Newline: Start a new line - if ((content[i] == '\r' && (i + 1 == content.Length || content[i + 1] != '\n')) || (content[i] != '\r' && ParserHelpers.IsNewLine(content[i]))) + if (ParserHelpers.IsNewLine(content[i])) { - PushNewLine(); + // \r on it's own: Start a new line, otherwise wait for \n + // Other Newline: Start a new line + if (content[i] == '\r' && i + 1 < content.Length && content[i + 1] == '\n') + { + i++; + } + + lineText = new string(content, lineStart, (i - lineStart) + 1); // +1 to include the current char + _lines.Add(new TextLine(lineStart, _lines.Count, lineText)); + + lineStart = i + 1; } } - } - private void PushNewLine() - { - _endLine = new TextLine(_endLine.End, _endLine.Index + 1); - _lines.Add(_endLine); - } - - private void AppendCore(char chr) - { - Debug.Assert(_lines.Count > 0); - _lines[_lines.Count - 1].Content.Append(chr); + lineText = new string(content, lineStart, content.Length - lineStart); // no +1 as content.Length points past the last char already + _lines.Add(new TextLine(lineStart, _lines.Count, lineText)); } private TextLine FindLine(int absoluteIndex) { - TextLine selected = null; + TextLine selected; - if (_currentLine == null) + if (_currentLine.IsDefault) { // Scan from line 0 selected = ScanLines(absoluteIndex, 0, _lines.Count); @@ -104,6 +102,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy selected = ScanLines(absoluteIndex, _currentLine.Index, _lines.Count); } } + else + { + selected = default; + } } else if (absoluteIndex < _currentLine.Start) { @@ -122,6 +124,10 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy selected = ScanLines(absoluteIndex, 0, _currentLine.Index); } } + else + { + selected = default; + } } else { @@ -129,7 +135,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy selected = _currentLine; } - Debug.Assert(selected == null || selected.Contains(absoluteIndex)); + Debug.Assert(selected.IsDefault || selected.Contains(absoluteIndex)); _currentLine = selected; return selected; } @@ -159,7 +165,7 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy } } - return null; + return default; } internal struct CharacterReference @@ -175,28 +181,26 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy public SourceLocation Location { get; } } - private class TextLine + private struct TextLine { - private StringBuilder _content = new StringBuilder(); - - public TextLine(int start, int index) + public TextLine(int start, int index, string content) { Start = start; Index = index; + Content = content; } - public StringBuilder Content - { - get { return _content; } - } + public string Content { get; } + + public bool IsDefault => Content == null; public int Length { get { return Content.Length; } } - public int Start { get; set; } - public int Index { get; set; } + public int Start { get; } + public int Index { get; } public int End { diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/TokenizerBackedParser.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/TokenizerBackedParser.cs index 00e49d87ec..ca374e0c1e 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/TokenizerBackedParser.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/Legacy/TokenizerBackedParser.cs @@ -16,25 +16,25 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy private readonly TokenizerView _tokenizer; private SyntaxListBuilder? _tokenBuilder; - // Following four high traffic methods cached as using method groups would cause allocation on every invocation. - protected static readonly Func IsSpacingToken = (token) => - { - return token.Kind == SyntaxKind.Whitespace; - }; - - protected static readonly Func IsSpacingTokenIncludingNewLines = (token) => - { - return IsSpacingToken(token) || token.Kind == SyntaxKind.NewLine; - }; - - protected static readonly Func IsSpacingTokenIncludingComments = (token) => - { - return IsSpacingToken(token) || token.Kind == SyntaxKind.CSharpComment; - }; - - protected static readonly Func IsSpacingTokenIncludingNewLinesAndComments = (token) => - { - return IsSpacingTokenIncludingNewLines(token) || token.Kind == SyntaxKind.CSharpComment; + // Following four high traffic methods cached as using method groups would cause allocation on every invocation. + protected static readonly Func IsSpacingToken = (token) => + { + return token.Kind == SyntaxKind.Whitespace; + }; + + protected static readonly Func IsSpacingTokenIncludingNewLines = (token) => + { + return IsSpacingToken(token) || token.Kind == SyntaxKind.NewLine; + }; + + protected static readonly Func IsSpacingTokenIncludingComments = (token) => + { + return IsSpacingToken(token) || token.Kind == SyntaxKind.CSharpComment; + }; + + protected static readonly Func IsSpacingTokenIncludingNewLinesAndComments = (token) => + { + return IsSpacingTokenIncludingNewLines(token) || token.Kind == SyntaxKind.CSharpComment; }; protected TokenizerBackedParser(LanguageCharacteristics language, ParserContext context) @@ -224,7 +224,19 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy protected internal bool NextIs(SyntaxKind type) { - return NextIs(token => token != null && type == token.Kind); + // Duplicated logic with NextIs(Func...) to prevent allocation + var cur = CurrentToken; + var result = false; + if (NextToken()) + { + result = (type == CurrentToken.Kind); + PutCurrentBack(); + } + + PutBack(cur); + EnsureCurrent(); + + return result; } protected internal bool NextIs(params SyntaxKind[] types) @@ -235,21 +247,17 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy protected internal bool NextIs(Func condition) { var cur = CurrentToken; + var result = false; if (NextToken()) { - var result = condition(CurrentToken); + result = condition(CurrentToken); PutCurrentBack(); - PutBack(cur); - EnsureCurrent(); - return result; - } - else - { - PutBack(cur); - EnsureCurrent(); } - return false; + PutBack(cur); + EnsureCurrent(); + + return result; } protected internal bool Was(SyntaxKind type) @@ -289,11 +297,11 @@ namespace Microsoft.AspNetCore.Razor.Language.Legacy protected internal IReadOnlyList ReadWhile(Func condition) { - if (!EnsureCurrent() || !condition(CurrentToken)) - { - return Array.Empty(); - } - + if (!EnsureCurrent() || !condition(CurrentToken)) + { + return Array.Empty(); + } + var result = new List(); do {