2

I am failing to understand why this doesn't seem to run the tasks in Parallel:

var tasks = new Task<MyReturnType>[mbis.Length];

for (int i = 0; i < tasks.Length; i++)
{
     tasks[i] = CAS.Service.GetAllRouterInterfaces(mbis[i], 3);
}

Parallel.ForEach(tasks, task => task.Start());

By stepping through the execution, I see that as soon as this line is evaluated:

tasks[i] = CAS.Service.GetAllRouterInterfaces(mbis[i], 3);

The task starts. I want to add all the new tasks to the list, and then execute them in parallel.

Wai Ha Lee
  • 8,598
  • 83
  • 57
  • 92
blgrnboy
  • 4,877
  • 10
  • 43
  • 94
  • Don't return "Cold Tasks" fix `GetAllRouterInterfaces` and make it return running tasks (or are they running already, its hard to tell from your question). – Scott Chamberlain Feb 25 '16 at 22:03
  • If `GetAllRouterInterfaces` is an `async` method returning a `Task` it should most likely be called `GetAllRouterInterfacesAsync` – Lukazoid Feb 25 '16 at 22:04

2 Answers2

4

If GetAllRouterInterfaces is an async method, the resulting Task will already be started (see this answer for further explanation).

This means that tasks will contain multiple tasks all of which are running in parallel without the subsequent call to Parallel.ForEach.

You may wish to wait for all the entries in tasks to complete, you can do this with an await Task.WhenAll(tasks);.

So you should end up with:

var tasks = new Task<MyReturnType>[mbis.Length];

for (int i = 0; i < tasks.Length; i++)
{
    tasks[i] = CAS.Service.GetAllRouterInterfaces(mbis[i], 3);
}

await Task.WhenAll(tasks);

Update from comments

It seems that despite GetAllRouterInterfaces being async and returning a Task it is still making synchronous POST requests (presumably before any other await). This would explain why you are getting minimal concurrency as each call to GetAllRouterInterfaces is blocking while this request is made. The ideal solution would be to make an aynchronous POST request, e.g:

await webclient.PostAsync(request).ConfigureAwait(false);

This will ensure your for loop is not blocked and the requests are made concurrently.

Further update after conversation

It seems you are unable to make the POST requests asynchronous and GetAllRouterInterfaces does not actually do any asynchronous work, due to this I have advised the following:

  1. Remove async from GetAllRouterInterfaces and change the return type to MyReturnType
  2. Call GetAllRouterInterfaces in parallel like so

    var routerInterfaces = mbis.AsParallel()
        .Select(mbi => CAS.Service.GetAllRouterInterfaces(mbi, 3));
    
Community
  • 1
  • 1
Lukazoid
  • 19,016
  • 3
  • 62
  • 85
  • GetAllRouterInterfaces returns a Task<>, and it does start doing things right away. For some reason, I am not seeing a benefit in time savings when I am running them in parallel vs. doing them one by one. Also, is there a different between Tasks.WaitAll(tasks.ToArray) and await Task.WhenAll(tasks)? – blgrnboy Feb 25 '16 at 22:11
  • 3
    @blgrnboy just because you run something in parallel does not mean it will be faster, especially if they are all trying to share a single resource like a disk or a server. – Scott Chamberlain Feb 25 '16 at 22:12
  • @blgrnboy, `Task.WaitAll()` is blocking. It will block thread until all tasks have finished. – monoh_ Feb 25 '16 at 22:13
  • As @ScottChamberlain says, not everything will be benefited by running in parallel, your `GetAllRouterInterface` may have something blocking or be bottlenecked. `await Task.WhenAll(tasks)` should be used in `async` methods whereas `Task.WaitAll` is for synchronous methods and will block the calling thread. – Lukazoid Feb 25 '16 at 22:14
  • @ScottChamberlain Yeah, I understand that part. They are all making REST calls, but I would assume at least some reduction in time, being that the resource is pretty heavily hit by others and doesn't typically slow down. – blgrnboy Feb 25 '16 at 22:14
  • I don't want to sidetrack the question too much but if you update your question with the contents of `GetAllRouterInterfaces` and the code which makes the `REST` call it may reveal something. – Lukazoid Feb 25 '16 at 22:19
  • I don't want to post all of that, but I will say this. GetAllRouterInterfaces is not necessarily parallel itself, it will generate some XmlContent, a make an HTTP Post with it. I was under the impression that as long as the calling method was trying to call this in parallel, it didn't matter what how the called method executed its work. – blgrnboy Feb 25 '16 at 22:22
  • Is your HTTP post being made asynchronously, like `await webclient.PostAsync(request)`? There are many ways to perform work in parallel but as `async` methods start immediately you could be blocking your `for` loop with each invocation of `GetAllRouterInterfaces` limiting the amount of concurrency. – Lukazoid Feb 25 '16 at 22:34
  • No, unfortunately, that is not being make asynchronously :( – blgrnboy Feb 25 '16 at 22:40
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/104589/discussion-between-lukazoid-and-blgrnboy). – Lukazoid Feb 25 '16 at 22:42
1

I don't know if I understand you the right way.

First of all, if GetAllRouterInterfaces is returns a Task you have to await the result.

With Parallel.ForEach you can't await tasks like as it is, but you can do something similar like this:

public async Task RunInParallel(IEnumerable<TWhatEver> mbisItems)
{
    //mbisItems == your parameter that you want to pass to GetAllRouterInterfaces

    //degree of cucurrency
    var concurrentTasks = 3;

    //Parallel.Foreach does internally something like this:
    await Task.WhenAll(
        from partition in Partitioner.Create(mbisItems).GetPartitions(concurrentTasks)
        select Task.Run(async delegate
        {
            using (partition)
                while (partition.MoveNext())
                {
                    var currentMbis = partition.Current;
                    var yourResult = await GetAllRouterInterfaces(currentMbis,3);
                }
        }
       ));
}
CaptainPlanet
  • 735
  • 4
  • 8