From f0330455c4d5ad771477e34ad93dc57974f781aa Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 7 Jan 2025 07:25:31 -0800 Subject: [PATCH 1/2] Change SupportedPlatformData to use ImmutableArrays for it's data I'm looking at a different change to completion and the usage of IEnumerable and List in SupportedPlatformData was tripping it up. It's easy enough to just have it use ImmutableArrays instead. One concern would be additional allocations due to the IEnumerable -> ImmutableArray conversion. However, I don't think is an issue as the IEnumerable member is always passed in from a List except from AbstractSignatureHelpProvider. In that case, it's a constructed Linq Select/Concat expression, which is reused multiple times (and potentially enumerated multiple times in SupportedPlatformData). It's extremely likely that it's better to just realize that array once. --- .../AbstractSymbolCompletionProvider.cs | 29 +++++++++++-------- .../Providers/SymbolCompletionItem.cs | 4 +-- .../CommonSemanticQuickInfoProvider.cs | 11 +++---- .../Shared/Utilities/SupportedPlatformData.cs | 13 ++++----- .../AbstractSignatureHelpProvider.cs | 6 ++-- 5 files changed, 34 insertions(+), 29 deletions(-) diff --git a/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs index cde4cc65aa669..6610dde75c4fb 100644 --- a/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs @@ -98,8 +98,8 @@ private ImmutableArray CreateItems( CompletionContext completionContext, ImmutableArray symbols, Func contextLookup, - Dictionary>? invalidProjectMap, - List? totalProjects) + Dictionary>? invalidProjectMap, + ImmutableArray totalProjects) { // We might get symbol w/o name but CanBeReferencedByName is still set to true, // need to filter them out. @@ -197,13 +197,13 @@ protected static bool TryFindFirstSymbolMatchesTargetTypes( private static SupportedPlatformData? ComputeSupportedPlatformData( CompletionContext completionContext, ImmutableArray symbols, - Dictionary>? invalidProjectMap, - List? totalProjects) + Dictionary>? invalidProjectMap, + ImmutableArray totalProjects) { SupportedPlatformData? supportedPlatformData = null; if (invalidProjectMap != null) { - List? invalidProjects = null; + ArrayBuilder? invalidProjects = null; foreach (var symbol in symbols) { if (invalidProjectMap.TryGetValue(symbol.Symbol, out invalidProjects)) @@ -211,7 +211,7 @@ protected static bool TryFindFirstSymbolMatchesTargetTypes( } if (invalidProjects != null) - supportedPlatformData = new SupportedPlatformData(completionContext.Document.Project.Solution, invalidProjects, totalProjects); + supportedPlatformData = new SupportedPlatformData(completionContext.Document.Project.Solution, invalidProjects.ToImmutable(), totalProjects); } return supportedPlatformData; @@ -299,7 +299,7 @@ private async Task> GetItemsAsync( if (relatedDocumentIds.IsEmpty) { var itemsForCurrentDocument = await GetSymbolsAsync(completionContext, syntaxContext, position, options, cancellationToken).ConfigureAwait(false); - return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: null); + return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: ImmutableArray.Empty); } using var _ = PooledDictionary.GetInstance(out var documentIdToIndex); @@ -315,10 +315,15 @@ private async Task> GetItemsAsync( var symbolToContextMap = UnionSymbols(contextAndSymbolLists); var missingSymbolsMap = FindSymbolsMissingInLinkedContexts(symbolToContextMap, contextAndSymbolLists); - var totalProjects = contextAndSymbolLists.Select(t => t.documentId.ProjectId).ToList(); + var totalProjects = contextAndSymbolLists.SelectAsArray(t => t.documentId.ProjectId); - return CreateItems( + var items = CreateItems( completionContext, [.. symbolToContextMap.Keys], symbol => symbolToContextMap[symbol], missingSymbolsMap, totalProjects); + + foreach (var (_, builder) in missingSymbolsMap) + builder.Free(); + + return items; } protected virtual bool IsExclusive() @@ -395,17 +400,17 @@ protected async Task> TryGetSymbolsForCon /// The symbols recommended in the active context. /// The symbols recommended in linked documents /// The list of projects each recommended symbol did NOT appear in. - private static Dictionary> FindSymbolsMissingInLinkedContexts( + private static Dictionary> FindSymbolsMissingInLinkedContexts( Dictionary symbolToContext, ImmutableArray<(DocumentId documentId, TSyntaxContext syntaxContext, ImmutableArray symbols)> linkedContextSymbolLists) { - var missingSymbols = new Dictionary>(LinkedFilesSymbolEquivalenceComparer.Instance); + var missingSymbols = new Dictionary>(LinkedFilesSymbolEquivalenceComparer.Instance); foreach (var (documentId, syntaxContext, symbols) in linkedContextSymbolLists) { var symbolsMissingInLinkedContext = symbolToContext.Keys.Except(symbols); foreach (var (symbol, _) in symbolsMissingInLinkedContext) - missingSymbols.GetOrAdd(symbol, m => []).Add(documentId.ProjectId); + missingSymbols.GetOrAdd(symbol, m => ArrayBuilder.GetInstance()).Add(documentId.ProjectId); } return missingSymbols; diff --git a/src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs b/src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs index a94c57660ac95..a4ce028eefdb4 100644 --- a/src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs +++ b/src/Features/Core/Portable/Completion/Providers/SymbolCompletionItem.cs @@ -236,8 +236,8 @@ private static void AddSupportedPlatforms(ArrayBuilder ProjectId.CreateFromSerialized(Guid.Parse(s)))], - candidateProjects.Split(s_projectSeperators).Select(s => ProjectId.CreateFromSerialized(Guid.Parse(s))).ToList()); + invalidProjects.Split(s_projectSeperators).SelectAsArray(s => ProjectId.CreateFromSerialized(Guid.Parse(s))), + candidateProjects.Split(s_projectSeperators).SelectAsArray(s => ProjectId.CreateFromSerialized(Guid.Parse(s)))); } return null; diff --git a/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs b/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs index 864c010dc711b..a01fc474a224f 100644 --- a/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs +++ b/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs @@ -10,6 +10,7 @@ using System.Threading.Tasks; using Microsoft.CodeAnalysis.Host; using Microsoft.CodeAnalysis.LanguageService; +using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.Utilities; @@ -86,9 +87,6 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf var semanticModel = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); var mainTokenInformation = BindToken(services, semanticModel, token, cancellationToken); - var candidateProjects = new List { document.Project.Id }; - var invalidProjects = new List(); - var candidateResults = new List<(DocumentId docId, TokenInformation tokenInformation)> { (document.Id, mainTokenInformation) @@ -103,7 +101,6 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf if (linkedToken != default) { // Not in an inactive region, so this file is a candidate. - candidateProjects.Add(linkedDocumentId.ProjectId); var linkedSymbols = BindToken(services, linkedModel, linkedToken, cancellationToken); candidateResults.Add((linkedDocumentId, linkedSymbols)); } @@ -117,7 +114,11 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf if (bestBinding.tokenInformation.Symbols.IsDefaultOrEmpty) return default; + // We calculate the set of projects that are candidates for the best binding + var candidateProjects = candidateResults.SelectAsArray(result => result.docId.ProjectId); + // We calculate the set of supported projects + var invalidProjects = ArrayBuilder.GetInstance(); candidateResults.Remove(bestBinding); foreach (var (docId, tokenInformation) in candidateResults) { @@ -126,7 +127,7 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf invalidProjects.Add(docId.ProjectId); } - var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects, candidateProjects); + var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects.ToImmutableAndFree(), candidateProjects); return (bestBinding.tokenInformation, supportedPlatforms); } diff --git a/src/Features/Core/Portable/Shared/Utilities/SupportedPlatformData.cs b/src/Features/Core/Portable/Shared/Utilities/SupportedPlatformData.cs index e03becf494801..eaee2a11be5f0 100644 --- a/src/Features/Core/Portable/Shared/Utilities/SupportedPlatformData.cs +++ b/src/Features/Core/Portable/Shared/Utilities/SupportedPlatformData.cs @@ -2,27 +2,26 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -#nullable disable - using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using Microsoft.CodeAnalysis.Shared.Extensions; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.Shared.Utilities; -internal sealed class SupportedPlatformData(Solution solution, List invalidProjects, IEnumerable candidateProjects) +internal sealed class SupportedPlatformData(Solution solution, ImmutableArray invalidProjects, ImmutableArray candidateProjects) { // Because completion finds lots of symbols that exist in // all projects, we'll instead maintain a list of projects // missing the symbol. - public readonly List InvalidProjects = invalidProjects; - public readonly IEnumerable CandidateProjects = candidateProjects; + public readonly ImmutableArray InvalidProjects = invalidProjects; + public readonly ImmutableArray CandidateProjects = candidateProjects; public readonly Solution Solution = solution; public IList ToDisplayParts() { - if (InvalidProjects == null || InvalidProjects.Count == 0) + if (InvalidProjects.Length == 0) return []; var builder = new List(); @@ -46,5 +45,5 @@ private static string Supported(bool supported) => supported ? FeaturesResources.Available : FeaturesResources.Not_Available; public bool HasValidAndInvalidProjects() - => InvalidProjects.Any() && InvalidProjects.Count != CandidateProjects.Count(); + => InvalidProjects.Length > 0 && InvalidProjects.Length != CandidateProjects.Length; } diff --git a/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs b/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs index 79bb502b59d1e..35ca09e00bead 100644 --- a/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs +++ b/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs @@ -254,7 +254,7 @@ private static SignatureHelpSymbolParameter ReplaceStructuralTypes( return itemsForCurrentDocument; } - var totalProjects = relatedDocuments.Select(d => d.Project.Id).Concat(document.Project.Id); + var totalProjects = relatedDocuments.Concat(document).SelectAsArray(d => d.Project.Id); var semanticModel = await document.ReuseExistingSpeculativeModelAsync(position, cancellationToken).ConfigureAwait(false); var compilation = semanticModel.Compilation; @@ -277,7 +277,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey || symbolKey = SymbolKey.Create(methodSymbol.OriginalDefinition, cancellationToken); } - var invalidProjectsForCurrentSymbol = new List(); + var invalidProjectsForCurrentSymbol = ArrayBuilder.GetInstance(); foreach (var relatedDocument in relatedDocuments) { // Try to resolve symbolKey in each related compilation, @@ -289,7 +289,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey || } } - var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol, totalProjects); + var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol.ToImmutableAndFree(), totalProjects); finalItems.Add(UpdateItem(item, platformData)); } From 59383e902ad7a8d86a14b1220cf85596d847b626 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 7 Jan 2025 08:13:01 -0800 Subject: [PATCH 2/2] prefer the using form of ArrayBuilder.GetInstance. --- .../Completion/Providers/AbstractSymbolCompletionProvider.cs | 2 +- .../Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs | 4 ++-- .../Portable/SignatureHelp/AbstractSignatureHelpProvider.cs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs b/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs index 6610dde75c4fb..768e5aaa02fb4 100644 --- a/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs +++ b/src/Features/Core/Portable/Completion/Providers/AbstractSymbolCompletionProvider.cs @@ -299,7 +299,7 @@ private async Task> GetItemsAsync( if (relatedDocumentIds.IsEmpty) { var itemsForCurrentDocument = await GetSymbolsAsync(completionContext, syntaxContext, position, options, cancellationToken).ConfigureAwait(false); - return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: ImmutableArray.Empty); + return CreateItems(completionContext, itemsForCurrentDocument, _ => syntaxContext, invalidProjectMap: null, totalProjects: []); } using var _ = PooledDictionary.GetInstance(out var documentIdToIndex); diff --git a/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs b/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs index a01fc474a224f..2b3d5c9125e5c 100644 --- a/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs +++ b/src/Features/Core/Portable/QuickInfo/CommonSemanticQuickInfoProvider.cs @@ -118,7 +118,7 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf var candidateProjects = candidateResults.SelectAsArray(result => result.docId.ProjectId); // We calculate the set of supported projects - var invalidProjects = ArrayBuilder.GetInstance(); + using var _ = ArrayBuilder.GetInstance(out var invalidProjects); candidateResults.Remove(bestBinding); foreach (var (docId, tokenInformation) in candidateResults) { @@ -127,7 +127,7 @@ internal abstract partial class CommonSemanticQuickInfoProvider : CommonQuickInf invalidProjects.Add(docId.ProjectId); } - var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects.ToImmutableAndFree(), candidateProjects); + var supportedPlatforms = new SupportedPlatformData(solution, invalidProjects.ToImmutableAndClear(), candidateProjects); return (bestBinding.tokenInformation, supportedPlatforms); } diff --git a/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs b/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs index 35ca09e00bead..3541d36653cda 100644 --- a/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs +++ b/src/Features/Core/Portable/SignatureHelp/AbstractSignatureHelpProvider.cs @@ -277,7 +277,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey || symbolKey = SymbolKey.Create(methodSymbol.OriginalDefinition, cancellationToken); } - var invalidProjectsForCurrentSymbol = ArrayBuilder.GetInstance(); + using var _ = ArrayBuilder.GetInstance(out var invalidProjectsForCurrentSymbol); foreach (var relatedDocument in relatedDocuments) { // Try to resolve symbolKey in each related compilation, @@ -289,7 +289,7 @@ symbolKeyItem.SymbolKey is not SymbolKey symbolKey || } } - var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol.ToImmutableAndFree(), totalProjects); + var platformData = new SupportedPlatformData(document.Project.Solution, invalidProjectsForCurrentSymbol.ToImmutableAndClear(), totalProjects); finalItems.Add(UpdateItem(item, platformData)); }