1

I've got a question about an overuse of async-await operators.
I have a list of Zoo instances and I need to insert each member of each of Zoo into a database separately. Here is the Zoo class definition:

class Zoo
{
public Dog Dog { get; set; }
public Cat Cat { get; set; }
public Bear Bear { get; set; }
}

The method to insert Zoo is provided below. Each instance of Zoo and its members should be validated or transformed before insertion, so I deal with them with Parallel.For:

public void Save(List<Zoo> zoos)
{
 Parallel.For(0, zoos.Count, new ParallelOptions { MaxDegreeOfParallelism = 4 }, async i =>
  {
  ...
  Task saveDog = Task.Run(async () => await InsertDogAsync(zoos[i], string connectionString));
  Task saveCat = Task.Run(async () => await InsertCatAsync(zoos[i], string connectionString));
  Task saveBear = Task.Run(async () => await InsertBearAsync(zoos[i], string connectionString));
  await Task.WhenAll(new Task[] { saveDog, saveCat, saveBear });
  ...
  });
}

I insert animals to the database using async methods. For example here it is for Dog:

private static async Task InsertDogAsync(Zoo zoo, string connectionString)
{
 using SqlConnection connection = new SqlConnection(connectionString);
 await connection.OpenAsync();
 ...
 using SqlCommand insertCommand = new SqlCommand("sp_insertDog", connection); // stored procedure
 ...
 await insertCommand.ExecuteNonQueryAsync();
}

My question is following: is there an overuse of async-await operators? As I realized, each await operator releases thread before it will be completed, but methods are called in a parallel task, so threads are used in a different tasks. Maybe it is more acceptable to remove some async-await, for example, from Task.Run lambda?

Misha
  • 59
  • 1
  • 5
  • 3
    If the tasks are IO bound which I assume they are then `Task.Run` is not really needed. Also I doubt you really need to do `Parallel.For` either. And the real issue isn't overuse of async/await, but under use as you should use it from the top to the bottom, so that `Save` method should be async and return a `Task` and whatever calls it should be async all the way to the top. Also `InsertDogAsync` should actually be async and return a `Task` as well. In fact that code will not compile as you cannot have `await` inside of a method that isn't `async`. – juharr Jun 30 '21 at 19:18
  • 3
    Might be that my C# is getting a bit far off me, but... why do you feel you have to wrap tasks within `Task.Run` calls? Why not going `Task saveDog = InsertDogAsync(zoos[i], connectionString);`? – Crono Jun 30 '21 at 19:20
  • @juharr I am sorry, I missed ```async Task``` at ```InsertDogAsync``` of course it returns ```Task```. – Misha Jun 30 '21 at 19:37
  • @Crono I don't know... I called them that way out of my habit. – Misha Jun 30 '21 at 19:41
  • @Misha I see :) It will likely feel a tad cleaner if you take these away IMHO. – Crono Jun 30 '21 at 19:46
  • A relevant set of points: https://stackoverflow.com/questions/44458083/task-parallel-library-mixed-with-async-await – Caius Jard Jun 30 '21 at 19:47
  • 1
    Few things: It looks like you're trying to do premature optimization. Don't worry we've all been there. 99% of the time you don't need to do anything in parallel like that. It will not make it faster, especially with database things. 2. You should try and use EntityFramework Core. It's a pain to set up at first, but you'll really like it. 3. I haven't really noticed too much a performance hit from using async-await too much, but I (almost) never do a Task.Run unless I have to. EF Core will take care of all that for you anyways – Chad K Jun 30 '21 at 20:35
  • 2
    I would **strongly suggest** batch inserts, which would preclude the necessity for parallelization in the first place, and be much much faster – Charlieface Jun 30 '21 at 20:35

1 Answers1

3

This is not a case of await overuse, it's a case of await misuse. The Parallel.ForEach method is not async-friendly, and feeding it with an async delegate is a bug: it doesn't do what you expect it to do. If you want to launch multiple asynchronous operation concurrently, while keeping the level of concurrency under control, the correct API to use is the Parallel.ForEachAsync method. This method will be introduced with the upcoming .NET 6, so for now you can utilize only home-made solutions, like the ones found in this question.

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104