Reduce allocations during completion in FilterToVisibleAndBrowsableSymbols (#77315)

* Reduce allocations during completion in FilterToVisibleAndBrowsableSymbols

This method was visible in allocations in one of the completion speedometer profiles I was looking at and has a couple of easily removable allocations.

Can combine two WhereAsArray calls
Can switch to using WhereAsArray(ImmutableAray) instead of the IEnumerable overload
Can use the pooling support in MetadataUnifyingSymbolHashSet
Can avoid EditorBrowsableInfo allocation when input symbols is empty
Can avoid the closure allocation that was present in RemoveOverriddenSymbolsWithinSet
Even with all these changes, the net benefit didn't prove large from the speedometer run, but I've done the work, it doesn't complicate things, and every bit helps.

Author: ToddGrun

src/Workspaces/Core/Portable/FindSymbols/FindReferences/MetadataUnifyingSymbolHashSet.cs

@@ -3,24 +3,32 @@
 // See the LICENSE file in the project root for more information.
 
 using System.Collections.Generic;
+using System.Diagnostics;
 using Microsoft.CodeAnalysis.PooledObjects;
 
 namespace Microsoft.CodeAnalysis.FindSymbols;
 
-internal sealed class MetadataUnifyingSymbolHashSet : HashSet<ISymbol>
+internal sealed class MetadataUnifyingSymbolHashSet : HashSet<ISymbol>, IPooled
 {
     private static readonly ObjectPool<MetadataUnifyingSymbolHashSet> s_metadataUnifyingSymbolHashSetPool = new(() => []);
 
     public MetadataUnifyingSymbolHashSet() : base(MetadataUnifyingEquivalenceComparer.Instance)
     {
     }
 
-    public static MetadataUnifyingSymbolHashSet AllocateFromPool()
-        => s_metadataUnifyingSymbolHashSetPool.Allocate();
+    public static PooledDisposer<MetadataUnifyingSymbolHashSet> GetInstance(out MetadataUnifyingSymbolHashSet instance)
+    {
+        instance = s_metadataUnifyingSymbolHashSetPool.Allocate();
+        Debug.Assert(instance.Count == 0);
+
+        return new PooledDisposer<MetadataUnifyingSymbolHashSet>(instance);
+    }
 
-    public static void ClearAndFree(MetadataUnifyingSymbolHashSet set)
+    public void Free(bool discardLargeInstances)
     {
-        set.Clear();
-        s_metadataUnifyingSymbolHashSetPool.Free(set);
+        // ignore discardLargeInstances as we don't limit our pooled hashset capacities
+        Clear();
+
+        s_metadataUnifyingSymbolHashSetPool.Free(this);
     }
 }

src/Workspaces/Core/Portable/Shared/Extensions/ISymbolExtensions.cs

@@ -645,38 +645,37 @@ private static bool ElementNameIs(XElement element, string name)
     public static ImmutableArray<T> FilterToVisibleAndBrowsableSymbols<T>(
         this ImmutableArray<T> symbols, bool hideAdvancedMembers, Compilation compilation) where T : ISymbol
     {
-        symbols = symbols.RemoveOverriddenSymbolsWithinSet();
+        if (symbols.Length == 0)
+            return [];
+
+        using var _ = MetadataUnifyingSymbolHashSet.GetInstance(out var overriddenSymbols);
+
+        foreach (var symbol in symbols)
+        {
+            var overriddenMember = symbol.GetOverriddenMember();
+            if (overriddenMember != null)
+                overriddenSymbols.Add(overriddenMember);
+        }
 
         // Since all symbols are from the same compilation, find the required attribute
         // constructors once and reuse.
         var editorBrowsableInfo = new EditorBrowsableInfo(compilation);
 
         // PERF: HasUnsupportedMetadata may require recreating the syntax tree to get the base class, so first
         // check to see if we're referencing a symbol defined in source.
-        return symbols.WhereAsArray((s, arg) =>
+        var filteredSymbols = symbols.WhereAsArray(static (s, arg) =>
             // Check if symbol is namespace (which is always visible) first to avoid realizing all locations
             // of each namespace symbol, which might end up allocating in LOH
+            !arg.overriddenSymbols.Contains(s) &&
             (s is INamespaceSymbol || s.Locations.Any(static loc => loc.IsInSource) || !s.HasUnsupportedMetadata) &&
             !s.IsDestructor() &&
             s.IsEditorBrowsable(
                 arg.hideAdvancedMembers,
                 arg.editorBrowsableInfo.Compilation,
                 arg.editorBrowsableInfo),
-            (hideAdvancedMembers, editorBrowsableInfo));
-    }
-
-    private static ImmutableArray<T> RemoveOverriddenSymbolsWithinSet<T>(this ImmutableArray<T> symbols) where T : ISymbol
-    {
-        var overriddenSymbols = new MetadataUnifyingSymbolHashSet();
-
-        foreach (var symbol in symbols)
-        {
-            var overriddenMember = symbol.GetOverriddenMember();
-            if (overriddenMember != null && !overriddenSymbols.Contains(overriddenMember))
-                overriddenSymbols.Add(overriddenMember);
-        }
+            arg: (hideAdvancedMembers, editorBrowsableInfo, overriddenSymbols));
 
-        return symbols.WhereAsArray(s => !overriddenSymbols.Contains(s));
+        return filteredSymbols;
     }
 
     public static ImmutableArray<T> FilterToVisibleAndBrowsableSymbolsAndNotUnsafeSymbols<T>(

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/IMethodSymbolExtensions.cs

@@ -163,13 +163,13 @@ public static IMethodSymbol RemoveInaccessibleAttributesAndAttributesOfTypes(
             method,
             containingType: method.ContainingType,
             explicitInterfaceImplementations: method.ExplicitInterfaceImplementations,
-            attributes: method.GetAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg),
+            attributes: method.GetAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg: arg),
             parameters: method.Parameters.SelectAsArray(static (p, arg) =>
                 CodeGenerationSymbolFactory.CreateParameterSymbol(
-                    p.GetAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg),
+                    p.GetAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg: arg),
                     p.RefKind, p.IsParams, p.Type, p.Name, p.IsOptional,
                     p.HasExplicitDefaultValue, p.HasExplicitDefaultValue ? p.ExplicitDefaultValue : null), arg),
-            returnTypeAttributes: method.GetReturnTypeAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg));
+            returnTypeAttributes: method.GetReturnTypeAttributes().WhereAsArray(static (a, arg) => !shouldRemoveAttribute(a, arg), arg: arg));
 
         static bool shouldRemoveAttribute(AttributeData a, (INamedTypeSymbol[] removeAttributeTypes, ISymbol accessibleWithin) arg)
             => arg.removeAttributeTypes.Any(attr => attr.Equals(a.AttributeClass)) ||

src/Workspaces/SharedUtilitiesAndExtensions/Workspace/Core/Extensions/IPropertySymbolExtensions.cs

@@ -59,7 +59,7 @@ public static IPropertySymbol RemoveInaccessibleAttributesAndAttributesOfTypes(
             property.Name,
             property.Parameters.SelectAsArray(static (p, arg) =>
                 CodeGenerationSymbolFactory.CreateParameterSymbol(
-                    p.GetAttributes().WhereAsArray(static (a, arg) => !ShouldRemoveAttribute(a, arg), arg),
+                    p.GetAttributes().WhereAsArray(static (a, arg) => !ShouldRemoveAttribute(a, arg), arg: arg),
                     p.RefKind, p.IsParams, p.Type, p.Name, p.IsOptional,
                     p.HasExplicitDefaultValue, p.HasExplicitDefaultValue ? p.ExplicitDefaultValue : null), arg),
             property.GetMethod,