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

Reduce allocations in AbstractProjectExtensionProvider.FilterExtensions #74112

Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ static ImmutableArray<CodeRefactoringProvider> GetProjectRefactorings(TextDocume
}

static ProjectCodeRefactoringProvider.ExtensionInfo GetExtensionInfo(ExportCodeRefactoringProviderAttribute attribute)
=> new(attribute.DocumentKinds, attribute.DocumentExtensions);
{
var kinds = EnumArrayConverter.FromStringArray<TextDocumentKind>(attribute.DocumentKinds);

return new(kinds, attribute.DocumentExtensions);
}
}

public async Task<bool> HasRefactoringsAsync(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ internal abstract class AbstractProjectExtensionProvider<TProvider, TExtension,
where TExportAttribute : Attribute
where TExtension : class
{
public record class ExtensionInfo(string[] DocumentKinds, string[]? DocumentExtensions);
public record class ExtensionInfo(ImmutableArray<TextDocumentKind> DocumentKinds, string[]? DocumentExtensions);

// Following CWTs are used to cache completion providers from projects' references,
// so we can avoid the slow path unless there's any change to the references.
Expand Down Expand Up @@ -87,48 +87,52 @@ public static ImmutableArray<TExtension> GetExtensions(TextDocument document, Fu

public static ImmutableArray<TExtension> FilterExtensions(TextDocument document, ImmutableArray<TExtension> extensions, Func<TExportAttribute, ExtensionInfo> getExtensionInfoForFiltering)
{
return extensions.WhereAsArray(ShouldIncludeExtension);
return extensions.WhereAsArray(ShouldIncludeExtension, (document, getExtensionInfoForFiltering));

bool ShouldIncludeExtension(TExtension extension)
static bool ShouldIncludeExtension(TExtension extension, (TextDocument, Func<TExportAttribute, ExtensionInfo>) args)
Copy link
Member

Choose a reason for hiding this comment

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

i have no problem with you inlining this method into WhereAsArray.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tried it, stared at it for a minute, and I personally like it better in the current form. Will gladly change it though if you feel fairly strongly or have an objective reason.

{
var (document, getExtensionInfoForFiltering) = args;
if (!s_extensionInfoMap.TryGetValue(extension, out var extensionInfo))
{
extensionInfo = s_extensionInfoMap.GetValue(extension,
new ConditionalWeakTable<TExtension, ExtensionInfo?>.CreateValueCallback(ComputeExtensionInfo));
}
extensionInfo = GetOrCreateExtensionInfo(extension, getExtensionInfoForFiltering);

if (extensionInfo == null)
return true;

if (!extensionInfo.DocumentKinds.Contains(document.Kind.ToString()))
if (extensionInfo.DocumentKinds.IndexOf(document.Kind) < 0)
return false;

if (document.FilePath != null &&
extensionInfo.DocumentExtensions != null &&
!extensionInfo.DocumentExtensions.Contains(PathUtilities.GetExtension(document.FilePath)))
Array.IndexOf(extensionInfo.DocumentExtensions, PathUtilities.GetExtension(document.FilePath)) < 0)
{
return false;
}

return true;
}

ExtensionInfo? ComputeExtensionInfo(TExtension extension)
static ExtensionInfo? GetOrCreateExtensionInfo(TExtension extension, Func<TExportAttribute, ExtensionInfo> getExtensionInfoForFiltering)
{
TExportAttribute? attribute;
try
{
var typeInfo = extension.GetType().GetTypeInfo();
attribute = typeInfo.GetCustomAttribute<TExportAttribute>();
}
catch
return s_extensionInfoMap.GetValue(extension,
new ConditionalWeakTable<TExtension, ExtensionInfo?>.CreateValueCallback(ComputeExtensionInfo));

ExtensionInfo? ComputeExtensionInfo(TExtension extension)
{
attribute = null;
}
TExportAttribute? attribute;
try
{
var typeInfo = extension.GetType().GetTypeInfo();
attribute = typeInfo.GetCustomAttribute<TExportAttribute>();
}
catch
{
attribute = null;
}

if (attribute == null)
return null;
return getExtensionInfoForFiltering(attribute);
if (attribute == null)
return null;
return getExtensionInfoForFiltering(attribute);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -989,7 +989,11 @@ private ImmutableDictionary<DiagnosticId, ImmutableArray<CodeFixProvider>> Compu
}

private static ProjectCodeFixProvider.ExtensionInfo GetExtensionInfo(ExportCodeFixProviderAttribute attribute)
=> new(attribute.DocumentKinds, attribute.DocumentExtensions);
{
var kinds = EnumArrayConverter.FromStringArray<TextDocumentKind>(attribute.DocumentKinds);

return new(kinds, attribute.DocumentExtensions);
}
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Jun 25, 2024

Choose a reason for hiding this comment

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

can we not have a helper for this? (since it seems duplicated).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!


private sealed class FixerComparer : IComparer<CodeFixProvider>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
// Licensed to the .NET Foundation under one or more agreements.
// 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;
using System.Collections.Immutable;

namespace Microsoft.CodeAnalysis.CodeFixesAndRefactorings;

internal static class EnumArrayConverter
{
public static ImmutableArray<TEnum> FromStringArray<TEnum>(string[] strings) where TEnum : struct, Enum
{
var enums = new FixedSizeArrayBuilder<TEnum>(strings.Length);
for (var i = 0; i < strings.Length; i++)
{
var s = strings[i];
if (!Enum.TryParse(s, out TEnum enumValue))
enumValue = default;

enums.Add(enumValue);
}

return enums.MoveToImmutable();
}
}
Loading