1

I'm new to C# threads and tasks and I'm trying to develop a workflow but without success probably because I'm mixing tasks with for iterations...

The point is:

I've got a bunch of lists, and inside each one there are some things to do, and need to make them work as much parallel and less blocking possible, and as soon as each subBunchOfThingsTodo is done ( it means every thing to do inside it is done parallely) it has do some business(DoSomethingAfterEveryThingToDoOfThisSubBunchOfThingsAreDone()).

e.g:

bunchOfSubBunchsOfThingsTodo

  • subBunchOfThingsTodo

    • ThingToDo1
    • ThingToDo2
  • subBunchOfThingsTodo

    • ThingToDo1
    • ThingToDo2
    • ThingToDo3
  • subBunchOfThingsTodo

    • ThingToDo1
    • ThingToDo2...

This is how I'm trying but unfortunately each iteration waits the previous one bunchOfThingsToDo and I need them to work in parallel. The same happens to the things to do , they wait the previous thing to start...

List<X> bunchOfSubBunchsOfThingsTodo = getBunchOfSubBunchsOfThingsTodo();     
foreach (var subBunchOfThingsToDo in bunchOfSubBunchsOfThingsTodo)
{
    int idSubBunchOfThingsToDo = subBunchOfThingsToDo.ThingsToDo.FirstOrDefault().IdSubBunchOfThingsToDo;
    
    var parent = Task.Factory.StartNew(() =>
    {
        foreach (var thingToDo in subBunchOfThingsToDo.ThingsToDo)
        {
            var child = Task.Factory.StartNew(() =>
            {
               //Do some stuff with thingToDo... Here I call several business methods
            });
        }
    });

    parent.Wait();
    DoSomethingAfterEveryThingToDoOfThisSubBunchOfThingsAreDone(idSubBunchOfThingsToDo);
}
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
fgc
  • 79
  • 1
  • 1
  • 13
  • Is this *business* CPU bound? – Stefan Mar 02 '18 at 12:02
  • 1
    If your "tasks" are independent from each other and big enough, then have a look at Parallel.ForEach. – gdir Mar 02 '18 at 12:04
  • 1
    [Use `Task.Run` not `Task.Factory.StartNew`](https://blog.stephencleary.com/2013/08/startnew-is-dangerous.html). not your issue but there are very specific instances when you should use StartNew and I'd guess this isn't one of them – Liam Mar 02 '18 at 12:04
  • The SubBunchOfThingsToDo are independent. I'm tried to use parallelFor but I cant make it work, Could you please write some code? Thanks – fgc Mar 02 '18 at 12:06
  • 2
    `Parallel.ForEach` is likely the way to go here – Liam Mar 02 '18 at 12:07
  • This is covered in the docs? https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/how-to-write-a-simple-parallel-foreach-loop – Liam Mar 02 '18 at 12:09
  • Your outter loop is going to wait on each loop because you've put the `Wait` inside it. Is this what you mean by *each iteration waits*? If you don't want it to wait (and you don't want to use `Parallel.ForEach` then you need to move that wait out of the loop. But you also need to store each Task in an array not just the last one in `parent`. So `Task[] parents` then `Task.WaitAll(parents);`. See this question https://stackoverflow.com/questions/19849847/use-task-waitall-to-handle-awaited-tasks – Liam Mar 02 '18 at 12:15
  • 1
    @fgc what is your *real* goal? Don't describe what you tried, describe what you *want*. Do you want to perform work in the *background* without blocking the UI? Or make a lot of remote,hence async, calls? Or perform some CPU-heavy jobs? Different requirements, different classes. For example, you could use `Parallel.ForEach(subBunchOfThingsToDo.ThingsToDo, thingToDo =>WorkKnowingItsParallelAlready(thingToDo))` for CPU-bound work – Panagiotis Kanavos Mar 02 '18 at 12:52
  • @fgc or you could use PLINQ - convert the double loops to a single query and add AsParallel() eg `from item in firstList.AsParallel() from innerItem in item.OtherList ...)` – Panagiotis Kanavos Mar 02 '18 at 12:54

2 Answers2

2

You may want to try using Task.WhenAll and playing with linq to generate a collection of hot tasks:

static async void ProcessThingsToDo(IEnumerable<ThingToDo> bunchOfThingsToDo)
{
    IEnumerable<Task> GetSubTasks(ThingToDo thing) 
        => thing.SubBunchOfThingsToDo.Select( async subThing => await Task.Run(subThing));

    var tasks = bunchOfThingsToDo
        .Select(async thing => await Task.WhenAll(GetSubTasks(thing)));

    await Task.WhenAll(tasks);
}

This way you are running each subThingToDo on a separate task and you get only one Task composed by all subtasks for each thingToDo

EDIT

ThingToDo is a rather simple class in this sample:

class ThingToDo
{
    public IEnumerable<Action> SubBunchOfThingsToDo { get; }
}
taquion
  • 2,667
  • 2
  • 18
  • 29
  • You missed that when all sub things in bunch are processed 'DoSomthingAfter..' must be called – gabba Mar 04 '18 at 09:51
-2

With minimum changes of your code you can try this way:

    var toWait = new List<Task>();
    List<X> bunchOfSubBunchsOfThingsTodo = getBunchOfSubBunchsOfThingsTodo();     

    foreach (var subBunchOfThingsToDo in bunchOfSubBunchsOfThingsTodo)
    {
        int idSubBunchOfThingsToDo = subBunchOfThingsToDo.ThingsToDo.FirstOrDefault().IdSubBunchOfThingsToDo;
    
        var parent = Task.Factory.StartNew(() =>
        {
            Parallel.ForEach(subBunchOfThingsToDo.ThingsToDo,
                thingToDo =>
                {
                        //Do some stuff with thingToDo... Here I call several business methods
                });
        });
    
        //parent.Wait();
        var handle = parent.ContinueWith((x) =>
        {
            DoSomethingAfterEveryThingToDoOfThisSubBunchOfThingsAreDone(idSubBunchOfThingsToDo);
        })
        .Start();

        toWait.Add(handle);
    }

    Task.WhenAll(toWait);
        

Thanks to downvoters team, that advised 'good' solution:

    var bunchOfSubBunchsOfThingsTodo = getBunchOfSubBunchsOfThingsTodo();
    var toWait = bunchOfSubBunchsOfThingsTodo
        .Select(subBunchOfThingsToDo =>
        {
            return Task.Run(() =>
            {
                int idSubBunchOfThingsToDo = subBunchOfThingsToDo.ThingsToDo.FirstOrDefault().IdSubBunchOfThingsToDo;

                Parallel.ForEach(subBunchOfThingsToDo.ThingsToDo,
                    thingToDo =>
                    {
                        //Do some stuff with thingToDo... Here I call several business methods
                    });

                DoSomethingAfterEveryThingToDoOfThisSubBunchOfThingsAreDone(idSubBunchOfThingsToDo);
            });
        });

    Task.WhenAll(toWait);
marc_s
  • 732,580
  • 175
  • 1,330
  • 1,459
gabba
  • 2,815
  • 2
  • 27
  • 48
  • @Liam, so question is about how to handle the sub bunch when it completes but didt stop a thread – gabba Mar 02 '18 at 12:18
  • @Liam, as you can see a made a description "With minimum changes", this is not best solution but a way to help questioner understand whats wrong. – gabba Mar 02 '18 at 12:20
  • Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/166097/discussion-between-gabba-and-liam). – gabba Mar 02 '18 at 12:20
  • @Liam, do I need to explain what questioner wants to do? If you look at code sample you will see loop by bunchOfSubBunchsOfThingsTodo, this loop waits on each iteration, so if we remove parent.wait and add continue with it will go – gabba Mar 02 '18 at 12:28
  • 1
    @gabba perhaps you should rethink what *your* code does? You use `Parallel.ForEach`, ie *tasks* to start *tasks*? Might as well use a simple loop. Those tasks are lost immediatelly and never attached to the "parent" task. ` parent.ContinueWith` or `parent.Wait()` will return immediatelly since the only thing it did was generate some fire-and-forget tasks. – Panagiotis Kanavos Mar 02 '18 at 12:46
  • @PanagiotisKanavos, thanks, I forget to remove Task from the original code – gabba Mar 02 '18 at 12:50
  • 1
    @gabba now remove the parent as well and use *only* `await Task.Run(()=>Parallel.ForEach(...)`. Remove that `.Start()`, you don't need cold tasks – Panagiotis Kanavos Mar 02 '18 at 12:52
  • @PanagiotisKanavos, thanks for advice (actually wrong advice), but now I want to add completely another way how to do the same. I don't want to rewrite this sample. Just want add another sample. If you want you can add your version – gabba Mar 02 '18 at 12:56
  • also as I stated in the comment on the question [don't use `Task.StartNew`](https://blog.stephencleary.com/2013/08/startnew-is-dangerous.html) – Liam Mar 02 '18 at 13:03
  • If you think it's wrong advice, just try to run this code – Panagiotis Kanavos Mar 02 '18 at 13:07
  • @PanagiotisKanavos, may be I didn't understand you? what code I should run? May be will be better to write another and correct answer? – gabba Mar 02 '18 at 13:11
  • `Parallel.ForEach(subBunchOfThingsToDo.ThingsToDo, thingToDo =>WorkKnowingItsParallelAlready(thingToDo))` or `from item in firstList.AsParallel() from innerItem in item.OtherList ...)`. Different requirements need different solutions. – Panagiotis Kanavos Mar 02 '18 at 13:12
  • @PanagiotisKanavos, and what will be in WorkKnowingItsParallelAlready? – gabba Mar 02 '18 at 13:13
  • Use your imagination. It's already running in parallel, you don't need to do anything more – Panagiotis Kanavos Mar 02 '18 at 13:14
  • *This* answer will fire a lot of tasks and never await for them to finish. It's a more complicated way to write `from item in firstList from innerItem in item.OtherList select Task.Run(()=>...)`. The query though will return an `IEnumerable` that you *can* await with `await Task.WhenAll(query)` – Panagiotis Kanavos Mar 02 '18 at 13:15
  • @PanagiotisKanavos, but you forget to run handling after group of Task )))) – gabba Mar 02 '18 at 13:18