Using Task.WhenAll for multiple async and fake async methods

Viewed 1615

A colleague of mine has refactored our controller methods so that all of our IO operations, including the synchronous ones, are encapsulated in separate tasks and then all those tasks are executed in parallel via Task.WhenAll. I can definitely understand the idea: we use more threads but all of our IO operations (and we can have quite a few) are executed with the speed of the slowest one, but I'm still unsure whether it is a correct path to go. Is it a valid approach or I'm missing something? Is the cost of using more threads going to be noticeable in a typical ASP.Net website application? Here is some sample code

public async Task<ActionResult> Foo() {
    var dataATask = _dataARepository.GetDataAsync();
    var dataBTask = Task.Run(_dataBRepository.GetData());
    await Task.WhenAll(dataATask, dataBTask);
    var viewModel = new ViewModel(dataATask.Result, dataBTask.Result);
    return View(viewModel);
}
4 Answers

In general you code is ok - it will consume more threads and a bit more CPU than original, but unless your site takes heavy load it would unlikely significantly impact overall performance. Obviously you need to measure it yourself for your particular load (including some sort of stress level load of 5-10x regular traffic).

Wrapping synchronous method in Task.Run is not best practice (see Should I expose asynchronous wrappers for synchronous methods?). It may work for you as long as trading extra threads for such behavior is acceptable for your case.

If you only have one synchronous operation left you can instead keep it synchronous and await the rest at the end of synchronous step saving that extra thread:

var dataATask = _dataARepository.GetDataAsync();
var dataBTaskResult = _dataBRepository.GetData();
await Task.WhenAll(dataATask); // or just await dataATask if you have only one.
var viewModel = new ViewModel(dataATask.Result, dataBTaskResult);

Consider these two approaches.

Original:

public async Task<ActionResult> Foo() 
{
    var dataATask = _dataARepository.GetDataAsync();
    var dataBTask = Task.Run(_dataBRepository.GetData());
    await Task.WhenAll(dataATask, dataBTask);
    var viewModel = new ViewModel(dataATask.Result, dataBTask.Result);
    return View(viewModel);
}

^This version will create a new thread to call _dataBRepository.GetData(). The additional thread will block until the call is complete. While waiting for the additional thread to complete, the main thread will yield control back to the ASP.NET pipeline, where it may handle some other user's request.

Different:

public async Task<ActionResult> Foo() 
{
    var dataATask = _dataARepository.GetDataAsync();
    var dataBResult = _dataBRepository.GetData();
    await dataATask;
    var viewModel = new ViewModel(dataATask.Result, dataBResult);
    return View(viewModel);
}

^This version does not spin up a separate thread for dataBRepository.GetData(). But it does block the main thread.

So your choice is:

  1. Spin up another thread, just so you can yield the main thread to some other task.
  2. Hang on to the main thread. If some other task needs a thread, it'll have to spin up its own.

In both cases you are using one thread at a time (for the most part). In both cases the transaction will complete in the time required by the slower of the two back-end transactions. But the original option spins up a new thread and yields the current one. This seems like extra work that is not needed.

On the other hand, if the action requires two or more synchronous back-end transactions, the original option would complete faster, because they could run in parallel. Assuming that they support it. If the back-end transactions are database transactions and use the same database, they are likely to block each other, in which case you still won't see any benefit.

Normally it is not useful to start a different thread to do some synchronous stuff, if all you do is wait until this thread is finished.

Compare:

async Task<int> ProcessAsync()
{
    var task = Task.Run(() => DoSomeHeavyCalculations());
    int calculationResult = await task;
    return calculationResult;
}

with

int ProcessAsync()
{
    return DoSomeHeavyCalculations();
}

Apart from that the async function uses more brainpower, it would limit re-usage of the function: only async callers can use it. Let your caller decide whether he wants to call you asynchronously or not. If he also has nothing to do but wait, he can let his caller decide, etc.

By the way, this is exactly what GetData does: it doesn't force callers like you to be asynchronous, it gives you the freedom to call it synchronous or use Task.Run to call it asynchronous.

However, if you function has something else to do during the heavy calculations, then it might be useful:

async Task<int> ProcessAsync()
{
    var task = Task.Run(() => DoSomeHeavyCalculations());

    // while the calculations are being performed, do something else:
    DoSomethingElse();

    return await task;
}

In your example: if there is only one "heavy calculation", do this yourself. If there are several heavy calculations that you can consider using Task.Run. But don't just order other threads to do something without doing anything yourself.

Related