Skip to content
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

Run providers in parallel when determining if we should show the lightbulb icon #73758

Merged
merged 2 commits into from
May 29, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -89,18 +89,58 @@ public async Task<bool> HasRefactoringsAsync(
CodeActionOptionsProvider options,
CancellationToken cancellationToken)
{
foreach (var provider in GetProviders(document))
{
cancellationToken.ThrowIfCancellationRequested();
// A token for controlling the inner work we do calling out to each provider. Once we have a single provider
// that returns a refactoring, we can cancel the work we're doing with all other providers.
using var linkedTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);

// This await will not complete until all providers have been called (though we'll attempt to bail out of any of
// them once we have a single refactoring found). So there's no concern here about produceItems running after
// linkedTokenSource has been diposed.
return await ProducerConsumer<VoidResult>.RunParallelAsync(
source: this.GetProviders(document),
produceItems: static async (provider, callback, args, cancellationToken) =>
{
var (@this, document, state, options, linkedTokenSource) = args;

var refactoring = await GetRefactoringFromProviderAsync(
document, state, provider, options, cancellationToken).ConfigureAwait(false);
// Do no work if either the outer request canceled, or another provider already found a refactoring.
if (cancellationToken.IsCancellationRequested || linkedTokenSource.Token.IsCancellationRequested)
return;

if (refactoring != null)
return true;
}
try
{
// We want to pass linkedTokenSource.Token here so that we can cancel the inner operation once the
// outer ProducerConsumer sees a single refactoring returned by any provider.
var refactoring = await @this.GetRefactoringFromProviderAsync(
document, state, provider, options, linkedTokenSource.Token).ConfigureAwait(false);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

linkedTokenSource.Token

Is it possible to get a ObjectDisposedException here if this has been disposed?

Copy link
Member Author

@CyrusNajmabadi CyrusNajmabadi May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should not be possible. we only dispose after the call to RunParallelAsync has completed. And it only completes Asher both the producer and consumer complete.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

took a minute to process, but yeah, that makes sense. Thanks!


// If we have a refactoring, send a single VoidResult value to the consumer so it can cancel the
// other concurrent operations, and can return 'true' to the caller to indicate that there are
// refactorings.
if (refactoring != null)
callback(default(VoidResult));
}
catch (OperationCanceledException)
{
// Ensure that the cancellation of the inner token doesn't bubble outside. We are not canceling the
// entire operation just because one provider succeeded and canceled the rest.
}
},
consumeItems: static async (items, args, cancellationToken) =>
{
// Try to consume from the results that produceItems is sending us. The moment we get a single result,
// we know we're done and we have at least one refactoring.
await foreach (var unused in items)
{
// Cancel all the other items that are still running (or are asked to run in the future).
args.linkedTokenSource.Cancel();
return true;
}

return false;
return true;
CyrusNajmabadi marked this conversation as resolved.
Show resolved Hide resolved
},
args: (this, document, state, options, linkedTokenSource),
// intentionally using the outer token here. The linked token is only used to cancel the inner operations.
cancellationToken).ConfigureAwait(false);
}

public async Task<ImmutableArray<CodeRefactoring>> GetRefactoringsAsync(
Expand Down
Loading