Converting a for loop into Task.Parallel.For

Question

I have a procedure bool IsExistImage(int i) . the task of the procedure to detect an image and return bool whether it exist or not.

i have a PDF of 100+ pages which i split and send only the file name through the method. file names are actually the pagenumber of the main PDF file. like 1,2,3,...,125,..

after detecting the image, my method correctly save the list of pages. For that i used this code:

ArrayList array1 = new ArrayList();
for(int i=1;i<pdf.length;i++)
{
   if(isExistImage(i))
   {
       array1.add(i);
   }
}

This process runs for more than 1 hours(obviously for the internal works in isExistImage() method.). I can assure you, that no object/variable are global out side the method scope.

So, to shorten the time, I used Task.Parallel For loop. here is what i did :

System.Threading.Tasks,Parallel.For(1,pdf.Length,i =>
{
    if(isExistImage(i))
        array1.Add(i);
}

But this is not working properly. Sometimes the image detection is right. But most of the time its wrong. When i use non parallel for loop, then it's always right.

I am not understanding what is the problem here. what should i apply here. Is there any technique i am missing?

Answer 1

Your problem is that ArrayList (and most other .Net collections) is not thread-safe.

There are several ways to fix this, but I think that in this case, the best option is to use PLINQ:

List<int> pagesWithImages = ParallelEnumerable.Range(1, pdf.Length)
    .Where(i => isExistImage(i))
    .ToList();

This will use multiple threads to call the (weirdly named) isExistImage method, which is exactly what you want, and then return a List<int> containing the indexes that matched the condition.

The returned list won't be sorted. If you want that, add AsOrdered() before the Where().

BTW, you really shouldn't be using ArrayList. If you want a list of integers, use List<int>.

Answer 2

1. ArrayList isn't thread safe; look into concurrent collections here.

2. is isExistImage thread safe? I.e. are you locking before updating any member variables??