Skip to content

Commit 49b30fd

Browse files
Clean up object pools and improve PooledHashSet<T> (#12406)
This represents some clean-up and a small performance improvement that I've intended to do for a long while. In fact, it's been sitting on my machine across a handful of branches for months. So, I decided to bring it together and submit a PR. Key Changes: - The custom pool classes are now instance classes rather than static classes. So, it's not possible to create an instance of an `ArrayBuilderPool<T>` rather than an `ObjectPool<ImmutableArray<T>.Builder>`. - Introduce a `SpecializedPools` static class that pulls together `StringHashSetPool`, `StringDictionaryPool<TValue>`, and `ReferenceEqualityHashSetPool<T>`. - Add `MaximumObjectSize` as a knob when creating new custom pools to avoid _always_ having to create a new `IPooledObjectPolicy<T>` implementation. - Introduce an `IPoolableObject` interface and way to create pools of `IPoolableObjects`. - Improve `PooledHashSet<T>` to avoid acquiring a `HashSet<T>` from a pool until there are at least two items. ---- CI Build: https://dev.azure.com/dnceng/internal/_build/results?buildId=2826683&view=results Test Insertion: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/684259 Toolset Run: https://dev.azure.com/dnceng/internal/_build/results?buildId=2826684&view=results
2 parents 657ebf8 + 1c5553e commit 49b30fd

File tree

79 files changed

+1620
-636
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

79 files changed

+1620
-636
lines changed

SpellingExclusions.dic

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,3 +7,4 @@ microsoft
77
vsls
88
Blazor
99
Metacode
10+
Poolable

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/ComponentTagHelperDescriptorProvider.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ private static TagHelperDescriptor CreateNameMatchingDescriptor(
125125
{
126126
metadata.IsGeneric = true;
127127

128-
using var cascadeGenericTypeAttributes = new PooledHashSet<string>(StringHashSetPool.Ordinal);
128+
using var cascadeGenericTypeAttributes = new PooledHashSet<string>(StringComparer.Ordinal);
129129

130130
foreach (var attribute in type.GetAttributes())
131131
{
@@ -610,7 +610,7 @@ private static void CreateContextParameter(TagHelperDescriptorBuilder builder, s
610610
// - are not indexers
611611
private static ImmutableArray<(IPropertySymbol property, PropertyKind kind)> GetProperties(INamedTypeSymbol type)
612612
{
613-
using var names = new PooledHashSet<string>(StringHashSetPool.Ordinal);
613+
using var names = new PooledHashSet<string>(StringComparer.Ordinal);
614614
using var results = new PooledArrayBuilder<(IPropertySymbol, PropertyKind)>();
615615

616616
var currentType = type;

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/CSharp/DefaultTagHelperDescriptorFactory.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,7 @@ private static bool IsPotentialDictionaryProperty(IPropertySymbol property)
375375
private static void CollectAccessibleProperties(
376376
INamedTypeSymbol typeSymbol, ref PooledArrayBuilder<IPropertySymbol> properties)
377377
{
378-
using var names = new PooledHashSet<string>(StringHashSetPool.Ordinal);
378+
using var names = new PooledHashSet<string>(StringComparer.Ordinal);
379379

380380
// Traverse the type hierarchy to find all accessible properties.
381381
var currentType = typeSymbol;

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/AllowedChildTagDescriptorBuilder_Pooling.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language;
88

99
public partial class AllowedChildTagDescriptorBuilder
1010
{
11-
internal static readonly ObjectPool<AllowedChildTagDescriptorBuilder> Pool = DefaultPool.Create(Policy.Instance);
11+
internal static readonly ObjectPool<AllowedChildTagDescriptorBuilder> Pool =
12+
DefaultPool.Create(static () => new AllowedChildTagDescriptorBuilder());
1213

1314
internal static AllowedChildTagDescriptorBuilder GetInstance(TagHelperDescriptorBuilder parent)
1415
{
@@ -26,15 +27,4 @@ private protected override void Reset()
2627
Name = null;
2728
DisplayName = null;
2829
}
29-
30-
private sealed class Policy : PooledBuilderPolicy<AllowedChildTagDescriptorBuilder>
31-
{
32-
public static readonly Policy Instance = new();
33-
34-
private Policy()
35-
{
36-
}
37-
38-
public override AllowedChildTagDescriptorBuilder Create() => new();
39-
}
4030
}

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/BoundAttributeDescriptorBuilder_Pooling.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language;
88

99
public partial class BoundAttributeDescriptorBuilder
1010
{
11-
internal static readonly ObjectPool<BoundAttributeDescriptorBuilder> Pool = DefaultPool.Create(Policy.Instance);
11+
internal static readonly ObjectPool<BoundAttributeDescriptorBuilder> Pool =
12+
DefaultPool.Create(static () => new BoundAttributeDescriptorBuilder());
1213

1314
internal static BoundAttributeDescriptorBuilder GetInstance(TagHelperDescriptorBuilder parent)
1415
{
@@ -36,15 +37,4 @@ private protected override void Reset()
3637
ContainingType = null;
3738
Parameters.Clear();
3839
}
39-
40-
private sealed class Policy : PooledBuilderPolicy<BoundAttributeDescriptorBuilder>
41-
{
42-
public static readonly Policy Instance = new();
43-
44-
private Policy()
45-
{
46-
}
47-
48-
public override BoundAttributeDescriptorBuilder Create() => new();
49-
}
5040
}

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/BoundAttributeParameterDescriptorBuilder_Pooling.cs

Lines changed: 2 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ namespace Microsoft.AspNetCore.Razor.Language;
88

99
public partial class BoundAttributeParameterDescriptorBuilder
1010
{
11-
internal static readonly ObjectPool<BoundAttributeParameterDescriptorBuilder> Pool = DefaultPool.Create(Policy.Instance);
11+
internal static readonly ObjectPool<BoundAttributeParameterDescriptorBuilder> Pool =
12+
DefaultPool.Create(static () => new BoundAttributeParameterDescriptorBuilder());
1213

1314
internal static BoundAttributeParameterDescriptorBuilder GetInstance(BoundAttributeDescriptorBuilder parent)
1415
{
@@ -29,15 +30,4 @@ private protected override void Reset()
2930
Name = null;
3031
PropertyName = null;
3132
}
32-
33-
private sealed class Policy : PooledBuilderPolicy<BoundAttributeParameterDescriptorBuilder>
34-
{
35-
public static readonly Policy Instance = new();
36-
37-
private Policy()
38-
{
39-
}
40-
41-
public override BoundAttributeParameterDescriptorBuilder Create() => new();
42-
}
4333
}

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentBindLoweringPass.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ private static void ProcessDuplicates(
217217
ref PooledArrayBuilder<IntermediateNodeReference<TagHelperDirectiveAttributeIntermediateNode>> references,
218218
ref PooledArrayBuilder<IntermediateNodeReference<TagHelperDirectiveAttributeParameterIntermediateNode>> parameterReferences)
219219
{
220-
using var _ = ReferenceEqualityHashSetPool<IntermediateNode>.GetPooledObject(out var parents);
220+
using var _ = SpecializedPools.GetPooledReferenceEqualityHashSet<IntermediateNode>(out var parents);
221221

222222
foreach (var reference in references)
223223
{
@@ -307,7 +307,7 @@ private static void ProcessDuplicateAttributes(IntermediateNode node)
307307
// If we still have duplicates at this point then they are genuine conflicts.
308308
// Use a hash set to quickly determine whether there are any duplicates.
309309
// If so, we need to do a more expensive pass to identify and remove them.
310-
using var _ = StringHashSetPool.Ordinal.GetPooledObject(out var duplicates);
310+
using var _ = SpecializedPools.GetPooledStringHashSet(out var duplicates);
311311

312312
foreach (var child in children)
313313
{
@@ -329,7 +329,7 @@ static void ReportDiagnosticAndRemoveDuplicates(IntermediateNode node)
329329
{
330330
var children = node.Children;
331331

332-
using var _ = StringDictionaryPool<ImmutableArray<AttributeInfo>.Builder>.Ordinal.GetPooledObject(out var duplicates);
332+
using var _ = SpecializedPools.GetPooledStringDictionary<ImmutableArray<AttributeInfo>.Builder>(out var duplicates);
333333

334334
for (var i = 0; i < children.Count; i++)
335335
{

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Components/ComponentEventHandlerLoweringPass.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ protected override void ExecuteCore(
3636
// For each event handler *usage* we need to rewrite the tag helper node to map to basic constructs.
3737
// Each usage will be represented by a tag helper property that is a descendant of either
3838
// a component or element.
39-
using var _ = ReferenceEqualityHashSetPool<IntermediateNode>.GetPooledObject(out var parents);
39+
using var _ = SpecializedPools.GetPooledReferenceEqualityHashSet<IntermediateNode>(out var parents);
4040
var references = documentNode.FindDescendantReferences<TagHelperDirectiveAttributeIntermediateNode>();
4141

4242
foreach (var reference in references)

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorIntermediateNodeLoweringPhase.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -187,7 +187,7 @@ private static IReadOnlyList<UsingReference> ImportDirectives(
187187

188188
private static void PostProcessImportedDirectives(DocumentIntermediateNode document)
189189
{
190-
using var _ = ReferenceEqualityHashSetPool<DirectiveDescriptor>.GetPooledObject(out var seenDirectives);
190+
using var _ = SpecializedPools.GetPooledReferenceEqualityHashSet<DirectiveDescriptor>(out var seenDirectives);
191191
var references = document.FindDescendantReferences<DirectiveIntermediateNode>();
192192

193193
for (var i = references.Length - 1; i >= 0; i--)

src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/DefaultRazorTagHelperContextDiscoveryPhase.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
using Microsoft.AspNetCore.Razor.Language.Legacy;
1313
using Microsoft.AspNetCore.Razor.Language.Syntax;
1414
using Microsoft.AspNetCore.Razor.PooledObjects;
15-
using Microsoft.Extensions.ObjectPool;
1615

1716
namespace Microsoft.AspNetCore.Razor.Language;
1817

@@ -73,7 +72,7 @@ internal static ReadOnlyMemory<char> GetMemoryWithoutGlobalPrefix(string s)
7372
return mem;
7473
}
7574

76-
internal abstract class DirectiveVisitor : SyntaxWalker
75+
internal abstract class DirectiveVisitor : SyntaxWalker, IPoolableObject
7776
{
7877
private bool _isInitialized;
7978
private string? _filePath;
@@ -173,7 +172,7 @@ internal sealed class TagHelperDirectiveVisitor : DirectiveVisitor
173172
/// A larger pool of <see cref="TagHelperDescriptor"/> lists to handle scenarios where tag helpers
174173
/// originate from a large number of assemblies.
175174
/// </summary>
176-
private static readonly ObjectPool<List<TagHelperDescriptor>> s_pool = ListPool<TagHelperDescriptor>.Create(100);
175+
private static readonly ListPool<TagHelperDescriptor> s_pool = ListPool<TagHelperDescriptor>.Create(poolSize: 100);
177176

178177
/// <summary>
179178
/// A map from assembly name to list of <see cref="TagHelperDescriptor"/>. Lists are allocated from and returned to

0 commit comments

Comments
 (0)