Converting a Parallel.ForEach loop into parallel tasks using lambda expression

Question

Need help converting this to a lamdba expression where I can use Task.WhenAll:

public void DoWork(string id)
{            
       var items = GetItems(id);  //takes time

       if (items == null)
       {
            return;
       }

       Parallel.ForEach(items, item =>
       {
            DoSomething(item); //takes time
            DoWork(item.subItemId);                    
       });           
}

I would like to change the Parallel For loop into tasks using a lambda expressions as suggested in Martin's answer in this thread: How to call an async method from within a loop without awaiting?

Not sure how to specify the multiple line after item =>. This doesn't seem to work:

public void DoWork(string id)
{            
       var items = GetItems(id);  //takes time

       if (items == null)
       {
           return;
       } 

       var tasks = items.Select(item => 
       {
           DoSomething(item)
           DoWork(item.subItemId)
       });      

       await Task.WhenAll(tasks);         
}

UPDATE:

Thank you all for your answers. If I have an if condition, would I be wasting tasks? Is there a better to code this? Do I need to be using Where instead?

var tasks = items.Select(async item => 
{ 
    if (item.HasWork)
    {
       await DoSomethingAsync(item);
       await DoWorkAsync(item.subItemId);
    }
});      

await Task.WhenAll(tasks);

Note that you should be somewhat wary of recursive `async` methods. It's probably worth transforming this method into being iterative, rather than recursive. — Servy, Jul 29 '14 at 15:31

score 7 · Accepted Answer · edited May 23 '17 at 12:13

7

It's easiest to use an async lambda:

public async Task DoWorkAsync(string id)
{            
  var items = GetItems(id);  //takes time

  if (items == null)
    return;

  var tasks = items.Select(async item => 
  { 
    await DoSomethingAsync(item);
    await DoWorkAsync(item.subItemId);
  });      

  await Task.WhenAll(tasks);         
}

Martin's original answer assumes you'd write your own async method instead of an async lambda, which may make the code clearer:

public async Task DoWorkAsync(string id)
{            
  var items = GetItems(id);  //takes time

  if (items == null)
    return;

  var tasks = items.Select(item => ProcessAsync(item));

  await Task.WhenAll(tasks);         
}

private async Task ProcessAsync(T item)
{
  await DoSomethingAsync(item);
  await DoWorkAsync(item.subItemId);
}

edited May 23 '17 at 12:13

Community

1
1

answered Jul 29 '14 at 15:28

Stephen Cleary

437,863
77
675
810

thanks. Question is, do I need to make DoSomething and DoWork as Async? Could I keep them as regular sync methods and follow Selman22's or Castro's answer, and still get the full benefit of parallel processing? – Prabhu Jul 29 '14 at 15:33
@Prabhu: You stated in a comment that you get the "compiler warning saying I am not using the await keyword for DoSomething and DoWork". So I infer they're already asynchronous; I just renamed them to [follow the naming convention](http://msdn.microsoft.com/en-us/library/hh873175.aspx). – Stephen Cleary Jul 29 '14 at 15:38
Sure. But DoSomething and DoWork are my methods, so I am able to make then non-async if it will make things simpler. Is there a benefit of keeping them async? – Prabhu Jul 29 '14 at 15:40
1

They should be asynchronous if they have asynchronous work to do (usually I/O). They should be synchronous if they have synchronous work to do (usually CPU computation). Let the *operation* determine how the *method* looks; don't force the method to be a certain way, imposing a mismatching signature onto the operation. – Stephen Cleary Jul 29 '14 at 15:42
do you see a potential problem with DoWorkAsync being recursive? – Prabhu Jul 29 '14 at 15:45
@Prabhu: I generally avoid recursive code in C#; C# is not a functional language, so there is a definite stack limit that you can run into (the rules for when tail-recursion optimization applies are horribly complex and not stable from version to version). `async` lets you escape some of that, but only if it truly acts asynchronously; e.g., if an awaited task is already complete, the method will act *synchronously*, and then you can really run into stack problems with recursive async. It's best to avoid recursion in C#. – Stephen Cleary Jul 29 '14 at 15:47
Thanks. I updated my question, had another doubt..if I have an if condition wrapping DoSomething and DoWork, then would I be creating empty tasks for some items (when the if is false)? Is there a better way to deal with the if condition? – Prabhu Jul 29 '14 at 15:52
A `Where` would be a bit more efficient, but not a groundbreaking performance improvement. – Stephen Cleary Jul 29 '14 at 17:33

Converting a Parallel.ForEach loop into parallel tasks using lambda expression

1 Answers1