To Task.Run or not to Task.Run

Question

Suppose I have an interface which includes an async method, and I have two different implementations of that interface. One of the two implementations is naturally async, and the other is not. What would be the "most correct" way of implementing the non-async method?

public interface ISomething {
    Task<Foo> DoSomethingAsync();
}

// Normal async implementation
public class Implementation1 : ISomething {
    async Task<Foo> ISomething.DoSomethingAsync() {
        return await DoSomethingElseAsync();
    }
}

// Non-async implementation
public class Implementation2 : ISomething {
    // Should it be:
    async Task<Foo> ISomething.DoSomethingAsync() {
        return await Task.Run(() => DoSomethingElse());
    }
    // Or:
    async Task<Foo> ISomething.DoSomethingAsync() {
        return DoSomethingElse();
    }
}

I try to keep up with Stephen Cleary's blog, and I know neither one of these actually provides any async benefits, and I'm ok with that. The second one seems more correct to me, since it doesn't pretend to be something it's not, but it does give a compiler warning, and those add up and get distracting.

This would all be inside ASP.NET (both web MVC and WebAPI), if that makes a difference.

Answer 1

You can forgo the async modifier altogether and use Task.FromResult to return a completed task synchronously:

Task<Foo> ISomething.DoSomethingAsync()
{
    return Task.FromResult(DoSomethingElse());
}

This takes care of the warning and has better performance as it doesn't need the state machine overhead of an async method.

However, this does change the semantics of exception handling a bit. If that's an issue then you should use the synchronous async method approach and accept the warning (or turn it off with a comment):

#pragma warning disable 1998
    async Task<Foo> ISomething.DoSomethingAsync() 
#pragma warning restore 1998
    {
        return DoSomethingElse();
    }

As Stephen Cleary suggested you can also take care of that warning (while keeping the method synchronous) by awaiting an already completed task:

async Task<Foo> ISomething.DoSomethingAsync() 
{
    await Task.FromResult(false); // or Task.CompletedTask in .Net 4.6
    return DoSomethingElse();
}

Answer 2

It really depends on what your method is doing:

• no I/O, neglible amount of cpu work
• cpu intensive work
• I/O intensive work

no I/O, neglible amount of cpu work

You should compute the result synchronously and create a Task holding the result.

Task<Foo> ISomething.DoSomethingAsync() {
    Foo result;
    // do something not hurting performance
    // no I/O here
    return Task.FromResult(result);
}

Note however that any exception will be thrown when the method is called, not when the task is awaited. For the latter case, which is compliant to the other types of work, you should use async nonetheless:

async Task<Foo> ISomething.DoSomethingAsync() {
    Foo result;
    // do something not hurting performance
    // no I/O here
    return result;
}

cpu intensive work

You should start a Task with Task.Run and do the cpu intensive work in the task.

Task<Foo> ISomething.DoSomethingAsync() {
    return Task.Run(() => {
        Foo result;
        // do some CPU intensive work here
        return result;
    });
}

I/O intensive work

You should use the async keyword and await any async I/O method. Do not use synchronous I/O methods.

async Task<Foo> ISomething.DoSomethingAsync() {
    Stream s = new ....

    // or any other async I/O operation
    var data = await s.ReadToEndAsync(); 

    return new Foo(data);
}