3

I have replaced a for loop in my code with a Parallel.For. The performance improvement is awesome (1/3 running time). I've tried to account for shared resources using an array to gather result codes. I then process the array out side the Parallel.For. Is this the most efficient way or will blocking still occur even if no iteration can ever share the same loop-index? Would a CompareExchange perform much better?

int[] pageResults = new int[arrCounter];
Parallel.For(0, arrCounter, i =>
{
   AlertToQueueInput input = new AlertToQueueInput();
   input.Message = Messages[i];

   pageResults[i] = scCommunication.AlertToQueue(input).ReturnCode;
});

foreach (int r in pageResults)
{
    if (r != 0 && outputPC.ReturnCode == 0) outputPC.ReturnCode = r;
}
stuartd
  • 70,509
  • 14
  • 132
  • 163
David
  • 65
  • 6

3 Answers3

6

It depends on whether you have any (valuable) side-effects in the main loop.

When the outputPC.ReturnCode is the only result, you can use PLINQ:

outputPC.ReturnCode = Messages
    .AsParallel()
    .Select(msg =>    
    {
       AlertToQueueInput input = new AlertToQueueInput();
       input.Message = msg;        
       return scCommunication.AlertToQueue(input).ReturnCode;
    })
    .FirstOrDefault(r => r != 0);

This assumes scCommunication.AlertToQueue() is thread-safe and you don't want to call it for the remaining items after the first error.

Note that FirstOrDefault() in PLinq is only efficient in Framework 4.5 and later.

H H
  • 263,252
  • 30
  • 330
  • 514
  • @Henk Holterman - are you suggesting this to drive my AlertToQueue calls in the main loop or to replace the second loop? – David May 20 '15 at 14:37
  • It replaces both loops. I have made an edit. It assumes you do not need to call any after an returncode != 0, it acts just like a break. – H H May 20 '15 at 15:59
  • I do need to continue after returncode != 0. This is really cool though. I can use this elsewhere. – David May 21 '15 at 12:53
3

You could replace:

foreach (int r in pageResults)
{
    if (r != 0 && outputPC.ReturnCode == 0) outputPC.ReturnCode = r;
}

with:

foreach (int r in pageResults)
{
    if (r != 0)
    {
        outputPC.ReturnCode = r;
        break;
    }
}

This will then stop the loop on the first fail.

David Arno
  • 42,717
  • 16
  • 86
  • 131
  • nice! It's not related to the parallel question. But definitely going to use that. – David May 20 '15 at 10:53
  • `Parallel.For` overload you used takes `Action` as loop body. Which means you can't return anything from it. That won't compile. – Sriram Sakthivel May 20 '15 at 11:04
  • @SriramSakthivel, damn you are right. Need to rethink that one as quitting the `Parallel.For` is the best option. – David Arno May 20 '15 at 11:06
  • @SriramSakthivel, I see someone else has already posted a working version of what I was trying to do, so I've just removed the broken code from my answer. – David Arno May 20 '15 at 11:08
3

I like David Arno's solution, but as I see you can improve the speed with putting the check inside the parallel loop and breaking directly from it. Anyway you put the main code to fail if any of iterations failed , so there is no need for further iterations.

Something like this:

Parallel.For(0, arrCounter, (i, loopState) =>
{
   AlertToQueueInput input = new AlertToQueueInput();
   input.Message = Messages[i];
   var code = scCommunication.AlertToQueue(input).ReturnCode;
    if (code != 0)
    {
        outputPC.ReturnCode = code ;
         loopState.Break();
    }

});

Upd 1:

If you need to save the result of all iterations you can do something like this:

int[] pageResults = new int[arrCounter];
Parallel.For(0, arrCounter, (i, loopState) =>
    {
       AlertToQueueInput input = new AlertToQueueInput();
       input.Message = Messages[i];
       var code = scCommunication.AlertToQueue(input).ReturnCode;
       pageResults[i] = code ;
        if (code != 0 && outputPC.ReturnCode == 0)
            outputPC.ReturnCode = code ;    
    });

It will save you from the foreach loop which is an improvement although a small one.

UPD 2:

just found this post and I think custom parallel is a good solution too. But it's your call to decide if it fits to your task.

Community
  • 1
  • 1
Max Novich
  • 1,169
  • 9
  • 20
  • Yes, this is better than my answer (and it works, unlike my updated answer :) – David Arno May 20 '15 at 11:09
  • I do want to send what pages I can after a page fails. My bad for not stipulating this in the original post. – David May 20 '15 at 11:21
  • @David I'm not sure I understood your comment. You need the array with codes for all of your pages anyway? – Max Novich May 20 '15 at 11:23
  • Yes. Does update 1 risk any performance hit for updating a shared variable within the Parallel.For? Or does the fact it can only be updated once prevent that? – David May 20 '15 at 11:48
  • I'm not sure. Theoretically one iteration can perform the check at the time the second one has passed it but yet not changed the value of the shared `val`. But this can occur only 1 time so I don't think it's problem, because you don't care about the value of the variable if it is not '0'. – Max Novich May 20 '15 at 11:52
  • Interesting. I'll try the custom parallel out and let you know my results. – David May 20 '15 at 14:48
  • I tried custom parallel. I got good results. However, the Parallel.For out performs it by a small margin using my particular dataset and environment setup – David May 20 '15 at 15:24