-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Improve processing in FindRefs #73253
Changes from 25 commits
d8d1b78
ecd1f39
94c0662
1042c63
269b8a3
cee9783
6e1adc1
f17f727
57c3ae6
2dedcbe
daa291e
3daa54f
1e351f5
4a6d2cb
de5f7e5
c8c6b37
99a8b91
6679c60
cc563f7
293f1ba
e8f85f9
40c0f65
bab6cf3
7917020
751af97
753ae44
d1689c3
91465a7
98f6cd7
8a43b5c
22b2dff
b99480c
298b1ec
03b0fe7
5bcb1ca
3bcce89
54639c6
2f64957
df96626
13b1f1c
3ae4a71
432b3fd
dc8b00d
d30867d
0a32ab9
48fdc8e
176e239
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,6 +5,7 @@ | |
#nullable disable | ||
|
||
using System.Collections.Generic; | ||
using System.Collections.Immutable; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Classification; | ||
|
@@ -73,8 +74,6 @@ public IStreamingProgressTracker ProgressTracker | |
// any of these. | ||
public ValueTask OnStartedAsync(CancellationToken cancellationToken) => default; | ||
public ValueTask OnCompletedAsync(CancellationToken cancellationToken) => default; | ||
public ValueTask OnFindInDocumentStartedAsync(Document document, CancellationToken cancellationToken) => default; | ||
public ValueTask OnFindInDocumentCompletedAsync(Document document, CancellationToken cancellationToken) => default; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FAR had a system to notify consumers "i'm startign a doc" and "i'm done with a doc". The original thinking was to use that for progress, to display which file we're looking at. but: we never hooked this up to naything at all. |
||
|
||
// More complicated forwarding functions. These need to map from the symbols | ||
// used by the FAR engine to the INavigableItems used by the streaming FAR | ||
|
@@ -107,17 +106,21 @@ public async ValueTask OnDefinitionFoundAsync(SymbolGroup group, CancellationTok | |
await context.OnDefinitionFoundAsync(definitionItem, cancellationToken).ConfigureAwait(false); | ||
} | ||
|
||
public async ValueTask OnReferenceFoundAsync(SymbolGroup group, ISymbol definition, ReferenceLocation location, CancellationToken cancellationToken) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. view with whitespcae off. |
||
public async ValueTask OnReferencesFoundAsync( | ||
ImmutableArray<(SymbolGroup group, ISymbol symbol, ReferenceLocation location)> references, CancellationToken cancellationToken) | ||
{ | ||
var definitionItem = await GetDefinitionItemAsync(group, cancellationToken).ConfigureAwait(false); | ||
var referenceItem = await location.TryCreateSourceReferenceItemAsync( | ||
classificationOptions, | ||
definitionItem, | ||
includeHiddenLocations: false, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
if (referenceItem != null) | ||
await context.OnReferenceFoundAsync(referenceItem, cancellationToken).ConfigureAwait(false); | ||
foreach (var (group, _, location) in references) | ||
{ | ||
var definitionItem = await GetDefinitionItemAsync(group, cancellationToken).ConfigureAwait(false); | ||
var referenceItem = await location.TryCreateSourceReferenceItemAsync( | ||
classificationOptions, | ||
definitionItem, | ||
includeHiddenLocations: false, | ||
cancellationToken).ConfigureAwait(false); | ||
|
||
if (referenceItem != null) | ||
await context.OnReferenceFoundAsync(referenceItem, cancellationToken).ConfigureAwait(false); | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ await context.ReportNoResultsAsync( | |
var (symbol, project) = symbolAndProjectOpt.Value; | ||
|
||
var solution = project.Solution; | ||
var bases = await FindBaseHelpers.FindBasesAsync(symbol, solution, cancellationToken).ConfigureAwait(false); | ||
var bases = FindBaseHelpers.FindBases(symbol, solution, cancellationToken); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. a lot of FAR and helpers becamse sync. will explain how later in pr. |
||
if (bases.Length == 0 && symbol is IMethodSymbol { MethodKind: MethodKind.Constructor } constructor) | ||
{ | ||
var nextConstructor = await FindNextConstructorInChainAsync(solution, constructor, cancellationToken).ConfigureAwait(false); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,29 +32,44 @@ public static async ValueTask<FindReferenceCache> GetCacheAsync(Document documen | |
|
||
static async Task<FindReferenceCache> ComputeCacheAsync(Document document, CancellationToken cancellationToken) | ||
{ | ||
var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the cache is an object we make for a document once we've decided it's a doc of interest and we're going ot be searching it. that means it already registered a 'hit' during the pass when we asked to find docments that could potnetially contain the symbols we're searching for (say because it got a hit in the bloomfilter of all the names in the doc). Once we've decided we're looking at the doc, it makes sense to get all the info we need up front, so that hte search in the doc doesn't ever ahve to go async. So we do this here. Note: teh txt is the most normal thing we need in a normal find refs. Say we're lookign for |
||
|
||
// Find-Refs is not impacted by nullable types at all. So get a nullable-disabled semantic model to avoid | ||
// unnecessary costs while binding. | ||
var model = await document.GetRequiredNullableDisabledSemanticModelAsync(cancellationToken).ConfigureAwait(false); | ||
var root = await document.GetRequiredSyntaxRootAsync(cancellationToken).ConfigureAwait(false); | ||
return new(document, model, root); | ||
|
||
// It's very costly to walk an entire tree. So if the tree is simple and doesn't contain | ||
// any unicode escapes in it, then we do simple string matching to find the tokens. | ||
var index = await SyntaxTreeIndex.GetRequiredIndexAsync(document, cancellationToken).ConfigureAwait(false); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similarly, we will have practically always computed thsi for all documents that were hits anyways (because that's how we figured out there might be a hit in the first place). So deferring this doesn't help at all, and this elides async from everything that follows. |
||
|
||
return new(document, text, model, root, index); | ||
} | ||
} | ||
|
||
public readonly Document Document; | ||
public readonly SourceText Text; | ||
public readonly SemanticModel SemanticModel; | ||
public readonly SyntaxNode Root; | ||
public readonly ISyntaxFactsService SyntaxFacts; | ||
public readonly SyntaxTreeIndex SyntaxTreeIndex; | ||
|
||
private readonly ConcurrentDictionary<SyntaxNode, SymbolInfo> _symbolInfoCache = []; | ||
private readonly ConcurrentDictionary<string, ImmutableArray<SyntaxToken>> _identifierCache; | ||
|
||
private ImmutableHashSet<string>? _aliasNameSet; | ||
private ImmutableArray<SyntaxToken> _constructorInitializerCache; | ||
|
||
private FindReferenceCache(Document document, SemanticModel semanticModel, SyntaxNode root) | ||
private FindReferenceCache( | ||
Document document, SourceText text, SemanticModel semanticModel, SyntaxNode root, SyntaxTreeIndex syntaxTreeIndex) | ||
{ | ||
Document = document; | ||
Text = text; | ||
SemanticModel = semanticModel; | ||
Root = root; | ||
SyntaxTreeIndex = syntaxTreeIndex; | ||
SyntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
|
||
_identifierCache = new(comparer: semanticModel.Language switch | ||
{ | ||
LanguageNames.VisualBasic => StringComparer.OrdinalIgnoreCase, | ||
|
@@ -81,10 +96,8 @@ public SymbolInfo GetSymbolInfo(SyntaxNode node, CancellationToken cancellationT | |
return null; | ||
} | ||
|
||
public async ValueTask<ImmutableArray<SyntaxToken>> FindMatchingIdentifierTokensAsync( | ||
Document document, | ||
string identifier, | ||
CancellationToken cancellationToken) | ||
public ImmutableArray<SyntaxToken> FindMatchingIdentifierTokens( | ||
string identifier, CancellationToken cancellationToken) | ||
{ | ||
if (identifier == "") | ||
{ | ||
|
@@ -99,53 +112,32 @@ public async ValueTask<ImmutableArray<SyntaxToken>> FindMatchingIdentifierTokens | |
if (_identifierCache.TryGetValue(identifier, out var result)) | ||
return result; | ||
|
||
// It's very costly to walk an entire tree. So if the tree is simple and doesn't contain | ||
// any unicode escapes in it, then we do simple string matching to find the tokens. | ||
var info = await SyntaxTreeIndex.GetRequiredIndexAsync(document, cancellationToken).ConfigureAwait(false); | ||
|
||
// If this document doesn't even contain this identifier (escaped or non-escaped) we don't have to search it at all. | ||
if (!info.ProbablyContainsIdentifier(identifier)) | ||
if (!this.SyntaxTreeIndex.ProbablyContainsIdentifier(identifier)) | ||
return []; | ||
|
||
return await ComputeAndCacheTokensAsync(this, document, identifier, info, cancellationToken).ConfigureAwait(false); | ||
// If the identifier was escaped in the file then we'll have to do a more involved search that actually | ||
// walks the root and checks all identifier tokens. | ||
// | ||
// otherwise, we can use the text of the document to quickly find candidates and test those directly. | ||
return this.SyntaxTreeIndex.ProbablyContainsEscapedIdentifier(identifier) | ||
? _identifierCache.GetOrAdd(identifier, _ => FindMatchingIdentifierTokensFromTree()) | ||
: _identifierCache.GetOrAdd(identifier, _ => FindMatchingIdentifierTokensFromText()); | ||
|
||
static async ValueTask<ImmutableArray<SyntaxToken>> ComputeAndCacheTokensAsync( | ||
FindReferenceCache cache, Document document, string identifier, SyntaxTreeIndex info, CancellationToken cancellationToken) | ||
{ | ||
var syntaxFacts = document.GetRequiredLanguageService<ISyntaxFactsService>(); | ||
var root = await cache.SemanticModel.SyntaxTree.GetRootAsync(cancellationToken).ConfigureAwait(false); | ||
|
||
// If the identifier was escaped in the file then we'll have to do a more involved search that actually | ||
// walks the root and checks all identifier tokens. | ||
// | ||
// otherwise, we can use the text of the document to quickly find candidates and test those directly. | ||
if (info.ProbablyContainsEscapedIdentifier(identifier)) | ||
{ | ||
return cache._identifierCache.GetOrAdd( | ||
identifier, _ => FindMatchingIdentifierTokensFromTree(syntaxFacts, identifier, root)); | ||
} | ||
else | ||
{ | ||
var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); | ||
return cache._identifierCache.GetOrAdd( | ||
identifier, _ => FindMatchingIdentifierTokensFromText(syntaxFacts, identifier, root, text, cancellationToken)); | ||
} | ||
} | ||
|
||
static bool IsMatch(ISyntaxFactsService syntaxFacts, string identifier, SyntaxToken token) | ||
=> !token.IsMissing && syntaxFacts.IsIdentifier(token) && syntaxFacts.TextMatch(token.ValueText, identifier); | ||
bool IsMatch(string identifier, SyntaxToken token) | ||
=> !token.IsMissing && this.SyntaxFacts.IsIdentifier(token) && this.SyntaxFacts.TextMatch(token.ValueText, identifier); | ||
|
||
static ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromTree( | ||
ISyntaxFactsService syntaxFacts, string identifier, SyntaxNode root) | ||
ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromTree() | ||
{ | ||
using var _ = ArrayBuilder<SyntaxToken>.GetInstance(out var result); | ||
using var obj = SharedPools.Default<Stack<SyntaxNodeOrToken>>().GetPooledObject(); | ||
|
||
var stack = obj.Object; | ||
stack.Push(root); | ||
stack.Push(this.Root); | ||
|
||
while (stack.TryPop(out var current)) | ||
{ | ||
cancellationToken.ThrowIfCancellationRequested(); | ||
if (current.IsNode) | ||
{ | ||
foreach (var child in current.AsNode()!.ChildNodesAndTokens().Reverse()) | ||
|
@@ -154,7 +146,7 @@ static ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromTree( | |
else if (current.IsToken) | ||
{ | ||
var token = current.AsToken(); | ||
if (IsMatch(syntaxFacts, identifier, token)) | ||
if (IsMatch(identifier, token)) | ||
result.Add(token); | ||
|
||
if (token.HasStructuredTrivia) | ||
|
@@ -172,19 +164,18 @@ static ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromTree( | |
return result.ToImmutableAndClear(); | ||
} | ||
|
||
static ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromText( | ||
ISyntaxFactsService syntaxFacts, string identifier, SyntaxNode root, SourceText sourceText, CancellationToken cancellationToken) | ||
ImmutableArray<SyntaxToken> FindMatchingIdentifierTokensFromText() | ||
CyrusNajmabadi marked this conversation as resolved.
Show resolved
Hide resolved
|
||
{ | ||
using var _ = ArrayBuilder<SyntaxToken>.GetInstance(out var result); | ||
|
||
var index = 0; | ||
while ((index = sourceText.IndexOf(identifier, index, syntaxFacts.IsCaseSensitive)) >= 0) | ||
while ((index = this.Text.IndexOf(identifier, index, this.SyntaxFacts.IsCaseSensitive)) >= 0) | ||
{ | ||
cancellationToken.ThrowIfCancellationRequested(); | ||
|
||
var token = root.FindToken(index, findInsideTrivia: true); | ||
var token = this.Root.FindToken(index, findInsideTrivia: true); | ||
var span = token.Span; | ||
if (span.Start == index && span.Length == identifier.Length && IsMatch(syntaxFacts, identifier, token)) | ||
if (span.Start == index && span.Length == identifier.Length && IsMatch(identifier, token)) | ||
result.Add(token); | ||
|
||
var nextIndex = index + identifier.Length; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FAR had a system to notify consumers "i'm startign a doc" and "i'm done with a doc". The original thinking was to use that for progress, to display which file we're looking at.
but: