0

In this example,

private async void Button1_Click(object sender, EventArgs e)
{
    if (condition) await Task.Run(Foo);
}

private void Foo()
{
    Thread.Sleep(5000);
}

sometimes condition is false, and the async method awaits nothing. But consider this example,

private async void Button1_Click(object sender, EventArgs e)
{
    await Task.Run(Foo);
}

private void Foo()
{
    if (condition) Thread.Sleep(5000);
}

where the method is always awaited, even if condition is false. I'm wondering what happens behind the scenes, if one is more preferable to the other, I mean objectively if there are compiler optimizations which make one preferable over the other. Assume condition can be checked from any thread, and has a very minimal impact to performance.

It seems that when the condition check is deferred, there is always a task being awaited, but when it's false in the handler, I have a situation which is close to one where the async method lacks an await operator, about which the compiler warns me. So it feels both wrong and right at the same time. Hoping someone can shed some objective light on this question.

djv
  • 15,168
  • 7
  • 48
  • 72
  • This is going to depend on what your condition actually is and how the relevant variables are set, so your question can't be answered as-is. Have you tried setting a breakpoint inside `Foo()` and checking the value of your condition? – Gabriel Luci Mar 04 '22 at 18:38
  • @GabrielLuci This is just a simplified example. In my actual case I am checking a `List.Any()` so the impact of the condition check is minimal – djv Mar 04 '22 at 18:39
  • Try setting a breakpoint and checking your condition then. If `Thread.Sleep` is running, then the condition is most certainly true. – Gabriel Luci Mar 04 '22 at 18:41
  • Side note: There's no value to using `FooAsync()` in the middle. You can just do `await Task.Run(Foo);` directly in the event handler. – Gabriel Luci Mar 04 '22 at 18:43
  • @GabrielLuci I appreciate the help, but in reality my code is doing other things than `Thread.Sleep` such as going to a database, and I have boiled it down to the essence to demonstrate the basic design. True about the `FooAsync()` and if I had really boiled it down I would only have done it the way you just suggested. You have a point that it may distract from my actual question. I will edit. – djv Mar 04 '22 at 18:46
  • I still think you need to set a breakpoint in `Foo()` and step through the code. If it's getting past your condition, it's because the condition is true. – Gabriel Luci Mar 04 '22 at 18:48
  • @GabrielLuci it may or may not be. The button is meant to update a database based on whether the user has modified a client side collection. The async method does the updating based on the condition. I don't know if I should check the condition on the UI, thus the async click handler runs synchronously, or whether I should defer the check so the handler always awaits Foo – djv Mar 04 '22 at 18:50
  • Your question does not appear to have anything to do with async or await. If the condition is false, the method is not called in the first place, so there is nothing to await. That is like asking what's the optimization difference between `if(condition) { x++; }` and `x++;`. Whether or not you should await what *has* been called is a [different question](https://stackoverflow.com/q/46053175/11683) altogether. – GSerg Mar 04 '22 at 18:51
  • @GSerg not exactly, t if the condition is false, then the code runs basically as an async method with no await, which I'm warned about in Visual Studio at design time. It warns me not to code it that way for the compiler, but what's the implication if it runs that way – djv Mar 04 '22 at 18:54

3 Answers3

3

Should async method always call await, if not what is the implication?

As I describe on my blog, async methods always begin executing synchronously, just like any other methods. await is where things may get asynchronous.

If a code path is taken where there is no await (or if the tasks awaited are already completed), then the method completes synchronously and returns an already-completed task to its caller. This is not a problem in practice, because "*Async" means "may be asynchronous", not "must be asynchronous".

In this specific example, you're using async void, but if this was an async Task method that was called a lot, in that case I'd recommend considering returning ValueTask instead, which would save some memory allocation for the Task whenever it completes synchronously.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • Does deferring the condition check to the task so it is always awaited have any benefit or detriment? I realize I am getting down into the weeds here but I wonder if there is a correct place to check, even if the performance is only marginally better one way or another. – djv Mar 04 '22 at 18:59
  • I feel like, yeah I should keep it in the click handler so sometimes no task is created, but if it's there and the condition is false, it's analogous to the situation I described in which I'm warned by the compiler. – djv Mar 04 '22 at 19:02
  • 1
    I would say place it in the location that makes the code more maintainable. If the maintainability isn't in question, then place it outside the `Task.Run`, since that way you won't be scheduling the check on a thread pool thread. – Stephen Cleary Mar 04 '22 at 19:03
  • Thanks, maintainability isn't a question, so I will place it in the button handler. – djv Mar 04 '22 at 19:12
2

The async keyword on the method is mearly an indicator to the compiler that the code inside the method should be converted into a state-machine with state transitions at the awaits.

The only technical down-side, that I am aware of, to having an async method that doesn't await anything, is the small amount of overhead introduced by the state-machine. It should have no negative effects, other than a very small performance impact, and a tiny bit of memory-pressure, that can most likely be ignored in your scenario.

Bradley Uffner
  • 16,641
  • 3
  • 39
  • 76
1

In your particular case Thread.Sleep(5000) is not the best way of pausing the execution. Thread.Sleep(5000) is an old API and it blocks the execution thread for a given time (5000ms). What does that mean? It means the thread won't be available for other tasks. Let's say, given your computer CPU output, you have 4 threads in total and you lock 1 of them for 5000ms - it is really bad especially if it is a web application that has to handle concurrent API requests. What to use instead? await Task.Delay(5000) it will "hang" the execution for a given time, yet the thread is going to come back to the thread pool and will be available for other tasks.

Now closer to your question. Wrapping non-asynchronous code into Task and awaiting won't do anything. Async/await was designed for I/O operations specifically for not blocking threads that are waiting for I/O to complete. Wrapping code into Task and not awaiting - is basically "fire and forget" which means your execution thread may continue to execute the next block without waiting for your Task method to complete. Why I say may is because if your code wrapper in Task is fast enough it would work synchronously

OlegI
  • 5,472
  • 4
  • 23
  • 31
  • If I had put `// some database code` instead, which is actually what it's doing, the question I'm asking is still the same: where should the condition be checked based on the implication that a task may or may not always be awaited. Thanks for answering. – djv Mar 04 '22 at 19:16
  • @djv it doesn't matter where you put the condition. Think of async-await as the propagation of Tasks in order to allow you to `await` the db call and the bottom level. Because if you use sync API instead of async when you call DB, it will block the thread. So async Task method is just a signature which allows you to call await inside the method, that's it – OlegI Mar 04 '22 at 19:19
  • I understand all that. But the compiler warns when an async method lacks an await operator. I just wondered if it's best practice to have the method always await or if it's ok to have it sometimes not which feels like the situation I'm being warned against. – djv Mar 04 '22 at 19:24
  • @djv it warns you because you don't need to use "async" keyword if you aren't awaiting anything in the method. If you use Task as return type you need to return Task or either await something (async Task). If you don't do either, then there is no need to use Task or Task<> as a return type or, if you want for some reason use it, just return Task.CompletedTask – OlegI Mar 04 '22 at 19:29
  • so by using "async" you enable "await". If you don't need to "await", don't use "async" – OlegI Mar 04 '22 at 19:31
  • Yes I do need await when the condition is true, because the code then goes out to sql and I don't want to block the UI. I got my answer to perform the condition check up front and be ok with the async method not awaiting anything when the condition is false. – djv Mar 04 '22 at 19:33