Mr. Boy
Mr. Boy

Reputation: 63720

Is it really more efficient to WhenAll() on a number of tiny awaitable methods?

Given an external API method signature like the following:

Task<int> ISomething.GetValueAsync(int x)

I often see code such as the following:

public async Task Go(ISomething i)
{
    int a = await i.GetValueAsync(1);
    int b = await i.GetValueAsync(2);
    int c = await i.GetValueAsync(3);
    int d = await i.GetValueAsync(4);
    
    Console.WriteLine($"{a} - {b} - {c} - {d}");
}

In code reviews it is sometimes suggested this is inefficient and should be rewritten:

public async Task Go(ISomething i)
{
    Task<int> ta = i.GetValueAsync(1);
    Task<int> tb = i.GetValueAsync(2);
    Task<int> tc = i.GetValueAsync(3);
    Task<int> td = i.GetValueAsync(4);
    
    await Task.WhenAll(ta,tb,tc,td);
    int a = ta.Result, b= tb.Result, c=tc.Result, d = td.Result;
    Console.WriteLine($"{a} - {b} - {c} - {d}");
}

I can see the logic behind allowing parallelisation but in reality is this worthwhile? It presumably adds some overhead to the scheduler and if the methods themselves are very lightweight, it seems likely that thread parallelisation would be more costly than the time saved. Further, on a busy server running many applications, it seems unlikely there would be lots of cores sitting idle.

I can't tell if this is always a good pattern to follow, or if it's an optimisation to make on a case-by-case basis? Do Microsoft (or anyone else) give good best practice advice? Should we always write Task - based code in this way as a matter of course?

Upvotes: 0

Views: 420

Answers (2)

Jeremy Lakeman
Jeremy Lakeman

Reputation: 11100

First, what does await actually do?

public async Task M() {
    await Task.Yield();
}

If the awaitable object has already completed, then execution continues immediately. Otherwise a callback delegate is added to the awaitable object. This callback will be invoked immediately when the task result is made available.

So what about Task.WhenAll, how does that work? The current implementation adds a callback to every incomplete task. That callback will decrement a counter atomically. When the counter reaches zero, the result will be made available.

No new I/O is scheduled, no continuations added to the thread pool. Just a small counter added to the end of each tasks processing. Your continuation will resume on whichever thread executed the last task.

If you are measuring a performance problem. I wouldn't worry about the overheads of Task.WhenAll.

Upvotes: 2

Stephen Cleary
Stephen Cleary

Reputation: 456407

if the methods themselves are very lightweight, it seems likely that thread parallelisation would be more costly than the time saved.

This is definitely an issue with parallel code (the parallel form of concurrency), but this is asynchronous concurrency. Presumably, GetValueAsync is a true asynchronous method, which generally implies I/O operations. And I/O operations tend to dwarf many local considerations. (Side note: the WhenAll approach actually causes fewer thread switches and less scheduling overhead, but it does increase memory overhead slightly).

So, this is a win in the general case. However, that does assume that the statements above are correct (i.e., GetValueAsync performs I/O).

As a contrary point, however, I have seen this sometimes used as a band-aid for an inadequate data access layer. If you're hitting a SQL database with four queries, then the best solution is usually to combine that data access into a single query rather than do four calls to the same SQL database concurrently.

Upvotes: 2

Related Questions