When is "too much" async and await? Should all methods return Task?

Question

I am building a project and using async and await methods. Everyone says that async application are built from the ground up, so should you really have any sync methods? Should all methods you return a Task so you can use the asynchronously?

Lets take a simple example, where by i am using Sql to load data into a collection, here is some code.

This code loads data from a table using ExecuteQueryAsync method, the method GetQuery constructs the SQL, but calling GetTableColumns. Once the SQL is generated and executed, i loop through the collection and populate each object by calling GetDataFromReader.

Should my non async methods, be async? Am i thinking in too much of a sync-way of programming and missing something?

public async Task<ICollection<MyObject>> ExecuteQueryAsync(Module module, List<SqlParameter> parameters)
{
    var result = new Collection<MyObject>();
    var query = GetQuery(module);

    using (var conn = new SqlConnection(_context.Database.Connection.ConnectionString))
    {
        await conn.OpenAsync();

        using (var cmd = new SqlCommand(query, conn))
        {
            if (parameters != null)
                cmd.Parameters.AddRange(parameters.ToArray());

            using (var dr = await cmd.ExecuteReaderAsync())
            {
                while (await dr.ReadAsync())
                {
                    result.Add(GetDataFromReader(module, dr));
                }

            }
        }

    }

    return result;
}

public string GetQuery(Module module)
{
    return "SELECT " + string.Join(",", GetTableColumns(module).ToArray()) + " FROM [TableA] ";
}

public List<string> GetTableColumns(Module module) 
{
    var columnNames = new List<string>();

    // get all list fields for the module
    var fields = (from a in module.Groups.SelectMany(a => a.Fields) select a).ToList();

    foreach (var field in fields)
    {
        if (field.Type == FieldType.List) {
            string query = "STUFF(";
            query += "(SELECT ';' + [Value] FROM [TableB] FOR XML PATH(''))";
            query += ", 1, 1, '') AS [" + field.ColumnName + "]";

            columnNames.Add(query);
        } else {
            columnNames.Add("[" + field.ColumnName + "]");
        }
    }

    return columnNames;
}

public MyObject GetDataFromReader(Module module, IDataReader dataReader)
{
    var entity = new MyObject();

    for (var i = 0; i < dataReader.FieldCount; i++)
    {
        object value = null;
        var fieldName = dataReader.GetName(i);

        if (!dataReader.IsDBNull(i))
        {
            value = dataReader.GetValue(i);
        }

        entity[fieldName] = value;
    }

    return entity;
}

Answer 1

If a method has no async operations inside it there's no benefit in making it async. You should only have async methods where you have an async operation (I/O, DB, etc.).

If your application has a lot of these I/O methods and they spread throughout your code base, that's not a bad thing. But don't just add the async keywords on synchronous methods.

In your specific case ExecuteQueryAsync benefits by being async as it allows using await cmd.ExecuteReaderAsync(). GetTableColumns and GetDataFromReader seem to be CPU intensive methods and they don't fit they async-await paradigm.

Answer 2

The philosophy behind "all async" is to facilitate non-blocking I/O.

That is, your primarily async code can potentially let the environment prioritize how your application or service is being executed and achieve as much parallelized execution as possible in a multi-threaded, multi-process system.

For example, ASP.NET Web API, ASP.NET MVC or even ASP.NET Web Forms (code-behind) can take advantage of all async to continue attending Web requests to other users while some async operation is being executed. Thus, even when a Web server like IIS or Katana might limit the number of concurrent requests, async operations are executed in a separate thread from the request thread, and this is allows the Web server to respond to other requests while async operations get a result and they need to continue:

// While WhateverAsync is being executed, current thread can be used 
// by a new request and so on.
// Obviously, this will work this way if WhateverAsync actually
// does its work in another thread...
await WhateverAsync();

So... do you need to implement everything asynchronously? Even when you return a Task you don't need to provide an asynchronous implementation:

public Task WhateverAsync()
{
    // This creates a fake Task object which 
    // simulates a Task that has already ended successfully
    // and without creating a child thread!
    // This, this method is a SYNCHRONOUS implementation unless
    // the whole method doesn't execute asynchronous operations.
    return Task.FromResult(true);
}

My point of view here is...

• ...implement everything returning a Task and using the Async suffix at the end of method's identifiers (WhateverAsync, WhoKnowsAsync, DoStuffAsync...)...

• ...unless you can be sure that the whole method will always execute a very simple things which can't block application/service's thread for a long time (long time can be few miliseconds, now imagine a code which doesn't block main app thread for 100ms whenever some method is called and your code can prioritize executing something while it awaits 100ms....). I would include here string manipulation, simple arithmetic operations, configuration methods...

If your code isn't async today, you can turn it into actual async operations without affecting the entire code base, as you only need to change Task.FromResult<T>(T result) calls to actually return an unfinished Task instance.

At the end of the day, your methods have an async signature and dependencies on them don't care if they're actually asynchronous, and these method implementations decide what's asynchronous or synchronous instead of giving this responsibility to the caller..