From 02ca58a2746ae2a88c30793b42ce04649b12c8d9 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 22 May 2025 11:23:38 -0700 Subject: [PATCH 1/2] Reduce costs in ComponentDirectiveVisitor.VisitRazorDirective Customer feedback ticket shows ~40% of CPU costs are in this method. Of the 32 seconds of CPU time in this method, 21 seconds are in it's calls to GetTypeNamespace and GetSpanWithoutGlobalPrefix. VisitRazorDirective is called many times (we'll call that number 'l'), depending on the number of directives in the page. Each call costs O(m x n) where m is the number of import statements in the page and n is the number of directives that aren't fully qualified. Evidently, for this customer on this page, this O(l * m * n) is too much. 1) GetTypeNamespace -- Reduced to O(n) calls by storing it's result in _nonFullyQualifiedComponents 2) GetSpanWithoutGlobalPrefix -- Reduced to O(l * m) by moving outside the innnermost loop Fixes: https://developercommunity.visualstudio.com/t/Razor-Development:-30-Second-CPU-Churn-A/10904811 --- .../DefaultRazorTagHelperBinderPhaseTest.cs | 16 ++++++-- ...aultRazorTagHelperContextDiscoveryPhase.cs | 40 ++++++++++--------- 2 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs b/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs index abea6f12392..932ed347b63 100644 --- a/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs +++ b/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs @@ -1255,11 +1255,17 @@ @using static SomeProject.SomeOtherFolder.Foo [InlineData("Project.Foo", "global::Project.Bar", false)] [InlineData("Project.Bar.Foo", "Project", false)] [InlineData("Bar.Foo", "Project", false)] - public void IsTypeInNamespace_WorksAsExpected(string typeName, string @namespace, bool expected) + public void IsTypeNamespaceInNormalizedNamespace_WorksAsExpected(string typeName, string @namespace, bool expected) { // Arrange & Act var descriptor = CreateComponentDescriptor(typeName, typeName, "Test.dll"); - var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeInNamespace(descriptor, @namespace); + + // Remove global:: prefix from namespace. + var normalizedNamespaceSpan = DefaultRazorTagHelperContextDiscoveryPhase.GetSpanWithoutGlobalPrefix(@namespace); + + var typeNamespace = descriptor.GetTypeNamespace(); + + var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeNamespaceInNormalizedNamespace(typeNamespace, normalizedNamespaceSpan); // Assert Assert.Equal(expected, result); @@ -1273,11 +1279,13 @@ public void IsTypeInNamespace_WorksAsExpected(string typeName, string @namespace [InlineData("Project.Foo", "Project.Bar", true)] [InlineData("Project.Bar.Foo", "Project", false)] [InlineData("Bar.Foo", "Project", false)] - public void IsTypeInScope_WorksAsExpected(string typeName, string currentNamespace, bool expected) + public void IsTypeNamespaceInScope_WorksAsExpected(string typeName, string currentNamespace, bool expected) { // Arrange & Act var descriptor = CreateComponentDescriptor(typeName, typeName, "Test.dll"); - var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeInScope(descriptor, currentNamespace); + var tagHelperTypeNamespace = descriptor.GetTypeNamespace(); + + var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeNamespaceInScope(tagHelperTypeNamespace, currentNamespace); // Assert Assert.Equal(expected, result); diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs index e349364cd1f..b46e4850497 100644 --- a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs @@ -58,7 +58,7 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, Cancellation codeDocument.SetPreTagHelperSyntaxTree(syntaxTree); } - private static ReadOnlySpan GetSpanWithoutGlobalPrefix(string s) + internal static ReadOnlySpan GetSpanWithoutGlobalPrefix(string s) { const string globalPrefix = "global::"; @@ -334,7 +334,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) internal sealed class ComponentDirectiveVisitor : DirectiveVisitor { - private readonly List _nonFullyQualifiedComponents = []; + private readonly List<(TagHelperDescriptor TagHelper, string TypeNamespace)> _nonFullyQualifiedComponents = []; private string? _filePath; private RazorSourceDocument? _source; @@ -363,7 +363,8 @@ public void Initialize(string filePath, IReadOnlyList tagHe continue; } - _nonFullyQualifiedComponents.Add(tagHelper); + var tagHelperTypeNamespace = tagHelper.GetTypeNamespace(); + _nonFullyQualifiedComponents.Add((tagHelper, tagHelperTypeNamespace)); if (currentNamespace is null) { @@ -374,12 +375,12 @@ public void Initialize(string filePath, IReadOnlyList tagHe { // If this is a child content tag helper, we want to add it if it's original type is in scope. // E.g, if the type name is `Test.MyComponent.ChildContent`, we want to add it if `Test.MyComponent` is in scope. - if (IsTypeInScope(tagHelper, currentNamespace)) + if (IsTypeNamespaceInScope(tagHelperTypeNamespace, currentNamespace)) { AddMatch(tagHelper); } } - else if (IsTypeInScope(tagHelper, currentNamespace)) + else if (IsTypeNamespaceInScope(tagHelperTypeNamespace, currentNamespace)) { // Also, if the type is already in scope of the document's namespace, using isn't necessary. AddMatch(tagHelper); @@ -455,7 +456,15 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - foreach (var tagHelper in _nonFullyQualifiedComponents) + if (_nonFullyQualifiedComponents.Count == 0) + { + // There aren't any non qualified components to add + continue; + } + + // Remove global:: prefix from namespace. + var normalizedNamespaceSpan = GetSpanWithoutGlobalPrefix(@namespace); + foreach (var (tagHelper, tagHelperTypeNamespace) in _nonFullyQualifiedComponents) { Debug.Assert(!tagHelper.IsComponentFullyQualifiedNameMatch, "We've already processed these."); @@ -463,12 +472,12 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) { // If this is a child content tag helper, we want to add it if it's original type is in scope of the given namespace. // E.g, if the type name is `Test.MyComponent.ChildContent`, we want to add it if `Test.MyComponent` is in this namespace. - if (IsTypeInNamespace(tagHelper, @namespace)) + if (IsTypeNamespaceInNormalizedNamespace(tagHelperTypeNamespace, normalizedNamespaceSpan)) { AddMatch(tagHelper); } } - else if (IsTypeInNamespace(tagHelper, @namespace)) + else if (IsTypeNamespaceInNormalizedNamespace(tagHelperTypeNamespace, normalizedNamespaceSpan)) { // If the type is at the top-level or if the type's namespace matches the using's namespace, add it. AddMatch(tagHelper); @@ -480,31 +489,24 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } } - internal static bool IsTypeInNamespace(TagHelperDescriptor tagHelper, string @namespace) + internal static bool IsTypeNamespaceInNormalizedNamespace(string typeNamespace, ReadOnlySpan namespaceWithoutGlobalPrefix) { - var typeNamespace = tagHelper.GetTypeNamespace(); - if (typeNamespace.IsNullOrEmpty()) { // Either the typeName is not the full type name or this type is at the top level. return true; } - // Remove global:: prefix from namespace. - var normalizedNamespaceSpan = GetSpanWithoutGlobalPrefix(@namespace); - - return normalizedNamespaceSpan.Equals(typeNamespace.AsSpan(), StringComparison.Ordinal); + return namespaceWithoutGlobalPrefix.Equals(typeNamespace.AsSpan(), StringComparison.Ordinal); } - // Check if the given type is already in scope given the namespace of the current document. + // Check if a type's namespace is already in scope given the namespace of the current document. // E.g, // If the namespace of the document is `MyComponents.Components.Shared`, // then the types `MyComponents.FooComponent`, `MyComponents.Components.BarComponent`, `MyComponents.Components.Shared.BazComponent` are all in scope. // Whereas `MyComponents.SomethingElse.OtherComponent` is not in scope. - internal static bool IsTypeInScope(TagHelperDescriptor tagHelper, string @namespace) + internal static bool IsTypeNamespaceInScope(string typeNamespace, string @namespace) { - var typeNamespace = tagHelper.GetTypeNamespace(); - if (typeNamespace.IsNullOrEmpty()) { // Either the typeName is not the full type name or this type is at the top level. From 01dfeb63059b6fad90100f20b2ab17c31521998c Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 27 May 2025 13:05:38 -0700 Subject: [PATCH 2/2] Don't walk all tagHelpers at the innermost loop. Instead, do a single search for those with a matching namespace and add those taghelper descriptors. --- .../DefaultRazorTagHelperBinderPhaseTest.cs | 26 ----- ...aultRazorTagHelperContextDiscoveryPhase.cs | 108 ++++++++++-------- .../Language/ReadOnlyMemoryOfCharComparer.cs | 42 +++++++ 3 files changed, 101 insertions(+), 75 deletions(-) create mode 100644 src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/ReadOnlyMemoryOfCharComparer.cs diff --git a/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs b/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs index 932ed347b63..b50d579fc1d 100644 --- a/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs +++ b/src/Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs @@ -1245,32 +1245,6 @@ @using static SomeProject.SomeOtherFolder.Foo Assert.Same(componentDescriptor, result); } - [Theory] - [InlineData("", "", true)] - [InlineData("Foo", "Project", true)] - [InlineData("Project.Foo", "Project", true)] - [InlineData("Project.Foo", "global::Project", true)] - [InlineData("Project.Bar.Foo", "Project.Bar", true)] - [InlineData("Project.Foo", "Project.Bar", false)] - [InlineData("Project.Foo", "global::Project.Bar", false)] - [InlineData("Project.Bar.Foo", "Project", false)] - [InlineData("Bar.Foo", "Project", false)] - public void IsTypeNamespaceInNormalizedNamespace_WorksAsExpected(string typeName, string @namespace, bool expected) - { - // Arrange & Act - var descriptor = CreateComponentDescriptor(typeName, typeName, "Test.dll"); - - // Remove global:: prefix from namespace. - var normalizedNamespaceSpan = DefaultRazorTagHelperContextDiscoveryPhase.GetSpanWithoutGlobalPrefix(@namespace); - - var typeNamespace = descriptor.GetTypeNamespace(); - - var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeNamespaceInNormalizedNamespace(typeNamespace, normalizedNamespaceSpan); - - // Assert - Assert.Equal(expected, result); - } - [Theory] [InlineData("", "", true)] [InlineData("Foo", "Project", true)] diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs index b46e4850497..2a57f87eeb4 100644 --- a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs @@ -58,18 +58,18 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, Cancellation codeDocument.SetPreTagHelperSyntaxTree(syntaxTree); } - internal static ReadOnlySpan GetSpanWithoutGlobalPrefix(string s) + internal static ReadOnlyMemory GetMemoryWithoutGlobalPrefix(string s) { const string globalPrefix = "global::"; - var span = s.AsSpan(); + var mem = s.AsMemory(); - if (span.StartsWith(globalPrefix.AsSpan(), StringComparison.Ordinal)) + if (mem.Span.StartsWith(globalPrefix.AsSpan(), StringComparison.Ordinal)) { - return span[globalPrefix.Length..]; + return mem[globalPrefix.Length..]; } - return span; + return mem; } internal abstract class DirectiveVisitor : SyntaxWalker @@ -241,7 +241,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - switch (GetSpanWithoutGlobalPrefix(addTagHelper.TypePattern)) + switch (GetMemoryWithoutGlobalPrefix(addTagHelper.TypePattern).Span) { case ['*']: AddMatches(nonComponentTagHelpers); @@ -287,7 +287,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - switch (GetSpanWithoutGlobalPrefix(removeTagHelper.TypePattern)) + switch (GetMemoryWithoutGlobalPrefix(removeTagHelper.TypePattern).Span) { case ['*']: RemoveMatches(nonComponentTagHelpers); @@ -334,7 +334,9 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) internal sealed class ComponentDirectiveVisitor : DirectiveVisitor { - private readonly List<(TagHelperDescriptor TagHelper, string TypeNamespace)> _nonFullyQualifiedComponents = []; + // The list values in this dictionary are pooled + private readonly Dictionary, List> _typeNamespaceToNonFullyQualifiedComponents = new Dictionary, List>(ReadOnlyMemoryOfCharComparer.Instance); + private List? _nonFullyQualifiedComponentsWithEmptyTypeNamespace; private string? _filePath; private RazorSourceDocument? _source; @@ -347,6 +349,7 @@ public void Initialize(string filePath, IReadOnlyList tagHe Debug.Assert(!IsInitialized); _filePath = filePath; + _nonFullyQualifiedComponentsWithEmptyTypeNamespace = ListPool.Default.Get(); foreach (var tagHelper in tagHelpers.AsEnumerable()) { @@ -363,26 +366,31 @@ public void Initialize(string filePath, IReadOnlyList tagHe continue; } - var tagHelperTypeNamespace = tagHelper.GetTypeNamespace(); - _nonFullyQualifiedComponents.Add((tagHelper, tagHelperTypeNamespace)); + var tagHelperTypeNamespace = tagHelper.GetTypeNamespace().AsMemory(); - if (currentNamespace is null) + if (tagHelperTypeNamespace.IsEmpty) { - continue; + _nonFullyQualifiedComponentsWithEmptyTypeNamespace.Add(tagHelper); } - - if (tagHelper.IsChildContentTagHelper) + else { - // If this is a child content tag helper, we want to add it if it's original type is in scope. - // E.g, if the type name is `Test.MyComponent.ChildContent`, we want to add it if `Test.MyComponent` is in scope. - if (IsTypeNamespaceInScope(tagHelperTypeNamespace, currentNamespace)) + if (!_typeNamespaceToNonFullyQualifiedComponents.TryGetValue(tagHelperTypeNamespace, out var tagHelpersList)) { - AddMatch(tagHelper); + tagHelpersList = ListPool.Default.Get(); + _typeNamespaceToNonFullyQualifiedComponents.Add(tagHelperTypeNamespace, tagHelpersList); } + + tagHelpersList.Add(tagHelper); + } + + if (currentNamespace is null) + { + continue; } - else if (IsTypeNamespaceInScope(tagHelperTypeNamespace, currentNamespace)) + + if (IsTypeNamespaceInScope(tagHelperTypeNamespace.Span, currentNamespace)) { - // Also, if the type is already in scope of the document's namespace, using isn't necessary. + // If the type is already in scope of the document's namespace, using isn't necessary. AddMatch(tagHelper); } } @@ -392,7 +400,18 @@ public void Initialize(string filePath, IReadOnlyList tagHe public override void Reset() { - _nonFullyQualifiedComponents.Clear(); + if (_nonFullyQualifiedComponentsWithEmptyTypeNamespace != null) + { + ListPool.Default.Return(_nonFullyQualifiedComponentsWithEmptyTypeNamespace); + _nonFullyQualifiedComponentsWithEmptyTypeNamespace = null; + } + + foreach (var (_, tagHelpers) in _typeNamespaceToNonFullyQualifiedComponents) + { + ListPool.Default.Return(tagHelpers); + } + + _typeNamespaceToNonFullyQualifiedComponents.Clear(); _filePath = null; _source = null; @@ -408,6 +427,8 @@ public override void Visit(RazorSyntaxTree tree) public override void VisitRazorDirective(RazorDirectiveSyntax node) { + var componentsWithEmptyTypeNamespace = _nonFullyQualifiedComponentsWithEmptyTypeNamespace.AssumeNotNull(); + var descendantLiterals = node.DescendantNodes(); foreach (var child in descendantLiterals) { @@ -456,30 +477,30 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - if (_nonFullyQualifiedComponents.Count == 0) + if (_typeNamespaceToNonFullyQualifiedComponents.Count == 0 && componentsWithEmptyTypeNamespace.Count == 0) { // There aren't any non qualified components to add continue; } - // Remove global:: prefix from namespace. - var normalizedNamespaceSpan = GetSpanWithoutGlobalPrefix(@namespace); - foreach (var (tagHelper, tagHelperTypeNamespace) in _nonFullyQualifiedComponents) + // Add all tag helpers that have an empty type namespace + foreach (var tagHelper in componentsWithEmptyTypeNamespace) { Debug.Assert(!tagHelper.IsComponentFullyQualifiedNameMatch, "We've already processed these."); - if (tagHelper.IsChildContentTagHelper) - { - // If this is a child content tag helper, we want to add it if it's original type is in scope of the given namespace. - // E.g, if the type name is `Test.MyComponent.ChildContent`, we want to add it if `Test.MyComponent` is in this namespace. - if (IsTypeNamespaceInNormalizedNamespace(tagHelperTypeNamespace, normalizedNamespaceSpan)) - { - AddMatch(tagHelper); - } - } - else if (IsTypeNamespaceInNormalizedNamespace(tagHelperTypeNamespace, normalizedNamespaceSpan)) + AddMatch(tagHelper); + } + + // Remove global:: prefix from namespace. + var normalizedNamespace = GetMemoryWithoutGlobalPrefix(@namespace); + + // Add all tag helpers with a matching namespace + if (_typeNamespaceToNonFullyQualifiedComponents.TryGetValue(normalizedNamespace, out var tagHelpers)) + { + foreach (var tagHelper in tagHelpers) { - // If the type is at the top-level or if the type's namespace matches the using's namespace, add it. + Debug.Assert(!tagHelper.IsComponentFullyQualifiedNameMatch, "We've already processed these."); + AddMatch(tagHelper); } } @@ -489,25 +510,14 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } } - internal static bool IsTypeNamespaceInNormalizedNamespace(string typeNamespace, ReadOnlySpan namespaceWithoutGlobalPrefix) - { - if (typeNamespace.IsNullOrEmpty()) - { - // Either the typeName is not the full type name or this type is at the top level. - return true; - } - - return namespaceWithoutGlobalPrefix.Equals(typeNamespace.AsSpan(), StringComparison.Ordinal); - } - // Check if a type's namespace is already in scope given the namespace of the current document. // E.g, // If the namespace of the document is `MyComponents.Components.Shared`, // then the types `MyComponents.FooComponent`, `MyComponents.Components.BarComponent`, `MyComponents.Components.Shared.BazComponent` are all in scope. // Whereas `MyComponents.SomethingElse.OtherComponent` is not in scope. - internal static bool IsTypeNamespaceInScope(string typeNamespace, string @namespace) + internal static bool IsTypeNamespaceInScope(ReadOnlySpan typeNamespace, string @namespace) { - if (typeNamespace.IsNullOrEmpty()) + if (typeNamespace.IsEmpty) { // Either the typeName is not the full type name or this type is at the top level. return true; diff --git a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/ReadOnlyMemoryOfCharComparer.cs b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/ReadOnlyMemoryOfCharComparer.cs new file mode 100644 index 00000000000..802b2fd23bb --- /dev/null +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/ReadOnlyMemoryOfCharComparer.cs @@ -0,0 +1,42 @@ +// 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; +using Microsoft.Extensions.Internal; + +namespace Microsoft.AspNetCore.Razor.Language; + +internal sealed class ReadOnlyMemoryOfCharComparer : IEqualityComparer> +{ + public static readonly ReadOnlyMemoryOfCharComparer Instance = new ReadOnlyMemoryOfCharComparer(); + + private ReadOnlyMemoryOfCharComparer() + { + } + + public static bool Equals(ReadOnlySpan x, ReadOnlyMemory y) + => x.SequenceEqual(y.Span); + + public bool Equals(ReadOnlyMemory x, ReadOnlyMemory y) + => x.Span.SequenceEqual(y.Span); + + public int GetHashCode(ReadOnlyMemory memory) + { +#if NET + return string.GetHashCode(memory.Span); +#else + // We don't rely on ReadOnlyMemory.GetHashCode() because it includes + // the index and length, but we just want a hash based on the characters. + var hashCombiner = HashCodeCombiner.Start(); + + foreach (var ch in memory.Span) + { + hashCombiner.Add(ch); + } + + return hashCombiner.CombinedHash; +#endif + } +}