Switch to cleaner patterns than explicit Task.Run #73471
Conversation
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
| @@ -106,7 +105,7 @@ public bool ExecuteCommand(TCommandArgs args, CommandExecutionContext context) | |||
| if (service == null) | |||
| return false; | |||
|
|
|||
| Contract.ThrowIfNull(document); | |||
| Roslyn.Utilities.Contract.ThrowIfNull(document); | |||
There was a problem hiding this comment.
conflict between us and VSThreading with ConfigureAwait on Task.Yield. so i removed our using, which meant having to qualify this.
| @@ -87,43 +88,41 @@ public async Task<(ImmutableArray<IMethodSymbol> symbols, bool isPartialResult)> | |||
| try | |||
| { | |||
| // Find applicable symbols in parallel | |||
| using var _1 = ArrayBuilder<Task<ImmutableArray<IMethodSymbol>?>>.GetInstance(out var tasks); | |||
| var peReferenceMethodSymbolsTask = ProducerConsumer<IMethodSymbol?>.RunParallelAsync( | |||
There was a problem hiding this comment.
@genlu for updated pattern.
| @@ -156,6 +156,9 @@ internal partial class CodeFixService : ICodeFixService | |||
| SortedDictionary<TextSpan, List<DiagnosticData>> spanToDiagnostics, | |||
| CancellationToken cancellationToken) | |||
| { | |||
| // Ensure we yield here so the caller can continue on. | |||
| await AwaitExtensions.ConfigureAwait(Task.Yield(), false); | |||
There was a problem hiding this comment.
we have a lot of usage of both Roslyn.Utilities and VSThreading in this file. so i couldn't comment out the using for either easily. So this was how i got past the ambiguity about which ConfigureAwait to call.
...n/Providers/ImportCompletionProvider/ExtensionMethodImportCompletionHelper.SymbolComputer.cs
Show resolved
Hide resolved
...erver/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs
Outdated
Show resolved
Hide resolved
...erver/Protocol/Features/Diagnostics/EngineV2/DiagnosticIncrementalAnalyzer_GetDiagnostics.cs
Outdated
Show resolved
Hide resolved
src/Workspaces/Core/Portable/CodeFixes/FixAllOccurrences/BatchFixAllProvider.cs
Outdated
Show resolved
Hide resolved
...paces/Core/Portable/Workspace/Solution/SolutionCompilationState.TranslationAction_Actions.cs
Outdated
Show resolved
Hide resolved
|
|
||
| return GetDiagnosticData(); | ||
| return await ProduceProjectDiagnosticsAsync( | ||
| [project], static _ => [], includeProjectNonLocalResult: true, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
note: thsi logic intentionally changed. the old logic said "if you give me a project id not in the solution, i give you all solution diagnostics". t he new logic says "if you give me a project not in the solution, you get back nothing". this mirrors the logic higher up in this file.
| @@ -253,6 +252,9 @@ private Task DelayAsync(CancellationToken cancellationToken) | |||
| private async Task FindResultsAsync( | |||
| IFindUsagesContext findContext, Document document, int position, CancellationToken cancellationToken) | |||
| { | |||
| // Ensure that we relinquish the thread so that the caller can proceed with their work. | |||
| await Task.Yield().ConfigureAwait(false); | |||
There was a problem hiding this comment.
Another API for this is:
await TaskScheduler.Default.SwitchTo(true);
There was a problem hiding this comment.
You can also call ConfigureAwait directly and not rely on it appearing as an extension method.
| @@ -110,9 +111,10 @@ public void RequestDiagnosticRefresh() | |||
| priorityProvider ??= new DefaultCodeActionRequestPriorityProvider(); | |||
|
|
|||
| // always make sure that analyzer is called on background thread. | |||
| return Task.Run(() => analyzer.GetDiagnosticsForSpanAsync( | |||
| await TaskScheduler.Default; | |||
There was a problem hiding this comment.
📝 This is slightly different from before. If the caller is already on the background thread, it will no longer yield. You can use SwitchTo(true) to force it to yield if that behavior is important.
There was a problem hiding this comment.
this was intentional. as the comment says, this is just to ensure that we're on the BG. if we're already there, no need to yield.
No description provided.