Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add a way to wait for a Task without throwing and without allocating #17148

Closed
GSPP opened this issue Apr 28, 2016 · 14 comments
Closed

Add a way to wait for a Task without throwing and without allocating #17148

GSPP opened this issue Apr 28, 2016 · 14 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Threading.Tasks
Milestone

Comments

@GSPP
Copy link

GSPP commented Apr 28, 2016

At the moment there does not seem to be ay direct API that you can use to wait for a task to complete without throwing in case the task completes cancelled or faulted.

This forces using exceptions for control flow:

try {
 task.Wait();
}
catch (AggEx) { } //Do nothing.

This is ugly and very slow. Exceptions seem to cost ~0.1ms of CPU time to process.

So if you are writing a port scanner using async IO you will find that you cannot scan more that 10k ports per second because the CPU is pegged from exception processing(!). Also, I don't know if exceptions scale across CPUs. I know that ExceptionDispatchInfo uses a global lock internally. So at least some of the processing does not scale at all.

It might not be too common to have many faulted tasks. But it is more likely to rely on a greater number of cancelled tasks.

I know of two workarounds:

Task.WhenAny(task); //params overload
task.CW(() => { }, ExecuteSynchronously).Wait();

Both allocate, add overhead and both make it unclear what you want to achieve.

Please add a simpler and faster way to wait for a task to complete regardless of the terminal state it ends up in.

The same problem exists with await. There is no direct way to wait for completion without throwing. Maybe ConfigureAwait can be extended with an AwaitOptions enum that has the values IgnoreContext, NoThrow.

@stephentoub stephentoub self-assigned this Apr 28, 2016
@GSPP
Copy link
Author

GSPP commented Apr 28, 2016

Did I open the same issue twice? This is funny.

@brian-reichle
Copy link

You can already do this with an extension method returning custom awaitable/awaiter structs that wrap ConfiguredTaskAwaitable.ConfiguredTaskAwaiter, you just need to proxy all the awaiter members except GetResult() (which should do nothing.)

public struct NonObservingAwaitable
{
    public NonObservingAwaitable(ConfiguredTaskAwaitable.ConfiguredTaskAwaiter innerAwaiter)
    {
        _innerAwaiter = innerAwaiter;
    }

    public NonObservingAwaiter GetAwaiter() => new NonObservingAwaiter(_innerAwaiter);

    public struct NonObservingAwaiter : ICriticalNotifyCompletion, INotifyCompletion
    {
        public NonObservingAwaiter(ConfiguredTaskAwaitable.ConfiguredTaskAwaiter innerAwaiter)
        {
            _innerAwaiter = innerAwaiter;
        }

        public void GetResult() { }
        public bool IsCompleted => _innerAwaiter.IsCompleted;
        public void OnCompleted(Action continuation) => _innerAwaiter.OnCompleted(continuation);
        public void UnsafeOnCompleted(Action continuation) => _innerAwaiter.UnsafeOnCompleted(continuation);

        readonly ConfiguredTaskAwaitable.ConfiguredTaskAwaiter _innerAwaiter;
    }

    readonly ConfiguredTaskAwaitable.ConfiguredTaskAwaiter _innerAwaiter;
}

public static class TaskUtils
{
   public static NonObservingAwaitable NonObserving(this Task task, bool continueOnCapturedContext = true)
    {
        return new NonObservingAwaitable(task.ConfigureAwait(continueOnCapturedContext).GetAwaiter());
    }
}

But If you do something like this, you need to be careful not to leave the exception unobserved.

var task = DoStuffAsync();
await task.NonObserving(false);

if (task.IsFaulted && IsIgnorable(task.Exception))
{
    return null;
}

return task.Result;

@GSPP
Copy link
Author

GSPP commented Apr 29, 2016

@brian-reichle this is new idea. But it only works if you want to await the task. Instead, you might want to Wait or await a combinator such as WhenAll. For all of those there are solutions but it's not nice.

@benaadams
Copy link
Member

WhenAll returns a Task so it still works:

var task = Task.WhenAll(...);
await task.NonObserving(false);

if (task.IsFaulted && IsIgnorable(task.Exception))
{
    return null;
}

return task.Result;

@GSPP
Copy link
Author

GSPP commented Apr 29, 2016

@benaadams you might not want the combined task to end up faulted. Maybe the task is supposed to signal completion, not successful completion. Other code might process that task not expecting any fault. I acknowledge that this is getting more contrived now...

@svick
Copy link
Contributor

svick commented Apr 29, 2016

@GSPP In that case, you can probably use Task.Factory.ContinueWhenAll(tasks, _ => {}).

@brian-reichle
Copy link

@GSPP, it was mostly posed as an alternative to your suggestion to extend ConfigureAwait and will be just as applicable in all the same situations.

In any case, this can still work to reduce the number of throws for multiple async requests, just use it in an async method that sanitizes the result of a single request before aggregating all the tasks. Of course, this won't help much with the number of allocations, but I suspect that aspect might be overrated.

If you find you really need to reduce the number of allocations per-request (and still want to use TPL), then you might need to do the combining yourself by awaiting multiple times in a single async method ... but there will probably be easier ways to 'trim the fat'.

var tasks = new[]
{
    DoStuffAsync(1),
    DoStuffAsync(2),
    DoStuffAsync(3),
    // ...
};

var result = new List<int>();
List<Exception> exceptions = null;

for (var i = 0; i < tasks.Length; i++)
{
    var task = tasks[i];
    await task.NonObserving(false);

    if (task.IsFaulted)
    {
        if (!IsIgnorable(task.Exception))
        {
            if (exceptions == null)
            {
                exceptions = new List<Exception>();
            }

            exceptions.Add(task.Exception);
        }
    }
    else
    {
        if (IsSuccessful(task.Result))
        {
            result.Add(i);
        }
    }
}

if (exceptions != null)
{
    throw new AggregateException(exceptions);
}

return result;

@stephentoub stephentoub removed their assignment Sep 30, 2016
@karelz
Copy link
Member

karelz commented Oct 13, 2016

We need API proposal. Scenario seems useful.

@danmoseley
Copy link
Member

@GSPP do you want to complete formal API proposal then we can review formally?

Docs on the API review process and see Jon's API request for a great example of a strong proposal. The more concrete the proposal is (e.g. has examples of usage, real-world scenarios, fleshed out API), the more discussion it will start and the better the chances of us being able to push the addition through the review process.

@GSPP
Copy link
Author

GSPP commented Mar 2, 2017

My time is too limited right now to act on this. Just leaving this note here so that others know this is "up for grabs".

@benaadams
Copy link
Member

Has this been resolved with ValueTask and IValueTaskSource?

@svick
Copy link
Contributor

svick commented Jun 22, 2018

@benaadams How exactly do they solve the issue of synchronously or asynchronously waiting for a Task without throwing that doesn't allocate?

@benaadams
Copy link
Member

Ah, good point, reread; is different issue :)

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 5.0 milestone Jan 31, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@stephentoub stephentoub removed the untriaged New issue has not been triaged by the area owner label Feb 25, 2020
@stephentoub stephentoub modified the milestones: 5.0, Future Feb 25, 2020
@stephentoub
Copy link
Member

This is possible now with ConfigureAwait as of #87067, e.g.

task.ConfigureAwait(ConfigureAwaitOptions.SuppressThrowing).GetAwaiter().GetResult();

While it's not the prettiest line of code ever, it gets the job done. I don't see us adding an additional mechanism, so I'm going to close this out. Thanks.

@ghost ghost locked as resolved and limited conversation to collaborators Jul 4, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Threading.Tasks
Projects
None yet
Development

No branches or pull requests

9 participants