I have a method in my view model
private async void SyncData(SyncMessage syncMessage)
{
if (syncMessage.State == SyncState.SyncContacts)
{
this.SyncContacts();
}
}
private async Task SyncContacts()
{
foreach(var contact in this.AllContacts)
{
// do synchronous data analysis
}
// ...
// AddContacts is an async method
CloudInstance.AddContacts(contactsToUpload);
}
When I call SyncData
from the UI commands and I'm syncing a large chunk of data UI freezes. But when I call SyncContacts
with this approach
private void SyncData(SyncMessage syncMessage)
{
if (syncMessage.State == SyncState.SyncContacts)
{
Task.Run(() => this.SyncContacts());
}
}
Everything is fine. Should not they be the same?
I was thinking that not using await for calling an async method creates a new thread.
Should not they be the same? I was thinking that not using await for
calling an async method creates a new thread.
No, async
does not magically allocate a new thread for it's method invocation. async-await
is mainly about taking advantage of naturally asynchronous APIs, such as a network call to a database or a remote web-service.
When you use Task.Run
, you explicitly use a thread-pool thread to execute your delegate. If you mark a method with the async
keyword, but don't await
anything internally, it will execute synchronously.
I'm not sure what your SyncContacts()
method actually does (since you haven't provided it's implementation), but marking it async
by itself will gain you nothing.
Edit:
Now that you've added the implementation, i see two things:
- I'm not sure how CPU intensive is your synchronous data analysis, but it may be enough for the UI to get unresponsive.
You're not awaiting your asynchronous operation. It needs to look like this:
private async Task SyncDataAsync(SyncMessage syncMessage)
{
if (syncMessage.State == SyncState.SyncContacts)
{
await this.SyncContactsAsync();
}
}
private Task SyncContactsAsync()
{
foreach(var contact in this.AllContacts)
{
// do synchronous data analysis
}
// ...
// AddContacts is an async method
return CloudInstance.AddContactsAsync(contactsToUpload);
}
What your line Task.Run(() => this.SyncContacts());
really does is creating a new task starting it and returning it to the caller (which is not used for any further purposes in your case). That's the reason why it will do its work in the background and the UI will keep working. If you need to (a)wait for the task to complete, you could use await Task.Run(() => this.SyncContacts());
. If you just want to ensure that SyncContacts has finished when you return your SyncData method, you could using the returning task and awaiting it at the end of your SyncData method. As it has been suggested in the comments: If you're not interested in whether the task has finished or not you just can return it.
However, Microsoft recommend to don't mix blocking code and async code and that async methods end with Async (https://msdn.microsoft.com/en-us/magazine/jj991977.aspx). Therefore, you should consider renaming your methods and don't mark methods with async, when you don't use the await keyword.
Just to clarify why the UI freezes - the work done in the tight foreach
loop is likely CPU-bound and will block the original caller's thread until the loop completes.
So, irrespective of whether the Task
returned from SyncContacts
is await
ed or not, the CPU bound work prior to calling AddContactsAsync
will still occur synchronously on, and block, the caller's thread.
private Task SyncContacts()
{
foreach(var contact in this.AllContacts)
{
// ** CPU intensive work here.
}
// Will return immediately with a Task which will complete asynchronously
return CloudInstance.AddContactsAsync(contactsToUpload);
}
(Re : No why async / return await
on SyncContacts
- see Yuval's point - making the method async and awaiting the result would have been wasteful in this instance)
For a WPF project, it should be OK to use Task.Run
to do the CPU bound work off the calling thread (but not so for MVC or WebAPI Asp.Net projects).
Also, assuming the contactsToUpload
mapping work is thread-safe, and that your app has full usage of the user's resources, you could also consider parallelizing the mapping to reduce overall execution time:
var contactsToUpload = this.AllContacts
.AsParallel()
.Select(contact => MapToUploadContact(contact));
// or simpler, .Select(MapToUploadContact);