6

I was reading this post about Parallel.ForEach where it was stated that "Parallel.ForEach is not compatible with passing in a async method."

So, to check I write this code:

static async Task Main(string[] args)
{
    var results = new ConcurrentDictionary<string, int>();

    Parallel.ForEach(Enumerable.Range(0, 100), async index =>
    {
        var res = await DoAsyncJob(index);
        results.TryAdd(index.ToString(), res);
    });         

    Console.ReadLine();
}

static async Task<int> DoAsyncJob(int i)
{
    Thread.Sleep(100);
    return await Task.FromResult(i * 10);
}

This code fills in the results dictionary concurrently.

By the way, I created a dictionary of type ConcurrentDictionary<string, int> because in case I have ConcurrentDictionary<int, int> when I explore its elements in debug mode I see that elements are sorted by the key and I thought that elenents was added consequently.

So, I want to know is my code is valid? If it "is not compatible with passing in a async method" why it works well?

9
  • 1
    Don't. Parallel.ForEach is made for CPU-intensive computations and doesn't recognize async methods. It doesn't await them, essentially converting them to async void fire-and-forget calls. Your method isn't async anyway, so it's not possible to say what the correct call would look like Commented Nov 27, 2018 at 10:01
  • @PanagiotisKanavos please post the source of your information. Thanks Commented Nov 27, 2018 at 10:03
  • 1
    I want to know is my code is valid?No. why it works well? It doesn't but you don't realize it because a) it doesn't perform any async work. Commented Nov 27, 2018 at 10:04
  • 1
    Parallel.ForEach - "Arrange for a number of threads to perform the following work in parallel" - async - "Well, there's no useful work for this thread to do until something else completes this awaitable". Even if it did work (it doesn't, it's entirely synchronous as Panagitotis says), you're combining things in an odd way that overallocates resources and then ignores them. Commented Nov 27, 2018 at 10:13
  • 1
    @Seabizkit there's nothing to convince about. There's no overload that accepts a Task. Without that, there's no way to await any async calls. async doesn't await anything, nor does it make anything run asynchronously. async void calls can't be awaited, that's why they are considered bugs outside event handlers. Commented Nov 27, 2018 at 10:37

3 Answers 3

10

This code works only because DoAsyncJob isn't really an asynchronous method. async doesn't make a method work asynchronously. Awaiting a completed task like that returned by Task.FromResult is synchronous too. async Task Main doesn't contain any asynchronous code, which results in a compiler warning.

An example that demonstrates how Parallel.ForEach doesn't work with asynchronous methods should call a real asynchronous method:

    static async Task Main(string[] args)
    {
        var results = new ConcurrentDictionary<string, int>();

        Parallel.ForEach(Enumerable.Range(0, 100), async index =>
        {
            var res = await DoAsyncJob(index);
            results.TryAdd(index.ToString(), res);
        });  
        Console.WriteLine($"Items in dictionary {results.Count}");
    }

    static async Task<int> DoAsyncJob(int i)
    {
        await Task.Delay(100);
        return i * 10;
    }

The result will be

Items in dictionary 0

Parallel.ForEach has no overload accepting a Func<Task>, it accepts only Action delegates. This means it can't await any asynchronous operations.

async index is accepted because it's implicitly an async void delegate. As far as Parallel.ForEach is concerned, it's just an Action<int>.

The result is that Parallel.ForEach fires off 100 tasks and never waits for them to complete. That's why the dictionary is still empty when the application terminates.

Sign up to request clarification or add additional context in comments.

7 Comments

You said in the comment that I can use list.Select(item=>Task.Run(...)) Why should i use Task.Run() in Select method? Can I use just like this: var tasks = Enumerable.Range(0, 100).Select(async index => { var res = await DoAsyncJob(index); results.TryAdd(index.ToString(), res); }); await Task.WhenAll(tasks);
Async is used to free a thread which would be waiting on "I/O" stuff, so in this case, would it not be freeing some threads since the operation in the foreach maybe db access for example... Are you saying that this would be the case if the caller aka Parallel.ForEach could return Task, and the fact that it doesn't... all the Task operations in Parallel are not able to implement the async lib correctly. Did i word that ok?
@DmitryS because you didn't explain what you really want to do. If you wanted to process some data items in the background without blocking, you could use .Select(..=>Task.Run..). The same would work if you wanted to test multiple algorithms and get the first result.
@Seabizkit Parallel.ForEach is specifically meant for data parallelism. That's why it doesn't provide any overloads for tasks, and actually uses the current thread for processing. It partitions the input data into as many partitions as there are cores and uses one task per partition to minimize cross-thread synchronization. It's a completely different problem from asynchronous or concurrent operations
@PanagiotisKanavos THANKS! i think that is getting a lot closer to the info I'm looking for.
|
4

An async method is one that starts and returns a Task.

Your code here

Parallel.ForEach(Enumerable.Range(0, 100), async index =>
{
    var res = await DoAsyncJob(index);
    results.TryAdd(index.ToString(), res);
});        

runs async methods 100 times in parallel. That's to say it parallelises the task creation, not the whole task. By the time ForEach has returned, your tasks are running but they are not necessarily complete.

You code works because DoAsyncJob() not actually asynchronous - your Task is completed upon return. Thread.Sleep() is a synchronous method. Task.Delay() is its asynchronous equivalent.

Understand the difference between CPU-bound and I/O-bound operations. As others have already pointed out, parallelism (and Parallel.ForEach) is for CPU-bound operations and asynchronous programming is not appropriate.

Comments

2

If you already have asynchronous work, you don't need Parallel.ForEach:

static async Task Main(string[] args)
{

    var results = await new Task.WhenAll(
        Enumerable.Range(0, 100)
        Select(i => DoAsyncJob(I)));

    Console.ReadLine();
}

Regarding your async job, you either go async all the way:

static async Task<int> DoAsyncJob(int i)
{
    await Task.Delay(100);
    return await Task.FromResult(i * 10);
}

Better yet:

static async Task<int> DoAsyncJob(int i)
{
    await Task.Delay(100);
    return i * 10;
}

or not at all:

static Task<int> DoAsyncJob(int i)
{
    Thread.Sleep(100);
    return Task.FromResult(i * 10);
}

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.