-
Notifications
You must be signed in to change notification settings - Fork 4.2k
Refactor searching and reporting results in navto. #73249
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
Merged
Merged
Changes from 39 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
6450bdc
Remove project
CyrusNajmabadi 1c7bbbd
in progress
CyrusNajmabadi a259470
Project removed
CyrusNajmabadi b14801b
Renames and arrays
CyrusNajmabadi 156dca7
in progress
CyrusNajmabadi 949dc93
in progress
CyrusNajmabadi 6bb84e2
in progress
CyrusNajmabadi 10a3642
in progress
CyrusNajmabadi 6d5be9e
in progress
CyrusNajmabadi b1ea829
COntinue
CyrusNajmabadi 6ba9cfc
Move to channels
CyrusNajmabadi e6ffd9c
Parallel
CyrusNajmabadi e79f9a9
in progrss
CyrusNajmabadi 72a217f
in progrss
CyrusNajmabadi fbeb9f7
Simplify
CyrusNajmabadi 950d652
Simplify
CyrusNajmabadi 0b6483b
Simplify
CyrusNajmabadi d484eef
in progress
CyrusNajmabadi f046fb9
in progress
CyrusNajmabadi 1973e1b
Pull out async
CyrusNajmabadi 87329d3
Simplify
CyrusNajmabadi bc0e3e6
cleanup
CyrusNajmabadi 3aec999
Simplify
CyrusNajmabadi b14f2f7
async
CyrusNajmabadi 962752f
simplify
CyrusNajmabadi c7c9f0c
Simplify
CyrusNajmabadi 23a55e9
Simplify
CyrusNajmabadi 3343271
Simplify
CyrusNajmabadi 096eb10
Simplify
CyrusNajmabadi 22d2101
Simplify
CyrusNajmabadi 7aad781
Simplify
CyrusNajmabadi 932b8e9
Simplify
CyrusNajmabadi b0f654d
in progress
CyrusNajmabadi 0c02539
Simplify
CyrusNajmabadi 513ff89
Simplify
CyrusNajmabadi 9aa4742
Simplify
CyrusNajmabadi ad9827e
Simplify
CyrusNajmabadi c5046c1
move
CyrusNajmabadi 267c8fe
cleanup
CyrusNajmabadi 391d97c
simplify
CyrusNajmabadi 43eca4d
simplify
CyrusNajmabadi 2106880
simplify
CyrusNajmabadi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
66 changes: 33 additions & 33 deletions
66
src/EditorFeatures/CSharpTest/NavigateTo/NavigateToSearcherTests.cs
Large diffs are not rendered by default.
Oops, something went wrong.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,10 +3,12 @@ | |
| // See the LICENSE file in the project root for more information. | ||
|
|
||
| using System; | ||
| using System.Collections.Immutable; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using Microsoft.CodeAnalysis.ErrorReporting; | ||
| using Microsoft.CodeAnalysis.NavigateTo; | ||
| using Microsoft.CodeAnalysis.Shared.Extensions; | ||
| using Microsoft.CodeAnalysis.Text.Shared.Extensions; | ||
| using Microsoft.VisualStudio.Language.NavigateTo.Interfaces; | ||
| using Microsoft.VisualStudio.Text.PatternMatching; | ||
|
|
@@ -18,11 +20,13 @@ internal partial class NavigateToItemProvider | |
| { | ||
| private class NavigateToItemProviderCallback : INavigateToSearchCallback | ||
| { | ||
| private readonly Solution _solution; | ||
| private readonly INavigateToItemDisplayFactory _displayFactory; | ||
| private readonly INavigateToCallback _callback; | ||
|
|
||
| public NavigateToItemProviderCallback(INavigateToItemDisplayFactory displayFactory, INavigateToCallback callback) | ||
| public NavigateToItemProviderCallback(Solution solution, INavigateToItemDisplayFactory displayFactory, INavigateToCallback callback) | ||
| { | ||
| _solution = solution; | ||
| _displayFactory = displayFactory; | ||
| _callback = callback; | ||
| } | ||
|
|
@@ -39,9 +43,41 @@ public void Done(bool isFullyLoaded) | |
| } | ||
| } | ||
|
|
||
| public Task AddItemAsync(Project project, INavigateToSearchResult result, CancellationToken cancellationToken) | ||
| public Task AddResultsAsync(ImmutableArray<INavigateToSearchResult> results, CancellationToken cancellationToken) | ||
|
Contributor
Author
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.
|
||
| { | ||
| ReportMatchResult(project, result); | ||
| foreach (var result in results) | ||
| { | ||
| var matchedSpans = result.NameMatchSpans.SelectAsArray(t => t.ToSpan()); | ||
|
|
||
| var patternMatch = new PatternMatch( | ||
| GetPatternMatchKind(result.MatchKind), | ||
| punctuationStripped: false, | ||
| result.IsCaseSensitive, | ||
| matchedSpans); | ||
|
|
||
| var project = _solution.GetRequiredProject(result.NavigableItem.Document.Project.Id); | ||
| var navigateToItem = new NavigateToItem( | ||
| result.Name, | ||
| result.Kind, | ||
| GetNavigateToLanguage(project.Language), | ||
| result.SecondarySort, | ||
| result, | ||
| patternMatch, | ||
| _displayFactory); | ||
|
|
||
| try | ||
| { | ||
| _callback.AddItem(navigateToItem); | ||
| } | ||
| catch (InvalidOperationException ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.Critical)) | ||
| { | ||
| // Mitigation for race condition in platform https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1534364 | ||
| // | ||
| // Catch this so that don't tear down OOP, but still report the exception so that we ensure this issue | ||
| // gets attention and is fixed. | ||
| } | ||
| } | ||
|
|
||
| return Task.CompletedTask; | ||
| } | ||
|
|
||
|
|
@@ -54,38 +90,6 @@ public void ReportIncomplete() | |
| { | ||
| } | ||
|
|
||
| private void ReportMatchResult(Project project, INavigateToSearchResult result) | ||
| { | ||
| var matchedSpans = result.NameMatchSpans.SelectAsArray(t => t.ToSpan()); | ||
|
|
||
| var patternMatch = new PatternMatch( | ||
| GetPatternMatchKind(result.MatchKind), | ||
| punctuationStripped: false, | ||
| result.IsCaseSensitive, | ||
| matchedSpans); | ||
|
|
||
| var navigateToItem = new NavigateToItem( | ||
| result.Name, | ||
| result.Kind, | ||
| GetNavigateToLanguage(project.Language), | ||
| result.SecondarySort, | ||
| result, | ||
| patternMatch, | ||
| _displayFactory); | ||
|
|
||
| try | ||
| { | ||
| _callback.AddItem(navigateToItem); | ||
| } | ||
| catch (InvalidOperationException ex) when (FatalError.ReportAndCatch(ex, ErrorSeverity.Critical)) | ||
| { | ||
| // Mitigation for race condition in platform https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1534364 | ||
| // | ||
| // Catch this so that don't tear down OOP, but still report the exception so that we ensure this issue | ||
| // gets attention and is fixed. | ||
| } | ||
| } | ||
|
|
||
| private static PatternMatchKind GetPatternMatchKind(NavigateToMatchKind matchKind) | ||
| => matchKind switch | ||
| { | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ | |
|
|
||
| using System; | ||
| using System.Collections.Concurrent; | ||
| using System.Collections.Generic; | ||
| using System.Collections.Immutable; | ||
| using System.Diagnostics; | ||
| using System.Diagnostics.CodeAnalysis; | ||
|
|
@@ -14,7 +13,6 @@ | |
| using Microsoft.CodeAnalysis.FindSymbols; | ||
| using Microsoft.CodeAnalysis.Host; | ||
| using Microsoft.CodeAnalysis.PatternMatching; | ||
| using Microsoft.CodeAnalysis.PooledObjects; | ||
| using Microsoft.CodeAnalysis.Remote; | ||
| using Microsoft.CodeAnalysis.Shared.Extensions; | ||
| using Microsoft.CodeAnalysis.Storage; | ||
|
|
@@ -63,25 +61,27 @@ public async Task SearchCachedDocumentsAsync( | |
| string searchPattern, | ||
| IImmutableSet<string> kinds, | ||
| Document? activeDocument, | ||
| Func<Project, INavigateToSearchResult, Task> onResultFound, | ||
| Func<ImmutableArray<INavigateToSearchResult>, Task> onResultsFound, | ||
| Func<Task> onProjectCompleted, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| if (cancellationToken.IsCancellationRequested) | ||
| return; | ||
|
|
||
| Contract.ThrowIfTrue(projects.IsEmpty); | ||
| Contract.ThrowIfTrue(projects.Select(p => p.Language).Distinct().Count() != 1); | ||
|
|
||
| Debug.Assert(priorityDocuments.All(d => projects.Contains(d.Project))); | ||
|
|
||
| var onItemFound = GetOnItemFoundCallback(solution, activeDocument, onResultFound, cancellationToken); | ||
| var onItemsFound = GetOnItemsFoundCallback(solution, activeDocument, onResultsFound, cancellationToken); | ||
|
|
||
| var documentKeys = projects.SelectManyAsArray(p => p.Documents.Select(DocumentKey.ToDocumentKey)); | ||
| var priorityDocumentKeys = priorityDocuments.SelectAsArray(DocumentKey.ToDocumentKey); | ||
|
|
||
| var client = await RemoteHostClient.TryGetClientAsync(solution.Services, cancellationToken).ConfigureAwait(false); | ||
| if (client != null) | ||
| { | ||
| var callback = new NavigateToSearchServiceCallback(onItemFound, onProjectCompleted); | ||
| var callback = new NavigateToSearchServiceCallback(onItemsFound, onProjectCompleted); | ||
| await client.TryInvokeAsync<IRemoteNavigateToSearchService>( | ||
| (service, callbackId, cancellationToken) => | ||
| service.SearchCachedDocumentsAsync(documentKeys, priorityDocumentKeys, searchPattern, [.. kinds], callbackId, cancellationToken), | ||
|
|
@@ -92,7 +92,7 @@ await client.TryInvokeAsync<IRemoteNavigateToSearchService>( | |
|
|
||
| var storageService = solution.Services.GetPersistentStorageService(); | ||
| await SearchCachedDocumentsInCurrentProcessAsync( | ||
| storageService, documentKeys, priorityDocumentKeys, searchPattern, kinds, onItemFound, onProjectCompleted, cancellationToken).ConfigureAwait(false); | ||
| storageService, documentKeys, priorityDocumentKeys, searchPattern, kinds, onItemsFound, onProjectCompleted, cancellationToken).ConfigureAwait(false); | ||
| } | ||
|
|
||
| public static async Task SearchCachedDocumentsInCurrentProcessAsync( | ||
|
|
@@ -101,16 +101,14 @@ public static async Task SearchCachedDocumentsInCurrentProcessAsync( | |
| ImmutableArray<DocumentKey> priorityDocumentKeys, | ||
| string searchPattern, | ||
| IImmutableSet<string> kinds, | ||
| Func<RoslynNavigateToItem, Task> onItemFound, | ||
| Func<ImmutableArray<RoslynNavigateToItem>, Task> onItemsFound, | ||
| Func<Task> onProjectCompleted, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| // Quick abort if OOP is now fully loaded. | ||
| if (!ShouldSearchCachedDocuments(out _, out _)) | ||
| return; | ||
|
|
||
| // If the user created a dotted pattern then we'll grab the last part of the name | ||
| var (patternName, patternContainer) = PatternMatcher.GetNameAndContainer(searchPattern); | ||
| var declaredSymbolInfoKindsSet = new DeclaredSymbolInfoKindSet(kinds); | ||
|
|
||
|
|
@@ -120,84 +118,56 @@ public static async Task SearchCachedDocumentsInCurrentProcessAsync( | |
|
|
||
| using var _1 = GetPooledHashSet(priorityDocumentKeys, out var priorityDocumentKeysSet); | ||
|
|
||
| // Sort the groups into a high pri group (projects that contain a high-pri doc), and low pri groups (those | ||
| // that don't). | ||
| // Sort the groups into a high pri group (projects that contain a high-pri doc), and low pri groups (those that | ||
| // don't), and process in that order. | ||
| using var _2 = GetPooledHashSet(groups.Where(g => g.Any(priorityDocumentKeysSet.Contains)), out var highPriorityGroups); | ||
| using var _3 = GetPooledHashSet(groups.Where(g => !highPriorityGroups.Contains(g)), out var lowPriorityGroups); | ||
|
CyrusNajmabadi marked this conversation as resolved.
Outdated
|
||
|
|
||
| await ProcessProjectGroupsAsync(highPriorityGroups).ConfigureAwait(false); | ||
| await ProcessProjectGroupsAsync(lowPriorityGroups).ConfigureAwait(false); | ||
|
|
||
| await PerformParallelSearchAsync( | ||
| highPriorityGroups.Concat(lowPriorityGroups), ProcessSingleProjectGroupAsync, onItemsFound, cancellationToken).ConfigureAwait(false); | ||
| return; | ||
|
|
||
| async Task ProcessProjectGroupsAsync(HashSet<IGrouping<ProjectKey, DocumentKey>> groups) | ||
| async ValueTask ProcessSingleProjectGroupAsync( | ||
| IGrouping<ProjectKey, DocumentKey> group, | ||
| Action<RoslynNavigateToItem> onItemFound, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| using var _ = ArrayBuilder<Task>.GetInstance(out var tasks); | ||
| if (cancellationToken.IsCancellationRequested) | ||
| return; | ||
|
|
||
| foreach (var group in groups) | ||
| tasks.Add(ProcessProjectGroupAsync(group)); | ||
|
|
||
| await Task.WhenAll(tasks).ConfigureAwait(false); | ||
| } | ||
|
|
||
| async Task ProcessProjectGroupAsync(IGrouping<ProjectKey, DocumentKey> group) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| await Task.Yield().ConfigureAwait(false); | ||
| var project = group.Key; | ||
|
|
||
| // Break the project into high-pri docs and low pri docs. | ||
| // Break the project into high-pri docs and low pri docs, and process in that order. | ||
| using var _1 = GetPooledHashSet(group.Where(priorityDocumentKeysSet.Contains), out var highPriDocs); | ||
|
CyrusNajmabadi marked this conversation as resolved.
Outdated
|
||
| using var _2 = GetPooledHashSet(group.Where(d => !highPriDocs.Contains(d)), out var lowPriDocs); | ||
|
|
||
| await SearchCachedDocumentsInCurrentProcessAsync( | ||
| storageService, patternName, patternContainer, declaredSymbolInfoKindsSet, | ||
| onItemFound, highPriDocs, cancellationToken).ConfigureAwait(false); | ||
| await ParallelForEachAsync( | ||
| highPriDocs.Concat(lowPriDocs), | ||
| cancellationToken, | ||
| async (documentKey, cancellationToken) => | ||
| { | ||
| var index = await GetIndexAsync(storageService, documentKey, cancellationToken).ConfigureAwait(false); | ||
| if (index == null) | ||
| return; | ||
|
|
||
| await SearchCachedDocumentsInCurrentProcessAsync( | ||
| storageService, patternName, patternContainer, declaredSymbolInfoKindsSet, | ||
| onItemFound, lowPriDocs, cancellationToken).ConfigureAwait(false); | ||
| ProcessIndex( | ||
| documentKey, document: null, patternName, patternContainer, declaredSymbolInfoKindsSet, | ||
| index, linkedIndices: null, onItemFound, cancellationToken); | ||
|
Contributor
Author
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. Importantly, searching each index is pure sync. |
||
| }).ConfigureAwait(false); | ||
|
|
||
| // done with project. Let the host know. | ||
| await onProjectCompleted().ConfigureAwait(false); | ||
| } | ||
| } | ||
|
|
||
| private static async Task SearchCachedDocumentsInCurrentProcessAsync( | ||
| IChecksummedPersistentStorageService storageService, | ||
| string patternName, | ||
| string? patternContainer, | ||
| DeclaredSymbolInfoKindSet kinds, | ||
| Func<RoslynNavigateToItem, Task> onItemFound, | ||
| HashSet<DocumentKey> documentKeys, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| using var _ = ArrayBuilder<Task>.GetInstance(out var tasks); | ||
|
|
||
| foreach (var documentKey in documentKeys) | ||
| { | ||
| tasks.Add(Task.Run(async () => | ||
| { | ||
| var index = await GetIndexAsync(storageService, documentKey, cancellationToken).ConfigureAwait(false); | ||
| if (index == null) | ||
| return; | ||
|
|
||
| await ProcessIndexAsync( | ||
| documentKey, document: null, patternName, patternContainer, kinds, onItemFound, index, cancellationToken).ConfigureAwait(false); | ||
| }, cancellationToken)); | ||
| } | ||
|
|
||
| await Task.WhenAll(tasks).ConfigureAwait(false); | ||
| } | ||
|
|
||
| private static Task<TopLevelSyntaxTreeIndex?> GetIndexAsync( | ||
| IChecksummedPersistentStorageService storageService, | ||
| DocumentKey documentKey, | ||
| CancellationToken cancellationToken) | ||
| { | ||
| cancellationToken.ThrowIfCancellationRequested(); | ||
| if (cancellationToken.IsCancellationRequested) | ||
| return SpecializedTasks.Null<TopLevelSyntaxTreeIndex>(); | ||
|
|
||
| // Retrieve the string table we use to dedupe strings. If we can't get it, that means the solution has | ||
| // fully loaded and we've switched over to normal navto lookup. | ||
| if (!ShouldSearchCachedDocuments(out var cachedIndexMap, out var stringTable)) | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
part of the ystem used to involve a callback that would pass along a project+item. The former is redundant given that everyone starts with a solution snapshot, and a navto item always contains a project-id in it anyways.