Skip to content

Reduce costs in ComponentDirectiveVisitor.VisitRazorDirective #11881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
May 28, 2025
Merged

Reduce costs in ComponentDirectiveVisitor.VisitRazorDirective #11881

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02ca58a
Reduce costs in ComponentDirectiveVisitor.VisitRazorDirective
ToddGrun May 22, 2025
01dfeb6
Don't walk all tagHelpers at the innermost loop. Instead, do a single…
ToddGrun May 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 4 additions & 22 deletions ...Compiler/Microsoft.AspNetCore.Razor.Language/test/DefaultRazorTagHelperBinderPhaseTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1245,26 +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 IsTypeInNamespace_WorksAsExpected(string typeName, string @namespace, bool expected)
{
// Arrange & Act
var descriptor = CreateComponentDescriptor(typeName, typeName, "Test.dll");
var result = DefaultRazorTagHelperContextDiscoveryPhase.ComponentDirectiveVisitor.IsTypeInNamespace(descriptor, @namespace);

// Assert
Assert.Equal(expected, result);
}

[Theory]
[InlineData("", "", true)]
[InlineData("Foo", "Project", true)]
Expand All @@ -1273,11 +1253,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);
Expand Down
118 changes: 65 additions & 53 deletions ...ft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,18 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument, Cancellation
codeDocument.SetPreTagHelperSyntaxTree(syntaxTree);
}

private static ReadOnlySpan<char> GetSpanWithoutGlobalPrefix(string s)
internal static ReadOnlyMemory<char> 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
Expand Down Expand Up @@ -241,7 +241,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node)
continue;
}

switch (GetSpanWithoutGlobalPrefix(addTagHelper.TypePattern))
switch (GetMemoryWithoutGlobalPrefix(addTagHelper.TypePattern).Span)
{
case ['*']:
AddMatches(nonComponentTagHelpers);
Expand Down Expand Up @@ -287,7 +287,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node)
continue;
}

switch (GetSpanWithoutGlobalPrefix(removeTagHelper.TypePattern))
switch (GetMemoryWithoutGlobalPrefix(removeTagHelper.TypePattern).Span)
{
case ['*']:
RemoveMatches(nonComponentTagHelpers);
Expand Down Expand Up @@ -334,7 +334,9 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node)

internal sealed class ComponentDirectiveVisitor : DirectiveVisitor
{
private readonly List<TagHelperDescriptor> _nonFullyQualifiedComponents = [];
// The list values in this dictionary are pooled
private readonly Dictionary<ReadOnlyMemory<char>, List<TagHelperDescriptor>> _typeNamespaceToNonFullyQualifiedComponents = new Dictionary<ReadOnlyMemory<char>, List<TagHelperDescriptor>>(ReadOnlyMemoryOfCharComparer.Instance);
private List<TagHelperDescriptor>? _nonFullyQualifiedComponentsWithEmptyTypeNamespace;

private string? _filePath;
private RazorSourceDocument? _source;
Expand All @@ -347,6 +349,7 @@ public void Initialize(string filePath, IReadOnlyList<TagHelperDescriptor> tagHe
Debug.Assert(!IsInitialized);

_filePath = filePath;
_nonFullyQualifiedComponentsWithEmptyTypeNamespace = ListPool<TagHelperDescriptor>.Default.Get();

foreach (var tagHelper in tagHelpers.AsEnumerable())
{
Expand All @@ -363,25 +366,31 @@ public void Initialize(string filePath, IReadOnlyList<TagHelperDescriptor> tagHe
continue;
}

_nonFullyQualifiedComponents.Add(tagHelper);
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 (IsTypeInScope(tagHelper, currentNamespace))
if (!_typeNamespaceToNonFullyQualifiedComponents.TryGetValue(tagHelperTypeNamespace, out var tagHelpersList))
{
AddMatch(tagHelper);
tagHelpersList = ListPool<TagHelperDescriptor>.Default.Get();
_typeNamespaceToNonFullyQualifiedComponents.Add(tagHelperTypeNamespace, tagHelpersList);
}

tagHelpersList.Add(tagHelper);
}
else if (IsTypeInScope(tagHelper, currentNamespace))

if (currentNamespace is null)
{
// Also, if the type is already in scope of the document's namespace, using isn't necessary.
continue;
}

if (IsTypeNamespaceInScope(tagHelperTypeNamespace.Span, currentNamespace))
{
// If the type is already in scope of the document's namespace, using isn't necessary.
AddMatch(tagHelper);
}
}
Expand All @@ -391,7 +400,18 @@ public void Initialize(string filePath, IReadOnlyList<TagHelperDescriptor> tagHe

public override void Reset()
{
_nonFullyQualifiedComponents.Clear();
if (_nonFullyQualifiedComponentsWithEmptyTypeNamespace != null)
{
ListPool<TagHelperDescriptor>.Default.Return(_nonFullyQualifiedComponentsWithEmptyTypeNamespace);
_nonFullyQualifiedComponentsWithEmptyTypeNamespace = null;
}

foreach (var (_, tagHelpers) in _typeNamespaceToNonFullyQualifiedComponents)
{
ListPool<TagHelperDescriptor>.Default.Return(tagHelpers);
}

_typeNamespaceToNonFullyQualifiedComponents.Clear();
_filePath = null;
_source = null;

Expand All @@ -407,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)
{
Expand Down Expand Up @@ -455,22 +477,30 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node)
continue;
}

foreach (var tagHelper in _nonFullyQualifiedComponents)
if (_typeNamespaceToNonFullyQualifiedComponents.Count == 0 && componentsWithEmptyTypeNamespace.Count == 0)
{
// There aren't any non qualified components to add
continue;
}

// 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 (IsTypeInNamespace(tagHelper, @namespace))
{
AddMatch(tagHelper);
}
}
else if (IsTypeInNamespace(tagHelper, @namespace))
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);
}
}
Expand All @@ -480,32 +510,14 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node)
}
}

internal static bool IsTypeInNamespace(TagHelperDescriptor tagHelper, 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.
return true;
}

// Remove global:: prefix from namespace.
var normalizedNamespaceSpan = GetSpanWithoutGlobalPrefix(@namespace);

return normalizedNamespaceSpan.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(ReadOnlySpan<char> typeNamespace, string @namespace)
{
var typeNamespace = tagHelper.GetTypeNamespace();

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;
Expand Down
42 changes: 42 additions & 0 deletions ...mpiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/ReadOnlyMemoryOfCharComparer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<ReadOnlyMemory<char>>
Copy link
Member

@DustinCampbell DustinCampbell May 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving this helper into Microsoft.AspNetCore.Razor.Utilities.Shared.

Copy link
Contributor Author

@ToddGrun ToddGrun May 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally had placed this under utilities, but hit a snag with it needing HashCodeCombiner which wasn't available there. Didn't want to start moving code around as part of this PR, so I just placed it in the most convenient location. Agreed that it should be more global though.

{
public static readonly ReadOnlyMemoryOfCharComparer Instance = new ReadOnlyMemoryOfCharComparer();

private ReadOnlyMemoryOfCharComparer()
{
}

public static bool Equals(ReadOnlySpan<char> x, ReadOnlyMemory<char> y)
=> x.SequenceEqual(y.Span);

public bool Equals(ReadOnlyMemory<char> x, ReadOnlyMemory<char> y)
=> x.Span.SequenceEqual(y.Span);

public int GetHashCode(ReadOnlyMemory<char> memory)
{
#if NET
return string.GetHashCode(memory.Span);
#else
// We don't rely on ReadOnlyMemory<char>.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
}
}
Loading