0

Let's say I have this sample code

var t = Float();
...
t.Wait();


async Task Float()
{
    while (LoopIsAllowed)
    {
        TryToComplete();
        System.Threading.Thread.Sleep(500);
    }
}

In this case code run synchronously and I have warning "async method lacks 'await'". I really need to run Float() async so this warning should almost always be considered. As I see it the problem here is that if I call Float() and await it latter the application will not work as intended and it will stuck on var t = Float() instead of t.Wait()

But I do not see any way to fix that other than like this:

async Task Float()
{
    while (LoopIsAllowed)
    {
        TryToComplete();
        await Task.Run(() => System.Threading.Thread.Sleep(500));
    }
}

Does this fix have any sense? In terms of memory and processor resources is it fine to call Task.Run or is there better way? As I see it after the fix calling var t = Float() will force the code to run synchronously until await Task.Run is reached. Then parent code will continue execution until t.Wait(). Only then it will continue to iterate through while (LoopIsAllowed) loop. And this behavior is exactly what I need.

Is this correct?

EDIT: What should I do if I do not have any delay in my code at all? And there is no other place to await. Should I just add delay? Or should I analyze code and add Task.Run somewhere around time consuming calculations blocks?

Alexei Levenkov
  • 98,904
  • 14
  • 127
  • 179
binariti
  • 93
  • 1
  • 1
  • 8
  • 5
    `await Task.Delay(500);`? – GSerg Sep 15 '21 at 19:49
  • 1
    You should use [`Task.Delay`](https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.task.delay?view=net-5.0) and `await Task.Delay(500);` as multiple tasks can (and probably are) running on the same thread – MindSwipe Sep 15 '21 at 19:49
  • 1
    When you use `Task.Run`, you are pulling a thread-pool thread out of the poll and scheduling your work onto that thread. `Task.Run(() => System.Threading.Thread.Sleep(500));` says, take a thread pool thread, put it completely asleep for a half second, let me know when it wakes up and then put the thread right back into the pool. As everyone has pointed out, use `Task.Delay`. In general, you should be `async` all the way down and `async` all the way up as well. Your `t = Float(); t.Wait();` code should really be in an `async` method as well: `await Float();` – Flydog57 Sep 15 '21 at 20:10
  • I've updated title to what I think you are asking (making CPU-bound code async) and resolved as duplicate of existing question on the topic. If you asking something else or duplicated does not address your question - please [edit] the question to clarify that and then the question possible will be re-opened for more answers. – Alexei Levenkov Sep 15 '21 at 20:27
  • Marked as duplicate: [Why use C# async/await for CPU-bound tasks](https://stackoverflow.com/questions/48928678/why-use-c-sharp-async-await-for-cpu-bound-tasks) – Theodor Zoulias Sep 15 '21 at 22:55

2 Answers2

1

Never use Thread.Sleep in async method.

use await Task.Delay(); instead of Thread.Sleep

async Task Float()
{
    while (LoopIsAllowed)
    {
        TryToComplete();
        await Task.Delay(500);
    }
}

and in main method use GetAwaiter().GetResult() instead of wait

var t = Float().GetAwaiter().GetResult();
Farhad Zamani
  • 5,381
  • 2
  • 16
  • 41
  • 1
    Preferably await the call to `Float` instead of using either, but yeah, `GetAwaiter().GetResult()` is preferred if you can't refactor your calling code to be async – MindSwipe Sep 15 '21 at 19:51
  • What should I do if I do not have delay in my code at all? And there is no other place to await. Should I add delay? Or should I analyze code and add Task.Run somewhere where there are some long calculations? – binariti Sep 15 '21 at 19:53
  • @binariti if you do not have delay in your code, you don't need to use `Task` and `async`. another options is `await Task.CompletedTask;` – Farhad Zamani Sep 15 '21 at 19:56
  • @binariti Then you should have `void Float()`, not have any funny business it it, and should have `var t = Task.Run(() => Float())` in the calling code. – GSerg Sep 15 '21 at 19:56
  • Thanks for the answer. What about my description of code flow in the question. Is it correct? – binariti Sep 15 '21 at 20:02
  • @binariti No, nothing will stop the parent from resuming as soon as the `Float` task is created. Which is what you should want in the first place. If you want to create a task in a suspended state and let it start actual work only after a certain event, well, that can be done too, but it's much easier to just create the task after it's known it's safe to start. – GSerg Sep 15 '21 at 20:10
0

If your intention is to introduce parallelism, the best place to offload synchronous work to the ThreadPool by using the Task.Run method is the upper level possible. This means that hiding Task.Runs deep inside method chains should generally be avoided. The reasoning is explained in this article: Should I expose asynchronous wrappers for synchronous methods? In your case the (not aptly named) Float method does synchronous work, so it should be a synchronous method with a void return type. You could then offload it to the ThreadPool like this:

Task t = Task.Run(() => Float());
//...
t.Wait();

The Task.Run is a clever little method that handles synchronous and asynchronous delegates (delegates with Task return type) equally well. So if you decide later to improve the efficiency (regarding utilization of threads) of the Float method by converting it to a mixed sync-async method, you can do it without changing anything in the calling site (beyond the name of the method that should now have the Async suffix):

async Task FloatAsync()
{
    while (LoopIsAllowed) // Potential code smell here. Is the LoopIsAllowed volatile?
    {
        TryToComplete();
        await Task.Delay(500);
    }
}
Task t = Task.Run(() => FloatAsync());
//...
t.Wait();
Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104