Provide a simple ArrayBuilder api that does not drop array builders w…

…hen they have a large number of elements in them Search related type sections in other documents when doing a document-filtered nav to search Use null for simplicity Apply suggestions from code review Keep around large array instances for speedometer Use a separate pool Make consistent Revert Expose similar api on string builder
dotnet · Feb 6, 2025 · 42afc4c · 42afc4c
1 parent 45350db
commit 42afc4c
Show file tree

Hide file tree

Showing 15 changed files with 122 additions and 75 deletions.
diff --git a/...eUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs b/...eUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs
@@ -745,10 +745,11 @@ private async Task<bool> ProcessSuppressMessageAttributesAsync(
             return false;
         }
 
-        using var pooledDeclarationNodes = SyntaxFacts.GetTopLevelAndMethodLevelMembers(root);
-        var declarationNodes = pooledDeclarationNodes.Object;
+        // Specifies false for discardLargeInstances as these objects commonly exceed the default ArrayBuilder capacity threshold.
+        using var _1 = ArrayBuilder<SyntaxNode>.GetInstance(discardLargeInstances: false, out var declarationNodes);
+        this.SyntaxFacts.AddTopLevelAndMethodLevelMembers(root, declarationNodes);
 
-        using var _ = PooledHashSet<ISymbol>.GetInstance(out var processedPartialSymbols);
+        using var _2 = PooledHashSet<ISymbol>.GetInstance(out var processedPartialSymbols);
         if (declarationNodes.Count > 0)
         {
             foreach (var node in declarationNodes)

diff --git a/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs b/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs
@@ -11,6 +11,7 @@
 using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
 using Microsoft.CodeAnalysis.Host.Mef;
 using Microsoft.CodeAnalysis.LanguageService;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Microsoft.VisualStudio.Commanding;
@@ -103,12 +104,11 @@ private bool ExecuteCommandImpl(EditorCommandArgs args, bool gotoNextMember, Com
     /// </summary>
     internal static int? GetTargetPosition(ISyntaxFactsService service, SyntaxNode root, int caretPosition, bool next)
     {
-        using var pooledMembers = service.GetMethodLevelMembers(root);
-        var members = pooledMembers.Object;
+        // Specifies false for discardLargeInstances as these objects commonly exceed the default ArrayBuilder capacity threshold.
+        using var _ = ArrayBuilder<SyntaxNode>.GetInstance(discardLargeInstances: false, out var members);
+        service.AddMethodLevelMembers(root, members);
         if (members.Count == 0)
-        {
             return null;
-        }
 
         var starts = members.Select(m => MemberStart(m)).ToArray();
         var index = Array.BinarySearch(starts, caretPosition);

diff --git a/...tures/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs b/...tures/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs
@@ -3,7 +3,6 @@
 // See the LICENSE file in the project root for more information.
 
 using System;
-using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
@@ -201,8 +200,10 @@ async Task ExecuteAnalyzersAsync(
                 }
 
                 var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>();
-                using var pooledMembers = syntaxFacts.GetMethodLevelMembers(root);
-                var members = pooledMembers.Object;
+
+                // Specifies false for discardLargeInstances as these objects commonly exceed the default ArrayBuilder capacity threshold.
+                using var _ = ArrayBuilder<SyntaxNode>.GetInstance(discardLargeInstances: false, out var members);
+                syntaxFacts.AddMethodLevelMembers(root, members);
 
                 var memberSpans = members.SelectAsArray(member => member.FullSpan);
                 var changedMemberId = members.IndexOf(changedMember);

diff --git a/...stics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs b/...stics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs
@@ -7,6 +7,7 @@
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.CodeAnalysis.LanguageService;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Microsoft.CodeAnalysis.Text;
 using Roslyn.Utilities;
@@ -47,8 +48,9 @@ static async Task<ImmutableArray<TextSpan>> CreateMemberSpansAsync(Document docu
                     var service = document.GetRequiredLanguageService<ISyntaxFactsService>();
                     var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false);
 
-                    using var pooledMembers = service.GetMethodLevelMembers(root);
-                    var members = pooledMembers.Object;
+                    // Specifies false for discardLargeInstances as these objects commonly exceed the default ArrayBuilder capacity threshold.
+                    using var _ = ArrayBuilder<SyntaxNode>.GetInstance(discardLargeInstances: false, out var members);
+                    service.AddMethodLevelMembers(root, members);
 
                     return members.SelectAsArray(m => m.FullSpan);
                 }

diff --git a/...es/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs b/...es/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs
@@ -23,13 +23,10 @@
 
 namespace Microsoft.CodeAnalysis.CSharp.LanguageService;
 
-internal class CSharpSyntaxFacts : ISyntaxFacts
+internal class CSharpSyntaxFacts : AbstractSyntaxFacts, ISyntaxFacts
 {
     internal static readonly CSharpSyntaxFacts Instance = new();
 
-    // Specifies false for trimOnFree as these objects commonly exceed the default ObjectPool threshold
-    private static readonly ObjectPool<List<SyntaxNode>> s_syntaxNodeListPool = new ObjectPool<List<SyntaxNode>>(() => [], trimOnFree: false);
-
     protected CSharpSyntaxFacts()
     {
     }
@@ -732,11 +729,7 @@ EnumMemberDeclarationSyntax or
     }
 
     public bool IsTopLevelNodeWithMembers([NotNullWhen(true)] SyntaxNode? node)
-    {
-        return node is BaseNamespaceDeclarationSyntax or
-               TypeDeclarationSyntax or
-               EnumDeclarationSyntax;
-    }
+        => node is BaseNamespaceDeclarationSyntax or BaseTypeDeclarationSyntax;
 
     private const string dotToken = ".";
 
@@ -889,30 +882,10 @@ private static void AppendTypeParameterList(StringBuilder builder, TypeParameter
         }
     }
 
-    public PooledObject<List<SyntaxNode>> GetTopLevelAndMethodLevelMembers(SyntaxNode? root)
-    {
-        var pooledObject = s_syntaxNodeListPool.GetPooledObject();
-        var list = pooledObject.Object;
-
-        AppendMembers(root, list, topLevel: true, methodLevel: true);
-
-        return pooledObject;
-    }
-
-    public PooledObject<List<SyntaxNode>> GetMethodLevelMembers(SyntaxNode? root)
-    {
-        var pooledObject = s_syntaxNodeListPool.GetPooledObject();
-        var list = pooledObject.Object;
-
-        AppendMembers(root, list, topLevel: false, methodLevel: true);
-
-        return pooledObject;
-    }
-
     public SyntaxList<SyntaxNode> GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration)
         => ((TypeDeclarationSyntax)typeDeclaration).Members;
 
-    private void AppendMembers(SyntaxNode? node, List<SyntaxNode> list, bool topLevel, bool methodLevel)
+    protected override void AppendMembers(SyntaxNode? node, ArrayBuilder<SyntaxNode> list, bool topLevel, bool methodLevel)
     {
         Debug.Assert(topLevel || methodLevel);
 

diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/CompilerExtensions.projitems
@@ -433,6 +433,7 @@
     <Compile Include="$(MSBuildThisFileDirectory)Services\SyntaxFacts\AbstractDocumentationCommentService.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Services\FileBannerFacts\AbstractFileBannerFacts.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Services\HeaderFacts\AbstractHeaderFacts.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)Services\SyntaxFacts\AbstractSyntaxFacts.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Services\SyntaxFacts\ExternalSourceInfo.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Services\SyntaxFacts\IAccessibilityFacts.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)Services\SyntaxFacts\IDocumentationCommentService.cs" />

diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/ArrayBuilder.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/ArrayBuilder.cs
@@ -6,11 +6,10 @@ namespace Microsoft.CodeAnalysis.PooledObjects;
 
 internal sealed partial class ArrayBuilder<T> : IPooled
 {
+    private static readonly ObjectPool<ArrayBuilder<T>> s_keepLargeInstancesPool = CreatePool();
+
     public static PooledDisposer<ArrayBuilder<T>> GetInstance(out ArrayBuilder<T> instance)
-    {
-        instance = GetInstance();
-        return new PooledDisposer<ArrayBuilder<T>>(instance);
-    }
+        => GetInstance(discardLargeInstances: true, out instance);
 
     public static PooledDisposer<ArrayBuilder<T>> GetInstance(int capacity, out ArrayBuilder<T> instance)
     {
@@ -23,4 +22,27 @@ public static PooledDisposer<ArrayBuilder<T>> GetInstance(int capacity, T fillWi
         instance = GetInstance(capacity, fillWithValue);
         return new PooledDisposer<ArrayBuilder<T>>(instance);
     }
+
+    public static PooledDisposer<ArrayBuilder<T>> GetInstance(bool discardLargeInstances, out ArrayBuilder<T> instance)
+    {
+        // If we're discarding large instances (the default behavior), then just use the normal pool.  If we're not, use
+        // a specific pool so that *other* normal callers don't accidentally get it and discard it.
+        instance = discardLargeInstances ? GetInstance() : s_keepLargeInstancesPool.Allocate();
+        return new PooledDisposer<ArrayBuilder<T>>(instance, discardLargeInstances);
+    }
+
+    void IPooled.Free(bool discardLargeInstances)
+    {
+        // If we're discarding large instances, use the default behavior (which already does that).  Otherwise, always
+        // clear and free the instance back to its originating pool.
+        if (discardLargeInstances)
+        {
+            Free();
+        }
+        else
+        {
+            this.Clear();
+            _pool?.Free(this);
+        }
+    }
 }
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/IPooled.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/IPooled.cs
@@ -6,5 +6,5 @@ namespace Microsoft.CodeAnalysis.PooledObjects;
 
 internal interface IPooled
 {
-    void Free();
+    void Free(bool discardLargeInstances);
 }
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledDictionary.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledDictionary.cs
@@ -11,4 +11,8 @@ public static PooledDisposer<PooledDictionary<K, V>> GetInstance(out PooledDicti
         instance = GetInstance();
         return new PooledDisposer<PooledDictionary<K, V>>(instance);
     }
+
+    // Nothing special to do here.
+    void IPooled.Free(bool discardLargeInstance)
+        => this.Free();
 }
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledDisposer.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledDisposer.cs
@@ -8,9 +8,9 @@
 namespace Microsoft.CodeAnalysis.PooledObjects;
 
 [NonCopyable]
-internal readonly struct PooledDisposer<TPoolable>(TPoolable instance) : IDisposable
+internal readonly struct PooledDisposer<TPoolable>(TPoolable instance, bool discardLargeInstances = true) : IDisposable
     where TPoolable : class, IPooled
 {
     void IDisposable.Dispose()
-        => instance?.Free();
+        => instance?.Free(discardLargeInstances);
 }
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledHashSet.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledHashSet.cs
@@ -13,4 +13,8 @@ public static PooledDisposer<PooledHashSet<T>> GetInstance(out PooledHashSet<T>
         instance = GetInstance();
         return new PooledDisposer<PooledHashSet<T>>(instance);
     }
+
+    // Nothing special to do here.
+    void IPooled.Free(bool discardLargeInstance)
+        => this.Free();
 }
diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledStringBuilder.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/ObjectPools/PooledStringBuilder.cs
@@ -8,10 +8,32 @@ namespace Microsoft.CodeAnalysis.PooledObjects;
 
 internal sealed partial class PooledStringBuilder : IPooled
 {
+    private static readonly ObjectPool<PooledStringBuilder> s_keepLargeInstancesPool = CreatePool();
+
     public static PooledDisposer<PooledStringBuilder> GetInstance(out StringBuilder instance)
+        => GetInstance(discardLargeInstances: true, out instance);
+
+    public static PooledDisposer<PooledStringBuilder> GetInstance(bool discardLargeInstances, out StringBuilder instance)
     {
-        var pooledInstance = GetInstance();
+        // If we're discarding large instances (the default behavior), then just use the normal pool.  If we're not, use
+        // a specific pool so that *other* normal callers don't accidentally get it and discard it.
+        var pooledInstance = discardLargeInstances ? GetInstance() : s_keepLargeInstancesPool.Allocate();
         instance = pooledInstance;
-        return new PooledDisposer<PooledStringBuilder>(pooledInstance);
+        return new PooledDisposer<PooledStringBuilder>(pooledInstance, discardLargeInstances);
+    }
+
+    void IPooled.Free(bool discardLargeInstances)
+    {
+        // If we're discarding large instances, use the default behavior (which already does that).  Otherwise, always
+        // clear and free the instance back to its originating pool.
+        if (discardLargeInstances)
+        {
+            Free();
+        }
+        else
+        {
+            this.Builder.Clear();
+            _pool.Free(this);
+        }
     }
 }
diff --git a/...es/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractSyntaxFacts.cs b/...es/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/AbstractSyntaxFacts.cs
@@ -0,0 +1,21 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using Microsoft.CodeAnalysis.PooledObjects;
+
+namespace Microsoft.CodeAnalysis.LanguageService;
+
+internal abstract class AbstractSyntaxFacts
+{
+    public void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result)
+        => AppendMembers(root, result, topLevel: true, methodLevel: true);
+
+    public void AddTopLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result)
+        => AppendMembers(root, result, topLevel: true, methodLevel: false);
+
+    public void AddMethodLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result)
+        => AppendMembers(root, result, topLevel: false, methodLevel: true);
+
+    protected abstract void AppendMembers(SyntaxNode? node, ArrayBuilder<SyntaxNode> list, bool topLevel, bool methodLevel);
+}
diff --git a/...orkspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/...orkspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs
@@ -7,6 +7,7 @@
 using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
 using System.Threading;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Text;
 
 namespace Microsoft.CodeAnalysis.LanguageService;
@@ -410,9 +411,12 @@ void GetPartsOfTupleExpression<TArgumentSyntax>(SyntaxNode node,
     SyntaxNode? ConvertToSingleLine(SyntaxNode? node, bool useElasticTrivia = false);
 
     // Violation.  This is a feature level API.
-    PooledObject<List<SyntaxNode>> GetTopLevelAndMethodLevelMembers(SyntaxNode? root);
+    void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result);
     // Violation.  This is a feature level API.
-    PooledObject<List<SyntaxNode>> GetMethodLevelMembers(SyntaxNode? root);
+    void AddTopLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result);
+    // Violation.  This is a feature level API.
+    void AddMethodLevelMembers(SyntaxNode? root, ArrayBuilder<SyntaxNode> result);
+
     SyntaxList<SyntaxNode> GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration);
 
     // Violation.  This is a feature level API.

diff --git a/...tilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/...tilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb
@@ -21,15 +21,13 @@ Imports Microsoft.CodeAnalysis.Editing
 
 Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
     Friend Class VisualBasicSyntaxFacts
+        Inherits AbstractSyntaxFacts
         Implements ISyntaxFacts
 
         Private Const DoesNotExistInVBErrorMessage = "This feature does not exist in VB"
 
         Public Shared ReadOnly Property Instance As New VisualBasicSyntaxFacts
 
-        ' Specifies false for trimOnFree as these objects commonly exceed the default ObjectPool threshold
-        Private Shared ReadOnly s_syntaxNodeListPool As ObjectPool(Of List(Of SyntaxNode)) = New ObjectPool(Of List(Of SyntaxNode))(Function() New List(Of SyntaxNode), trimOnFree:=False)
-
         Protected Sub New()
         End Sub
 
@@ -888,24 +886,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
             Return TextSpan.FromBounds(list.First.SpanStart, list.Last.Span.End)
         End Function
 
-        Public Function GetTopLevelAndMethodLevelMembers(root As SyntaxNode) As PooledObject(Of List(Of SyntaxNode)) Implements ISyntaxFacts.GetTopLevelAndMethodLevelMembers
-            Dim pooledList = PooledObject(Of List(Of SyntaxNode)).Create(s_syntaxNodeListPool)
-            Dim list = pooledList.Object
-
-            AppendMembers(root, list, topLevel:=True, methodLevel:=True)
-
-            Return pooledList
-        End Function
-
-        Public Function GetMethodLevelMembers(root As SyntaxNode) As PooledObject(Of List(Of SyntaxNode)) Implements ISyntaxFacts.GetMethodLevelMembers
-            Dim pooledList = PooledObject(Of List(Of SyntaxNode)).Create(s_syntaxNodeListPool)
-            Dim list = pooledList.Object
-
-            AppendMembers(root, list, topLevel:=False, methodLevel:=True)
-
-            Return pooledList
-        End Function
-
         Public Function GetMembersOfTypeDeclaration(typeDeclaration As SyntaxNode) As SyntaxList(Of SyntaxNode) Implements ISyntaxFacts.GetMembersOfTypeDeclaration
             Return DirectCast(typeDeclaration, TypeBlockSyntax).Members
         End Function
@@ -1050,7 +1030,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
             End If
         End Sub
 
-        Private Sub AppendMembers(node As SyntaxNode, list As List(Of SyntaxNode), topLevel As Boolean, methodLevel As Boolean)
+        Protected Overrides Sub AppendMembers(node As SyntaxNode, list As ArrayBuilder(Of SyntaxNode), topLevel As Boolean, methodLevel As Boolean)
             Debug.Assert(topLevel OrElse methodLevel)
 
             For Each member In node.GetMembers()
@@ -1998,6 +1978,18 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService
             Return DirectCast(node, LiteralExpressionSyntax).Token
         End Function
 
+        Private Sub ISyntaxFacts_AddTopLevelAndMethodLevelMembers(root As SyntaxNode, result As ArrayBuilder(Of SyntaxNode)) Implements ISyntaxFacts.AddTopLevelAndMethodLevelMembers
+            AddTopLevelAndMethodLevelMembers(root, result)
+        End Sub
+
+        Private Sub ISyntaxFacts_AddTopLevelMembers(root As SyntaxNode, result As ArrayBuilder(Of SyntaxNode)) Implements ISyntaxFacts.AddTopLevelMembers
+            AddTopLevelMembers(root, result)
+        End Sub
+
+        Private Sub ISyntaxFacts_AddMethodLevelMembers(root As SyntaxNode, result As ArrayBuilder(Of SyntaxNode)) Implements ISyntaxFacts.AddMethodLevelMembers
+            AddMethodLevelMembers(root, result)
+        End Sub
+
 #End Region
 
     End Class