From 3e68ef0d9cae4414eaa190acfc0a64cf355d6953 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Thu, 8 Aug 2024 14:37:42 -0700 Subject: [PATCH] Remove most usage of "new ArrayBuilder" Most callers of this were intending to instead use ArrayBuilder.GetInstance (as that actually is what pulls items from the pool). Otherwise, the code would create a new ArrayBuilder (and consequently the ImmutableArray.Builder), reallocate until the array hit the appropriate size, and likely return the item to a pool that would never be pulled from. --- .../Core/IntelliSense/AsyncCompletion/FilterSet.cs | 4 ++-- .../ExpressionCompiler/HoistedStateMachineLocalTests.cs | 7 +++++-- .../CSharp/Test/ExpressionCompiler/LocalsTests.cs | 6 +++--- .../Debugger/Engine/DkmInspectionSession.cs | 2 +- .../SemanticSearch/AbstractSemanticSearchService.cs | 2 +- .../EditAndContinue/ActiveStatementsDescription.cs | 6 +++--- .../DidChangeConfigurationNotificationHandler.cs | 2 +- .../Formatting/AbstractFormatDocumentHandlerBase.cs | 4 ++-- .../Handler/Formatting/FormatDocumentOnTypeHandler.cs | 4 ++-- .../Handler/SignatureHelp/SignatureHelpHandler.cs | 5 +++-- src/Scripting/CSharpTest/ObjectFormatterTests.cs | 4 +++- src/VisualStudio/LiveShare/Impl/ProjectsHandler.cs | 9 +++++---- .../PerfMargin/DataModel.cs | 2 +- .../Handler/Formatting/FormatDocumentOnTypeHandler.cs | 6 +++--- .../Core/Portable/Options/GlobalOptionService.cs | 2 +- .../CoreTest/UtilityTest/NameGeneratorTests.cs | 4 ++-- 16 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/FilterSet.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/FilterSet.cs index 6477cfdd23667..817c456bf6c85 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/FilterSet.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/FilterSet.cs @@ -131,7 +131,7 @@ private static CompletionFilter CreateCompletionFilter( public (ImmutableArray filters, int data) GetFiltersAndAddToSet(RoslynCompletionItem item) { - var listBuilder = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var listBuilder); var vectorForSingleItem = new BitVector32(); if (item.Flags.IsExpanded()) @@ -150,7 +150,7 @@ private static CompletionFilter CreateCompletionFilter( } } - return (listBuilder.ToImmutableAndFree(), vectorForSingleItem.Data); + return (listBuilder.ToImmutableAndClear(), vectorForSingleItem.Data); } // test only diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/HoistedStateMachineLocalTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/HoistedStateMachineLocalTests.cs index 9ca94da5e118e..111e9ce3d2ba4 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/HoistedStateMachineLocalTests.cs +++ b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/HoistedStateMachineLocalTests.cs @@ -1390,9 +1390,12 @@ static IEnumerable M() private static string[] GetLocalNames(EvaluationContext context) { string unused; - var locals = new ArrayBuilder(); + var locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: false, typeName: out unused, testData: null); - return locals.Select(l => l.LocalName).ToArray(); + var result = locals.Select(l => l.LocalName).ToArray(); + + locals.Free(); + return result; } } } diff --git a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs index 6648928c9825f..94d13e31b1d0a 100644 --- a/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs +++ b/src/ExpressionEvaluator/CSharp/Test/ExpressionCompiler/LocalsTests.cs @@ -3863,7 +3863,7 @@ static void DummySequencePoint() EvaluationContext context; context = CreateMethodContext(runtime, "C.d__0.MoveNext", atLineNumber: 500); string unused; - var locals = new ArrayBuilder(); + var locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: true, typeName: out unused, testData: null); var names = locals.Select(l => l.LocalName).ToArray(); // The order must confirm the order of the arguments in the method signature. @@ -3877,7 +3877,7 @@ static void DummySequencePoint() EvaluationContext context; context = CreateMethodContext(runtime, "C.d__0.MoveNext", atLineNumber: 500); string unused; - var locals = new ArrayBuilder(); + var locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: true, typeName: out unused, testData: null); var names = locals.Select(l => l.LocalName).ToArray(); // The problem is not fixed in versions before 4.5: the order of arguments can be wrong. @@ -3913,7 +3913,7 @@ static void DummySequencePoint() EvaluationContext context; context = CreateMethodContext(runtime, "C.d__0.MoveNext", atLineNumber: 500); string unused; - var locals = new ArrayBuilder(); + var locals = ArrayBuilder.GetInstance(); context.CompileGetLocals(locals, argumentsOnly: true, typeName: out unused, testData: null); var names = locals.Select(l => l.LocalName).ToArray(); // The order must confirm the order of the arguments in the method signature. diff --git a/src/ExpressionEvaluator/Core/Test/ResultProvider/Debugger/Engine/DkmInspectionSession.cs b/src/ExpressionEvaluator/Core/Test/ResultProvider/Debugger/Engine/DkmInspectionSession.cs index 6c7aa75250eda..da86707bc504b 100644 --- a/src/ExpressionEvaluator/Core/Test/ResultProvider/Debugger/Engine/DkmInspectionSession.cs +++ b/src/ExpressionEvaluator/Core/Test/ResultProvider/Debugger/Engine/DkmInspectionSession.cs @@ -71,7 +71,7 @@ internal bool Equals(InstanceAndMethod other) internal Dispatcher(ImmutableArray items) { _implementations = items; - _calls = new ArrayBuilder(); + _calls = ArrayBuilder.GetInstance(); } internal TResult Invoke(object instance, MethodId method, Func f) diff --git a/src/Features/Core/Portable/SemanticSearch/AbstractSemanticSearchService.cs b/src/Features/Core/Portable/SemanticSearch/AbstractSemanticSearchService.cs index 3194bd7e2095d..81a53d7d5c53f 100644 --- a/src/Features/Core/Portable/SemanticSearch/AbstractSemanticSearchService.cs +++ b/src/Features/Core/Portable/SemanticSearch/AbstractSemanticSearchService.cs @@ -388,7 +388,7 @@ private static bool TryGetFindMethod(Assembly queryAssembly, [NotNullWhen(true)] { try { - var candidates = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var candidates); foreach (var candidate in type.GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Static | BindingFlags.Instance)) { diff --git a/src/Features/TestUtilities/EditAndContinue/ActiveStatementsDescription.cs b/src/Features/TestUtilities/EditAndContinue/ActiveStatementsDescription.cs index c7dcbbf56cf69..46b1ff47b7d2e 100644 --- a/src/Features/TestUtilities/EditAndContinue/ActiveStatementsDescription.cs +++ b/src/Features/TestUtilities/EditAndContinue/ActiveStatementsDescription.cs @@ -52,8 +52,8 @@ public ActiveStatementsDescription(string oldMarkedSource, string newMarkedSourc var activeStatementCount = Math.Max(OldStatements.Length, (newActiveStatementMarkers.Length == 0) ? -1 : newActiveStatementMarkers.Max(m => m.Id)); - var newMappedSpans = new ArrayBuilder(); - var newMappedRegions = new ArrayBuilder>(); + using var _1 = ArrayBuilder.GetInstance(out var newMappedSpans); + using var _2 = ArrayBuilder>.GetInstance(out var newMappedRegions); var newExceptionRegionMarkers = SourceMarkers.GetExceptionRegions(newMarkedSource); newMappedSpans.ZeroInit(activeStatementCount); @@ -137,7 +137,7 @@ internal static ImmutableArray GetUnmappedActiveStateme { var map = new Dictionary>(); - var activeStatements = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var activeStatements); var sourceIndex = 0; foreach (var markedSource in markedSources) diff --git a/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs b/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs index 07786e46a47de..914489a045c28 100644 --- a/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Configuration/DidChangeConfigurationNotificationHandler.cs @@ -85,7 +85,7 @@ private async Task RefreshOptionsAsync(CancellationToken cancellationToken) RoslynDebug.Assert(configurationsFromClient.Length == SupportedOptions.Sum(option => option is IPerLanguageValuedOption ? 2 : 1)); // LSP ensures the order of result from client should match the order we sent from server. - var optionsToUpdate = new ArrayBuilder>(); + using var _ = ArrayBuilder>.GetInstance(out var optionsToUpdate); for (var i = 0; i < configurationsFromClient.Length; i++) { diff --git a/src/LanguageServer/Protocol/Handler/Formatting/AbstractFormatDocumentHandlerBase.cs b/src/LanguageServer/Protocol/Handler/Formatting/AbstractFormatDocumentHandlerBase.cs index 45f13c67d6d03..331c3971595bf 100644 --- a/src/LanguageServer/Protocol/Handler/Formatting/AbstractFormatDocumentHandlerBase.cs +++ b/src/LanguageServer/Protocol/Handler/Formatting/AbstractFormatDocumentHandlerBase.cs @@ -42,9 +42,9 @@ internal abstract class AbstractFormatDocumentHandlerBase(); + using var _ = ArrayBuilder.GetInstance(out var edits); edits.AddRange(textChanges.Select(change => ProtocolConversions.TextChangeToTextEdit(change, text))); - return edits.ToArrayAndFree(); + return edits.ToArray(); } public abstract LSP.TextDocumentIdentifier GetTextDocumentIdentifier(RequestType request); diff --git a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs index f15993645ca9c..87efbd4a7a72c 100644 --- a/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs +++ b/src/LanguageServer/Protocol/Handler/Formatting/FormatDocumentOnTypeHandler.cs @@ -72,9 +72,9 @@ public FormatDocumentOnTypeHandler(IGlobalOptionService globalOptions) return []; } - var edits = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var edits); edits.AddRange(textChanges.Select(change => ProtocolConversions.TextChangeToTextEdit(change, documentSyntax.Text))); - return edits.ToArrayAndFree(); + return edits.ToArray(); } } } diff --git a/src/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs b/src/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs index 9984c8f58804c..055fba092cd05 100644 --- a/src/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs +++ b/src/LanguageServer/Protocol/Handler/SignatureHelp/SignatureHelpHandler.cs @@ -136,9 +136,10 @@ private static string GetSignatureText(SignatureHelpItem item) return sb.ToString(); } + private static ClassifiedTextElement GetSignatureClassifiedText(SignatureHelpItem item) { - var taggedTexts = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var taggedTexts); taggedTexts.AddRange(item.PrefixDisplayParts); @@ -160,7 +161,7 @@ private static ClassifiedTextElement GetSignatureClassifiedText(SignatureHelpIte taggedTexts.AddRange(item.SuffixDisplayParts); taggedTexts.AddRange(item.DescriptionParts); - return new ClassifiedTextElement(taggedTexts.ToArrayAndFree().Select(part => new ClassifiedTextRun(part.Tag.ToClassificationTypeName(), part.Text))); + return new ClassifiedTextElement(taggedTexts.ToArray().Select(part => new ClassifiedTextRun(part.Tag.ToClassificationTypeName(), part.Text))); } } } diff --git a/src/Scripting/CSharpTest/ObjectFormatterTests.cs b/src/Scripting/CSharpTest/ObjectFormatterTests.cs index 07201a7ada581..2f5baa40fb5c2 100644 --- a/src/Scripting/CSharpTest/ObjectFormatterTests.cs +++ b/src/Scripting/CSharpTest/ObjectFormatterTests.cs @@ -861,7 +861,7 @@ public void DebuggerProxy_DiagnosticBag() [Fact] public void DebuggerProxy_ArrayBuilder() { - var obj = new ArrayBuilder(); + var obj = ArrayBuilder.GetInstance(); obj.AddRange(new[] { 1, 2, 3, 4, 5 }); var str = s_formatter.FormatObject(obj, SingleLineOptions); @@ -875,6 +875,8 @@ public void DebuggerProxy_ArrayBuilder() "4", "5" ); + + obj.Free(); } [Fact, WorkItem(8542, "https://github.com/dotnet/roslyn/issues/8452")] diff --git a/src/VisualStudio/LiveShare/Impl/ProjectsHandler.cs b/src/VisualStudio/LiveShare/Impl/ProjectsHandler.cs index 5e1e1961cd06f..39413cf743dfc 100644 --- a/src/VisualStudio/LiveShare/Impl/ProjectsHandler.cs +++ b/src/VisualStudio/LiveShare/Impl/ProjectsHandler.cs @@ -21,11 +21,11 @@ internal class ProjectsHandler : ILspRequestHandler { public async Task HandleAsync(object param, RequestContext requestContext, CancellationToken cancellationToken) { - var projects = new ArrayBuilder(); + using var _1 = ArrayBuilder.GetInstance(out var projects); + using var _2 = ArrayBuilder.GetInstance(out var externalUris); var solution = requestContext.Context; foreach (var project in solution.Projects) { - var externalUris = new ArrayBuilder(); foreach (var sourceFile in project.Documents) { var uri = new Uri(sourceFile.FilePath); @@ -37,7 +37,7 @@ public async Task HandleAsync(object param, RequestContext r } } #pragma warning disable 0612 - await requestContext.ProtocolConverter.RegisterExternalFilesAsync(externalUris.ToArrayAndFree()).ConfigureAwait(false); + await requestContext.ProtocolConverter.RegisterExternalFilesAsync(externalUris.ToArray()).ConfigureAwait(false); #pragma warning restore 0612 var lspProject = new CustomProtocol.Project @@ -48,9 +48,10 @@ public async Task HandleAsync(object param, RequestContext r }; projects.Add(lspProject); + externalUris.Clear(); } - return projects.ToArrayAndFree(); + return projects.ToArray(); } } } diff --git a/src/VisualStudio/VisualStudioDiagnosticsToolWindow/PerfMargin/DataModel.cs b/src/VisualStudio/VisualStudioDiagnosticsToolWindow/PerfMargin/DataModel.cs index 1659f16293a5c..d99edc1fdd2a8 100644 --- a/src/VisualStudio/VisualStudioDiagnosticsToolWindow/PerfMargin/DataModel.cs +++ b/src/VisualStudio/VisualStudioDiagnosticsToolWindow/PerfMargin/DataModel.cs @@ -22,7 +22,7 @@ public DataModel() where !field.IsSpecialName select field; - var builder = new ArrayBuilder(); + using var _ = ArrayBuilder.GetInstance(out var builder); var features = new Dictionary(); var root = new ActivityLevel("All"); diff --git a/src/VisualStudio/Xaml/Impl/Implementation/LanguageServer/Handler/Formatting/FormatDocumentOnTypeHandler.cs b/src/VisualStudio/Xaml/Impl/Implementation/LanguageServer/Handler/Formatting/FormatDocumentOnTypeHandler.cs index b5952b67e4583..f534a21a07941 100644 --- a/src/VisualStudio/Xaml/Impl/Implementation/LanguageServer/Handler/Formatting/FormatDocumentOnTypeHandler.cs +++ b/src/VisualStudio/Xaml/Impl/Implementation/LanguageServer/Handler/Formatting/FormatDocumentOnTypeHandler.cs @@ -33,12 +33,12 @@ public FormatDocumentOnTypeHandler() public async Task HandleRequestAsync(DocumentOnTypeFormattingParams request, RequestContext context, CancellationToken cancellationToken) { - var edits = new ArrayBuilder(); if (string.IsNullOrEmpty(request.Character)) { - return edits.ToArrayAndFree(); + return []; } + using var _ = ArrayBuilder.GetInstance(out var edits); var document = context.Document; var formattingService = document?.Project.Services.GetService(); if (document != null && formattingService != null) @@ -53,7 +53,7 @@ public async Task HandleRequestAsync(DocumentOnTypeFormattingParams } } - return edits.ToArrayAndFree(); + return edits.ToArray(); } } } diff --git a/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs b/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs index a8e2021cdc3b2..e3b8e60784b30 100644 --- a/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs +++ b/src/Workspaces/Core/Portable/Options/GlobalOptionService.cs @@ -188,7 +188,7 @@ public bool SetGlobalOptions(ImmutableArray> o private bool SetGlobalOptions(OneOrMany> options) { - var changedOptions = new ArrayBuilder<(OptionKey2, object?)>(options.Count); + using var _ = ArrayBuilder<(OptionKey2, object?)>.GetInstance(options.Count, out var changedOptions); var persisters = GetOptionPersisters(); lock (_gate) diff --git a/src/Workspaces/CoreTest/UtilityTest/NameGeneratorTests.cs b/src/Workspaces/CoreTest/UtilityTest/NameGeneratorTests.cs index 96610a53a0bbc..711561fa13d48 100644 --- a/src/Workspaces/CoreTest/UtilityTest/NameGeneratorTests.cs +++ b/src/Workspaces/CoreTest/UtilityTest/NameGeneratorTests.cs @@ -96,10 +96,10 @@ public void EnsureUniquenessInPlaceCanUseNotIncludingFirst10(string[] names, str private static void VerifyEnsureUniquenessInPlace(string[] names, bool[]? isFixed, Func? canUse, bool isCaseSensitive, string[] expectedResult) { - var namesBuilder = new ArrayBuilder(); + using var _1 = ArrayBuilder.GetInstance(out var namesBuilder); namesBuilder.AddRange(names); - var isFixedBuilder = new ArrayBuilder(); + using var _2 = ArrayBuilder.GetInstance(out var isFixedBuilder); isFixedBuilder.AddRange(isFixed ?? Enumerable.Repeat(false, names.Length)); NameGenerator.EnsureUniquenessInPlace(namesBuilder, isFixedBuilder, canUse, isCaseSensitive);