From e8f03f49eb539e68225e149062f38e58b5092435 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 25 Jun 2024 13:45:15 -0700 Subject: [PATCH 1/2] Reduce allocations in Razor's DirectiveVisitor Noticed in customer trace, ~35% of allocations over a nearly two minute period are in razor's sourcegenerator's call to the TagHelperDirectiveVisitor. As the DirectiveVisitor construction and usage are scoped to the calling method (DefaultRazorTagHelperContextDiscoveryPhase.ExecuteCore), it's easy enough to make the DirectiveVisitor disposable and to make the DirectiveVisitor use a pooled array for it's TagHelperDescriptor array. --- ...aultRazorTagHelperContextDiscoveryPhase.cs | 42 ++++++++++++------- 1 file changed, 27 insertions(+), 15 deletions(-) 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 7d458653948..a7cfe710074 100644 --- a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs @@ -69,6 +69,8 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument) var context = TagHelperDocumentContext.Create(tagHelperPrefix, matches.ToImmutableArray()); codeDocument.SetTagHelperContext(context); codeDocument.SetPreTagHelperSyntaxTree(syntaxTree); + + visitor.Dispose(); } private static bool MatchesDirective(TagHelperDescriptor descriptor, string typePattern, string assemblyName) @@ -106,32 +108,34 @@ private static ReadOnlySpan RemoveGlobalPrefix(in ReadOnlySpan span) return span; } - internal abstract class DirectiveVisitor(HashSet matches) : SyntaxWalker + internal abstract class DirectiveVisitor(HashSet matches) : SyntaxWalker, IDisposable { protected readonly HashSet Matches = matches; public abstract string TagHelperPrefix { get; } public abstract void Visit(RazorSyntaxTree tree); + + public abstract void Dispose(); } internal sealed class TagHelperDirectiveVisitor : DirectiveVisitor { - private readonly List _tagHelpers; + private readonly PooledObject.Builder> _tagHelpers; private string _tagHelperPrefix; public TagHelperDirectiveVisitor(IReadOnlyList tagHelpers, HashSet matches) : base(matches) { // We don't want to consider components in a view document. - _tagHelpers = new(); + _tagHelpers = ArrayBuilderPool.GetPooledObject(); for (var i = 0; i < tagHelpers.Count; i++) { var tagHelper = tagHelpers[i]; if (!tagHelper.IsAnyComponentDocumentTagHelper()) { - _tagHelpers.Add(tagHelper); + _tagHelpers.Object.Add(tagHelper); } } } @@ -165,13 +169,13 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - if (!AssemblyContainsTagHelpers(addTagHelper.AssemblyName, _tagHelpers)) + if (!AssemblyContainsTagHelpers(addTagHelper.AssemblyName, _tagHelpers.Object)) { // No tag helpers in the assembly. continue; } - foreach (var tagHelper in _tagHelpers) + foreach (var tagHelper in _tagHelpers.Object) { if (MatchesDirective(tagHelper, addTagHelper.TypePattern, addTagHelper.AssemblyName)) { @@ -189,13 +193,13 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } - if (!AssemblyContainsTagHelpers(removeTagHelper.AssemblyName, _tagHelpers)) + if (!AssemblyContainsTagHelpers(removeTagHelper.AssemblyName, _tagHelpers.Object)) { // No tag helpers in the assembly. continue; } - foreach (var tagHelper in _tagHelpers) + foreach (var tagHelper in _tagHelpers.Object) { if (MatchesDirective(tagHelper, removeTagHelper.TypePattern, removeTagHelper.AssemblyName)) { @@ -217,7 +221,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } } - private static bool AssemblyContainsTagHelpers(string assemblyName, List tagHelpers) + private static bool AssemblyContainsTagHelpers(string assemblyName, ImmutableArray.Builder tagHelpers) { foreach (var tagHelper in tagHelpers) { @@ -229,11 +233,16 @@ private static bool AssemblyContainsTagHelpers(string assemblyName, List _notFullyQualifiedComponents; + private readonly PooledObject.Builder> _notFullyQualifiedComponents; private readonly string _filePath; private RazorSourceDocument _source; @@ -242,7 +251,7 @@ public ComponentDirectiveVisitor(string filePath, IReadOnlyList(capacity: tagHelpers.Count); + _notFullyQualifiedComponents = ArrayBuilderPool.GetPooledObject(); for (var i = 0; i < tagHelpers.Count; i++) { @@ -260,7 +269,7 @@ public ComponentDirectiveVisitor(string filePath, IReadOnlyList Date: Tue, 25 Jun 2024 16:12:14 -0700 Subject: [PATCH 2/2] Switch from ArrayBuilderPool to ListPool --- ...aultRazorTagHelperContextDiscoveryPhase.cs | 61 +++++++++++-------- 1 file changed, 35 insertions(+), 26 deletions(-) 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 a7cfe710074..7957b1a66dc 100644 --- a/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs +++ b/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs @@ -53,24 +53,25 @@ protected override void ExecuteCore(RazorCodeDocument codeDocument) visitor = new TagHelperDirectiveVisitor(descriptors, matches); } - if (codeDocument.GetImportSyntaxTrees() is { IsDefault: false } imports) + using (visitor) { - foreach (var import in imports) + if (codeDocument.GetImportSyntaxTrees() is { IsDefault: false } imports) { - visitor.Visit(import); + foreach (var import in imports) + { + visitor.Visit(import); + } } - } - visitor.Visit(syntaxTree); + visitor.Visit(syntaxTree); - // This will always be null for a component document. - var tagHelperPrefix = visitor.TagHelperPrefix; + // This will always be null for a component document. + var tagHelperPrefix = visitor.TagHelperPrefix; - var context = TagHelperDocumentContext.Create(tagHelperPrefix, matches.ToImmutableArray()); - codeDocument.SetTagHelperContext(context); - codeDocument.SetPreTagHelperSyntaxTree(syntaxTree); - - visitor.Dispose(); + var context = TagHelperDocumentContext.Create(tagHelperPrefix, matches.ToImmutableArray()); + codeDocument.SetTagHelperContext(context); + codeDocument.SetPreTagHelperSyntaxTree(syntaxTree); + } } private static bool MatchesDirective(TagHelperDescriptor descriptor, string typePattern, string assemblyName) @@ -121,21 +122,21 @@ internal abstract class DirectiveVisitor(HashSet matches) : internal sealed class TagHelperDirectiveVisitor : DirectiveVisitor { - private readonly PooledObject.Builder> _tagHelpers; + private List _tagHelpers; private string _tagHelperPrefix; public TagHelperDirectiveVisitor(IReadOnlyList tagHelpers, HashSet matches) : base(matches) { // We don't want to consider components in a view document. - _tagHelpers = ArrayBuilderPool.GetPooledObject(); + _tagHelpers = ListPool.Default.Get(); for (var i = 0; i < tagHelpers.Count; i++) { var tagHelper = tagHelpers[i]; if (!tagHelper.IsAnyComponentDocumentTagHelper()) { - _tagHelpers.Object.Add(tagHelper); + _tagHelpers.Add(tagHelper); } } } @@ -169,13 +170,13 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) continue; } - if (!AssemblyContainsTagHelpers(addTagHelper.AssemblyName, _tagHelpers.Object)) + if (!AssemblyContainsTagHelpers(addTagHelper.AssemblyName, _tagHelpers)) { // No tag helpers in the assembly. continue; } - foreach (var tagHelper in _tagHelpers.Object) + foreach (var tagHelper in _tagHelpers) { if (MatchesDirective(tagHelper, addTagHelper.TypePattern, addTagHelper.AssemblyName)) { @@ -193,13 +194,13 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } - if (!AssemblyContainsTagHelpers(removeTagHelper.AssemblyName, _tagHelpers.Object)) + if (!AssemblyContainsTagHelpers(removeTagHelper.AssemblyName, _tagHelpers)) { // No tag helpers in the assembly. continue; } - foreach (var tagHelper in _tagHelpers.Object) + foreach (var tagHelper in _tagHelpers) { if (MatchesDirective(tagHelper, removeTagHelper.TypePattern, removeTagHelper.AssemblyName)) { @@ -221,7 +222,7 @@ public override void VisitRazorDirective(RazorDirectiveSyntax node) } } - private static bool AssemblyContainsTagHelpers(string assemblyName, ImmutableArray.Builder tagHelpers) + private static bool AssemblyContainsTagHelpers(string assemblyName, List tagHelpers) { foreach (var tagHelper in tagHelpers) { @@ -236,13 +237,17 @@ private static bool AssemblyContainsTagHelpers(string assemblyName, ImmutableArr public override void Dispose() { - _tagHelpers.Dispose(); + if (_tagHelpers != null) + { + ListPool.Default.Return(_tagHelpers); + _tagHelpers = null; + } } } internal sealed class ComponentDirectiveVisitor : DirectiveVisitor { - private readonly PooledObject.Builder> _notFullyQualifiedComponents; + private List _notFullyQualifiedComponents; private readonly string _filePath; private RazorSourceDocument _source; @@ -251,7 +256,7 @@ public ComponentDirectiveVisitor(string filePath, IReadOnlyList.GetPooledObject(); + _notFullyQualifiedComponents = ListPool.Default.Get(); for (var i = 0; i < tagHelpers.Count; i++) { @@ -269,7 +274,7 @@ public ComponentDirectiveVisitor(string filePath, IReadOnlyList.Default.Return(_notFullyQualifiedComponents); + _notFullyQualifiedComponents = null; + } } } }