From 1ff0978f62d521668a04cba8e14985a8b158cfca Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 16 Aug 2021 13:12:42 -0700 Subject: [PATCH] Remove some low hanging allocations * [x] In the ordinary case, diagnostics aren't produced by builders. The current pattern results in allocating a HashSet and an empty array for this case. Removing some of these allocations * [x] ViewComponentTagHelperPass does not need to operate on .razor files, but does some unnecessary work. We can avoid it --- .../CreateNewOnMetadataUpdateAttributePass.cs | 5 ++- .../src/ModelExpressionPass.cs | 9 ++++- .../src/ViewComponentTagHelperPass.cs | 9 ++++- .../test/ModelExpressionPassTest.cs | 7 +++- ...DefaultAllowedChildTagDescriptorBuilder.cs | 22 +++++----- .../DefaultBoundAttributeDescriptorBuilder.cs | 40 ++++++++++++------- ...oundAttributeParameterDescriptorBuilder.cs | 23 +++++++---- ...faultRequiredAttributeDescriptorBuilder.cs | 30 +++++++++----- ...DefaultTagMatchingRuleDescriptorBuilder.cs | 30 +++++++++----- .../src/HtmlConventions.cs | 20 ++++++++-- 10 files changed, 132 insertions(+), 63 deletions(-) diff --git a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/CreateNewOnMetadataUpdateAttributePass.cs b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/CreateNewOnMetadataUpdateAttributePass.cs index cfac39e14293..9c81968d164b 100644 --- a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/CreateNewOnMetadataUpdateAttributePass.cs +++ b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/CreateNewOnMetadataUpdateAttributePass.cs @@ -16,9 +16,10 @@ internal sealed class CreateNewOnMetadataUpdateAttributePass : IntermediateNodeP { protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode) { - if (FileKinds.IsComponent(codeDocument.GetFileKind())) + if (documentNode.DocumentKind != RazorPageDocumentClassifierPass.RazorPageDocumentKind && + documentNode.DocumentKind != MvcViewDocumentClassifierPass.MvcViewDocumentKind) { - // Hot reload does not apply to components. + // Not a MVC file. Skip. return; } diff --git a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ModelExpressionPass.cs b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ModelExpressionPass.cs index ff4490563a0e..6bbceb73f878 100644 --- a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ModelExpressionPass.cs +++ b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ModelExpressionPass.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; @@ -15,6 +15,13 @@ public class ModelExpressionPass : IntermediateNodePassBase, IRazorOptimizationP protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode) { + if (documentNode.DocumentKind != RazorPageDocumentClassifierPass.RazorPageDocumentKind && + documentNode.DocumentKind != MvcViewDocumentClassifierPass.MvcViewDocumentKind) + { + // Not a MVC file. Skip. + return; + } + var visitor = new Visitor(); visitor.Visit(documentNode); } diff --git a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ViewComponentTagHelperPass.cs b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ViewComponentTagHelperPass.cs index cc0b873e060a..e9b2191d44fe 100644 --- a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ViewComponentTagHelperPass.cs +++ b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/src/ViewComponentTagHelperPass.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System.Collections.Generic; @@ -15,6 +15,13 @@ public class ViewComponentTagHelperPass : IntermediateNodePassBase, IRazorOptimi protected override void ExecuteCore(RazorCodeDocument codeDocument, DocumentIntermediateNode documentNode) { + if (documentNode.DocumentKind != RazorPageDocumentClassifierPass.RazorPageDocumentKind && + documentNode.DocumentKind != MvcViewDocumentClassifierPass.MvcViewDocumentKind) + { + // Not a MVC file. Skip. + return; + } + var @namespace = documentNode.FindPrimaryNamespace(); var @class = documentNode.FindPrimaryClass(); if (@namespace == null || @class == null) diff --git a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/ModelExpressionPassTest.cs b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/ModelExpressionPassTest.cs index 3465e29ef591..9f79d631a573 100644 --- a/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/ModelExpressionPassTest.cs +++ b/src/Razor/Microsoft.AspNetCore.Mvc.Razor.Extensions/test/ModelExpressionPassTest.cs @@ -170,7 +170,10 @@ private DocumentIntermediateNode CreateIRDocument(RazorEngine engine, RazorCodeD } } - return codeDocument.GetDocumentIntermediateNode(); + var irNode = codeDocument.GetDocumentIntermediateNode(); + irNode.DocumentKind = MvcViewDocumentClassifierPass.MvcViewDocumentKind; + + return irNode; } private TagHelperIntermediateNode FindTagHelperNode(IntermediateNode node) @@ -205,4 +208,4 @@ public override void VisitTagHelper(TagHelperIntermediateNode node) } } } -} \ No newline at end of file +} diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultAllowedChildTagDescriptorBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultAllowedChildTagDescriptorBuilder.cs index eeb5de244218..595f782e5206 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultAllowedChildTagDescriptorBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultAllowedChildTagDescriptorBuilder.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; @@ -36,10 +36,10 @@ public override RazorDiagnosticCollection Diagnostics public AllowedChildTagDescriptor Build() { - var validationDiagnostics = Validate(); - var diagnostics = new HashSet(validationDiagnostics); + var diagnostics = Validate(); if (_diagnostics != null) { + diagnostics ??= new(); diagnostics.UnionWith(_diagnostics); } @@ -47,31 +47,35 @@ public AllowedChildTagDescriptor Build() var descriptor = new DefaultAllowedChildTagDescriptor( Name, displayName, - diagnostics.ToArray()); + diagnostics?.ToArray() ?? Array.Empty()); return descriptor; } - private IEnumerable Validate() + private HashSet Validate() { + HashSet diagnostics = null; if (string.IsNullOrWhiteSpace(Name)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidRestrictedChildNullOrWhitespace(_parent.GetDisplayName()); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else if (Name != TagHelperMatchingConventions.ElementCatchAllName) { foreach (var character in Name) { - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidRestrictedChild(_parent.GetDisplayName(), Name, character); - - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } + + return diagnostics; } } } diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeDescriptorBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeDescriptorBuilder.cs index 8bae2309d7a3..a9b9eb456e22 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeDescriptorBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeDescriptorBuilder.cs @@ -92,10 +92,10 @@ public override void BindAttributeParameter(Action(validationDiagnostics); + var diagnostics = Validate(); if (_diagnostics != null) { + diagnostics ??= new(); diagnostics.UnionWith(_diagnostics); } @@ -125,7 +125,7 @@ public BoundAttributeDescriptor Build() CaseSensitive, parameters, new Dictionary(Metadata), - diagnostics.ToArray()) + diagnostics?.ToArray() ?? Array.Empty()) { IsEditorRequired = IsEditorRequired, }; @@ -159,13 +159,15 @@ private string GetDisplayName() return Name; } - private IEnumerable Validate() + private HashSet Validate() { // data-* attributes are explicitly not implemented by user agents and are not intended for use on // the server; therefore it's invalid for TagHelpers to bind to them. const string DataDashPrefix = "data-"; var isDirectiveAttribute = this.IsDirectiveAttribute(); + HashSet diagnostics = null; + if (string.IsNullOrWhiteSpace(Name)) { if (IndexerAttributeNamePrefix == null) @@ -174,7 +176,8 @@ private IEnumerable Validate() _parent.GetDisplayName(), GetDisplayName()); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } else @@ -186,7 +189,8 @@ private IEnumerable Validate() GetDisplayName(), Name); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } StringSegment name = Name; @@ -201,13 +205,14 @@ private IEnumerable Validate() GetDisplayName(), Name); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } for (var i = 0; i < name.Length; i++) { var character = name[i]; - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidBoundAttributeName( _parent.GetDisplayName(), @@ -215,7 +220,8 @@ private IEnumerable Validate() name.Value, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } @@ -229,7 +235,8 @@ private IEnumerable Validate() GetDisplayName(), IndexerAttributeNamePrefix); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else if (IndexerAttributeNamePrefix.Length > 0 && string.IsNullOrWhiteSpace(IndexerAttributeNamePrefix)) { @@ -237,7 +244,8 @@ private IEnumerable Validate() _parent.GetDisplayName(), GetDisplayName()); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else { @@ -253,13 +261,14 @@ private IEnumerable Validate() GetDisplayName(), indexerPrefix.Value); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } for (var i = 0; i < indexerPrefix.Length; i++) { var character = indexerPrefix[i]; - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidBoundAttributePrefix( _parent.GetDisplayName(), @@ -267,11 +276,14 @@ private IEnumerable Validate() indexerPrefix.Value, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } } + + return diagnostics; } private void EnsureAttributeParameterBuilders() diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeParameterDescriptorBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeParameterDescriptorBuilder.cs index c93115fe4536..82670124298b 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeParameterDescriptorBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultBoundAttributeParameterDescriptorBuilder.cs @@ -1,6 +1,7 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Collections.Generic; using System.Linq; @@ -51,10 +52,10 @@ public override RazorDiagnosticCollection Diagnostics public BoundAttributeParameterDescriptor Build() { - var validationDiagnostics = Validate(); - var diagnostics = new HashSet(validationDiagnostics); + var diagnostics = Validate(); if (_diagnostics != null) { + diagnostics ??= new(); diagnostics.UnionWith(_diagnostics); } var descriptor = new DefaultBoundAttributeParameterDescriptor( @@ -66,7 +67,7 @@ public BoundAttributeParameterDescriptor Build() GetDisplayName(), CaseSensitive, new Dictionary(Metadata), - diagnostics.ToArray()); + diagnostics?.ToArray() ?? Array.Empty()); return descriptor; } @@ -81,28 +82,34 @@ private string GetDisplayName() return $":{Name}"; } - private IEnumerable Validate() + private HashSet Validate() { + HashSet diagnostics = null; if (string.IsNullOrWhiteSpace(Name)) { + var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidBoundAttributeParameterNullOrWhitespace(_parent.Name); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else { foreach (var character in Name) { - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidBoundAttributeParameterName( _parent.Name, Name, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } + + return diagnostics; } } } diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRequiredAttributeDescriptorBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRequiredAttributeDescriptorBuilder.cs index 9e1b98d10767..93f714d5ef08 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRequiredAttributeDescriptorBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultRequiredAttributeDescriptorBuilder.cs @@ -45,10 +45,10 @@ public override RazorDiagnosticCollection Diagnostics public RequiredAttributeDescriptor Build() { - var validationDiagnostics = Validate(); - var diagnostics = new HashSet(validationDiagnostics); + var diagnostics = Validate(); if (_diagnostics != null) { + diagnostics ??= new(); diagnostics.UnionWith(_diagnostics); } @@ -60,7 +60,7 @@ public RequiredAttributeDescriptor Build() Value, ValueComparisonMode, displayName, - diagnostics.ToArray(), + diagnostics?.ToArray() ?? Array.Empty(), new Dictionary(Metadata)); return rule; @@ -71,39 +71,47 @@ private string GetDisplayName() return NameComparisonMode == RequiredAttributeDescriptor.NameComparisonMode.PrefixMatch ? string.Concat(Name, "...") : Name; } - private IEnumerable Validate() + private HashSet Validate() { + HashSet diagnostics = null; + if (string.IsNullOrWhiteSpace(Name)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedAttributeNameNullOrWhitespace(); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else { - var name = Name; + var name = new StringSegment(Name); var isDirectiveAttribute = this.IsDirectiveAttribute(); if (isDirectiveAttribute && name.StartsWith("@", StringComparison.Ordinal)) { - name = name.Substring(1); + name = name.Subsegment(1); } else if (isDirectiveAttribute) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidRequiredDirectiveAttributeName(GetDisplayName(), Name); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } - foreach (var character in name) + for (var i = 0; i < name.Length; i++) { - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + var character = name[i]; + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedAttributeName(Name, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } + + return diagnostics; } } } diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultTagMatchingRuleDescriptorBuilder.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultTagMatchingRuleDescriptorBuilder.cs index f17769422e18..944ecc800a45 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultTagMatchingRuleDescriptorBuilder.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/DefaultTagMatchingRuleDescriptorBuilder.cs @@ -1,4 +1,4 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; @@ -65,10 +65,10 @@ public override void Attribute(Action config public TagMatchingRuleDescriptor Build() { - var validationDiagnostics = Validate(); - var diagnostics = new HashSet(validationDiagnostics); + var diagnostics = Validate(); if (_diagnostics != null) { + diagnostics ??= new(); diagnostics.UnionWith(_diagnostics); } @@ -90,28 +90,32 @@ public TagMatchingRuleDescriptor Build() TagStructure, CaseSensitive, requiredAttributes, - diagnostics.ToArray()); + diagnostics?.ToArray() ?? Array.Empty()); return rule; } - private IEnumerable Validate() + private HashSet Validate() { + HashSet diagnostics = null; + if (string.IsNullOrWhiteSpace(TagName)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedTagNameNullOrWhitespace(); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else if (TagName != TagHelperMatchingConventions.ElementCatchAllName) { foreach (var character in TagName) { - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedTagName(TagName, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } @@ -122,21 +126,25 @@ private IEnumerable Validate() { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedParentTagNameNullOrWhitespace(); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } else { foreach (var character in ParentTag) { - if (char.IsWhiteSpace(character) || HtmlConventions.InvalidNonWhitespaceHtmlCharacters.Contains(character)) + if (char.IsWhiteSpace(character) || HtmlConventions.IsInvalidNonWhitespaceHtmlCharacters(character)) { var diagnostic = RazorDiagnosticFactory.CreateTagHelper_InvalidTargetedParentTagName(ParentTag, character); - yield return diagnostic; + diagnostics ??= new(); + diagnostics.Add(diagnostic); } } } } + + return diagnostics; } private void EnsureRequiredAttributeBuilders() diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/HtmlConventions.cs b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/HtmlConventions.cs index 29e99c2df1c2..c73e8fc8e617 100644 --- a/src/Razor/Microsoft.AspNetCore.Razor.Language/src/HtmlConventions.cs +++ b/src/Razor/Microsoft.AspNetCore.Razor.Language/src/HtmlConventions.cs @@ -1,8 +1,7 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; using System.Text.RegularExpressions; namespace Microsoft.AspNetCore.Razor.Language @@ -10,6 +9,8 @@ namespace Microsoft.AspNetCore.Razor.Language public static class HtmlConventions { private const string HtmlCaseRegexReplacement = "-$1$2"; + private static readonly char[] InvalidNonWhitespaceHtmlCharacters = + new[] { '@', '!', '<', '/', '?', '[', '>', ']', '=', '"', '\'', '*' }; // This matches the following AFTER the start of the input string (MATCH). // Any letter/number followed by an uppercase letter then lowercase letter: 1(Aa), a(Aa), A(Aa) @@ -21,8 +22,19 @@ public static class HtmlConventions RegexOptions.None, TimeSpan.FromMilliseconds(500)); - internal static IReadOnlyCollection InvalidNonWhitespaceHtmlCharacters { get; } = new List( - new[] { '@', '!', '<', '/', '?', '[', '>', ']', '=', '"', '\'', '*' }); + + internal static bool IsInvalidNonWhitespaceHtmlCharacters(char testChar) + { + foreach (var c in InvalidNonWhitespaceHtmlCharacters) + { + if (c == testChar) + { + return true; + } + } + + return false; + } /// /// Converts from pascal/camel case to lower kebab-case.