CODEWITHSUNDEEP
c#performancelinqforeach
Houlahan
Houlahan

Reputation: 783

Speeding up iterating through two foreach loops

Trying speed up iterating though two foreach loops at the moment it takes about 15 seconds` 

foreach (var prodCost in Settings.ProdCostsAndQtys)
{
    foreach (var simplified in Settings.SimplifiedPricing
        .Where(simplified => prodCost.improd.Equals(simplified.PPPROD) && 
               prodCost.pplist.Equals(simplified.PPLIST)))
    {
        prodCost.pricecur = simplified.PPP01;
        prodCost.priceeur = simplified.PPP01;
    }
}

Basically the ProdCostsAndQtys list is a list of objects which has 5 properties, the size of the list is 798677

The SimplifiedPricing list is a list of objects with 44 properties, the size of this list is 347 but is more than likely going to get a lot bigger (hence wanting to get the best performance now).

The loop iterates through all the objects in the first list within the second loop if the two conditions match they replace the two properties from the first loop with the second loop.

Upvotes: 1

Views: 210

Answers (3)

Tim Schmelter
Tim Schmelter

Reputation: 460058

A join should be more efficient:

var toUpdate = from pc in Settings.ProdCostsAndQtys
               join s in Settings.SimplifiedPricing
               on new { prod=pc.improd, list=pc.pplist } equals new { prod=s.PPPROD, list=s.PPLIST }
               select new { prodCost = pc, simplified = s };
foreach (var pcs in toUpdate)
{
    pcs.prodCost.pricecur = pcs.simplified.PPP01;
    pcs.prodCost.priceeur = pcs.simplified.PPP01;
}

Upvotes: 4

Bas
Bas

Reputation: 27085

It seems that your SimplifiedPricing is a smaller lookup list and the outer loop iterates on a larger list. It looks to me as if the main source of delay is the Equals check for each item on the smaller list to match each item in the larger list. Also, when you have a match, you update the value in the larger list, so updating multiple times looks redundant.

Considering this, I would suggest building up a Dictionary for the items in the smaller list, increasing memory consumption but drastically speeding up lookup times. First we need something to hold the key of this dictionary. I will assume that the improd and pplist are integers, but it does not matter for this case:

public struct MyKey
{
    public readonly int Improd;
    public readonly int Pplist;

    public MyKey(int improd, int pplist)
    {
        Improd = improd;
        Pplist = pplist;
    }

    public override int GetHashCode()
    {
        return Improd.GetHashCode() ^ Pplist.GetHashCode();
    }

    public override bool Equals(object obj)
    {
        if (!(obj is MyKey)) return false;

        var other = (MyKey)obj;
        return other.Improd.Equals(this.Improd) && other.Pplist.Equals(this.Pplist);
    }
}

Now that we have something that compares Pplist and Improd in one go, we can use it as a key for a dictionary containing the SimplifiedPricing.

IReadOnlyDictionary<MyKey, SimplifiedPricing> simplifiedPricingLookup =
    (from sp in Settings.SimplifiedPricing
     group sp by new MyKey(sp.PPPROD, sp.PPLIST) into g
     select new {key = g.Key, value = g.Last()}).ToDictionary(o => o.key, o => o.value);

Notice the IReadOnlyDictionary. This is to show our intent of not modifying this dictionary after its creation, allowing us to safely parallelize the main loop:

Parallel.ForEach(Settings.ProdCostsAndQtys, c =>
{
    SimplifiedPricing value;
    if (simplifiedPricingLookup.TryGetValue(new MyKey(c.improd, c.pplist), out value))
    {
        c.pricecur = value.PPP01;
        c.priceeur = value.PPP01;
    }
});

This should change your single-threaded O(n²) loop to a parallelized O(n) loop, with a slight overhead for creating the simplifiedPricingLookup dictionary.

Upvotes: 4

realbart
realbart

Reputation: 3965

You could make use of multiple threads with parallel.Foreach:

Parallel.ForEach(Settings.ProdCostsAndQtys, prodCost =>
{
    foreach (var simplified in Settings.SimplifiedPricing
      .Where(simplified => 
        prodCost.improd.Equals(simplified.PPPROD) && 
        prodCost.pplist.Equals(simplified.PPLIST))
    {
        prodCost.pricecur = simplified.PPP01;
        prodCost.priceeur = simplified.PPP01;
    }
}

However, this only applies if you have the lists in memory. There are far more efficient mechanisms for updating the lists in the database. Also, using linq join might make the code more readable at neglectible performance cost.

Upvotes: 2

Related Questions