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.

Sign up for GitHub

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

Remove an enumerator allocation in SymbolCompletionItem.CreateWorker #76375

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Remove an enumerator allocation in SymbolCompletionItem.CreateWorker #76375

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion ...harp/Portable/Completion/CompletionProviders/AttributeNamedParameterCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ select SymbolCompletionItem.CreateWithSymbolId(
displayText: p.Name.ToIdentifierToken().ToString(),
displayTextSuffix: displayTextSuffix,
insertionText: null,
symbols: ImmutableArray.Create(p),
symbols: [p],
contextPosition: token.SpanStart,
sortText: p.Name,
rules: CompletionItemRules.Default);
Expand Down
2 changes: 1 addition & 1 deletion ...letion/CompletionProviders/FunctionPointerUnmanagedCallingConventionCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private static void AddTypes(HashSet<CompletionItem> completionItems, int contex
completionItems.Add(
SymbolCompletionItem.CreateWithSymbolId(
displayName,
ImmutableArray.Create(type),
[type],
rules: CompletionItemRules.Default,
contextPosition));
}
Expand Down
2 changes: 1 addition & 1 deletion ...atures/CSharp/Portable/Completion/CompletionProviders/NamedParameterCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ public override async Task ProvideCompletionsAsync(CompletionContext context)
context.AddItem(SymbolCompletionItem.CreateWithSymbolId(
displayText: escapedName,
displayTextSuffix: ColonString,
symbols: ImmutableArray.Create(parameter),
symbols: [parameter],
rules: s_rules.WithMatchPriority(SymbolMatchPriority.PreferNamedArgument),
contextPosition: token.SpanStart,
filterText: escapedName));
Expand Down
2 changes: 1 addition & 1 deletion src/Features/CSharp/Portable/Completion/CompletionProviders/TupleNameCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ private static void AddItems(ImmutableArray<INamedTypeSymbol> inferredTypes, int
context.AddItem(SymbolCompletionItem.CreateWithSymbolId(
displayText: field.Name,
displayTextSuffix: ColonString,
symbols: ImmutableArray.Create(field),
symbols: [field],
rules: CompletionItemRules.Default,
contextPosition: spanStart,
filterText: field.Name));
Expand Down
26 changes: 2 additions & 24 deletions src/Features/Core/Portable/Completion/CommonCompletionUtilities.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,28 +82,6 @@ public static bool IsStartingNewWord(SourceText text, int characterPosition, Fun
return true;
}

public static Func<CancellationToken, Task<CompletionDescription>> CreateDescriptionFactory(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

public static Func<CancellationToken, Task> CreateDescriptionFactory(

These don't appear to be used anymore.

SolutionServices workspaceServices,
SemanticModel semanticModel,
int position,
ISymbol symbol,
SymbolDescriptionOptions options)
{
return CreateDescriptionFactory(workspaceServices, semanticModel, position, options, [symbol]);
}

public static Func<CancellationToken, Task<CompletionDescription>> CreateDescriptionFactory(
SolutionServices workspaceServices, SemanticModel semanticModel, int position, SymbolDescriptionOptions options, IReadOnlyList<ISymbol> symbols)
{
return c => CreateDescriptionAsync(workspaceServices, semanticModel, position, symbols, options, supportedPlatforms: null, cancellationToken: c);
}

public static Func<CancellationToken, Task<CompletionDescription>> CreateDescriptionFactory(
SolutionServices workspaceServices, SemanticModel semanticModel, int position, IReadOnlyList<ISymbol> symbols, SymbolDescriptionOptions options, SupportedPlatformData supportedPlatforms)
{
return c => CreateDescriptionAsync(workspaceServices, semanticModel, position, symbols, options, supportedPlatforms: supportedPlatforms, cancellationToken: c);
}

public static async Task<CompletionDescription> CreateDescriptionAsync(
SolutionServices workspaceServices, SemanticModel semanticModel, int position, ISymbol symbol, int overloadCount, SymbolDescriptionOptions options, SupportedPlatformData? supportedPlatforms, CancellationToken cancellationToken)
{
Expand Down Expand Up @@ -170,13 +148,13 @@ public static async Task<CompletionDescription> CreateDescriptionAsync(
}

public static Task<CompletionDescription> CreateDescriptionAsync(
SolutionServices workspaceServices, SemanticModel semanticModel, int position, IReadOnlyList<ISymbol> symbols, SymbolDescriptionOptions options, SupportedPlatformData? supportedPlatforms, CancellationToken cancellationToken)
SolutionServices workspaceServices, SemanticModel semanticModel, int position, ImmutableArray<ISymbol> symbols, SymbolDescriptionOptions options, SupportedPlatformData? supportedPlatforms, CancellationToken cancellationToken)
{
// Lets try to find the first non-obsolete symbol (overload) and fall-back
// to the first symbol if all are obsolete.
var symbol = symbols.FirstOrDefault(s => !s.IsObsolete()) ?? symbols[0];

return CreateDescriptionAsync(workspaceServices, semanticModel, position, symbol, overloadCount: symbols.Count - 1, options, supportedPlatforms, cancellationToken);
return CreateDescriptionAsync(workspaceServices, semanticModel, position, symbol, overloadCount: symbols.Length - 1, options, supportedPlatforms, cancellationToken);
}

private static void AddOverloadPart(List<TaggedText> textContentBuilder, int overloadCount, bool isGeneric)
Expand Down
2 changes: 1 addition & 1 deletion src/Features/Core/Portable/Completion/Providers/AbstractPartialTypeCompletionProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ private CompletionItem CreateCompletionItem(INamedTypeSymbol symbol, TSyntaxCont
displayText: displayText,
displayTextSuffix: suffix,
insertionText: insertionText,
symbols: ImmutableArray.Create(symbol),
symbols: [symbol],
contextPosition: context.Position,
properties: GetProperties(symbol, context),
rules: CompletionItemRules.Default);
Expand Down
32 changes: 15 additions & 17 deletions src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,17 +22,17 @@ internal static class SymbolCompletionItem
{
private const string InsertionTextProperty = "InsertionText";

private static readonly Action<IReadOnlyList<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> s_addSymbolEncoding = AddSymbolEncoding;
private static readonly Action<IReadOnlyList<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> s_addSymbolInfo = AddSymbolInfo;
private static readonly Action<ImmutableArray<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> s_addSymbolEncoding = AddSymbolEncoding;
private static readonly Action<ImmutableArray<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> s_addSymbolInfo = AddSymbolInfo;
private static readonly char[] s_projectSeperators = [';'];

private static CompletionItem CreateWorker(
string displayText,
string? displayTextSuffix,
IReadOnlyList<ISymbol> symbols,
ImmutableArray<ISymbol> symbols,
CompletionItemRules rules,
int contextPosition,
Action<IReadOnlyList<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> symbolEncoder,
Action<ImmutableArray<ISymbol>, ArrayBuilder<KeyValuePair<string, string>>> symbolEncoder,
string? sortText = null,
string? insertionText = null,
string? filterText = null,
Expand All @@ -50,16 +50,14 @@ private static CompletionItem CreateWorker(
builder.AddRange(properties);

if (insertionText != null)
{
builder.Add(KeyValuePairUtil.Create(InsertionTextProperty, insertionText));
}

builder.Add(KeyValuePairUtil.Create("ContextPosition", contextPosition.ToString()));
AddSupportedPlatforms(builder, supportedPlatforms);
symbolEncoder(symbols, builder);

tags = tags.NullToEmpty();
if (symbols.All(symbol => symbol.IsObsolete()) && !tags.Contains(WellKnownTags.Deprecated))
if (!tags.Contains(WellKnownTags.Deprecated) && symbols.All(static symbol => symbol.IsObsolete()))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

!tags.Contains(WellKnownTags.Deprecated)

order switched here as the IsObsolete checks allocate quite a bit. I doubt the caller has this tag set already in tags, but this seemed nicer.

tags = tags.Add(WellKnownTags.Deprecated);

var firstSymbol = symbols[0];
Expand All @@ -80,10 +78,10 @@ private static CompletionItem CreateWorker(
return item;
}

private static void AddSymbolEncoding(IReadOnlyList<ISymbol> symbols, ArrayBuilder<KeyValuePair<string, string>> properties)
private static void AddSymbolEncoding(ImmutableArray<ISymbol> symbols, ArrayBuilder<KeyValuePair<string, string>> properties)
=> properties.Add(KeyValuePairUtil.Create("Symbols", EncodeSymbols(symbols)));

private static void AddSymbolInfo(IReadOnlyList<ISymbol> symbols, ArrayBuilder<KeyValuePair<string, string>> properties)
private static void AddSymbolInfo(ImmutableArray<ISymbol> symbols, ArrayBuilder<KeyValuePair<string, string>> properties)
{
var symbol = symbols[0];
var isGeneric = symbol.GetArity() > 0;
Expand All @@ -107,13 +105,13 @@ public static bool GetShouldProvideParenthesisCompletion(CompletionItem item)
return false;
}

public static string EncodeSymbols(IReadOnlyList<ISymbol> symbols)
public static string EncodeSymbols(ImmutableArray<ISymbol> symbols)
{
if (symbols.Count > 1)
if (symbols.Length > 1)
{
return string.Join("|", symbols.Select(EncodeSymbol));
}
else if (symbols.Count == 1)
else if (symbols.Length == 1)
{
return EncodeSymbol(symbols[0]);
}
Expand Down Expand Up @@ -270,7 +268,7 @@ public static bool TryGetInsertionText(CompletionItem item, [NotNullWhen(true)]
// COMPAT OVERLOAD: This is used by IntelliCode.
public static CompletionItem CreateWithSymbolId(
string displayText,
IReadOnlyList<ISymbol> symbols,
ImmutableArray<ISymbol> symbols,
CompletionItemRules rules,
int contextPosition,
string? sortText = null,
Expand Down Expand Up @@ -302,7 +300,7 @@ public static CompletionItem CreateWithSymbolId(
public static CompletionItem CreateWithSymbolId(
string displayText,
string? displayTextSuffix,
IReadOnlyList<ISymbol> symbols,
ImmutableArray<ISymbol> symbols,
CompletionItemRules rules,
int contextPosition,
string? sortText = null,
Expand All @@ -326,7 +324,7 @@ public static CompletionItem CreateWithSymbolId(
public static CompletionItem CreateWithNameAndKind(
string displayText,
string displayTextSuffix,
IReadOnlyList<ISymbol> symbols,
ImmutableArray<ISymbol> symbols,
CompletionItemRules rules,
int contextPosition,
string? sortText = null,
Expand Down Expand Up @@ -357,12 +355,12 @@ internal static bool GetSymbolIsGeneric(CompletionItem item)
=> item.TryGetProperty("IsGeneric", out var v) && bool.TryParse(v, out var isGeneric) && isGeneric;

public static async Task<CompletionDescription> GetDescriptionAsync(
CompletionItem item, IReadOnlyList<ISymbol> symbols, Document document, SemanticModel semanticModel, SymbolDescriptionOptions options, CancellationToken cancellationToken)
CompletionItem item, ImmutableArray<ISymbol> symbols, Document document, SemanticModel semanticModel, SymbolDescriptionOptions options, CancellationToken cancellationToken)
{
var position = GetDescriptionPosition(item);
var supportedPlatforms = GetSupportedPlatforms(item, document.Project.Solution);

if (symbols.Count != 0)
if (symbols.Length != 0)
{
return await CommonCompletionUtilities.CreateDescriptionAsync(document.Project.Solution.Services, semanticModel, position, symbols, options, supportedPlatforms, cancellationToken).ConfigureAwait(false);
}
Expand Down
3 changes: 2 additions & 1 deletion src/Features/Core/Portable/ExternalAccess/Pythia/Api/PythiaCompletionProviderBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

namespace Microsoft.CodeAnalysis.ExternalAccess.Pythia.Api;

Expand Down Expand Up @@ -42,7 +43,7 @@ public static CompletionItem CreateSymbolCompletionItem(
SupportedPlatformData? supportedPlatforms = null,
ImmutableDictionary<string, string>? properties = null,
ImmutableArray<string> tags = default)
=> SymbolCompletionItem.CreateWithSymbolId(displayText, displayTextSuffix: null, symbols, rules, contextPosition, sortText, insertionText,
=> SymbolCompletionItem.CreateWithSymbolId(displayText, displayTextSuffix: null, symbols.ToImmutableArray(), rules, contextPosition, sortText, insertionText,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ToImmutableArray

This is potentially an allocation where it wasn't before, but only if they weren't using an ImmutableArray and it's limited to Pythia.

filterText, displayTextPrefix: null, inlineDescription: null, glyph: null, supportedPlatforms, properties.AsImmutableOrNull(), tags);

public static ImmutableArray<SymbolDisplayPart> CreateRecommendedKeywordDisplayParts(string keyword, string toolTip)
Expand Down
2 changes: 1 addition & 1 deletion ...ortable/IntroduceUsingStatement/AbstractIntroduceUsingStatementCodeRefactoringProvider.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,7 @@ private static TStatementSyntax FindSiblingStatementContainingLastUsage(
private static void AddReferencedLocalVariables(
HashSet<ISymbol> referencedVariables,
SyntaxNode node,
IReadOnlyList<ISymbol> localVariables,
ArrayBuilder<ISymbol> localVariables,
SemanticModel semanticModel,
ISyntaxFactsService syntaxFactsService,
CancellationToken cancellationToken)
Expand Down
Loading