Runtime: Add awaitable ThreadPool.SwitchTo()

Created on 26 Jan 2017  路  26Comments  路  Source: dotnet/runtime

Task.Run breaks the execution flow by forcing the code into a delegate, it's basically callback hell (since it's the first callback in the chain). Instead, for the situations where we want to stay in the await pattern, ThreadPool.Yield() would be a nice to have API. It would turn code that looks like this:

```C#
await Task.Run(() => DoSomething());

```C#
await ThreadPool.Yield();

// Now on a thread pool thread so we can call some user code
DoSomething();

/cc @stephentoub

api-needs-work area-System.Threading

Most helpful comment

SwitchTo sounds fine to me.

However, C# has since gained the ability to have awaits in catch/finally blocks, making that largely a moot argument.

If we ever get async disposable maybe that pattern could be more natural.

C# await using (TaskScheduler.Current.Preserve()) { await TaskScheduler.Default.SwitchTo(); // Do thread pool stuff } // Revert to the original scheduler

All 26 comments

SwitchTo?

Was keeping in line with Task.Yield()

I agree this is a pretty reasonable thing to add. In the original async/await CTP, we actually had methods for doing this, IIRC as extensions on a TaskScheduler and SynchronizationContext, e.g.
```C#
await TaskScheduler.Default.SwitchTo(); // continuation will be queued to the thread pool

We removed these because you couldn't use an await inside of a catch/finally, which meant it was difficult to return to the original context if you were trying to do something structured like:
```C#
var origScheduler = TaskScheduler.Current;
await TaskScheduler.Default.SwitchTo();
try
{
    ... // runs on thread pool
}
finally { await origScheduler; }

and as a result you could end up in situations where error related logic would run on the "wrong" context; in a sense we were concerned we were adding APIs that would lead to dangerous situations. However, C# has since gained the ability to have awaits in catch/finally blocks, making that largely a moot argument.

I would personally also err on the side of using a name more like SwitchTo. The Yield naming was intended to suggest yielding and coming back to the current context, so using it to mean "go to that other context" might be a little confusing.

SwitchTo sounds fine to me.

However, C# has since gained the ability to have awaits in catch/finally blocks, making that largely a moot argument.

If we ever get async disposable maybe that pattern could be more natural.

C# await using (TaskScheduler.Current.Preserve()) { await TaskScheduler.Default.SwitchTo(); // Do thread pool stuff } // Revert to the original scheduler

Should it check if already on threadpool/scheduler and immediately complete if so?

Should it check if already on threadpool/scheduler and immediately complete if so?

It would depend on your scenario. For example, if you were using it to ensure that a caller of your async API wasn't blocked while doing some long computation, then you'd want it to always queue. But if you were using it to ensure you were running on a context that owned done resources only accessible to that context, you wouldn't care and for perf might want it to nop.

@davidfowl, now we have IAsyncDisposable and this still looks nice, indeed:

await using (TaskScheduler.Current.Preserve())
{
    await TaskScheduler.Default.SwitchTo();
    // Do thread pool stuff
} // Revert to the original scheduler

But I feel it is no different from:

await Task.Run(async() => {
    // do thread pool stuff 
    // (or async stuff without sync context)
}); // Revert to the original sync context - without ConfigireAwait(false)

Is there any subtle difference?

Is there any subtle difference?

Potentially perf, but functionally I'd expect them to be the same. That said, the former doesn't exist, so it's hard to say for sure.

Wouldn't it be possible to have the SwitchTo to pass in your TaskScheduler. The return value is an async dispobable that contains the previous TaskScheduler.

A use case is where you have a limited access to a resource like a database connection or external API service. Together with the async disposable, you can create something like the following:

private TaskScheduler _limitedResource = new LimitedTaskScheduler(10);

await using (Task.SwitchTo(_limitedResource))
{
  // Use resource with limited concurrent access
}

Placing the SwitchTo on the Task would make it more visible for people who are not aware of the existence of the TaskScheduler.

An overload to this method can accept a boolean indicating whether you must always switch or only if you are running on a different context than the provided one.

Could the API take a cancellation token? Making the SwitchTo implementation aware of cancellation could avoid queuing work on the thread pool when already canceled.

It would be a single call, replacing:

cancellationToken.ThrowIfCancellationRequested();
await TaskScheduler.Default.SwitchTo();
cancellationToken.ThrowIfCancellationRequested();

Usage:

public static async IAsyncEnumerable<string> EnumerateFilesAsync(
    string path,
    string searchPattern,
    SearchOption searchOption,
    [EnumeratorCancellation] CancellationToken cancellationToken)
{
    await TaskScheduler.Default.SwitchTo(cancellationToken);

    foreach (var file in Directory.EnumerateFiles(path, searchPattern, searchOption))
    {
        yield return file;

        await TaskScheduler.Default.SwitchTo(cancellationToken);
    }
}

Maybe not relevant to how .NET Core would implement it, but this implementation actually passes the cancellation token to TaskFactory.StartNew when switching to a non-default task scheduler: https://gist.github.com/jnm2/46a642d2c9f2794ece0b095ba3d96270

@noseratio
```C#
await using (TaskScheduler.Current.Preserve())
{
await TaskScheduler.Default.SwitchTo();
// Do thread pool stuff
} // Revert to the original scheduler

It could be simplified to 
```C#
await using (await TaskScheduler.Default.SwitchTo())
{
}

I don't like having await TaskScheduler.Default.SwitchTo() produce an IAsyncDisposable. It's much rarer that I'll want to revert to the original scheduler. In the 99% case, I'll get IDE warnings that I'm not disposing a disposable object.

It must implement it via async disposable pattern, with DisposeAsync returning a struct instead of ValueTask.

The IDE will (should!) still warn when a type follows the async disposable pattern and is not disposed, and I wouldn't like that for this API.

Maybe _ = await TaskScheduler.Default.SwitchTo(); would be okay as a way to silence, but the name SwitchTo doesn't make it obvious what is being discarded. And we're making the 99% case less optimal for the sake of the 1%. It seems more fitting for the rarer case to use the additional .Preserve() call. On top of that, the name Preserve is significantly more self-documenting

Is TaskSheduler.SwitchTo still planned for 6.0?

It'd be really useful where we deal we a 3rd party code that we don't want to execute on any custom synchronization context.
E.g., I really like the simplicity of the first option from the below. IMO, it clearly indicates the intent, with minimum boilerplate code, and it's also free of some corner cases (unlike the last option):

// switch to the thread pool explicitly for the rest of the async method
await TaskScheduler.Default.SwitchTo();
await RunOneWorkflowAsync();
await RunAnotherWorkflowAsync();

// execute RunOneWorkflowAsync on the thread pool 
// and stay there for the rest of the async method
await Task.Run(RunOneWorkflowAsync).ConfigureAwait(false);
await RunAnotherWorkflowAsync();

```C#
await Task.Run(async () =>
{
// start on the thread pool
await RunOneWorkflowAsync();
await RunAnotherWorkflowAsync();
}).ConfigureAwait(false);
// continue on the thread pool for the rest of the async method

```csharp
// start on whatever the current synchronization context is
await RunOneWorkflowAsync().ConfigureAwait(false);
// continue on the thread pool for the rest of the async method,
// unless everything inside `RunOneWorkflowAsync` has completed synchronously
await RunAnotherWorkflowAsync();

I have an experimental implementation of SwitchTo as an extension method (gist), it needs some work and tests and I'd be happy to contribute that as a PR for 6.0.

@noseratio the syntax you want already works if you reference the Microsoft.VisualStudio.Threading Nuget package.

the syntax you want already works if you reference the Microsoft.VisualStudio.Threading Nuget package.

@AArnott thanks for the pointer, didn't know that had also been open-sourced! 馃憤

It's good to see the alwaysYield param there as well, mindlike! Do you think supressing ExecutionContext flow for non-default task schedulers is an overkill?

I'm curious to know if this SwitchTo pattern get used a lot in Visual Studio itself. I generally like it much more than async lambda to Task.Run. The latter IMHO should only be used for synchronous lambdas which do some CPU-bound work.

I don't know why I'd want to suppress ExecutionContext flow. But I'm open to learning. The whole distinction between safe and unsafe awaiters is one I'm not particularly strong on.

We only explicitly call SwitchTo when we want to use alwaysYield: true. Otherwise we just await on the TaskScheduler directly (since we made that awaitable as well).
And yes, we generally prefer await TaskScheduler.Default; over await Task.Run because not only the code is cleaner, but we avoid allocating another closure, delegate, and async state machine since the calling method already has all those that can be reused. However, using await taskScheduler causes us to lose our caller's context. So if we want to return to our caller's context (e.g. the main thread) then using await Task.Run is an easy way to do some amount of work on the threadpool but then return to our caller's context when we're done.

I don't know why I'd want to suppress ExecutionContext flow. But I'm open to learning. The whole distinction between safe and unsafe awaiters is one I'm not particularly strong on.

This explained things for me: https://devblogs.microsoft.com/pfxteam/whats-new-for-parallelism-in-net-4-5-beta/
Ctrl+F for For those of you familiar with ExecutionContext.

I don't know why you'd want to suppress the execution context flow as part of this either. Seems entirely unrelated here. Maybe there's some common scenario I'm missing?

I think we could do a better job of suppressing ExecutionFlow in some of our awaiters: https://github.com/microsoft/vs-threading/pull/689

I don't know why you'd want to suppress the execution context flow as part of this either. Seems entirely unrelated here. Maybe there's some common scenario I'm missing?

@davidfowl and @AArnott I might be wrong, but I think it might be a sensible optimization for ICriticalNotifyCompletion.UnsafeOnCompleted. We don't have to flow execution context there, but Task.Factory.StartNew would still do that, adding a bit of unneeded overhead. I believe suppressing the flow explicitly tells Task.Factory.StartNew to not do that.

This is not a concern for TaskScheduler.Default, for which can just use ThreadPool.UnsafeQueueUserWorkItem.

And yes, we generally prefer await TaskScheduler.Default; over await Task.Run because not only the code is cleaner, but we avoid allocating another closure, delegate, and async state machine since the calling method already has all those that can be reused. However, using await taskScheduler causes us to lose our caller's context.

That makes sense. I usually use await TaskScheduler.Default at the beginning of the methods that don't touch the UI or ViewModel at all.

I think it might be a sensible optimization for ICriticalNotifyCompletion.UnsafeOnCompleted. We don't have to flow execution context there, but Task.Factory.StartNew would still do that, adding a bit of unneeded overhead

See https://github.com/microsoft/vs-threading/pull/689#discussion_r497711508. Someone could do the appropriate measurements, but my guess is that for .NET Core this would actually be a net negative rather than positive.

See microsoft/vs-threading#689 (comment). Someone could do the appropriate measurements, but my guess is that for .NET Core this would actually be a net negative rather than positive.

That comment is a great insight, thank you. I suppose it explains why there so few uses of SuppressFlow in .NET Core.

I suppose it explains why there so few uses of SuppressFlow in .NET Core.

Yes, it's use now isn't about throughout or allocation, but entirely about object lifetime, i.e. ensuring that ExecutionContext isn't captured unnecessarily into something that may live for a long time, that doesn't need access to the context (and won't be calling unknown code that might), and thus shouldn't forcibly extend the lifetime of values stored into async locals captured by the EC.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

iCodeWebApps picture iCodeWebApps  路  3Comments

matty-hall picture matty-hall  路  3Comments

bencz picture bencz  路  3Comments

yahorsi picture yahorsi  路  3Comments

omajid picture omajid  路  3Comments