CODEWITHSUNDEEP
c#arraysforeachparallel-processing
Pelle Liljendal
Pelle Liljendal

Reputation: 63

Correct way to use indexer in Parallel.For

I have a method which generates a (Waveform) bitmap based on a specific (Waveform)-function (in the examples below I am simply using Math.Sin to simplify things). Up until now this method was "serial" (no threads) however some of the functions used are relatively time consuming so I tried using Parallel.For and Parallel.ForEach, but I guess the index variable I use must get "corrupted" (or at least have another value than what I expect) the graphics being generated will either contain "spikes" or strange lines between points that are not neighbours.

Here first is my serial version (which works):

Point[] points = new Point[rect.Width];
byte[] ptTypes = new byte[rect.Width];
for (int i = 0; i < rect.Width; i++)
{
    double phase = MathUtil.WrapRad((MathUtil.PI2 / (rect.Width / 1d)) * i);
    double value = waveform.ValueAtPhase(phase);
    newPoint = new Point(rect.Left + i,
        rect.Top + (int) (halfHeight + (value * -1d * halfHeight * scaleY)));
    points[i] = newPoint;
    if (i == 0)
        ptTypes[i] = (byte)PathPointType.Start;
    else
        ptTypes[i] = (byte)PathPointType.Line;
}
using (GraphicsPath wavePath = new GraphicsPath(points, ptTypes))
{
    gph.DrawPath(wavePen, wavePath);
}

As you can see the code simply uses 2 arrays (one for Points and one for PointTypes) so the order these values are inserted into the array should not matter, as long as the values are inserted into the correct elements of the arrays.

Next example is using Parallel.For (to simplify the examples I have omitted the creation of the arrays and the actual draw method):

Parallel.For(0, rect.Width,
       i =>
{
    double phase = MathUtil.WrapRad((MathUtil.PI2 / (rect.Width / 1d)) * i);
    double value = Math.Sin(phase);//waveform.ValueAtPhase(phase);
    newPoint = new Point(rect.Left + i,
        rect.Top + (int)(halfHeight + (value * -1d * halfHeight * scaleY)));
    points[i] = newPoint;
    if (i == 0)
        ptTypes[i] = (byte)PathPointType.Start;
    else
        ptTypes[i] = (byte)PathPointType.Line;
});

Lastly I tried using a Partitioner with a Parallel.ForEach loop, but that did not fix the problem either:

var rangePartitioner = Partitioner.Create(0, rect.Width);
Parallel.ForEach(rangePartitioner, (range, loopState) =>
{
    for (int i = range.Item1; i < range.Item2; i++)
    {
        double phase = MathUtil.WrapRad((MathUtil.PI2 / (rect.Width / 1d)) * i);
        double value = Math.Sin(phase);//waveform.ValueAtPhase(phase);
        newPoint = new Point(rect.Left + i,
            rect.Top + (int)(halfHeight + (value * -1d * halfHeight * scaleY)));
        points[i] = newPoint;
        if (i == 0)
            ptTypes[i] = (byte)PathPointType.Start;
        else
            ptTypes[i] = (byte)PathPointType.Line;
    }
});

Pelle

Upvotes: 2

Views: 105

Answers (1)

Rob
Rob

Reputation: 27357

newPoint = new Point(rect.Left + i, rect.Top + (int)(halfHeight + (value * -1d * halfHeight * scaleY)));

newPoint is not scoped to your for() loop - it's likely threads are updating it before you get to the next line points[i] = newPoint;

Change it to var newPoint = ...

Otherwise, your Parallel.For looks fine.

Also, does this have different behavior?

Math.Sin(phase);//waveform.ValueAtPhase(phase);

Providing ValueAtPhase does not modify anything, you should be able to use it within the loop.

Upvotes: 2

Related Questions