CODEWITHSUNDEEP
c#parallel-processing
ebvtrnog
ebvtrnog

Reputation: 4367

C# parallelism in LINQ

I would like to know if this is a safe way of calculating x in the code below.

public static IEnumerable<Object> Parse(Object obj, ref int x)
{
    x += 10;
    return new List<Object>();
}

public static void Query(List<Object> obj)
{
    int x = 0;

    var result = obj
        .AsParallel()
        .Select(o => Parse(o, ref x))
        .Aggregate((a, b) => a.Concat(b));
}

This is a shortened version of my code. I want the x to be some kind of a static counter for all parallel executions of Parse. I hope this is not confusing.

Upvotes: 2

Views: 111

Answers (3)

Mrinal Kamboj
Mrinal Kamboj

Reputation: 11482

I would suggest a different approach to tackle the issue, as suggested earlier introducing a synchronization construct in the Parallel code would impact its working, if you still need it then your original code need something like Interlocked / lock to make it thread safe, however

A better way would be each thread have a local counter and aggregate that at the end, something like this:

public class MyClass
{
  public int x;
  public object o;
}

public static IEnumerable<MyClass> Parse(Object obj)
{
    MyClass c = new MyClass();
    c.x += 10;
    c.o  = <some new object>
    // Add c to instance of List<MyClass>
    return new List<MyClass>();
}

public static void Query(List<Object> obj)
{          
    var result = obj
        .AsParallel()
        .Select(o => Parse(o))

   // result is of type IEnumerable<MyClass>

   var sum = result.Sum(a=>a.x);

   var aggregate = result.Aggregate((a, b) => a.o.Concat(b.o));
}

This is a lock / synchronization free solution which has no performance hit and there's no race condition. Always for threading, try to make things local for a thread and later on apply a function like sum for every individual thread variable.

Upvotes: 2

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726479

Your code has a race condition. Even though variable x is passed by reference, it remains the same variable in all concurrent executions, so adding ten to it needs to be atomic.

One way to fix this would be using Interlocked.Add method instead of +=:

public static IEnumerable<Object> Parse(Object obj, ref int x)
{
    Interlocked.Add(ref x, 10);
    return new List<Object>();
}

Upvotes: 4

Amit
Amit

Reputation: 46323

Definitely not safe. You need to use the Interlocked class.

public static IEnumerable<Object> Parse(Object obj, ref int x)
{
    Interlocked.Add(ref x, 10);
    return new List<Object>();
}

Upvotes: 4

Related Questions