From a35b8c17a7a21a4eeca6944d8380565bf5c75601 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Fri, 16 Aug 2024 10:44:26 -0700 Subject: [PATCH 1/6] Reduce usage in CSharpSyntaxFacts.AppendMembers (again) A recent change(https://github.com/dotnet/roslyn/pull/74596) modified this code to take in an ArrayBuilder to allow callers to easily use pooled arrays. However, the size of the data populated into those arrays commonly exceeds the ArrayBuilder reuse threshold, those reducing the allocation benefit realized in those codepaths. This PR changes the callers to instead use a pooled list, thus avoiding the threshold issue. Current scrolling speedometer results for the typing scenario still show this as allocating around 35 MB (which was about 3.5% at the time of the earlier PR). My hope is that this PR will get rid of nearly all those allocations this time. --- ...UnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs | 6 ++++-- .../InternalUtilities/EnumerableExtensions.cs | 11 ++++++++--- .../Core/Editor/GoToAdjacentMemberCommandHandler.cs | 5 +++-- ...crementalAnalyzer.IncrementalMemberEditAnalyzer.cs | 4 +++- ...lyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs | 6 ++++-- .../AbstractSemanticModelReuseLanguageService.cs | 10 +++++++--- .../CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs | 6 +++--- .../Core/Services/SyntaxFacts/ISyntaxFacts.cs | 5 ++--- .../Services/SyntaxFacts/VisualBasicSyntaxFacts.vb | 6 +++--- 9 files changed, 37 insertions(+), 22 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs index 58d4f897614cd..25db9465e3b5c 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs @@ -745,9 +745,11 @@ private async Task ProcessSuppressMessageAttributesAsync( return false; } - using var _1 = ArrayBuilder.GetInstance(out var declarationNodes); + using var pooledDeclarationNodes = SharedPools.Default>().GetPooledObject(); + var declarationNodes = pooledDeclarationNodes.Object; + SyntaxFacts.AddTopLevelAndMethodLevelMembers(root, declarationNodes); - using var _2 = PooledHashSet.GetInstance(out var processedPartialSymbols); + using var _ = PooledHashSet.GetInstance(out var processedPartialSymbols); if (declarationNodes.Count > 0) { foreach (var node in declarationNodes) diff --git a/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs b/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs index 3b49120cac4ec..b63210c0d8d2d 100644 --- a/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs +++ b/src/Compilers/Core/Portable/InternalUtilities/EnumerableExtensions.cs @@ -9,6 +9,7 @@ using System.Collections.ObjectModel; using System.Diagnostics; using System.Linq; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis; @@ -385,11 +386,15 @@ public static ImmutableArray SelectAsArray(this IRead if (source == null) return ImmutableArray.Empty; - var builder = ArrayBuilder.GetInstance(source.Count); + var builder = new TResult[source.Count]; + var index = 0; foreach (var item in source) - builder.Add(selector(item)); + { + builder[index] = selector(item); + index++; + } - return builder.ToImmutableAndFree(); + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } public static ImmutableArray SelectManyAsArray(this IEnumerable? source, Func> selector) diff --git a/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs b/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs index 7b5dbe8689e1e..db7ad1c65fab8 100644 --- a/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs +++ b/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs @@ -5,12 +5,12 @@ #nullable disable using System; +using System.Collections.Generic; using System.ComponentModel.Composition; using System.Linq; 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,7 +103,8 @@ private bool ExecuteCommandImpl(EditorCommandArgs args, bool gotoNextMember, Com /// internal static int? GetTargetPosition(ISyntaxFactsService service, SyntaxNode root, int caretPosition, bool next) { - using var _ = ArrayBuilder.GetInstance(out var members); + using var pooledMembers = SharedPools.Default>().GetPooledObject(); + var members = pooledMembers.Object; service.AddMethodLevelMembers(root, members); if (members.Count == 0) diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs index e5836fdc41428..2c92f3e1c1ea5 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs @@ -3,6 +3,7 @@ // 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; @@ -215,7 +216,8 @@ async Task ExecuteAnalyzersAsync( return null; } - using var _ = ArrayBuilder.GetInstance(out var members); + using var pooledMembers = SharedPools.Default>().GetPooledObject(); + var members = pooledMembers.Object; var syntaxFacts = document.GetRequiredLanguageService(); syntaxFacts.AddMethodLevelMembers(root, members); diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs index 8147b2f9577e7..a2b8004d41673 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs @@ -2,11 +2,11 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. +using System.Collections.Generic; using System.Collections.Immutable; 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,7 +47,9 @@ static async Task> CreateMemberSpansAsync(Document docu var service = document.GetRequiredLanguageService(); var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - using var _ = ArrayBuilder.GetInstance(out var members); + using var pooledMembers = SharedPools.Default>().GetPooledObject(); + var members = pooledMembers.Object; + service.AddMethodLevelMembers(root, members); return members.SelectAsArray(m => m.FullSpan); } diff --git a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs index 690cda2ca16d6..b878fc141b06e 100644 --- a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs +++ b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs @@ -3,12 +3,12 @@ // See the LICENSE file in the project root for more information. using System; +using System.Collections.Generic; using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Internal.Log; using Microsoft.CodeAnalysis.LanguageService; -using Microsoft.CodeAnalysis.PooledObjects; namespace Microsoft.CodeAnalysis.SemanticModelReuse; @@ -106,7 +106,9 @@ protected SyntaxNode GetPreviousBodyNode(SyntaxNode previousRoot, SyntaxNode cur } else { - using var _1 = ArrayBuilder.GetInstance(out var currentMembers); + using var pooledCurrentMembers = SharedPools.Default>().GetPooledObject(); + var currentMembers = pooledCurrentMembers.Object; + this.SyntaxFacts.AddMethodLevelMembers(currentRoot, currentMembers); var index = currentMembers.IndexOf(currentBodyNode); if (index < 0) @@ -115,7 +117,9 @@ protected SyntaxNode GetPreviousBodyNode(SyntaxNode previousRoot, SyntaxNode cur return null; } - using var _2 = ArrayBuilder.GetInstance(out var previousMembers); + using var pooledPreviousMembers = SharedPools.Default>().GetPooledObject(); + var previousMembers = pooledPreviousMembers.Object; + this.SyntaxFacts.AddMethodLevelMembers(previousRoot, previousMembers); if (currentMembers.Count != previousMembers.Count) { diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs index 7f1e15590e970..dcaafb026c298 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs @@ -896,12 +896,12 @@ private static void AppendTypeParameterList(StringBuilder builder, TypeParameter } } - public void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, ArrayBuilder list) + public void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, List list) { AppendMembers(root, list, topLevel: true, methodLevel: true); } - public void AddMethodLevelMembers(SyntaxNode? root, ArrayBuilder list) + public void AddMethodLevelMembers(SyntaxNode? root, List list) { AppendMembers(root, list, topLevel: false, methodLevel: true); } @@ -909,7 +909,7 @@ public void AddMethodLevelMembers(SyntaxNode? root, ArrayBuilder lis public SyntaxList GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration) => ((TypeDeclarationSyntax)typeDeclaration).Members; - private void AppendMembers(SyntaxNode? node, ArrayBuilder list, bool topLevel, bool methodLevel) + private void AppendMembers(SyntaxNode? node, List list, bool topLevel, bool methodLevel) { Debug.Assert(topLevel || methodLevel); diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs index 5fac8b710de4e..ae79c462967e3 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs @@ -7,7 +7,6 @@ using System.Diagnostics.CodeAnalysis; using System.Threading; using Microsoft.CodeAnalysis.Text; -using Microsoft.CodeAnalysis.PooledObjects; #if CODE_STYLE using Microsoft.CodeAnalysis.Internal.Editing; @@ -410,9 +409,9 @@ void GetPartsOfTupleExpression(SyntaxNode node, SyntaxNode? ConvertToSingleLine(SyntaxNode? node, bool useElasticTrivia = false); // Violation. This is a feature level API. - void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, ArrayBuilder list); + void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, List list); // Violation. This is a feature level API. - void AddMethodLevelMembers(SyntaxNode? root, ArrayBuilder list); + void AddMethodLevelMembers(SyntaxNode? root, List list); SyntaxList GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration); // Violation. This is a feature level API. diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index ab22c60526d1a..a560a3918ad15 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -895,11 +895,11 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService Return TextSpan.FromBounds(list.First.SpanStart, list.Last.Span.End) End Function - Public Sub AddTopLevelAndMethodLevelMembers(root As SyntaxNode, list As ArrayBuilder(Of SyntaxNode)) Implements ISyntaxFacts.AddTopLevelAndMethodLevelMembers + Public Sub AddTopLevelAndMethodLevelMembers(root As SyntaxNode, list As List(Of SyntaxNode)) Implements ISyntaxFacts.AddTopLevelAndMethodLevelMembers AppendMembers(root, list, topLevel:=True, methodLevel:=True) End Sub - Public Sub AddMethodLevelMembers(root As SyntaxNode, list As ArrayBuilder(Of SyntaxNode)) Implements ISyntaxFacts.AddMethodLevelMembers + Public Sub AddMethodLevelMembers(root As SyntaxNode, list As List(Of SyntaxNode)) Implements ISyntaxFacts.AddMethodLevelMembers AppendMembers(root, list, topLevel:=False, methodLevel:=True) End Sub @@ -1047,7 +1047,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService End If End Sub - Private Sub AppendMembers(node As SyntaxNode, list As ArrayBuilder(Of SyntaxNode), topLevel As Boolean, methodLevel As Boolean) + Private Sub AppendMembers(node As SyntaxNode, list As List(Of SyntaxNode), topLevel As Boolean, methodLevel As Boolean) Debug.Assert(topLevel OrElse methodLevel) For Each member In node.GetMembers() From 94c3a55254a07fec4c913cd35b12d7133bf4226b Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 19 Aug 2024 09:54:04 -0700 Subject: [PATCH 2/6] Use arrays directly instead of ArrayBuilder in certain situations --- .../Collections/ImmutableArrayExtensions.cs | 167 +++++++++++++----- 1 file changed, 126 insertions(+), 41 deletions(-) diff --git a/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs b/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs index 2fbb864964308..6778fcbd7310b 100644 --- a/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs +++ b/src/Compilers/Core/Portable/Collections/ImmutableArrayExtensions.cs @@ -352,14 +352,14 @@ public static async ValueTask> SelectAsArrayAsync.Empty; - var builder = ArrayBuilder.GetInstance(array.Length); + var builder = new TResult[array.Length]; - foreach (var item in array) + for (var i = 0; i < array.Length; i++) { - builder.Add(await selector(item, cancellationToken).ConfigureAwait(false)); + builder[i] = await selector(array[i], cancellationToken).ConfigureAwait(false); } - return builder.ToImmutableAndFree(); + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } /// @@ -370,14 +370,14 @@ public static async ValueTask> SelectAsArrayAsync.Empty; - var builder = ArrayBuilder.GetInstance(array.Length); + var builder = new TResult[array.Length]; - foreach (var item in array) + for (var i = 0; i < array.Length; i++) { - builder.Add(await selector(item, arg, cancellationToken).ConfigureAwait(false)); + builder[i] = await selector(array[i], arg, cancellationToken).ConfigureAwait(false); } - return builder.ToImmutableAndFree(); + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } public static ValueTask> SelectManyAsArrayAsync(this ImmutableArray source, Func>> selector, TArg arg, CancellationToken cancellationToken) @@ -432,13 +432,13 @@ public static ImmutableArray ZipAsArray(this Immutable return ImmutableArray.Create(map(self[0], other[0]), map(self[1], other[1]), map(self[2], other[2]), map(self[3], other[3])); default: - var builder = ArrayBuilder.GetInstance(self.Length); + var builder = new TResult[self.Length]; for (int i = 0; i < self.Length; i++) { - builder.Add(map(self[i], other[i])); + builder[i] = map(self[i], other[i]); } - return builder.ToImmutableAndFree(); + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } } @@ -815,44 +815,124 @@ internal static ImmutableArray Concat(this ImmutableArray first, Immuta internal static ImmutableArray Concat(this ImmutableArray first, ImmutableArray second, ImmutableArray third) { - var builder = ArrayBuilder.GetInstance(first.Length + second.Length + third.Length); - builder.AddRange(first); - builder.AddRange(second); - builder.AddRange(third); - return builder.ToImmutableAndFree(); + var builder = new T[first.Length + second.Length + third.Length]; + var index = 0; + + foreach (var item in first) + { + builder[index++] = item; + } + + foreach (var item in second) + { + builder[index++] = item; + } + + foreach (var item in third) + { + builder[index++] = item; + } + + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } internal static ImmutableArray Concat(this ImmutableArray first, ImmutableArray second, ImmutableArray third, ImmutableArray fourth) { - var builder = ArrayBuilder.GetInstance(first.Length + second.Length + third.Length + fourth.Length); - builder.AddRange(first); - builder.AddRange(second); - builder.AddRange(third); - builder.AddRange(fourth); - return builder.ToImmutableAndFree(); + var builder = new T[first.Length + second.Length + third.Length + fourth.Length]; + var index = 0; + + foreach (var item in first) + { + builder[index++] = item; + } + + foreach (var item in second) + { + builder[index++] = item; + } + + foreach (var item in third) + { + builder[index++] = item; + } + + foreach (var item in fourth) + { + builder[index++] = item; + } + + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } internal static ImmutableArray Concat(this ImmutableArray first, ImmutableArray second, ImmutableArray third, ImmutableArray fourth, ImmutableArray fifth) { - var builder = ArrayBuilder.GetInstance(first.Length + second.Length + third.Length + fourth.Length + fifth.Length); - builder.AddRange(first); - builder.AddRange(second); - builder.AddRange(third); - builder.AddRange(fourth); - builder.AddRange(fifth); - return builder.ToImmutableAndFree(); + var builder = new T[first.Length + second.Length + third.Length + fourth.Length + fifth.Length]; + var index = 0; + + foreach (var item in first) + { + builder[index++] = item; + } + + foreach (var item in second) + { + builder[index++] = item; + } + + foreach (var item in third) + { + builder[index++] = item; + } + + foreach (var item in fourth) + { + builder[index++] = item; + } + + foreach (var item in fifth) + { + builder[index++] = item; + } + + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } internal static ImmutableArray Concat(this ImmutableArray first, ImmutableArray second, ImmutableArray third, ImmutableArray fourth, ImmutableArray fifth, ImmutableArray sixth) { - var builder = ArrayBuilder.GetInstance(first.Length + second.Length + third.Length + fourth.Length + fifth.Length + sixth.Length); - builder.AddRange(first); - builder.AddRange(second); - builder.AddRange(third); - builder.AddRange(fourth); - builder.AddRange(fifth); - builder.AddRange(sixth); - return builder.ToImmutableAndFree(); + var builder = new T[first.Length + second.Length + third.Length + fourth.Length + fifth.Length + sixth.Length]; + var index = 0; + + foreach (var item in first) + { + builder[index++] = item; + } + + foreach (var item in second) + { + builder[index++] = item; + } + + foreach (var item in third) + { + builder[index++] = item; + } + + foreach (var item in fourth) + { + builder[index++] = item; + } + + foreach (var item in fifth) + { + builder[index++] = item; + } + + foreach (var item in sixth) + { + builder[index++] = item; + } + + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } internal static ImmutableArray Concat(this ImmutableArray first, T second) @@ -872,15 +952,20 @@ internal static ImmutableArray AddRange(this ImmutableArray self, in Te return self.Add(items[0]); } - var builder = ArrayBuilder.GetInstance(self.Length + items.Count); - builder.AddRange(self); + var builder = new T[self.Length + items.Count]; + var index = 0; + + foreach (var item in self) + { + builder[index++] = item; + } foreach (var item in items) { - builder.Add(item); + builder[index++] = item; } - return builder.ToImmutableAndFree(); + return ImmutableCollectionsMarshal.AsImmutableArray(builder); } /// From 86986e1953836ff4f97b8f9d1062dade97d8524c Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Mon, 19 Aug 2024 10:28:02 -0700 Subject: [PATCH 3/6] Add explicit scope to using stmts to reduce pool contention. --- ...stractSemanticModelReuseLanguageService.cs | 42 ++++++++++++------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs index b878fc141b06e..acfe097a6778c 100644 --- a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs +++ b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs @@ -106,28 +106,38 @@ protected SyntaxNode GetPreviousBodyNode(SyntaxNode previousRoot, SyntaxNode cur } else { - using var pooledCurrentMembers = SharedPools.Default>().GetPooledObject(); - var currentMembers = pooledCurrentMembers.Object; + var currentMembersCount = 0; + var index = 0; - this.SyntaxFacts.AddMethodLevelMembers(currentRoot, currentMembers); - var index = currentMembers.IndexOf(currentBodyNode); - if (index < 0) + // Explicitly scope these pooled objects to allow earlier disposal and reduced pool pressure. + using (var pooledMembers = SharedPools.Default>().GetPooledObject()) { - Debug.Fail($"Unhandled member type in {nameof(GetPreviousBodyNode)}"); - return null; - } + var currentMembers = pooledMembers.Object; - using var pooledPreviousMembers = SharedPools.Default>().GetPooledObject(); - var previousMembers = pooledPreviousMembers.Object; + this.SyntaxFacts.AddMethodLevelMembers(currentRoot, currentMembers); + index = currentMembers.IndexOf(currentBodyNode); + if (index < 0) + { + Debug.Fail($"Unhandled member type in {nameof(GetPreviousBodyNode)}"); + return null; + } - this.SyntaxFacts.AddMethodLevelMembers(previousRoot, previousMembers); - if (currentMembers.Count != previousMembers.Count) - { - Debug.Fail("Member count shouldn't have changed as there were no top level edits."); - return null; + currentMembersCount = currentMembers.Count; } - return previousMembers[index]; + using (var pooledMembers = SharedPools.Default>().GetPooledObject()) + { + var previousMembers = pooledMembers.Object; + + this.SyntaxFacts.AddMethodLevelMembers(previousRoot, previousMembers); + if (currentMembersCount != previousMembers.Count) + { + Debug.Fail("Member count shouldn't have changed as there were no top level edits."); + return null; + } + + return previousMembers[index]; + } } } From 31c6079c02dc9743b29f7398759e8038d3c597fd Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 20 Aug 2024 11:00:30 -0700 Subject: [PATCH 4/6] Have (CSharp/VisualBasic)SyntaxFacts classes own the pool. *Hopefully*, this will finally get the allocation reductions, as having each caller own what pool they use was giving me grief. --- ...aryPragmaSuppressionsDiagnosticAnalyzer.cs | 3 +- .../GoToAdjacentMemberCommandHandler.cs | 4 +- ...lAnalyzer.IncrementalMemberEditAnalyzer.cs | 5 +-- ...crementalMemberEditAnalyzer_MemberSpans.cs | 3 +- ...stractSemanticModelReuseLanguageService.cs | 40 +++++++------------ .../Services/SyntaxFacts/CSharpSyntaxFacts.cs | 17 +++++++- .../Core/Services/SyntaxFacts/ISyntaxFacts.cs | 4 +- .../SyntaxFacts/VisualBasicSyntaxFacts.vb | 23 +++++++++-- 8 files changed, 56 insertions(+), 43 deletions(-) diff --git a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs index 25db9465e3b5c..195d145731738 100644 --- a/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs +++ b/src/Analyzers/Core/Analyzers/RemoveUnnecessarySuppressions/AbstractRemoveUnnecessaryPragmaSuppressionsDiagnosticAnalyzer.cs @@ -745,10 +745,9 @@ private async Task ProcessSuppressMessageAttributesAsync( return false; } - using var pooledDeclarationNodes = SharedPools.Default>().GetPooledObject(); + using var pooledDeclarationNodes = SyntaxFacts.GetTopLevelAndMethodLevelMembers(root); var declarationNodes = pooledDeclarationNodes.Object; - SyntaxFacts.AddTopLevelAndMethodLevelMembers(root, declarationNodes); using var _ = PooledHashSet.GetInstance(out var processedPartialSymbols); if (declarationNodes.Count > 0) { diff --git a/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs b/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs index db7ad1c65fab8..923049a4c1d8b 100644 --- a/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs +++ b/src/EditorFeatures/Core/Editor/GoToAdjacentMemberCommandHandler.cs @@ -103,10 +103,8 @@ private bool ExecuteCommandImpl(EditorCommandArgs args, bool gotoNextMember, Com /// internal static int? GetTargetPosition(ISyntaxFactsService service, SyntaxNode root, int caretPosition, bool next) { - using var pooledMembers = SharedPools.Default>().GetPooledObject(); + using var pooledMembers = service.GetMethodLevelMembers(root); var members = pooledMembers.Object; - - service.AddMethodLevelMembers(root, members); if (members.Count == 0) { return null; diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs index 2c92f3e1c1ea5..98e0dc4c68628 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer.cs @@ -216,11 +216,10 @@ async Task ExecuteAnalyzersAsync( return null; } - using var pooledMembers = SharedPools.Default>().GetPooledObject(); + var syntaxFacts = document.GetRequiredLanguageService(); + using var pooledMembers = syntaxFacts.GetMethodLevelMembers(root); var members = pooledMembers.Object; - var syntaxFacts = document.GetRequiredLanguageService(); - syntaxFacts.AddMethodLevelMembers(root, members); var memberSpans = members.SelectAsArray(member => member.FullSpan); var changedMemberId = members.IndexOf(changedMember); diff --git a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs index a2b8004d41673..c76750e900a4c 100644 --- a/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs +++ b/src/LanguageServer/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer.IncrementalMemberEditAnalyzer_MemberSpans.cs @@ -47,10 +47,9 @@ static async Task> CreateMemberSpansAsync(Document docu var service = document.GetRequiredLanguageService(); var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - using var pooledMembers = SharedPools.Default>().GetPooledObject(); + using var pooledMembers = service.GetMethodLevelMembers(root); var members = pooledMembers.Object; - service.AddMethodLevelMembers(root, members); return members.SelectAsArray(m => m.FullSpan); } } diff --git a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs index acfe097a6778c..f6f726fd43779 100644 --- a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs +++ b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs @@ -106,38 +106,26 @@ protected SyntaxNode GetPreviousBodyNode(SyntaxNode previousRoot, SyntaxNode cur } else { - var currentMembersCount = 0; - var index = 0; + using var pooledCurrentMembers = this.SyntaxFacts.GetMethodLevelMembers(currentRoot); + var currentMembers = pooledCurrentMembers.Object; - // Explicitly scope these pooled objects to allow earlier disposal and reduced pool pressure. - using (var pooledMembers = SharedPools.Default>().GetPooledObject()) + var index = currentMembers.IndexOf(currentBodyNode); + if (index < 0) { - var currentMembers = pooledMembers.Object; - - this.SyntaxFacts.AddMethodLevelMembers(currentRoot, currentMembers); - index = currentMembers.IndexOf(currentBodyNode); - if (index < 0) - { - Debug.Fail($"Unhandled member type in {nameof(GetPreviousBodyNode)}"); - return null; - } - - currentMembersCount = currentMembers.Count; + Debug.Fail($"Unhandled member type in {nameof(GetPreviousBodyNode)}"); + return null; } - using (var pooledMembers = SharedPools.Default>().GetPooledObject()) - { - var previousMembers = pooledMembers.Object; - - this.SyntaxFacts.AddMethodLevelMembers(previousRoot, previousMembers); - if (currentMembersCount != previousMembers.Count) - { - Debug.Fail("Member count shouldn't have changed as there were no top level edits."); - return null; - } + var pooledPreviousMembers = this.SyntaxFacts.GetMethodLevelMembers(previousRoot); + var previousMembers = pooledPreviousMembers.Object; - return previousMembers[index]; + if (currentMembers.Count != previousMembers.Count) + { + Debug.Fail("Member count shouldn't have changed as there were no top level edits."); + return null; } + + return previousMembers[index]; } } diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs index dcaafb026c298..081625a453edf 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/CSharp/Services/SyntaxFacts/CSharpSyntaxFacts.cs @@ -31,6 +31,9 @@ internal class CSharpSyntaxFacts : ISyntaxFacts { internal static readonly CSharpSyntaxFacts Instance = new(); + // Specifies false for trimOnFree as these objects commonly exceed the default ObjectPool threshold + private static readonly ObjectPool> s_syntaxNodeListPool = new ObjectPool>(() => [], trimOnFree: false); + protected CSharpSyntaxFacts() { } @@ -896,14 +899,24 @@ private static void AppendTypeParameterList(StringBuilder builder, TypeParameter } } - public void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, List list) + public PooledObject> GetTopLevelAndMethodLevelMembers(SyntaxNode? root) { + var pooledObject = s_syntaxNodeListPool.GetPooledObject(); + var list = pooledObject.Object; + AppendMembers(root, list, topLevel: true, methodLevel: true); + + return pooledObject; } - public void AddMethodLevelMembers(SyntaxNode? root, List list) + public PooledObject> GetMethodLevelMembers(SyntaxNode? root) { + var pooledObject = s_syntaxNodeListPool.GetPooledObject(); + var list = pooledObject.Object; + AppendMembers(root, list, topLevel: false, methodLevel: true); + + return pooledObject; } public SyntaxList GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs index ae79c462967e3..074aa4e482618 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Services/SyntaxFacts/ISyntaxFacts.cs @@ -409,9 +409,9 @@ void GetPartsOfTupleExpression(SyntaxNode node, SyntaxNode? ConvertToSingleLine(SyntaxNode? node, bool useElasticTrivia = false); // Violation. This is a feature level API. - void AddTopLevelAndMethodLevelMembers(SyntaxNode? root, List list); + PooledObject> GetTopLevelAndMethodLevelMembers(SyntaxNode? root); // Violation. This is a feature level API. - void AddMethodLevelMembers(SyntaxNode? root, List list); + PooledObject> GetMethodLevelMembers(SyntaxNode? root); SyntaxList GetMembersOfTypeDeclaration(SyntaxNode typeDeclaration); // Violation. This is a feature level API. diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index a560a3918ad15..339793ba0c259 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -27,6 +27,9 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService 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 @@ -895,12 +898,26 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService Return TextSpan.FromBounds(list.First.SpanStart, list.Last.Span.End) End Function - Public Sub AddTopLevelAndMethodLevelMembers(root As SyntaxNode, list As List(Of SyntaxNode)) Implements ISyntaxFacts.AddTopLevelAndMethodLevelMembers + 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) - End Sub - Public Sub AddMethodLevelMembers(root As SyntaxNode, list As List(Of SyntaxNode)) Implements ISyntaxFacts.AddMethodLevelMembers + 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 + + Private Shared Sub Releaser(pool As ObjectPool(Of List(Of SyntaxNode)), obj As List(Of SyntaxNode)) + pool.ClearAndFree(obj, pool.TrimOnFree) End Sub Public Function GetMembersOfTypeDeclaration(typeDeclaration As SyntaxNode) As SyntaxList(Of SyntaxNode) Implements ISyntaxFacts.GetMembersOfTypeDeclaration From 8cab6a8a936ad1d110ee77a3b910b235d4af87de Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 20 Aug 2024 11:03:36 -0700 Subject: [PATCH 5/6] add back using --- .../AbstractSemanticModelReuseLanguageService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs index f6f726fd43779..0d4712b4ce423 100644 --- a/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs +++ b/src/Workspaces/Core/Portable/SemanticModelReuse/AbstractSemanticModelReuseLanguageService.cs @@ -116,7 +116,7 @@ protected SyntaxNode GetPreviousBodyNode(SyntaxNode previousRoot, SyntaxNode cur return null; } - var pooledPreviousMembers = this.SyntaxFacts.GetMethodLevelMembers(previousRoot); + using var pooledPreviousMembers = this.SyntaxFacts.GetMethodLevelMembers(previousRoot); var previousMembers = pooledPreviousMembers.Object; if (currentMembers.Count != previousMembers.Count) From ed9cd8338e3fc71182dd7f65f1fa9409ef709018 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 20 Aug 2024 11:06:18 -0700 Subject: [PATCH 6/6] Remove method was using for testing --- .../Services/SyntaxFacts/VisualBasicSyntaxFacts.vb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb index 339793ba0c259..bc31c6403f9a3 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/VisualBasic/Services/SyntaxFacts/VisualBasicSyntaxFacts.vb @@ -916,10 +916,6 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.LanguageService Return pooledList End Function - Private Shared Sub Releaser(pool As ObjectPool(Of List(Of SyntaxNode)), obj As List(Of SyntaxNode)) - pool.ClearAndFree(obj, pool.TrimOnFree) - End Sub - Public Function GetMembersOfTypeDeclaration(typeDeclaration As SyntaxNode) As SyntaxList(Of SyntaxNode) Implements ISyntaxFacts.GetMembersOfTypeDeclaration Return DirectCast(typeDeclaration, TypeBlockSyntax).Members End Function