Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
gafter opened this issue Apr 14, 2015 · 17 comments
Open
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature Request Tenet-Performance Regression in measured performance of the product from goals.
Milestone

Comments

@gafter
Copy link
Member

gafter commented Apr 14, 2015

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.

@jaredpar @VSadov

@VSadov
Copy link
Member

VSadov commented Apr 14, 2015

This is basically the tail-call optimization for async methods.

@pharring

@svick
Copy link
Contributor

svick commented Apr 14, 2015

Won't this change the semantics when e itself throws an exception (instead of returning a faulted Task)? Specifically, return e bubbles the exception up the call stack, while return await e puts it into the returned Task.

Though I guess something like this would work:

try
{
    return e;
}
catch (Exception ex)
{
    return Task.FromException(ex);
}

@jaredpar
Copy link
Member

@svick yes it would change the call stack for exceptions of this nature.

@gafter
Copy link
Member Author

gafter commented Apr 15, 2015

@svick Given the proposed constraints, the callers will just bubble the exception up, so the end result is the same other than the fact that the call stack is different.

@svick
Copy link
Contributor

svick commented Apr 15, 2015

@gafter I don't understand. Consider the following code:

Task<int> E()
{
    throw new Exception();
}

async Task<int> ReturnAwaitE()
{
    return await E();
}

Task<int> ReturnE()
{
    return E();
}var t = ReturnAwaitE();
await t.ContinueWith(_ => {}); // await t, but don't throw if it's faulted
Console.WriteLine(t.Exception);

This code writes out the exception, but if I replace ReturnAwaitE(), with ReturnE(), it throws instead; so the end result is very different here.

Doesn't ReturnAwaitE fulfill your constraints (the method ends with return await, there is no other await, there is no try)? And isn't ReturnE how the optimized version would look like?

@jaredpar
Copy link
Member

@svick this feature would not change exception semantics in that way. To do so would turn an optimization into a breaking change. The implementation of this would have to take exceptions into account when generating the code.

Even in this case though the compiler can generate much more efficient code than the state machine that is generated today:

Task<int> ReturnE()
{
  try {
    return E();
  } catch  (Exception e) {
    return Task<int>.FromException(e);
  }
}

@pharring
Copy link
Contributor

👍 on the idea.
Note that the transform is valid even in the presence of .ConfigureAwait(false)
I had thought of doing this via an analyzer.
I think, in many cases, the extra try/catch is unnecessary. If an exception is thrown from an "await"-able method (instead of returning a faulted Task), then doesn't this indicate an exception in the task handling machinery? That's the sort of exception I'd be happy to let go up to the caller.

@svick
Copy link
Contributor

svick commented Apr 15, 2015

@jaredpar Yeah, that breaking change was what I was afraid of. It wasn't clear to me from the initial description that this was what the optimization would do.

@jaredpar
Copy link
Member

@pharring if there is another await in the function then the existing state machine code will handle it. If the return await call though is the only await then the compiler needs to generate the extra try / catch in many cases. Otherwise it changes the observable behavior of the method.

@pharring
Copy link
Contributor

BTW, Task.FromException is an internal method.

@stephentoub
Copy link
Member

@pharring, it was in 4.5, it isn't anymore.

@stephentoub
Copy link
Member

ps Note, though, that the try/catch will also need to specialize catching an OperationCanceledException and returning a canceled task for that case, as that's what the async machinery does today.

@stephentoub
Copy link
Member

pps And for it to actually remain semantically equivalent, it'd have to appropriately handle ExecutionContext. In particular, inside of an async method, changes made to ExecutionContext (e.g. putting something into the logical call context) will not be visible to the caller of the async method upon the return of the synchronous invocation.

@gafter gafter added Enhancement Tenet-Performance Regression in measured performance of the product from goals. labels Apr 16, 2015
@gafter gafter added this to the 1.1 milestone Apr 16, 2015
@pharring
Copy link
Contributor

pharring commented May 7, 2015

This was proposed a couple of times already

@khellang
Copy link
Member

khellang commented May 7, 2015

👍 on this optimization!

@jaredpar jaredpar modified the milestones: 1.1, 2.0 Jul 31, 2015
@GSPP
Copy link

GSPP commented Aug 22, 2015

Is the C# team willing to take a dependency on the internals of TaskAwaiter here? Previously, that class was opaque and there wasn't any compiler knowledge about it built-in. From the languages point of view that class could do anything at all when awaited.

@jaredpar jaredpar removed this from the 2.0 milestone Nov 23, 2015
@dpoeschl dpoeschl added this to the 2.0 milestone Dec 3, 2015
@yaakov-h
Copy link
Member

Won't this remove the method in question from an exception stack trace?

Is there a way that this optimisation could be achieved whilst still preserving the stack trace?

@gafter gafter removed this from the 2.0 (Preview 1) milestone Jun 20, 2016
@gafter gafter modified the milestones: 2.0 (RC), 2.0 (Preview 1) Jun 20, 2016
@gafter gafter modified the milestones: 3.0, 2.0 (RC) Jul 19, 2016
@jaredpar jaredpar modified the milestones: 16.0, Unknown Sep 7, 2018
@gafter gafter added Code Gen Quality Room for improvement in the quality of the compiler's generated code and removed Concept-Code Quality Improvement labels Sep 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Code Gen Quality Room for improvement in the quality of the compiler's generated code Feature Request Tenet-Performance Regression in measured performance of the product from goals.
Projects
None yet
Development

No branches or pull requests

10 participants