Reputation: 2237
My question is little theoretical I want to know whether List<object>
is thread-safe if I used Parallel.For
in this way. Please see below:
public static List<uint> AllPrimesParallelAggregated(uint from, uint to)
{
List<uint> result = new List<uint>();
Parallel.For((int)from, (int)to,
() => new List<uint>(), // Local state initializer
(i, pls, local) => // Loop body
{
if (IsPrime((uint)i))
{
local.Add((uint)i);
}
return local;
},
local => // Local to global state combiner
{
lock (result)
{
result.AddRange(local);
}
});
return result;
}
Is local
list is thread safe? Whether I've the correct data in result
list without data is being alter due to multiple threads as using I'm having normal loop?
Note: I'm not worry about list order. I want to know about length of the list and data.
Upvotes: 7
Views: 2160
Reputation: 52240
Is this solution thread-safe? Technically yes, in reality no.
The very notion of a thread-safe List
(as opposed to a queue or a bag) is that the list is safe with respect to order, or, more strictly, index, since there is no key other than an ascending integer. In a parallel world, it's sort of a nonsensical concept, when you think about it. That is why the System.Collections.Concurrent namespace contains a ConcurrentBag
and a ConcurrentQueue
but no ConcurrentList
.
Since you are asking about the thread safety of a list, I am assuming the requirement for your software is to generate a list that is in ascending order. If that is the case, no, your solution will not work. While the code is technically thread-safe, the threads may finish in any order and your variable result
will end up non-sorted.
If you wish to use parallel computation, you must store your results in a bag, then when all threads are finished sort the bag to generate the ordered list. Otherwise you must perform the computation in series.
And since you have to use a bag anyway, you may as well use ConcurrentBag, and then you won't have to bother with the lock{}
statement.
Upvotes: 7
Reputation: 19149
List Is not thread safe. but your current algorithm works. as it was explained in other answer and comments.
This is the description about localInit of For.Parallel
The
<paramref name="localInit"/>
delegate is invoked once for each thread that participates in the loop's execution and returns the initial local state for each of those threads. These initial states are passed to the first
IMO You are adding unnecessary complexity inside your loop. I would use ConcurrentBag
instead. which is thread safe by design.
ConcurrentBag<uint> result = new ConcurrentBag<uint>();
Parallel.For((long) from, (long) to,
(i, PLS) =>
{
if (IsPrime((uint)i))
{
result.Add((uint)i); // this is thread safe. don't worry
}
});
return result.OrderBy(I => I).ToList(); // order if that matters
All public and protected members of ConcurrentBag are thread-safe and may be used concurrently from multiple threads.
Upvotes: 3
Reputation: 14007
A List<T>
is not thread-safe. For the way you are using it, thread safety is not required though. You need thread safety when you access a resource concurrently from multiple threads. You are not doing that, since you are working on local lists.
At the end you are adding the contents of the local lists into the result
variable. Since you use lock
for this operation, you are thread-safe within that block.
So your solution is probably fine.
Upvotes: 2