async and await: are they bad?

Question

We recently developed a site based on SOA but this site ended up having terrible load and performance issues when it went under load. I posted a question related this issue here:

ASP.NET website becomes unresponsive under load

The site is made of an API (WEB API) site which is hosted on a 4-node cluster and a web site which is hosted on another 4-node cluster and makes calls to the API. Both are developed using ASP.NET MVC 5 and all actions/methods are based on async-await method.

After running the site under some monitoring tools such as NewRelic, investigating several dump files and profiling the worker process, it turned out that under a very light load (e.g. 16 concurrent users) we ended up having around 900 threads which utilized 100% of CPU and filled up the IIS thread queue!

Even though we managed to deploy the site to the production environment by introducing heaps of caching and performance amendments many developers in our team believe that we have to remove all async methods and covert both API and the web site to normal Web API and Action methods which simply return an Action result.

I personally am not happy with approach because my gut feeling is that we have not used the async methods properly otherwise it means that Microsoft has introduced a feature that basically is rather destructive and unusable!

Do you know any reference that clears it out that where and how async methods should/can be used? How we should use them to avoid such dramas? e.g. Based on what I read on MSDN I believe the API layer should be async but the web site could be a normal no-async ASP.NET MVC site.

Update:

Here is the async method that makes all the communications with the API.

public static async Task<T> GetApiResponse<T>(object parameters, string action, CancellationToken ctk)
{
        using (var httpClient = new HttpClient())
        {
            httpClient.BaseAddress = new Uri(BaseApiAddress);

            var formatter = new JsonMediaTypeFormatter();

            return
                await
                    httpClient.PostAsJsonAsync(action, parameters, ctk)
                        .ContinueWith(x => x.Result.Content.ReadAsAsync<T>(new[] { formatter }).Result, ctk);
        }
    }

Is there anything silly with this method? Note that when we converted all method to non-async methods we got a heaps better performance.

Here is a sample usage (I've cut the other bits of the code which was related to validation, logging etc. This code is the body of a MVC action method).

In our service wrapper:

public async static Task<IList<DownloadType>> GetSupportedContentTypes()
{
  string userAgent = Request.UserAgent;
  var parameters = new { Util.AppKey, Util.StoreId, QueryParameters = new { UserAgent = userAgent } };
  var taskResponse = await  Util.GetApiResponse<ApiResponse<SearchResponse<ProductItem>>>(
                    parameters,
                    "api/Content/ContentTypeSummary",
                    default(CancellationToken));
                    return task.Data.Groups.Select(x => x.DownloadType()).ToList();
 }

And in the Action:

public async Task<ActionResult> DownloadTypes()
    {
        IList<DownloadType> supportedTypes = await ContentService.GetSupportedContentTypes();

Answer 1

Is there anything silly with this method? Note that when we converted all method to non-async methods we got a heaps better performance.

I can see at least two things going wrong here:

public static async Task<T> GetApiResponse<T>(object parameters, string action, CancellationToken ctk)
{
        using (var httpClient = new HttpClient())
        {
            httpClient.BaseAddress = new Uri(BaseApiAddress);

            var formatter = new JsonMediaTypeFormatter();

            return
                await
                    httpClient.PostAsJsonAsync(action, parameters, ctk)
                        .ContinueWith(x => x.Result.Content
                            .ReadAsAsync<T>(new[] { formatter }).Result, ctk);
        }
    }

Firstly, the lambda you're passing to ContinueWith is blocking:

x => x.Result.Content.ReadAsAsync<T>(new[] { formatter }).Result

This is equivalent to:

x => { 
    var task = x.Result.Content.ReadAsAsync<T>(new[] { formatter });
    task.Wait();
    return task.Result;
};

Thus, you're blocking a pool thread on which the lambda is happened to be executed. This effectively kills the advantage of the naturally asynchronous ReadAsAsync API and reduces the scalability of your web app. Watch out for other places like this in your code.

Secondly, an ASP.NET request is handled by a server thread with a special synchronization context installed on it, AspNetSynchronizationContext. When you use await for continuation, the continuation callback will be posted to the same synchronization context, the compiler-generated code will take care of this. OTOH, when you use ContinueWith, this doesn't happen automatically.

Thus, you need to explicitly provide the correct task scheduler, remove the blocking .Result (this will return a task) and Unwrap the nested task:

return
    await
        httpClient.PostAsJsonAsync(action, parameters, ctk).ContinueWith(
            x => x.Result.Content.ReadAsAsync<T>(new[] { formatter }), 
            ctk,
            TaskContinuationOptions.None, 
            TaskScheduler.FromCurrentSynchronizationContext()).Unwrap();

That said, you really don't need such added complexity of ContinueWith here:

var x = await httpClient.PostAsJsonAsync(action, parameters, ctk);
return await x.Content.ReadAsAsync<T>(new[] { formatter });

The following article by Stephen Toub is highly relevant:

"Async Performance: Understanding the Costs of Async and Await".

If I have to call an async method in a sync context, where using await is not possible, what is the best way of doing it?

You almost never should need to mix await and ContinueWith, you should stick with await. Basically, if you use async, it's got to be async "all the way".

For the server-side ASP.NET MVC / Web API execution environment, it simply means the controller method should be async and return a Task or Task<>, check this. ASP.NET keeps track of pending tasks for a given HTTP request. The request is not getting completed until all tasks have been completed.

If you really need to call an async method from a synchronous method in ASP.NET, you can use AsyncManager like this to register a pending task. For classic ASP.NET, you can use PageAsyncTask.

At worst case, you'd call task.Wait() and block, because otherwise your task might continue outside the boundaries of that particular HTTP request.

For client side UI apps, some different scenarios are possible for calling an async method from synchronous method. For example, you can use ContinueWith(action, TaskScheduler.FromCurrentSynchronizationContext()) and fire an completion event from action (like this).

Answer 2

async and await should not create a large number of threads, particularly not with just 16 users. In fact, it should help you make better use of threads. The purpose of async and await in MVC is to actually give up the thread pool thread when it's busy processing IO bound tasks. This suggests to me that you are doing something silly somewhere, such as spawning threads and then waiting indefinitely.

Still, 900 threads is not really a lot, and if they're using 100% cpu, then they're not waiting.. they're chewing on something. It's this something that you should be looking into. You said you have used tools like NewRelic, well what did they point to as the source of this CPU usage? What methods?

If I were you, I would first prove that merely using async and await are not the cause of your problems. Simply create a simple site that mimics the behavior and then run the same tests on it.

Second, take a copy of your app, and start stripping stuff out and then running tests against it. See if you can track down where the problem is exactly.