I have a .net core API which has a controller that builds an aggregated object to return. the object it creates is made of data that comes from 3 method calls to a service class. These are all independent of each other and can be run in isolation from each other. Currently I am using tasks to improve the performance of this controller. the current version looks something like this...
[HttpGet]
public IActionResult myControllerAction()
{
var data1 = new sometype1();
var data2 = new sometype2();
var data3 = new List<sometype3>();
var t1 = new Task(() => { data1 = service.getdata1(); });
t1.Start();
var t2 = new Task(() => { data2 = service.getdata2(); });
t2.Start();
var t3 = new Task(() => { data3 = service.getdata2(); });
t3.Start();
Task.WaitAll(t1, t2, t3);
var data = new returnObject
{
d1 = data1,
d2 = data2,
d2 = data3
};
return Ok(data);
}
This works well however I am wondering if using tasks is the best solution here? Would using async/await be a better idea and more accepted way?
For example should the controller be marked as async and an await put on each call to the the service methods?
service
not offer asynchronous alternatives for the getdata
methods? Also, generally you should prefer Task.Run()
over creating tasks and then manually starting them.
Task.WaitAll
, since you are parking a thread with that line.
Start
on them. They aren't threads. Just use Task.Run
. Then use await Task.WhenAll
This works well however I am wondering if using tasks is the best solution here? Would using async/await be a better idea and more accepted way?
Yes, absolutely. Doing parallel processing on ASP.NET consumes multiple threads per request, which can severely impact your scalability. Asynchronous processing is far superior for I/O.
To use async
, first start with your lowest-level call, somewhere inside your service. It's probably doing an HTTP call at some point; change that to use asynchronous HTTP calls (e.g., HttpClient
). Then let async
grow naturally from there.
Eventually, you'll end up with asynchronous getdata1Async
, getdata2Async
, and getdata3Async
methods, which can be consumed concurrently as such:
[HttpGet]
public async Task<IActionResult> myControllerAction()
{
var t1 = service.getdata1Async();
var t2 = service.getdata2Async();
var t3 = service.getdata3Async();
await Task.WhenAll(t1, t2, t3);
var data = new returnObject
{
d1 = await t1,
d2 = await t2,
d3 = await t3
};
return Ok(data);
}
With this approach, while the three service calls are in progress, myControllerAction
uses zero threads instead of four.
[HttpGet]
public async Task<IActionResult> GetAsync()
{
var t1 = Task.Run(() => service.getdata1());
var t2 = Task.Run(() => service.getdata2());
var t3 = Task.Run(() => service.getdata3());
await Task.WhenAll(t1, t2, t3);
var data = new returnObject
{
d1 = t1.Status == TaskStatus.RanToCompletion ? t1.Result : null,
d2 = t2.Status == TaskStatus.RanToCompletion ? t2.Result : null,
d3 = t3.Status == TaskStatus.RanToCompletion ? t3.Result : null
};
return Ok(data);
}
Your action thread is currently blocked when you are waiting for tasks. Use TaskWhenAll to return awaitable Task object. Thus with async method you can await for tasks instead of blocking thread. Instead of creating local variables and assigning them in tasks, you can use Task
RanToCompletion
. Are you anticipating early cancellation ? Had any of the calls faulted, await Task.WhenAll
would raise an exception
As i understand, you want this to execute in parallel, so I don't think there is anything wrong with your code. As Gabriel mentioned, you could await the the finishing of the tasks.
[HttpGet]
public async Task<IActionResult> myControllerAction()
{
var data1 = new sometype1();
var data2 = new sometype2();
var data3 = new List<sometype3>();
var t1 = Task.Run(() => { data1 = service.getdata1(); });
var t2 = Task.Run(() => { data2 = service.getdata2(); });
var t3 = Task.Run(() => { data3 = service.getdata3(); });
await Task.WhenAll(t1, t2, t3); // otherwise a thread will be blocked here
var data = new returnObject
{
d1 = data1,
d2 = data2,
d2 = data3
};
return Ok(data);
}
You could also use the results of the tasks to save some lines of codes and make the code overall "better" (see comments):
[HttpGet]
public async Task<IActionResult> myControllerAction()
{
var t1 = Task.Run(() => service.getdata1() );
var t2 = Task.Run(() => service.getdata2() );
var t3 = Task.Run(() => service.getdata3() );
await Task.WhenAll(t1, t2, t3); // otherwise a thread will be blocked here
var data = new returnObject
{
d1 = t1.Result,
d2 = t2.Result,
d2 = t3.Result
};
return Ok(data);
}
Success story sharing
await
each task. I preferTask.WhenAll
even in this case because it makes the code clearer. It's also a bit more efficient (only resuming on the context once instead of 3 times), but my main reason is code clarity.Process
just doesn't have a very async-friendly API. :/