Skip to content

Commit

Permalink
Use pooled objects in more locations (#10598)
Browse files Browse the repository at this point in the history
Cleaning up some places that are newing up Lists intead of using the object pool. I also moved a few things to ImmutableArray instead of List since they weren't mutated after initial collection
  • Loading branch information
ryzngard authored Jul 11, 2024
1 parent 0fb08aa commit 7b511df
Show file tree
Hide file tree
Showing 34 changed files with 368 additions and 319 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void ApplyCapabilities(VSInternalServerCapabilities serverCapabilities, V

var character = request.Character;

using var _ = ListPool<IOnAutoInsertProvider>.GetPooledObject(out var applicableProviders);
using var applicableProviders = new PooledArrayBuilder<IOnAutoInsertProvider>();
foreach (var provider in _onAutoInsertProviders)
{
if (provider.TriggerCharacter == character)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis.CSharp;
using Microsoft.CodeAnalysis.CSharp.Syntax;
using Microsoft.CodeAnalysis.Text;
Expand Down Expand Up @@ -38,7 +39,7 @@ public static async Task<TextEdit[]> GetUsingStatementEditsAsync(RazorCodeDocume
var oldUsings = await FindUsingDirectiveStringsAsync(originalCSharpText, cancellationToken).ConfigureAwait(false);
var newUsings = await FindUsingDirectiveStringsAsync(changedCSharpText, cancellationToken).ConfigureAwait(false);

var edits = new List<TextEdit>();
using var edits = new PooledArrayBuilder<TextEdit>();
foreach (var usingStatement in newUsings.Except(oldUsings))
{
// This identifier will be eventually thrown away.
Expand All @@ -48,7 +49,7 @@ public static async Task<TextEdit[]> GetUsingStatementEditsAsync(RazorCodeDocume
edits.AddRange(workspaceEdit.DocumentChanges!.Value.First.First().Edits);
}

return [.. edits];
return edits.ToArray();
}

private static async Task<IEnumerable<string>> FindUsingDirectiveStringsAsync(SourceText originalCSharpText, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Text.Json;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.AspNetCore.Razor.Threading;
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
using Microsoft.CodeAnalysis.Razor.Workspaces;
Expand Down Expand Up @@ -52,16 +54,16 @@ internal sealed class DefaultCSharpCodeActionProvider(LanguageServerFeatureOptio

private readonly LanguageServerFeatureOptions _languageServerFeatureOptions = languageServerFeatureOptions;

public Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(
public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(
RazorCodeActionContext context,
IEnumerable<RazorVSInternalCodeAction> codeActions,
ImmutableArray<RazorVSInternalCodeAction> codeActions,
CancellationToken cancellationToken)
{
// Used to identify if this is VSCode which doesn't support
// code action resolve.
if (!context.SupportsCodeActionResolve)
{
return SpecializedTasks.AsNullable(SpecializedTasks.EmptyReadOnlyList<RazorVSInternalCodeAction>());
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

var tree = context.CodeDocument.GetSyntaxTree();
Expand All @@ -72,7 +74,7 @@ internal sealed class DefaultCSharpCodeActionProvider(LanguageServerFeatureOptio
? SupportedImplicitExpressionCodeActionNames
: SupportedDefaultCodeActionNames;

var results = new List<RazorVSInternalCodeAction>();
using var results = new PooledArrayBuilder<RazorVSInternalCodeAction>();

foreach (var codeAction in codeActions)
{
Expand All @@ -94,7 +96,7 @@ internal sealed class DefaultCSharpCodeActionProvider(LanguageServerFeatureOptio
}
}

return Task.FromResult<IReadOnlyList<RazorVSInternalCodeAction>?>(results);
return Task.FromResult(results.ToImmutable());

static bool CanDeserializeTo<T>(object? data)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
using System.Diagnostics.CodeAnalysis;
using System.Linq;
Expand All @@ -12,6 +13,7 @@
using Microsoft.AspNetCore.Razor.Language.Syntax;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Razor;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.AspNetCore.Razor.Threading;
using Microsoft.CodeAnalysis.ExternalAccess.Razor;
using Microsoft.CodeAnalysis.Razor.Workspaces;
Expand All @@ -36,32 +38,32 @@ internal sealed class TypeAccessibilityCodeActionProvider : ICSharpCodeActionPro
"IDE1007"
};

public Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(
public Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(
RazorCodeActionContext context,
IEnumerable<RazorVSInternalCodeAction> codeActions,
ImmutableArray<RazorVSInternalCodeAction> codeActions,
CancellationToken cancellationToken)
{
if (context.Request?.Context?.Diagnostics is null)
{
return SpecializedTasks.AsNullable(SpecializedTasks.EmptyReadOnlyList<RazorVSInternalCodeAction>());
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

if (!codeActions.Any())
if (codeActions.IsEmpty)
{
return SpecializedTasks.AsNullable(SpecializedTasks.EmptyReadOnlyList<RazorVSInternalCodeAction>());
return SpecializedTasks.EmptyImmutableArray<RazorVSInternalCodeAction>();
}

var results = context.SupportsCodeActionResolve
? ProcessCodeActionsVS(context, codeActions)
: ProcessCodeActionsVSCode(context, codeActions);

var orderedResults = results.OrderBy(codeAction => codeAction.Title).ToArray();
return Task.FromResult<IReadOnlyList<RazorVSInternalCodeAction>?>(orderedResults);
var orderedResults = results.Sort(static (x, y) => StringComparer.CurrentCulture.Compare(x.Title, y.Title));
return Task.FromResult(orderedResults);
}

private static IEnumerable<RazorVSInternalCodeAction> ProcessCodeActionsVSCode(
private static ImmutableArray<RazorVSInternalCodeAction> ProcessCodeActionsVSCode(
RazorCodeActionContext context,
IEnumerable<RazorVSInternalCodeAction> codeActions)
ImmutableArray<RazorVSInternalCodeAction> codeActions)
{
var diagnostics = context.Request.Context.Diagnostics.Where(diagnostic =>
diagnostic is { Severity: DiagnosticSeverity.Error, Code: { } code } &&
Expand All @@ -70,10 +72,10 @@ private static IEnumerable<RazorVSInternalCodeAction> ProcessCodeActionsVSCode(

if (diagnostics is null || !diagnostics.Any())
{
return Array.Empty<RazorVSInternalCodeAction>();
return [];
}

var typeAccessibilityCodeActions = new List<RazorVSInternalCodeAction>();
using var typeAccessibilityCodeActions = new PooledArrayBuilder<RazorVSInternalCodeAction>();

foreach (var diagnostic in diagnostics)
{
Expand Down Expand Up @@ -142,14 +144,14 @@ private static IEnumerable<RazorVSInternalCodeAction> ProcessCodeActionsVSCode(
}
}

return typeAccessibilityCodeActions;
return typeAccessibilityCodeActions.ToImmutable();
}

private static IEnumerable<RazorVSInternalCodeAction> ProcessCodeActionsVS(
private static ImmutableArray<RazorVSInternalCodeAction> ProcessCodeActionsVS(
RazorCodeActionContext context,
IEnumerable<RazorVSInternalCodeAction> codeActions)
ImmutableArray<RazorVSInternalCodeAction> codeActions)
{
var typeAccessibilityCodeActions = new List<RazorVSInternalCodeAction>(1);
using var typeAccessibilityCodeActions = new PooledArrayBuilder<RazorVSInternalCodeAction>();

foreach (var codeAction in codeActions)
{
Expand Down Expand Up @@ -199,7 +201,7 @@ private static IEnumerable<RazorVSInternalCodeAction> ProcessCodeActionsVS(
}
}

return typeAccessibilityCodeActions;
return typeAccessibilityCodeActions.ToImmutable();

static bool TryGetOwner(RazorCodeActionContext context, [NotNullWhen(true)] out SyntaxNode? owner)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ internal sealed class CodeActionEndpoint(

cancellationToken.ThrowIfCancellationRequested();

using var _ = ArrayBuilderPool<SumType<Command, CodeAction>>.GetPooledObject(out var commandsOrCodeActions);
using var commandsOrCodeActions = new PooledArrayBuilder<SumType<Command, CodeAction>>();

// Grouping the code actions causes VS to sort them into groups, rather than just alphabetically sorting them
// by title. The latter is bad for us because it can put "Remove <div>" at the top in some locales, and our fully
Expand Down Expand Up @@ -201,12 +201,12 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetDelegatedCodeAc
providers = _htmlCodeActionProviders;
}

return await FilterCodeActionsAsync(context, codeActions, providers, cancellationToken).ConfigureAwait(false);
return await FilterCodeActionsAsync(context, codeActions.ToImmutableArray(), providers, cancellationToken).ConfigureAwait(false);
}

private RazorVSInternalCodeAction[] ExtractCSharpCodeActionNamesFromData(RazorVSInternalCodeAction[] codeActions)
{
using var _ = ArrayBuilderPool<RazorVSInternalCodeAction>.GetPooledObject(out var actions);
using var actions = new PooledArrayBuilder<RazorVSInternalCodeAction>();

foreach (var codeAction in codeActions)
{
Expand Down Expand Up @@ -239,20 +239,19 @@ private RazorVSInternalCodeAction[] ExtractCSharpCodeActionNamesFromData(RazorVS

private static async Task<ImmutableArray<RazorVSInternalCodeAction>> FilterCodeActionsAsync(
RazorCodeActionContext context,
RazorVSInternalCodeAction[] codeActions,
ImmutableArray<RazorVSInternalCodeAction> codeActions,
IEnumerable<ICodeActionProvider> providers,
CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

using var _ = ArrayBuilderPool<Task<IReadOnlyList<RazorVSInternalCodeAction>?>>.GetPooledObject(out var tasks);

using var tasks = new PooledArrayBuilder<Task<ImmutableArray<RazorVSInternalCodeAction>>>();
foreach (var provider in providers)
{
tasks.Add(provider.ProvideAsync(context, codeActions, cancellationToken));
}

return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken).ConfigureAwait(false);
return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutable(), cancellationToken).ConfigureAwait(false);
}

// Internal for testing
Expand Down Expand Up @@ -304,35 +303,32 @@ private async Task<ImmutableArray<RazorVSInternalCodeAction>> GetRazorCodeAction
{
cancellationToken.ThrowIfCancellationRequested();

using var _ = ArrayBuilderPool<Task<IReadOnlyList<RazorVSInternalCodeAction>?>>.GetPooledObject(out var tasks);
using var tasks = new PooledArrayBuilder<Task<ImmutableArray<RazorVSInternalCodeAction>>>();

foreach (var provider in _razorCodeActionProviders)
{
tasks.Add(provider.ProvideAsync(context, cancellationToken));
}

return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutableArray(), cancellationToken).ConfigureAwait(false);
return await ConsolidateCodeActionsFromProvidersAsync(tasks.ToImmutable(), cancellationToken).ConfigureAwait(false);
}

private static async Task<ImmutableArray<RazorVSInternalCodeAction>> ConsolidateCodeActionsFromProvidersAsync(
ImmutableArray<Task<IReadOnlyList<RazorVSInternalCodeAction>?>> tasks,
ImmutableArray<Task<ImmutableArray<RazorVSInternalCodeAction>>> tasks,
CancellationToken cancellationToken)
{
var results = await Task.WhenAll(tasks).ConfigureAwait(false);

using var _ = ArrayBuilderPool<RazorVSInternalCodeAction>.GetPooledObject(out var codeActions);
using var codeActions = new PooledArrayBuilder<RazorVSInternalCodeAction>();

cancellationToken.ThrowIfCancellationRequested();

foreach (var result in results)
{
if (result is not null)
{
codeActions.AddRange(result);
}
codeActions.AddRange(result);
}

return codeActions.ToImmutableArray();
return codeActions.ToImmutable();
}

private static ImmutableHashSet<string> GetAllAvailableCodeActionNames()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See License.txt in the project root for license information.

using System;
using System.Collections.Frozen;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Diagnostics;
Expand All @@ -23,9 +24,9 @@ internal sealed class CodeActionResolveEndpoint(
IEnumerable<HtmlCodeActionResolver> htmlCodeActionResolvers,
ILoggerFactory loggerFactory) : IRazorDocumentlessRequestHandler<CodeAction, CodeAction>
{
private readonly ImmutableDictionary<string, IRazorCodeActionResolver> _razorCodeActionResolvers = CreateResolverMap(razorCodeActionResolvers);
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _csharpCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(csharpCodeActionResolvers);
private readonly ImmutableDictionary<string, BaseDelegatedCodeActionResolver> _htmlCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(htmlCodeActionResolvers);
private readonly FrozenDictionary<string, IRazorCodeActionResolver> _razorCodeActionResolvers = CreateResolverMap(razorCodeActionResolvers);
private readonly FrozenDictionary<string, BaseDelegatedCodeActionResolver> _csharpCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(csharpCodeActionResolvers);
private readonly FrozenDictionary<string, BaseDelegatedCodeActionResolver> _htmlCodeActionResolvers = CreateResolverMap<BaseDelegatedCodeActionResolver>(htmlCodeActionResolvers);
private readonly ILogger _logger = loggerFactory.GetOrCreateLogger<CodeActionResolveEndpoint>();

public bool MutatesSolutionState => false;
Expand Down Expand Up @@ -111,7 +112,7 @@ internal Task<CodeAction> ResolveCSharpCodeActionAsync(CodeAction codeAction, Ra
internal Task<CodeAction> ResolveHtmlCodeActionAsync(CodeAction codeAction, RazorCodeActionResolutionParams resolutionParams, CancellationToken cancellationToken)
=> ResolveDelegatedCodeActionAsync(_htmlCodeActionResolvers, codeAction, resolutionParams, cancellationToken);

private async Task<CodeAction> ResolveDelegatedCodeActionAsync(ImmutableDictionary<string, BaseDelegatedCodeActionResolver> resolvers, CodeAction codeAction, RazorCodeActionResolutionParams resolutionParams, CancellationToken cancellationToken)
private async Task<CodeAction> ResolveDelegatedCodeActionAsync(FrozenDictionary<string, BaseDelegatedCodeActionResolver> resolvers, CodeAction codeAction, RazorCodeActionResolutionParams resolutionParams, CancellationToken cancellationToken)
{
if (resolutionParams.Data is not JsonElement csharpParamsObj)
{
Expand Down Expand Up @@ -140,10 +141,10 @@ private async Task<CodeAction> ResolveDelegatedCodeActionAsync(ImmutableDictiona
return resolvedCodeAction;
}

private static ImmutableDictionary<string, T> CreateResolverMap<T>(IEnumerable<T> codeActionResolvers)
private static FrozenDictionary<string, T> CreateResolverMap<T>(IEnumerable<T> codeActionResolvers)
where T : ICodeActionResolver
{
using var _ = DictionaryPool<string, T>.GetPooledObject(out var resolverMap);
using var _ = StringDictionaryPool<T>.GetPooledObject(out var resolverMap);

foreach (var resolver in codeActionResolvers)
{
Expand All @@ -155,7 +156,7 @@ private static ImmutableDictionary<string, T> CreateResolverMap<T>(IEnumerable<T
resolverMap[resolver.Action] = resolver;
}

return resolverMap.ToImmutableDictionary();
return resolverMap.ToFrozenDictionary();
}

private static string GetCodeActionId(RazorCodeActionResolutionParams resolutionParams) =>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.Language;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
using Microsoft.AspNetCore.Razor.LanguageServer.Formatting;
using Microsoft.AspNetCore.Razor.PooledObjects;
using Microsoft.CodeAnalysis.Razor.DocumentMapping;
using Microsoft.CodeAnalysis.Razor.Workspaces;
using Microsoft.VisualStudio.LanguageServer.Protocol;
Expand All @@ -17,13 +19,12 @@ internal sealed class DefaultHtmlCodeActionProvider(IRazorDocumentMappingService
{
private readonly IRazorDocumentMappingService _documentMappingService = documentMappingService;

public async Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(
public async Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(
RazorCodeActionContext context,
IEnumerable<RazorVSInternalCodeAction> codeActions,
ImmutableArray<RazorVSInternalCodeAction> codeActions,
CancellationToken cancellationToken)
{
var results = new List<RazorVSInternalCodeAction>();

using var results = new PooledArrayBuilder<RazorVSInternalCodeAction>(codeActions.Length);
foreach (var codeAction in codeActions)
{
if (codeAction.Edit is not null)
Expand All @@ -38,7 +39,7 @@ internal sealed class DefaultHtmlCodeActionProvider(IRazorDocumentMappingService
}
}

return results;
return results.ToImmutable();
}

public static async Task RemapAndFixHtmlCodeActionEditAsync(IRazorDocumentMappingService documentMappingService, RazorCodeDocument codeDocument, CodeAction codeAction, CancellationToken cancellationToken)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license. See License.txt in the project root for license information.

using System.Collections.Generic;
using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Razor.LanguageServer.CodeActions.Models;
Expand All @@ -17,5 +18,5 @@ internal interface ICodeActionProvider
/// The list of code actions returned from all providers will be combined together in a list. A null result and an empty
/// result are effectively the same.
/// </remarks>
Task<IReadOnlyList<RazorVSInternalCodeAction>?> ProvideAsync(RazorCodeActionContext context, IEnumerable<RazorVSInternalCodeAction> codeActions, CancellationToken cancellationToken);
Task<ImmutableArray<RazorVSInternalCodeAction>> ProvideAsync(RazorCodeActionContext context, ImmutableArray<RazorVSInternalCodeAction> codeActions, CancellationToken cancellationToken);
}
Loading

0 comments on commit 7b511df

Please sign in to comment.