From 4b634ccf875ea793a9a2aa3df806fbe5df245f05 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 2 May 2024 18:56:39 -0700 Subject: [PATCH 1/5] Switch to a producer/consumer queue to search for add-using results. --- .../AbstractAddImportFeatureService.cs | 54 +++++++++++-------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs index 84a7d0ffe9b66..2c74ab801a67b 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs @@ -10,6 +10,7 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Threading; +using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.CodeAnalysis.AddPackage; using Microsoft.CodeAnalysis.CodeActions; @@ -20,6 +21,7 @@ using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Remote; using Microsoft.CodeAnalysis.Shared.Extensions; +using Microsoft.CodeAnalysis.Shared.Utilities; using Microsoft.CodeAnalysis.SymbolSearch; using Microsoft.CodeAnalysis.Text; using Roslyn.Utilities; @@ -215,28 +217,33 @@ private static async Task FindResultsInUnreferencedProjectSourceSymbolsAsync( var viableUnreferencedProjects = GetViableUnreferencedProjects(project); - // Search all unreferenced projects in parallel. - using var _ = ArrayBuilder.GetInstance(out var findTasks); - // Create another cancellation token so we can both search all projects in parallel, // but also stop any searches once we get enough results. using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - foreach (var unreferencedProject in viableUnreferencedProjects) - { - if (!unreferencedProject.SupportsCompilation) - continue; - - // Search in this unreferenced project. But don't search in any of its' - // direct references. i.e. we don't want to search in its metadata references - // or in the projects it references itself. We'll be searching those entities - // individually. - findTasks.Add(ProcessReferencesAsync( - allSymbolReferences, maxResults, linkedTokenSource, - finder.FindInSourceSymbolsInProjectAsync(projectToAssembly, unreferencedProject, exact, linkedTokenSource.Token))); - } - - await Task.WhenAll(findTasks).ConfigureAwait(false); + // Defer to the ProducerConsumer. We're search the unreferenced projects in parallel. As we get results, we'll + // add them to the 'allSymbolReferences' queue. If we get enough results, we'll cancel all the other work. + await ProducerConsumer>.RunAsync( + ProducerConsumerOptions.SingleReaderOptions, + produceItems: static (onItemsFound, args) => RoslynParallel.ForEachAsync( + args.viableUnreferencedProjects, + args.linkedTokenSource.Token, + async (project, cancellationToken) => + { + // Search in this unreferenced project. But don't search in any of its' direct references. i.e. we + // don't want to search in its metadata references or in the projects it references itself. We'll be + // searching those entities individually. + var references = await args.finder.FindInSourceSymbolsInProjectAsync( + args.projectToAssembly, project, args.exact, cancellationToken).ConfigureAwait(false); + onItemsFound(references); + }), + consumeItems: static async (symbolReferencesEnumerable, args) => + { + await foreach (var symbolReferences in symbolReferencesEnumerable) + ProcessReferences(args.allSymbolReferences, args.maxResults, symbolReferences, args.linkedTokenSource); + }, + args: (projectToAssembly, allSymbolReferences, maxResults, finder, exact, viableUnreferencedProjects, linkedTokenSource), + linkedTokenSource.Token).ConfigureAwait(false); } private async Task FindResultsInUnreferencedMetadataSymbolsAsync( @@ -314,13 +321,14 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( return result.ToImmutableAndFree(); } - private static async Task ProcessReferencesAsync( + private static void ProcessReferences( ConcurrentQueue allSymbolReferences, int maxResults, - CancellationTokenSource linkedTokenSource, - Task> task) + ImmutableArray foundReferences, + CancellationTokenSource linkedTokenSource) { - AddRange(allSymbolReferences, await task.ConfigureAwait(false)); + linkedTokenSource.Token.ThrowIfCancellationRequested(); + AddRange(allSymbolReferences, foundReferences); // If we've gone over the max amount of items we're looking for, attempt to cancel all existing work that is // still searching. @@ -419,7 +427,7 @@ int IEqualityComparer.GetHashCode(PortableExecutabl private static HashSet GetViableUnreferencedProjects(Project project) { var solution = project.Solution; - var viableProjects = new HashSet(solution.Projects); + var viableProjects = new HashSet(solution.Projects.Where(p => p.SupportsCompilation)); // Clearly we can't reference ourselves. viableProjects.Remove(project); From 62f73cd8598f084b30249fc2bc3a6206f77de9bb Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 2 May 2024 19:04:27 -0700 Subject: [PATCH 2/5] Switch to a producer/consumer queue to search for add-using results. --- .../AbstractAddImportFeatureService.cs | 64 +++++++++++-------- 1 file changed, 39 insertions(+), 25 deletions(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs index 2c74ab801a67b..600aae28f0f22 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs @@ -237,11 +237,8 @@ await ProducerConsumer>.RunAsync( args.projectToAssembly, project, args.exact, cancellationToken).ConfigureAwait(false); onItemsFound(references); }), - consumeItems: static async (symbolReferencesEnumerable, args) => - { - await foreach (var symbolReferences in symbolReferencesEnumerable) - ProcessReferences(args.allSymbolReferences, args.maxResults, symbolReferences, args.linkedTokenSource); - }, + consumeItems: static (symbolReferencesEnumerable, args) => + ProcessReferencesAsync(args.allSymbolReferences, args.maxResults, symbolReferencesEnumerable, args.linkedTokenSource), args: (projectToAssembly, allSymbolReferences, maxResults, finder, exact, viableUnreferencedProjects, linkedTokenSource), linkedTokenSource.Token).ConfigureAwait(false); } @@ -253,7 +250,7 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( { // Only do this if none of the project searches produced any results. We may have a // lot of metadata to search through, and it would be good to avoid that if we can. - if (allSymbolReferences.Count > 0) + if (!allSymbolReferences.IsEmpty) return; // Keep track of the references we've seen (so that we don't process them multiple times @@ -264,29 +261,36 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( var newReferences = GetUnreferencedMetadataReferences(project, seenReferences); - // Search all metadata references in parallel. - using var _ = ArrayBuilder.GetInstance(out var findTasks); - // Create another cancellation token so we can both search all projects in parallel, // but also stop any searches once we get enough results. using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - foreach (var (referenceProject, reference) in newReferences) - { - var compilation = referenceToCompilation.GetOrAdd( - reference, r => CreateCompilation(project, r)); + // Defer to the ProducerConsumer. We're search the unreferenced projects in parallel. As we get results, we'll + // add them to the 'allSymbolReferences' queue. If we get enough results, we'll cancel all the other work. + await ProducerConsumer>.RunAsync( + ProducerConsumerOptions.SingleReaderOptions, + produceItems: static (onItemsFound, args) => RoslynParallel.ForEachAsync( + args.newReferences, + args.linkedTokenSource.Token, + async (tuple, cancellationToken) => + { + var (referenceProject, reference) = tuple; + var compilation = args.referenceToCompilation.GetOrAdd( + reference, r => CreateCompilation(args.project, r)); - // Ignore netmodules. First, they're incredibly esoteric and barely used. - // Second, the SymbolFinder API doesn't even support searching them. - if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol assembly) - { - findTasks.Add(ProcessReferencesAsync( - allSymbolReferences, maxResults, linkedTokenSource, - finder.FindInMetadataSymbolsAsync(assembly, referenceProject, reference, exact, linkedTokenSource.Token))); - } - } + // Ignore netmodules. First, they're incredibly esoteric and barely used. + // Second, the SymbolFinder API doesn't even support searching them. + if (compilation.GetAssemblyOrModuleSymbol(reference) is not IAssemblySymbol assembly) + return; - await Task.WhenAll(findTasks).ConfigureAwait(false); + var references = await args.finder.FindInMetadataSymbolsAsync( + assembly, referenceProject, reference, args.exact, args.linkedTokenSource.Token).ConfigureAwait(false); + onItemsFound(references); + }), + consumeItems: static (symbolReferencesEnumerable, args) => + ProcessReferencesAsync(args.allSymbolReferences, args.maxResults, symbolReferencesEnumerable, args.linkedTokenSource), + args: (referenceToCompilation, project, allSymbolReferences, maxResults, finder, exact, newReferences, linkedTokenSource), + linkedTokenSource.Token).ConfigureAwait(false); } /// @@ -297,7 +301,7 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( private static ImmutableArray<(Project, PortableExecutableReference)> GetUnreferencedMetadataReferences( Project project, HashSet seenReferences) { - var result = ArrayBuilder<(Project, PortableExecutableReference)>.GetInstance(); + using var _ = ArrayBuilder<(Project, PortableExecutableReference)>.GetInstance(out var result); var solution = project.Solution; foreach (var p in solution.Projects) @@ -318,7 +322,17 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( } } - return result.ToImmutableAndFree(); + return result.ToImmutableAndClear(); + } + + private static async Task ProcessReferencesAsync( + ConcurrentQueue allSymbolReferences, + int maxResults, + IAsyncEnumerable> reader, + CancellationTokenSource linkedTokenSource) + { + await foreach (var symbolReferences in reader) + ProcessReferences(allSymbolReferences, maxResults, symbolReferences, linkedTokenSource); } private static void ProcessReferences( From c6674b10d41ab03347593ef082a94d7ca0719814 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 2 May 2024 19:05:12 -0700 Subject: [PATCH 3/5] Clean --- .../Core/Portable/AddImport/AbstractAddImportFeatureService.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs index 600aae28f0f22..149030dd3123e 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs @@ -10,7 +10,6 @@ using System.Linq; using System.Runtime.CompilerServices; using System.Threading; -using System.Threading.Channels; using System.Threading.Tasks; using Microsoft.CodeAnalysis.AddPackage; using Microsoft.CodeAnalysis.CodeActions; From 6b2354ef7ba0582ccc6d41873a95728956418c13 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 2 May 2024 19:06:13 -0700 Subject: [PATCH 4/5] Fix comment --- .../Core/Portable/AddImport/AbstractAddImportFeatureService.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs index 149030dd3123e..0f1d0d91d3c9b 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs @@ -264,7 +264,7 @@ private async Task FindResultsInUnreferencedMetadataSymbolsAsync( // but also stop any searches once we get enough results. using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - // Defer to the ProducerConsumer. We're search the unreferenced projects in parallel. As we get results, we'll + // Defer to the ProducerConsumer. We're search the metadata references in parallel. As we get results, we'll // add them to the 'allSymbolReferences' queue. If we get enough results, we'll cancel all the other work. await ProducerConsumer>.RunAsync( ProducerConsumerOptions.SingleReaderOptions, From 6ee0cb6054c9fbabaf4ab9467f01e817d3480c57 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 2 May 2024 19:07:20 -0700 Subject: [PATCH 5/5] Simplify --- .../AbstractAddImportFeatureService.cs | 33 ++++++++----------- 1 file changed, 13 insertions(+), 20 deletions(-) diff --git a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs index 0f1d0d91d3c9b..7cf284b733d81 100644 --- a/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs +++ b/src/Features/Core/Portable/AddImport/AbstractAddImportFeatureService.cs @@ -331,28 +331,21 @@ private static async Task ProcessReferencesAsync( CancellationTokenSource linkedTokenSource) { await foreach (var symbolReferences in reader) - ProcessReferences(allSymbolReferences, maxResults, symbolReferences, linkedTokenSource); - } - - private static void ProcessReferences( - ConcurrentQueue allSymbolReferences, - int maxResults, - ImmutableArray foundReferences, - CancellationTokenSource linkedTokenSource) - { - linkedTokenSource.Token.ThrowIfCancellationRequested(); - AddRange(allSymbolReferences, foundReferences); - - // If we've gone over the max amount of items we're looking for, attempt to cancel all existing work that is - // still searching. - if (allSymbolReferences.Count >= maxResults) { - try - { - linkedTokenSource.Cancel(); - } - catch (ObjectDisposedException) + linkedTokenSource.Token.ThrowIfCancellationRequested(); + AddRange(allSymbolReferences, symbolReferences); + + // If we've gone over the max amount of items we're looking for, attempt to cancel all existing work that is + // still searching. + if (allSymbolReferences.Count >= maxResults) { + try + { + linkedTokenSource.Cancel(); + } + catch (ObjectDisposedException) + { + } } } }