426

I would like to ask you on your opinion about the correct architecture when to use Task.Run. I am experiencing laggy UI in our WPF .NET 4.5 application (with Caliburn Micro framework).

Basically I am doing (very simplified code snippets):

public class PageViewModel : IHandle<SomeMessage>
{
   ...

   public async void Handle(SomeMessage message)
   {
      ShowLoadingAnimation();

      // Makes UI very laggy, but still not dead
      await this.contentLoader.LoadContentAsync();

      HideLoadingAnimation();
   }
}

public class ContentLoader
{
    public async Task LoadContentAsync()
    {
        await DoCpuBoundWorkAsync();
        await DoIoBoundWorkAsync();
        await DoCpuBoundWorkAsync();

        // I am not really sure what all I can consider as CPU bound as slowing down the UI
        await DoSomeOtherWorkAsync();
    }
}

From the articles/videos I read/saw, I know that await async is not necessarily running on a background thread and to start work in the background you need to wrap it with await Task.Run(async () => ... ). Using async await does not block the UI, but still it is running on the UI thread, so it is making it laggy.

Where is the best place to put Task.Run?

Should I just

  1. Wrap the outer call because this is less threading work for .NET

  2. , or should I wrap only CPU-bound methods internally running with Task.Run as this makes it reusable for other places? I am not sure here if starting work on background threads deep in core is a good idea.

Ad (1), the first solution would be like this:

public async void Handle(SomeMessage message)
{
    ShowLoadingAnimation();
    await Task.Run(async () => await this.contentLoader.LoadContentAsync());
    HideLoadingAnimation();
}

// Other methods do not use Task.Run as everything regardless
// if I/O or CPU bound would now run in the background.

Ad (2), the second solution would be like this:

public async Task DoCpuBoundWorkAsync()
{
    await Task.Run(() => {
        // Do lot of work here
    });
}

public async Task DoSomeOtherWorkAsync(
{
    // I am not sure how to handle this methods -
    // probably need to test one by one, if it is slowing down UI
}
Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Lukas K
  • 6,037
  • 4
  • 23
  • 31
  • 2
    BTW, the line in (1) `await Task.Run(async () => await this.contentLoader.LoadContentAsync());` should simply be `await Task.Run( () => this.contentLoader.LoadContentAsync() );`. AFAIK you don't gain anything by adding a second `await` and `async` inside `Task.Run`. And since you aren't passing parameters, that simplifies slightly more to `await Task.Run( this.contentLoader.LoadContentAsync );`. – ToolmakerSteve Jun 27 '18 at 06:25
  • there is actually a little difference if you have second await inside. See this [article](https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md). I found it very useful, just with this particular point I do not agree and prefer returning directly Task instead of awaiting. (as you suggest in your comment) – Lukas K Jan 11 '19 at 21:16
  • If you have just a sequence of synchronous methods, you can use the pattern `await Task.Run(() => { RunAnySynchronousMethod(); return Task.CompletedTask; });` inside your async method (e.g. async controller method, test method etc). – Matt Dec 02 '20 at 09:46
  • Related: [await Task.Run vs await](https://stackoverflow.com/questions/38739403/await-task-run-vs-await/) – Theodor Zoulias Sep 02 '21 at 12:36

2 Answers2

479

Note the guidelines for performing work on a UI thread, collected on my blog:

  • Don't block the UI thread for more than 50ms at a time.
  • You can schedule ~100 continuations on the UI thread per second; 1000 is too much.

There are two techniques you should use:

1) Use ConfigureAwait(false) when you can.

E.g., await MyAsync().ConfigureAwait(false); instead of await MyAsync();.

ConfigureAwait(false) tells the await that you do not need to resume on the current context (in this case, "on the current context" means "on the UI thread"). However, for the rest of that async method (after the ConfigureAwait), you cannot do anything that assumes you're in the current context (e.g., update UI elements).

For more information, see my MSDN article Best Practices in Asynchronous Programming.

2) Use Task.Run to call CPU-bound methods.

You should use Task.Run, but not within any code you want to be reusable (i.e., library code). So you use Task.Run to call the method, not as part of the implementation of the method.

So purely CPU-bound work would look like this:

// Documentation: This method is CPU-bound.
void DoWork();

Which you would call using Task.Run:

await Task.Run(() => DoWork());

Methods that are a mixture of CPU-bound and I/O-bound should have an Async signature with documentation pointing out their CPU-bound nature:

// Documentation: This method is CPU-bound.
Task DoWorkAsync();

Which you would also call using Task.Run (since it is partially CPU-bound):

await Task.Run(() => DoWorkAsync());
Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 5
    Thanks for your fast resp! I know the link you posted and saw the videos that are referenced in your blog. Actually that is why I posted this question - in the video is said (same as in your response) you should not use Task.Run in core code. But my problem is, that I need to wrap such method every time I use it to not slow down the responsivnes (please note all my code is async and is not blocking, but without Thread.Run it is just laggy). I am as well confused whether it is better approach to just wrap the CPU bound methods (many Task.Run calls) or completly wrap everything in one Task.Run? – Lukas K Aug 02 '13 at 14:16
  • 17
    All your library methods should be using `ConfigureAwait(false)`. If you do that first, then you may find that `Task.Run` is completely unnecessary. If you *do* still need `Task.Run`, then it doesn't make much difference to the runtime in this case whether you call it once or many times, so just do what's most natural for your code. – Stephen Cleary Aug 02 '13 at 14:21
  • 4
    I don't understand how the first technique will help him. Even if you use `ConfigureAwait(false)` on your cpu-bound method, it's still the UI thread that's going to do the cpu-bound method, and only everything after might be done on a TP thread. Or did I misunderstand something? – Drake Oct 05 '15 at 19:46
  • 2
    @Darius: I recommend both techniques used together. This isn't two answers in one; it's one answer with two parts. – Stephen Cleary Oct 05 '15 at 19:57
  • @StephenCleary If a method is CPU-bound, it should be called using `Task.Run`. When it's mixed (also I/O bound), it should be async and called using `Task.Run`. Could you explain why? I can't really see the need for Async signature. – user4205580 Nov 14 '15 at 10:00
  • `DoWorkAsync` apparently uses await in its body, so when we call it using `Task.Run`, the `Task.Run` can finish before `DoWorkAsync` does all its work. So the lines after `await Task.Run(() => DoWorkAsync())` will be executed, even though `DoWorkAsync` hasn't finished yet. – user4205580 Nov 14 '15 at 10:11
  • I've just tested it: to make sure that no other lines are executed before `DoWorkAsync` finishes its work completely, one should call `await Task.Run(async () => await DoWorkAsync());` – user4205580 Nov 14 '15 at 10:37
  • 7
    @user4205580: No, `Task.Run` understands asynchronous signatures, so it won't complete until `DoWorkAsync` is complete. The extra `async`/`await` is unnecessary. I explain more of the "why" in my [blog series on `Task.Run` etiquette](http://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-even-in.html). – Stephen Cleary Nov 14 '15 at 16:17
  • 1
    @StephenCleary Ok, makes sense why it didn't work with `await Task.Run(() => {DoWorkAsync();})`, but it works if I use `Task.Run(() => DoWorkAsync())` – user4205580 Nov 14 '15 at 16:34
  • 2
    @user4205580: Yes, the extra brackets would make that lambda return `void` instead of `Task`/`Task`. So `() => { return DoWorkAsync(); }` should also work. – Stephen Cleary Nov 14 '15 at 17:41
  • 2
    Thanks. I've read a few of your articles, also the one that attempts to explain why using `Task.Run` is bad at the implementation level. I'd say sometimes the `Task.Run` is necessary (of course the point here is to wrap as few lines as possible in that call, not the whole body of my async method). If my async method doesn't use existing async methods, then there's no other way than just use `await Task.Run` in its code to make it truly async, right? Standard C# async methods use it I guess. – user4205580 Nov 14 '15 at 20:13
  • 3
    @user4205580: No. The vast majority of "core" async methods do not use it internally. The *normal* way to implement a "core" async method is to use `TaskCompletionSource` or one of its shorthand notations such as `FromAsync`. I have a [blog post that goes into more detail why async methods don't require threads](http://blog.stephencleary.com/2013/11/there-is-no-thread.html). – Stephen Cleary Nov 14 '15 at 21:08
  • You just need to call await DoWorkAsync(); in your example. No need to add another Task.Run(() => ... – Michael Puckett II Feb 16 '17 at 13:47
  • @MichaelPuckettII: The final example where I call `DoWorkAsync` inside `Task.Run` is only if `DoWorkAsync` contains CPU-bound code as well as I/O-bound code. The `Task.Run` would be required to prevent blocking the UI thread on the CPU-bound portion(s). For more information, see my [series on `Task.Run` etiquette](http://blog.stephencleary.com/2013/10/taskrun-etiquette-and-proper-usage.html). – Stephen Cleary Feb 16 '17 at 14:17
  • As I look into this I keep wondering if Task was the type you meant to put on DoWorkAsync() or if there was meant to be more understanding behind it because I can't seem to get that flow to be correct. – Michael Puckett II Feb 16 '17 at 18:40
  • Oh well. I can't get this to paste code for some reason. – Michael Puckett II Feb 16 '17 at 18:52
  • @MichaelPuckettII See my example [here](http://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-even-in.html). – Stephen Cleary Feb 16 '17 at 19:04
  • @StephenCleary why is it bad practice to use `Task.Run()` on I/O bound processes? – Shaul Behr Oct 10 '19 at 09:45
  • @ShaulBehr: It's always ideal to use asynchronous code for I/O. In a client GUI app, using `Task.Run` to block a thread on I/O is an acceptable workaround, causing a minor but acceptable performance hit in exchange for a more responsive UI. Using `Task.Run` on the *server* side is where the "bad practice" comes in: it's almost always a bad idea, causing a decrease in performance for no benefit at all. – Stephen Cleary Oct 10 '19 at 12:26
  • Not sure, but shouldn't the last code snippet be `await Task.Run(async () => await DoWorkAsync());`? Is it a typo, or am I missing something? – Eugene Podskal Dec 11 '19 at 14:42
  • 2
    @EugenePodskal: It can be written either way. [Eliding `async` and `await`](https://blog.stephencleary.com/2016/12/eliding-async-await.html) is common when it's just a simple passthrough like this. – Stephen Cleary Dec 11 '19 at 17:49
  • Sorry for bothering, forgot about https://blog.stephencleary.com/2016/12/eliding-async-await.html. A few recent async-await bugs in our code probably just made me unreasonably suspicious to absent `await`'s. – Eugene Podskal Dec 12 '19 at 09:49
  • @EugenePodskal: I believe there are some analyzers that can catch things like missing `await`s. – Stephen Cleary Dec 12 '19 at 14:05
  • 1
    [async: "There is no thread!"](https://blog.stephencleary.com/2013/11/there-is-no-thread.html) - awsome article. I like it! – Matt Dec 02 '20 at 13:49
  • @StephenCleary Some minor detail I am missing from that is whether a method running a CPU bound `Task` async should itself await or just return that `Task`. I usually go for: `return Task.Run(() => doCpuWork(), cancellationToken);` for any `${SyncMethod}Async()` implementation. Should I prefer `return await Task.Run(() => doCpuWork(), cancellationToken);` plus the `async` key word? I figured the caller is expected to `await` if appropriate anyway. – Beltway Oct 06 '21 at 09:24
  • 1
    @Beltway: See [eliding `async` and `await`](https://blog.stephencleary.com/2016/12/eliding-async-await.html). – Stephen Cleary Oct 06 '21 at 12:25
  • Stephen do you still believe that the advice *"Use `ConfigureAwait(false)` when you can."* is a good advice, in the context of a question that asks about the use of `Task.Run` in a GUI application? If you do, could you please make a trivial edit in the answer because I want to reassess my vote? :-) – Theodor Zoulias Feb 03 '22 at 05:40
  • @TheodorZoulias: My answer was twofold because both approaches are valid. `Task.Run` is good for offloading blocking/synchronous work, and `ConfigureAwait(false)` (applied to code *outside* the `Task.Run`, of course) prevents unnecessary continuations hitting the UI thread - what Stephen Toub refers to as the thousand paper cuts. I did run into this in my first async desktop app: sluggish behavior just due to too many continuations hitting the UI thread. Most apps that's not a problem, though. – Stephen Cleary Feb 03 '22 at 06:52
  • 1
    Stephen my objection is that this advice, although it has merits, is not directly related with the question asked. Here is a question where the advice would be relevant: [Why would I bother to use Task.ConfigureAwait(false)?](https://stackoverflow.com/questions/27851073/) The advice is positioned prominently inside the question, long before any mention to the `Task.Run`, which is what the OP is interested for. So I don't think that it's a good answer honestly, and my vote (from May 17 '20) doesn't reflect well my current opinion. – Theodor Zoulias Feb 03 '22 at 07:15
  • The intention of my answer is both/and, not either/or. Both are recommended; it's not two different answers. – Stephen Cleary Feb 03 '22 at 15:39
  • In the last snippet: `await Task.Run(() => DoWorkAsync());` - what's the difference between that and `await DoWorkAsync();`? – Jeppe Apr 15 '22 at 08:43
  • `Task.Run` begins executing the asynchronous method on a thread pool thread. – Stephen Cleary Apr 15 '22 at 13:20
22

One issue with your ContentLoader is that internally it operates sequentially. A better pattern is to parallelize the work and then sychronize at the end, so we get

public class PageViewModel : IHandle<SomeMessage>
{
   ...

   public async void Handle(SomeMessage message)
   {
      ShowLoadingAnimation();

      // makes UI very laggy, but still not dead
      await this.contentLoader.LoadContentAsync(); 

      HideLoadingAnimation();   
   }
}

public class ContentLoader 
{
    public async Task LoadContentAsync()
    {
        var tasks = new List<Task>();
        tasks.Add(DoCpuBoundWorkAsync());
        tasks.Add(DoIoBoundWorkAsync());
        tasks.Add(DoCpuBoundWorkAsync());
        tasks.Add(DoSomeOtherWorkAsync());

        await Task.WhenAll(tasks).ConfigureAwait(false);
    }
}

Obviously, this doesn't work if any of the tasks require data from other earlier tasks, but should give you better overall throughput for most scenarios.

Paul Hatcher
  • 7,342
  • 1
  • 54
  • 51
  • 1
    Isn't .ConfigureAwait(false) incorrect here? You actually do want to resume on the UI thread after the LoadContentAsync method finishes. Or did I totally misunderstand how this works? – Achilles P. Sep 24 '21 at 22:01