I came across some lines of code today that, boiled down, amounts to something like this:
using (var db = new AppDbContext())
{
var foo = await db.Foos
.Where(m => m.Name == "BuzzBaz" )
.FirstAsync();
foo.Name = "Quxx";
db.SaveChanges();
}
Notice the select query is async
but the call to SaveChanges() is not. It's synchronous!
Could this potentially create any problems? Should I just refactor this to use db.SaveChangesAsync();
and all is well? Or maybe just call First()
instead of FirstAsync()
? Or perhaps it's harmless and might have some benefit to overall system performance?
I found some documentation that makes me think just removing async
altogether is probably a good bet here. https://docs.microsoft.com/en-us/ef/ef6/fundamentals/async
In most applications using async will have no noticeable benefits and even could be detrimental. Use tests, profiling and common sense to measure the impact of async in your particular scenario before committing to it.
Your code as it is "correct", meaning it does not have a fundamental flaw.
Let's analyze what happens, and my 2 cents on them :
When doing your query to the db (the FirstAsync
), the use of await
means the instructions will be executed sequentially, but the thread won't be blocked (asynchronous call) and can be used for other things. This is probably a good thing, if this is a server application you may want your threads be able to process other requests while waiting for the DB response.
The use of non-async SaveChanges
will however block the thread. This in itself is not an error, but since it is also an I/O operation on db, it might block the thread for some significant time. It would indeed seem more "consistent" if you also await
with an async version here as well.
Is that an issue ? It depends. Is your application heavily used ? Are your users experiencing some low reactivity on heavy load ? Then in this case it might be a potential improvement to use async on save.
Otherwise, and as suggested on the linked MSDN article, until you know it has an impact, my advice would be to not worry too much.
The mix of usages is not per se a problem.
Personnally, I would go for an async SaveChanges as well, for consistency and imagining that a DB write is some I/O operation that could be slow.
If it's a desktop application, just make sure that you are not blocking the UI thread.
Apart from the coding consistency -which is quite important- consider the following:
If the answer is that it could be, stick to async usage. Front end UI will be blocked during the save, making it non responsive. Web APIs make use of IO threads which enables better thread management: Simple description of worker and I/O threads in .NET
If it a library make sure to use the ConfigureAwait(false)
for all async function calls.
I usually make everything async and if need be for a sync implementation, I prefer to make code duplicates with a sync version.