CODEWITHSUNDEEP
c#parallel.foreach
Salizar Marxx
Salizar Marxx

Reputation: 933

Inconsistent behavior with parallel foreach

I have a collection of objects that need to be run through a set of rules to determine if those objects adhere/match the given rule collection.

initially this was written as a Sql Link statement:

From x in objects From y in rules

I started by turning that statement into a ForEach/inner ForEach statement. The resulting output lined up with expected results.

Having used Parallel.ForEach in numerous other situations I figured it would be a good place to use it here.

objects.ForEach(o => {
    var obj = o.Value;
    var ruleCount = 0;
    //eventRules.ForEach(r => {
    Parallel.ForEach(eventRules, r => {
        var matchedDetail = r.GetMatchDetails(obj, _ruleContext);
        var matched = matchedDetail.Matched;
        if (matchedDetail.Matched)
        {
            var result = CreateResult(obj, r, matchedDetail);
            matchedResults.Add(result);
        }
        ruleCount += 1;
    });
    Console.WriteLine($"{obj.object_id} {eventRules.Count()} {ruleCount}");
});

84052 83 82
35135 83 82
37576 83 83
38772 83 81
80513 83 81
95824 83 83
99402 83 82
24626 83 83
30711 83 82
96613 83 83
63487 83 83
78497 83 83
81404 83 83
93719 83 82
36600 83 83
68544 83 83
78685 83 81

After noticing the results didn't match after adding the Parallel.ForEach I added the additional ruleCount markers and output. this indicated that randomly I would see 3 or 4 rules not being ran/returned. Substituting the standard ForEach back in, results in a very consistent, albeit slower, run. My understanding from MSDN documentation and other sources indicates that Parallel.ForEach should wait until all actions are complete before returning to the caller.

As an aside I've tested this with 4.5.1, and 4.6.1 both showed the same behavior.

Upvotes: 1

Views: 246

Answers (1)

Jakub Lortz
Jakub Lortz

Reputation: 14896

Some of the operations performed are not thread-safe.

One of them is ruleCount += 1;. You can replace it with Interlocked.Increment to make it atomic and thread-safe.

Interlocked.Increment(ref ruleCount);

Another (possible) one is matchedResults.Add(result);. I can't see the type of the matchedResults collection, but you should make sure it's a thread-safe collection (for example ConcurrentQueue). You could also use lock to synchronize the access to a regular collection, but using a thread-safe collection is a better idea.

Upvotes: 3

Related Questions