I tryed to refactor a nested sequential for loop into a nested Parallel.For loop.
But following the recommended parallel patterns and locks, the overall result was too low compared with the sequential result.
The problem was caused by a wrong or inconsistent use of BigInteger calculation methods.
For BigInteger you need to use ++-operator or BigInteger methods like BigInteger.Add().
My sources:
- How to: Write a Parallel.For Loop with Thread-Local Variables
- Threading in C# - Parallel Programming - The Parallel Class - For and ForEach
Please find sample code below:
internal static class Program
{
static Object lockObj = new Object();
static void Main()
{
//target result: 575
NestedLoopAggregationTest();
return;
}
private static void NestedLoopAggregationTest()
{
BigInteger totalSequential = 0;
BigInteger totalRecomandedPattern = 0;
BigInteger totalAntiPattern = 0;
const int iEnd1 = 5;
const int iEnd2 = 10;
const int iEnd3 = 15;
for (int iCn1 = 1; iCn1 <= iEnd1; iCn1++)
{
for (int iCn2 = 1; iCn2 <= iEnd2; iCn2++)
{
for (int iCn3 = iCn2 - 1; iCn3 <= iEnd3; iCn3++)
{
totalSequential++;
}
}
}
Parallel.For(1, iEnd1 + 1, (iCn1) =>
{
Parallel.For(1, iEnd2 + 1, (iCn2) =>
{
Parallel.For<BigInteger>(iCn2 - 1, iEnd3 + 1, () => 0, (iCn3, state, subtotal) =>
{
//Solution:
//for BigInteger use ++-operator or BigInteger.Add()
subtotal = BigInteger.Add(subtotal, 1);
return subtotal;
},
(subtotal) =>
{
lock (lockObj)
{
totalRecomandedPattern = BigInteger.Add(totalRecomandedPattern, subtotal);
}
}
);
});
});
MessageBox.Show(totalSequential.ToString() + Environment.NewLine + totalRecomandedPattern.ToString() +
}
}
Your current parallel implementation requires a lock every time
subtotalis modified in the inner loop. This modified approach is faster than both your serial and parallel implementaions because it avoids a lock in the innermost loop:I increased each of the endpoints by a factor of 10 so the runtime is long enough to be measured on my hardware, then got the following average times: