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

Jump to bottom

Reduce allocations due to multiple levels of ImmutableArray<FinderLocations> created during find references invocations. #73118

Merged
merged 2 commits into from
Apr 20, 2024
Merged

Reduce allocations due to multiple levels of ImmutableArray<FinderLocations> created during find references invocations. #73118

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
67c0ce3
Reduce allocations due to multiple levels of ImmutableArray<FinderLoc…
ToddGrun Apr 20, 2024
bfec0ab
Clear location to be in inner loop
ToddGrun Apr 20, 2024
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
30 changes: 22 additions & 8 deletions src/Features/Core/Portable/ChangeSignature/DelegateInvokeMethodReferenceFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,11 @@ protected override Task DetermineDocumentsToSearchAsync<TData>(
return Task.CompletedTask;
}

protected override async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected override async ValueTask FindReferencesInDocumentAsync<TData>(
IMethodSymbol methodSymbol,
FindReferencesDocumentState state,
Action<FinderLocation, TData> processResult,
TData processResultData,
FindReferencesSearchOptions options,
CancellationToken cancellationToken)
{
Expand All @@ -89,7 +91,15 @@ protected override async ValueTask<ImmutableArray<FinderLocation>> FindReference
var root = state.Root;
var nodes = root.DescendantNodes();

using var _ = ArrayBuilder<SyntaxNode>.GetInstance(out var convertedAnonymousFunctions);
var invocations = nodes.Where(syntaxFacts.IsInvocationExpression)
.Where(e => state.SemanticModel.GetSymbolInfo(e, cancellationToken).Symbol?.OriginalDefinition == methodSymbol);

foreach (var node in invocations)
{
var finderLocation = CreateFinderLocation(node, state, cancellationToken);
processResult(finderLocation, processResultData);
Copy link
Member

Choose a reason for hiding this comment

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

nit: i would be ok with this as a single line.

}

foreach (var node in nodes)
{
if (!syntaxFacts.IsAnonymousFunctionExpression(node))
Expand All @@ -103,14 +113,17 @@ protected override async ValueTask<ImmutableArray<FinderLocation>> FindReference
}

if (convertedType == methodSymbol.ContainingType)
convertedAnonymousFunctions.Add(node);
{
var finderLocation = CreateFinderLocation(node, state, cancellationToken);
processResult(finderLocation, processResultData);
}
}

var invocations = nodes.Where(syntaxFacts.IsInvocationExpression)
.Where(e => state.SemanticModel.GetSymbolInfo(e, cancellationToken).Symbol?.OriginalDefinition == methodSymbol);
return;

return invocations.Concat(convertedAnonymousFunctions).SelectAsArray(
node => new FinderLocation(
static FinderLocation CreateFinderLocation(SyntaxNode node, FindReferencesDocumentState state, CancellationToken cancellationToken)
{
return new FinderLocation(
node,
new ReferenceLocation(
state.Document,
Expand All @@ -119,6 +132,7 @@ protected override async ValueTask<ImmutableArray<FinderLocation>> FindReference
isImplicit: false,
GetSymbolUsageInfo(node, state, cancellationToken),
GetAdditionalFindUsagesProperties(node, state),
CandidateReason.None)));
CandidateReason.None));
}
}
}
17 changes: 11 additions & 6 deletions src/Workspaces/Core/Portable/FindSymbols/FindReferences/FindReferencesSearchEngine.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ private async Task ProcessProjectAsync(Project project, ImmutableArray<ISymbol>
{
await finder.DetermineDocumentsToSearchAsync(
symbol, globalAliases, project, _documents,
static (doc, documents) => documents.Add(doc),
StandardCallbacks<Document>.AddToHashSet,
Copy link
Member

Choose a reason for hiding this comment

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

bute.

foundDocuments,
_options, cancellationToken).ConfigureAwait(false);

Expand Down Expand Up @@ -272,12 +272,15 @@ private async Task ProcessDocumentAsync(
// just grab those once here and hold onto them for the lifetime of this call.
var cache = await FindReferenceCache.GetCacheAsync(document, cancellationToken).ConfigureAwait(false);

// scratch hashset to place results in. Populated/inspected/cleared in inner loop.
Copy link
Member

Choose a reason for hiding this comment

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

this does not look like a hashset :)

using var _ = ArrayBuilder<FinderLocation>.GetInstance(out var foundReferenceLocations);

foreach (var symbol in symbols)
{
var globalAliases = TryGet(symbolToGlobalAliases, symbol);
var state = new FindReferencesDocumentState(cache, globalAliases);

await ProcessDocumentAsync(symbol, state).ConfigureAwait(false);
await ProcessDocumentAsync(symbol, state, foundReferenceLocations).ConfigureAwait(false);
}
}
finally
Expand All @@ -286,7 +289,7 @@ private async Task ProcessDocumentAsync(
}

async Task ProcessDocumentAsync(
ISymbol symbol, FindReferencesDocumentState state)
ISymbol symbol, FindReferencesDocumentState state, ArrayBuilder<FinderLocation> foundReferenceLocations)
{
cancellationToken.ThrowIfCancellationRequested();

Expand All @@ -297,11 +300,13 @@ async Task ProcessDocumentAsync(
var group = _symbolToGroup[symbol];
foreach (var finder in _finders)
{
var references = await finder.FindReferencesInDocumentAsync(
symbol, state, _options, cancellationToken).ConfigureAwait(false);
foreach (var (_, location) in references)
await finder.FindReferencesInDocumentAsync(
symbol, state, StandardCallbacks<FinderLocation>.AddToArrayBuilder, foundReferenceLocations, _options, cancellationToken).ConfigureAwait(false);
foreach (var (_, location) in foundReferenceLocations)
await _progress.OnReferenceFoundAsync(group, symbol, location, cancellationToken).ConfigureAwait(false);
}

foundReferenceLocations.Clear();
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions ...rtable/FindSymbols/FindReferences/FindReferencesSearchEngine_FindReferencesInDocuments.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,16 +101,21 @@ async ValueTask PerformSearchInDocumentAsync(
async ValueTask PerformSearchInDocumentWorkerAsync(
ISymbol symbol, FindReferencesDocumentState state)
{
// Scratch buffer to place references for each finder. Cleared at the end of every loop iteration.
using var _ = ArrayBuilder<FinderLocation>.GetInstance(out var referencesForFinder);

// Always perform a normal search, looking for direct references to exactly that symbol.
foreach (var finder in _finders)
{
var references = await finder.FindReferencesInDocumentAsync(
symbol, state, _options, cancellationToken).ConfigureAwait(false);
foreach (var (_, location) in references)
await finder.FindReferencesInDocumentAsync(
symbol, state, StandardCallbacks<FinderLocation>.AddToArrayBuilder, referencesForFinder, _options, cancellationToken).ConfigureAwait(false);
foreach (var (_, location) in referencesForFinder)
{
var group = await ReportGroupAsync(symbol, cancellationToken).ConfigureAwait(false);
await _progress.OnReferenceFoundAsync(group, symbol, location, cancellationToken).ConfigureAwait(false);
}

referencesForFinder.Clear();
Copy link
Member

Choose a reason for hiding this comment

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

it's interesting how close this pattern is to the pipewriting/reading stuff i just did. this might get cleaner (in the future) with a channel that is pulling off the items and reporting to them to the progress, while the main find is pushing into the channel.

}

// Now, for symbols that could involve inheritance, look for references to the same named entity, and
Expand Down
21 changes: 12 additions & 9 deletions ...s/Core/Portable/FindSymbols/FindReferences/Finders/AbstractMemberScopedReferenceFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,23 +49,24 @@ protected sealed override Task DetermineDocumentsToSearchAsync<TData>(
return Task.CompletedTask;
}

protected sealed override async ValueTask<ImmutableArray<FinderLocation>> FindReferencesInDocumentAsync(
protected sealed override async ValueTask FindReferencesInDocumentAsync<TData>(
TSymbol symbol,
FindReferencesDocumentState state,
Action<FinderLocation, TData> processResult,
TData processResultData,
FindReferencesSearchOptions options,
CancellationToken cancellationToken)
{
var container = GetContainer(symbol);
if (container != null)
return await FindReferencesInContainerAsync(symbol, container, state, cancellationToken).ConfigureAwait(false);

if (symbol.ContainingType != null && symbol.ContainingType.IsScriptClass)
{
await FindReferencesInContainerAsync(symbol, container, state, processResult, processResultData, cancellationToken).ConfigureAwait(false);
}
else if (symbol.ContainingType != null && symbol.ContainingType.IsScriptClass)
{
var tokens = await FindMatchingIdentifierTokensAsync(state, symbol.Name, cancellationToken).ConfigureAwait(false);
return await FindReferencesInTokensAsync(symbol, state, tokens, cancellationToken).ConfigureAwait(false);
await FindReferencesInTokensAsync(symbol, state, tokens, processResult, processResultData, cancellationToken).ConfigureAwait(false);
}

return [];
}

private static ISymbol? GetContainer(ISymbol symbol)
Expand Down Expand Up @@ -96,10 +97,12 @@ protected sealed override async ValueTask<ImmutableArray<FinderLocation>> FindRe
return null;
}

private ValueTask<ImmutableArray<FinderLocation>> FindReferencesInContainerAsync(
private ValueTask FindReferencesInContainerAsync<TData>(
TSymbol symbol,
ISymbol container,
FindReferencesDocumentState state,
Action<FinderLocation, TData> processResult,
TData processResultData,
CancellationToken cancellationToken)
{
var service = state.Document.GetRequiredLanguageService<ISymbolDeclarationService>();
Expand All @@ -118,6 +121,6 @@ private ValueTask<ImmutableArray<FinderLocation>> FindReferencesInContainerAsync
}
}

return FindReferencesInTokensAsync(symbol, state, tokens.ToImmutable(), cancellationToken);
return FindReferencesInTokensAsync(symbol, state, tokens.ToImmutable(), processResult, processResultData, cancellationToken);
}
}
2 changes: 0 additions & 2 deletions ...indSymbols/FindReferences/Finders/AbstractMethodOrPropertyOrEventSymbolReferenceFinder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,8 @@

using System.Collections.Immutable;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.LanguageService;
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;

namespace Microsoft.CodeAnalysis.FindSymbols.Finders;

Expand Down
Loading
Loading