CODEWITHSUNDEEP
c#task-parallel-libraryparallel.foreach
user10479
user10479

Reputation: 810

Parallel.For and Parallel.ForEach not running to conclusion

Here's a failing test. How can I confirm that the loops run the correct number of times?

    public Random Randomator { get; set; }
    public const int TimesToRun = 1000000;

    [TestMethod]
    public void ThrowTheDice()
    {
        Randomator = new Random();

        var resultsParallel = new Dictionary<int, int>
        {
            {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}
        };

        var resultsParallelForEach = new Dictionary<int, int>
        {
            {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}
        };

        var stopwatch = new Stopwatch();
        stopwatch.Start();
        Parallel.For(0, TimesToRun, ctr =>
        {
            var val = ThrowDice();
            if (!resultsParallel.ContainsKey(val))
                throw new ArgumentOutOfRangeException();

            var existing = resultsParallel[val];
            resultsParallel[val] = existing + 1;
        });

        stopwatch.Stop();
        var parallelTime = stopwatch.Elapsed;

        stopwatch = new Stopwatch();
        stopwatch.Start();
        var numbers = Enumerable.Range(0, TimesToRun);
        Parallel.ForEach(numbers, ctr =>
        {
            var val = ThrowDice();
            if (!resultsParallel.ContainsKey(val))
                throw new ArgumentOutOfRangeException();

            var existing = resultsParallelForEach[val];
            resultsParallelForEach[val] = existing + 1;
        });

        stopwatch.Stop();
        var parallelForEachTime = stopwatch.Elapsed;

        var parallelTotal = resultsParallel.Sum(x => x.Value);
        var parallelForEachTotal = resultsParallelForEach.Sum(x => x.Value);

        Assert.AreEqual(parallelTotal, TimesToRun);
        Assert.AreEqual(parallelForEachTotal, TimesToRun);
    }

    public int ThrowDice()
    {
        return Randomator.Next(1, 7);
    }

Upvotes: 0

Views: 881

Answers (3)

Volker Wollmann
Volker Wollmann

Reputation: 1

You may use a semaphore to serialize the concurrent Access to resultsParallel, resultsParallelForEach:

public class Example { public static Random Randomator { get; set; } public const int TimesToRun = 1000000;

    public static Semaphore semaphore;


    public static void ThrowTheDice()
    {
        Randomator = new Random();

        semaphore = new Semaphore(1, 1);

        var resultsParallel = new Dictionary<int, int>
    {
        {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}
    };

        var resultsParallelForEach = new Dictionary<int, int>
    {
        {1, 0}, {2, 0}, {3, 0}, {4, 0}, {5, 0}, {6, 0}
    };

        var stopwatch = new Stopwatch();
        stopwatch.Start();
        Parallel.For(0, TimesToRun, ctr =>
        {
            var val = ThrowDice();
            if (!resultsParallel.ContainsKey(val))
                throw new ArgumentOutOfRangeException();

            semaphore.WaitOne();

            var existing = resultsParallel[val];
            resultsParallel[val] = existing + 1;

            semaphore.Release();
        });

        stopwatch.Stop();
        var parallelTime = stopwatch.Elapsed;

        stopwatch = new Stopwatch();
        stopwatch.Start();
        var numbers = Enumerable.Range(0, TimesToRun);
        Parallel.ForEach(numbers, ctr =>
        {
            var val = ThrowDice();
            if (!resultsParallel.ContainsKey(val))
                throw new ArgumentOutOfRangeException();

            semaphore.WaitOne();

            var existing = resultsParallelForEach[val];
            resultsParallelForEach[val] = existing + 1;

            semaphore.Release();
        });

        stopwatch.Stop();
        var parallelForEachTime = stopwatch.Elapsed;

        var parallelTotal = resultsParallel.Sum(x => x.Value);
        var parallelForEachTotal = resultsParallelForEach.Sum(x => x.Value);

        Debug.Assert(parallelTotal == TimesToRun);
        Debug.Assert(parallelForEachTotal == TimesToRun);
    }

    public static int ThrowDice()
    {
        return Randomator.Next(1, 7);
    }
}

Upvotes: 0

Georg
Georg

Reputation: 5771

You are using hashtable implementations that are not thread-safe. Thus, you only proof you did a mistake. Use ConcurrentDictionary instead, which is thread-safe:

var resultsParallel = new ConcurrentDictionary<int, int>();

var stopwatch = new Stopwatch();
stopwatch.Start();
Parallel.For(0, TimesToRun, ctr =>
{
    var val = ThrowDice();
    resultsParallel.AddOrUpdate(val, 1, (key, old) => old + 1);
});

Upvotes: 1

Damien_The_Unbeliever
Damien_The_Unbeliever

Reputation: 239646

In parallel, you're running these lines:

var existing = resultsParallel[val];
resultsParallel[val] = existing + 1;

There's no guarantee that only one thread/task is running those lines at the same time, for any particular val value. So two threads can read the value 2, add 1, and store the value 3. You need to use thread-safe methods for accumulating your totals.

E.g. you could use an overload of Parallel.For that allows each thread to build up its own copy of the results separately and then has a final combining step to allow you to compute the total results:

public static ParallelLoopResult For<TLocal>(
    long fromInclusive,
    long toExclusive,
    Func<TLocal> localInit,
    Func<long, ParallelLoopState, TLocal, TLocal> body,
    Action<TLocal> localFinally
)

Upvotes: 8

Related Questions