Microsoft.AspNetCore.Identity UserManager GetUserA

Question

I've been reading the source code of UserManager.cs and I've stepped into the following block:

        public virtual Task<TUser> GetUserAsync(ClaimsPrincipal principal)
    {
        if (principal == null)
        {
            throw new ArgumentNullException(nameof(principal));
        }
        var id = GetUserId(principal);
        return id == null ? Task.FromResult<TUser>(null) : FindByIdAsync(id);
    }

I've been very curious if there is a reason why the above is not like this:

        public virtual async Task<TUser> GetUserAsync(ClaimsPrincipal principal)
    {
        if (principal == null)
        {
            throw new ArgumentNullException(nameof(principal));
        }
        var id = GetUserId(principal);
        return id == null ? await Task.FromResult<TUser>(null) : await FindByIdAsync(id);
    }

As you can see I've only added async/await.

*I'm implementing the UserManager class with another ORM.

Answer 1

Since I didn't write it I cannot know for certain. ;) But I guess it's regarding the return await-anti pattern.

There's even an issue for it on GitHub, where the request is that Roslyn handles it when compiling.

Optimize an async method that ends "return await e" to be non-async "return e"

this optimization would improve the performance of an async method whose only await appears at the end. Not when producing debug code and not when nested in a try block.

To quote the excellent comment from damien_the_unbeliever, which he posted on this question

await is a way of pausing the current method until some other piece of code has done its job. If all that your method is going to do when it's resumed is to say "I'm done" then why did you go to all of the effort? That being said, the actual anti-pattern is return await if that's the only await in the method (as it is here) and it's not within a try block with a finally or a using block (in which case there is additional code to run after return

When using await/async one should be aware of the overhead it creates.

Take these two classes for example:

public class BaseClass
{
    public async Task<int> GetIdAsync()
    {
        return await Task.Run(() => 100);
    }
}

public class Foo : BaseClass
{
    public Task<int> GetId()
    {
        return GetIdAsync();
    }
}


public class Bar : BaseClass
{
    public async Task<int> GetId()
    {
        return await GetIdAsync();
    }
}

If you decompile the code from Bar with ILSpy will look something like this:

public class Bar : BaseClass
{
    [DebuggerStepThrough, AsyncStateMachine(typeof(Bar.<GetId>d__0))]
    public Task<int> GetId()
    {
        Bar.<GetId>d__0 <GetId>d__ = new Bar.<GetId>d__0();
        <GetId>d__.<>4__this = this;
        <GetId>d__.<>t__builder = AsyncTaskMethodBuilder<int>.Create();
        <GetId>d__.<>1__state = -1;
        AsyncTaskMethodBuilder<int> <>t__builder = <GetId>d__.<>t__builder;
        <>t__builder.Start<Bar.<GetId>d__0>(ref <GetId>d__);
        return <GetId>d__.<>t__builder.Task;
    }
}

While Foo looks like this:

public class Foo : BaseClass
{
    public Task<int> GetId()
    {
        return base.GetIdAsync();
    }
}

So, to sum up, if you're simply going to return the value from one Task, there's really no reason to await it. Instead you should return the Task and let the caller await that Task instead. Awaiting the Task extends the stack and creates an overhead which only affects performance negatively.