Thread local BigInteger variable in nested Parallel.For is not processed for aggregation with standard patterns?

107 Views Asked by At

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:

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() + 
        }           
    }
1

There are 1 best solutions below

1
Eric J. On

Your current parallel implementation requires a lock every time subtotal is 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:

Parallel.For(1, iEnd1 + 1, (iCn1) =>
{
    Parallel.For(1, iEnd2 + 1, (iCn2) =>
    {
        BigInteger subtotal = 0;
        for (var iCnt3 = iCn2 - 1; iCnt3 < iEnd3 + 1; iCnt3++)
        {
            //Solution:
            //for BigInteger use ++-operator or BigInteger.Add()
            subtotal = BigInteger.Add(subtotal, 1);
        }
        lock (lockObj)
        {
            totalRecomandedPatternModified = BigInteger.Add(totalRecomandedPatternModified, subtotal);
        }
    });
});

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:

Serial:   9ms
Parallel: 11ms
Modified: 2ms