-
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
Move more code over to ProducerConsumer model. #73324
Conversation
@@ -120,14 +120,15 @@ private static void ClearCachedData() | |||
|
|||
// 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. | |||
await PerformParallelSearchAsync( | |||
await ProducerConsumer<RoslynNavigateToItem>.RunParallelAsync( |
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.
inlined as requested :)
@@ -101,7 +101,7 @@ private static void ClearCachedData() | |||
ImmutableArray<DocumentKey> priorityDocumentKeys, | |||
string searchPattern, | |||
IImmutableSet<string> kinds, | |||
Func<ImmutableArray<RoslynNavigateToItem>, Task> onItemsFound, | |||
Func<ImmutableArray<RoslynNavigateToItem>, VoidResult, CancellationToken, Task> onItemsFound, |
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.
need the VoidResult for the 'args'. we don't actually end up needing any args ourselves, so this is how we just indicate it's unused.
also, i found that things wer ecleaner if i made it so that 'cancellation' was consistently passed along as an actual arg to the lambdas, instead of needing to package it in the 'args' parameter.
/// <code>Parallel.ForEachAsync</code>, allowing for parallel processing of the items, with a preference towards | ||
/// earlier items. | ||
/// </summary> | ||
private static Task PerformParallelSearchAsync<T>( |
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.
removed. inlined in the 3 places that use it.
@@ -94,19 +111,6 @@ internal static partial class FixAllContextHelper | |||
return await GetDocumentDiagnosticsToFixAsync( | |||
fixAllContext.Solution, allDiagnostics, fixAllContext.CancellationToken).ConfigureAwait(false); | |||
|
|||
async Task AddDocumentDiagnosticsAsync(ConcurrentDictionary<ProjectId, ImmutableArray<Diagnostic>> diagnostics, Project projectToFix) |
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.
inlined.
} | ||
await foreach (var (project, diagnostics) in results) | ||
{ | ||
if (diagnostics.Any()) |
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.
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.
agreed. can do in followup.
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.
@jasonmalinowski For review when you get back. |
Followup to #73323.