3

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://learn.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.

ooXei1sh
  • 3,459
  • 2
  • 27
  • 47

2 Answers2

2

Your code, as it is, 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.

Pac0
  • 21,465
  • 8
  • 65
  • 74
  • The use of `await` means the statement will be executed **asynchronously** and the the execution flow will return to the caller. Whether the thread is blocked or not depends on the caller. – Paulo Morgado Jan 27 '20 at 22:58
  • @PauloMorgado I'm never sure of the proper words to use, but for the sake of being clear for other readers : `async Task MainFunction() { Console.WriteLine("before"); await Foo(); Console.WriteLine("after") } ` and `async Task Foo() { // long processing during several seconds ... and then : Console.WriteLine("processed!"): }` Will reliably result in the output : "before", "processed!", "after". The execution flow looks pretty "synchronous" to me, that what `await` does (make the asynchronous call looks synchronous for the execution flow). – Pac0 Jan 28 '20 at 08:06
  • Aren't you confusing sequential with synchronous? – Paulo Morgado Jan 28 '20 at 09:21
  • @PauloMorgado Most probably ! Thanks for the word, I'll edit my answer – Pac0 Jan 28 '20 at 10:08
1

Apart from the coding consistency -which is quite important- consider the following:

  1. Is this a DAL library? Are you sure who is going to consume it and are you sure that code reusability will never be a concern?

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.

  1. Even with await, the call is asynchronous, in terms of rubbings running on another thread, while the original is waiting. Most times this is negligible as performance but why not?

I usually make everything async and if need be for a sync implementation, I prefer to make code duplicates with a sync version.

Athanasios Kataras
  • 25,191
  • 4
  • 32
  • 61