0

Inside the Parallel.ForEach method when I use an asynchronous method DownloadWebsiteAsync to get an external resource the calling method executeAsync_Click does not await the RunDownloadParallelAsync method and prints the result before RunDownloadParallelAsync finishes its work. When I change DownloadWebsiteAsync to DownloadWebsiteSync (which works in a synchronous way) everything is working well.

private async void executeAsync_Click(object sender, RoutedEventArgs e)
{
    var watch = System.Diagnostics.Stopwatch.StartNew();

    var results = await DemoMethods.RunDownloadParallelAsync();
    PrintResults(results);

    watch.Stop();
    var elapsedMs = watch.ElapsedMilliseconds;

    resultsWindow.Text += $"Total execution time: { elapsedMs }";
}

public static async Task<List<WebsiteDataModel>> RunDownloadParallelAsync()
{
    List<string> websites = PrepData();
    List<WebsiteDataModel> output = new List<WebsiteDataModel>();
    
    await Task.Run(() =>
        Parallel.ForEach<string>(websites, async (site) =>
        {
            WebsiteDataModel results =await DownloadWebsiteAsync(site);
            output.Add(results);
        })
    );
    return output;
}
Palle Due
  • 5,929
  • 4
  • 17
  • 32
mark
  • 73
  • 4
  • I don't see a need for parallelism here. You could simply loop through all your websites, call `DownloadWebsiteAsync`, place the resulting `Task` in a list (say `downloadTasks`), and after calling `DownloadWebsiteAsync` on all websites (after the loop) call `await Task.WhenAll(downloadTasks)` and then get the results like described [here](https://stackoverflow.com/a/17197786/9363973) – MindSwipe Sep 22 '20 at 13:05
  • You do not need `Parallel.ForEach` if your methods are `async`, this adds nothing. What you're doing now is starting a separate thread which starts threads which start tasks, and to make things worse this still doesn't work correctly since `output.Add` isn't thread safe! Consider `Task.WhenAll` instead. – Jeroen Mostert Sep 22 '20 at 13:05
  • If you wish to rate limit the number of requests that can run together you can do something like this: https://devblogs.microsoft.com/pfxteam/implementing-a-simple-foreachasync-part-2/ . If you only have a small number of tasks, you might be better off just using `Task.WhenAll` – Jason Sep 22 '20 at 13:29
  • The `Parallel.ForEach` is not async-friendly. @Servy [this](https://stackoverflow.com/questions/15136542/parallel-foreach-with-asynchronous-lambda) is the correct duplicate IMHO. – Theodor Zoulias Sep 22 '20 at 19:01
  • @TheodorZoulias You just linked another question with the same solution. Of course it's a frequently asked question, so searching on the title leads to a ton of duplicates. – Servy Sep 22 '20 at 19:10
  • @Servy yes, but the [current](https://stackoverflow.com/questions/46211283/c-sharp-parallel-foreach-and-async-combination) duplicate with 1 answer is less likely to be helpful to the OP than the [more popular](https://stackoverflow.com/questions/15136542/parallel-foreach-with-asynchronous-lambda) duplicate with 7 answers. – Theodor Zoulias Sep 22 '20 at 19:24

2 Answers2

4

Parallel and async generally don't work well together.

Here you want to allow multiple HTTP requests to happen simultaneously, but throwing multiple threads at the job will not help.

Instead, just use async:

public static async Task<IEnumerable<WebsiteDataModel>> RunDownloadParallelAsync()
{
    return await Task.WhenAll(PrepData().Select(DownloadWebsiteAsync));
}

Each request will be sent without waiting for the previous response.

Johnathan Barclay
  • 18,599
  • 1
  • 22
  • 35
0

If you need to limit the number of simultaneous "workers", use an ActionBlock from TPL DataFlow.

Paulo Morgado
  • 14,111
  • 3
  • 31
  • 59