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.
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.