Parallel.foreach doesnt process all items

Question

Im having a problem here. im trying to use Parallel.foreach to convert my datatable to a list object. like this .

public List<ProductList> GetProductList(DataTable table)
{
    List<ProductList> list = new List<ProductList>();
    Parallel.ForEach(table.AsEnumerable(), item =>
    {

            string sku = item["product_sku"].ToString();
            //int skus = Convert.ToInt16(item["product_sku"]);

            string price = item["product_price"].ToString();

            string tweakerID = item["ID"].ToString();
            string finalPrice = item["FinalPrice"].ToString();
            list.Add(new ProductList() { SKU = sku, Price = price, ID = id, FinalPrice = finalPrice });


    });





    list.RemoveAll(item => item == null);

    return list;
}

i have over 65000 product rows. and after this . there are only about 63000 products added in to the list. but the result is not a fix number. for example last three times that i ran this code i got 63202 , 64025 , 62920 . and everytime its a new number.

i also get no exception .

MadOX is right about List<T> not being thread safe, but I'm wondering if doing this in a normal foreach might be just a fast if not faster. Write some code and benchmark each way to see which is more efficient. Another alternative would have to been to use a lock. — Kevin
– Kevin, Commented Dec 17, 2016 at 14:41
I actually did. and in parallel.foreach i got 60k+ rows in 1 or 2 sec . but in normal foreach . i get it in 100-150 sec. its really an improvement — Saeed Asgari
– Saeed Asgari, Commented Dec 17, 2016 at 14:48

madoxdev · Accepted Answer · 2016-12-17 13:41:00Z

15

Thats because List<T> is not concurent safe. Try that: ConcurrentBag<T> instead.

ConcurentBag exists in System.Collections.Concurrent namespace, which contains few more thread safe collections.

All items are processed but not all are added to List. Reason for that is, when two threads try to add different items on exactly same time, list is not able to deal with it, and saves only one of them.

answered Dec 17, 2016 at 13:41

madoxdev

3,9301 gold badge26 silver badges42 bronze badges

Sign up to request clarification or add additional context in comments.

6 Comments

Saeed Asgari Over a year ago

It works . thanks . but i get a little bit slower performance. but its okey . thanks MadOX

Saeed Asgari Over a year ago

Im gonna select your answer as solved. i have to wait about 10 min to do that

madoxdev Over a year ago

It is slower as it creates lock object on Add method, so sometimes threads must wait.

Saeed Asgari Over a year ago

but it doesnt have list.RemoveAll(item => item == null); how can i do this with ConcurentBag . to remove all null objects?

madoxdev Over a year ago

It cannot be null, as you are adding always new Product. I think that RemoveAll is not necessary and probably exists because parallel operations added some Nulls to your non concurent list

|

Collectives™ on Stack Overflow

Parallel.foreach doesnt process all items

1 Answer 1

6 Comments

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

6 Comments

Related