Can we add new elements to a list using Parallel.ForEach()?

Question

My code does very simple stuff

list already has elements. I have approximately 25000 elements (and I'm expecting to have more) in the list and each element is small (DateTime).

List<DateTime> newList = new List<DateTime>();
Parallel.ForEach(list, l => newlist.Add(new DateTime(l.Ticks + 5000)));

i.e, based on each element, I'm creating new elements and adding them to a different list. But, this doesn't seem to be a good programming approach. I hit this exceptions some times, but not everytime.

IndexOutOfRangeException : {"Index was outside the bounds of the array."}

Can we add elements to a list using Parallel.ForEach()? If yes, why do I hit the error? If no, why?

Solution 1

What you would really want in this situation is more like this:

newlist = list.AsParallel().Select(l => new DateTime(l.Ticks + 5000)).ToList();

Although you should measure the performance to see if this situation even benefits from parallelization.

Solution 2

Try a thread local variable with a final result that adds all thread local variables to the newList as such...

Parallel.ForEach(list, () => DateTime.MinValue, (l, state, date) =>
{
    date = new DateTime(l.Ticks+5000);
    return date;
},
finalresult =>
{
   lock (newList)
   {
       newList.Add(finalresult);
   }
});

The first param is your old list the second Param is the initial value of each thread (I just initialized to datetime min). the third param block is as follows- the l is the same as in your code; the state is a Paralleloption object of which you can exit out of parallel loop if you choose; the last is the stand in variable that represents the thread local variable. The finalresult param represents the end result of each thread local variable and is called for each thread - it is there you can place a lock of the newList and add to the newList shared variable. In theory this works. I have used similar coding in my own code. Hope this helps you or someone else.

Solution 3

This will effectively call List<T>.Add concurrently, yet according to MSDN documentation for List<T>:

"Any instance members are not guaranteed to be thread safe."

Even if it were (thread safe), this is far too cheap to benefit from parallel execution (as opposed to overhead of parallel execution). Did you actually measure your performance? 25000 elements is not that many.

Solution 4

As everyone has mentioned, there seems to be no case for doing this parallel. It will certainly be far, far slower. However, for completion, the reason this sometimes fails is there is no lock on the list object that's being written to by multiple threads. Add this:

object _locker = new object();
List<DateTime> newList = new List<DateTime>();
Parallel.ForEach(list, l => lock (_locker) newlist.Add(new DateTime(l.Ticks + 5000)));

Solution 5

There simply is not enough work to do for this to warrant using Parallel.ForEach and also List<T> is not thread safe, so you would have to lock if you wanted to add to the same list in parallel. Just use a regular for loop.

View more solutions

Comments

[dansan]

According to Albahari talking about Parallel.Foreach in c#5, this is the way to go. PLINQ takes care of much of the work for you.

[Gabe]

@dansan: While the code is sound, the amount of work to be done in the method call is so small that it's quite likely that it would be no improvement (or even go slower).

[dansan]

Yes, understood. I know that there's quite the performance threshold before any baked-in parallelization actually improves performance. I'm simply noting that of the methods available in c# 5 (and discussed here), this is the most correct one unless you need more low-level control for some reason.