Using async/await or task in web api controller (.net core)

原文：https://stackoverflow.com/questions/41953102/using-async-await-or-task-in-web-api-controller-net-core

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?

1

Does service not offer asynchronous alternatives for the getdata methods? Also, generally you should prefer Task.Run() over creating tasks and then manually starting them. 

– Damien_The_Unbeliever

 Jan 31 '17 at 8:47

• 2
  async/await does not help with parallelism in that sense. In fact, it would serialize your 3 service requests. It would help, however, when you do Task.WaitAll, since you are parking a thread with that line. 

  – Gabriel Garcia

   Jan 31 '17 at 8:49
• 1
  There's no reason at all to create cold tasks then call Start on them. They aren't threads. Just use Task.Run. Then use await Task.WhenAll 

  – Panagiotis Kanavos

   Jan 31 '17 at 9:06

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.

Is it not redundant to do await Task.WhenAll() and await for each task in the returnObject? Not sure why it wouldn't just be sufficient to do one or the other. Can you please explain? :) 

– susieloo_

 Jul 11 '17 at 23:45

• 9
  If the return types are different, you have to await each task. I prefer Task.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. 

  – Stephen Cleary

   Jul 12 '17 at 0:46
•  
  @StephenCleary How to use zero threads if my API need to call System.Diagnostics.Process with WaitForExit? I am planning to implement stackoverflow.com/a/10789196/241004 as workaround. Do you have better idea? 

  – Jawad Al Shaikh

   Oct 4 '17 at 8:25
• 1
  @JawadAlShaikh: No, that's about the best you can do. Process just doesn't have a very async-friendly API. :/ 

  – Stephen Cleary

   Oct 4 '17 at 10:41
• 1
  @StephenPatten: Yes. 

  – Stephen Cleary

   Jan 24 '18 at 15:29

[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);
}

1. 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.
2. Instead of creating local variables and assigning them in tasks, you can use Task<T> to return results of required type.
3. Instead of creating and running tasks, use Task<TResult>.Run method
4. I recommend to use convention for action names - if action accepts GET request, it's name should starts with Get
5. Next, you should check whether tasks completed successfully. It is done by checking task status. In my sample I used null values for return object properties if some of tasks do not complete successfully. You can use another approach - e.g. return error if some of tasks failed.

I think you need to explain the check for RanToCompletion. Are you anticipating early cancellation ? Had any of the calls faulted, await Task.WhenAll would raise an exception 

– Panagiotis Kanavos

 Jan 31 '17 at 9:08 

•  
  Are your sure you can just rename the get method in WebApi? In MVC you can't without altering behavior. 

  – Sefe

   Jan 31 '17 at 9:43
•  
  @Sefe yep, you are right. Didn't notice it mvc core, will revert some web-api related changes in a moment 

  – Sergey Berezovskiy

   Jan 31 '17 at 9:44

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);
}

The first snippet contains several problems - initializing variables with throwaway objects, then capturing those variables. 

– Panagiotis Kanavos

 Jan 31 '17 at 9:10

•  
  I agree that this is not optimal, that's why I provided a "better" solution with my 2nd code snippet. Anyways, I don't think that the 1st snipped has real problems. The compiler will make the throwaway local variables to private fields which should work just fine. (Again, I know it's not ideal.) 

  – Thomas D.

   Jan 31 '17 at 11:34
•  
  At least you should use Task for the method type because of async usage 

  – Can PERK

   Feb 19 '19 at 18:26
•  
  @CanPERK: you are right, I have added the suggestion. 

  – Thomas D.

   Feb 22 '19 at 16:43