361
votes

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
}
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
there is actually a little difference if you have second await inside. See this article. 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
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

2 Answers

415
votes

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());
17
votes

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.