Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use pooled objects in more locations #10598

Merged
merged 7 commits into from
Jul 11, 2024
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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(
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
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(
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
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.OrderBy(codeAction => codeAction.Title).ToImmutableArray();
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
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 ImmutableArray<RazorVSInternalCodeAction>.Empty;
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@DustinCampbell this had a list set to 1 and I'm not really sure why... I think I wouldn't need to do that here because that would just keep it on the stack?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super weird! I would just ignore the capacity in that case.

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);
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
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,7 +141,7 @@ 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);
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
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();
ryzngard marked this conversation as resolved.
Show resolved Hide resolved
}

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
Loading