Always using Async in an ASP.NET MVC Controller

Question

I recently inherited an ASP.NET MVC project. In that project, the developer is using async everywhere. I'm trying to assess if its a good idea or not. Specifically, I'm reviewing the controller code at the moment.

In the controllers, the developer wrote the stuff like the following:

public async Task<ActionResult> Index()
{
  return View();
}

Is there any advantage to this instead of the traditional version:

public ActionResult Index()
{
  return View();
}

I could understand using async if await was used in the controller code. A lot of times, its not being used though. Is there any justification for this approach?

Answer 1

No. That's not a good idea to use Async everywhere.

Regarding the missing await the compiler issues a warning when an Async method does not contain an Await operator. An Async method without an await will run synchronously, which makes the semantics incorrect.

For your specific example, the operation is simple and short running, and that's why using Async/Await does not bring any benefits, and it shouldn't be used, so you are perfectly fine using:

public ActionResult Index()
{
    return View();
}

Async/Await should be used for controller methods that perform some kind of IO (e.g. Network requests, Database access etc.). If a controller method is doing CPU bound work (e.g. math. calculations), then using Async/Await can actually make performance worse, unless your measurements prove me wrong.

The old rule applies: Measure twice, cut once.

Answer 2

There is no sense to make action/method if there is no await anywhere in this method. Making method async means that "under the box" (by compiler) would be created state machine which makes a little overhead versus pure void version (really small overhead but if you have milion requests per second that can make a difference ;)). If you are scared that there could be await introduced soon (you better have good reason for "being scared") and you have a lot of code dependend on this method (for instance it's abstract class method/ it belongs to interface) you should keep method returning Task<ActionResult> but without marking it asynchronous. Example:

public Task<ActionResult> MyAction() {

    return Task.FromResult<ActionResult>(View());
}

This Task.FromResult<T> method returns completed task - which prevents compiler creating asynchronous state machine.

Back to your main question - seems like people who wrote this code didn't know what they're doing - marking method async doesn't mean that method works "asynchronously".