1
\$\begingroup\$

I'm filling a list of items from the database, which is taking extremly long time to run (Approx 60 seconds)

Here is the code with the relevant breakdown

97% of the performance issues come in the foreach loop at the bottom. Is there a better way to match list items on the nested object.

private List<ROPSetting> TestMethod() 
{

    using DataSet ds = DB.GetDs($"EXEC GetSupplyChainROP_C @DateFrom={DateTime.Now.AddYears(-1).ToDbQuote()}");
    // load DS takes 3 seconds. 

    using var itemReader = ds.Tables[0].CreateDataReader();

    var items = itemReader.ParseList<ROPSetting>().ToList();

    using var usageReader = ds.Tables[1].CreateDataReader();
    var usageItems = usageReader.ParseList<ItemUsage>().ToList();

    // 97% of the performance impact is below.  Is there a better way to match 
    foreach (var itm in items)
    {
        itm.Usage = usageItems
                     .Where(a => a.ItemCode == itm.ItemCode && a.WarehouseCode == itm.WarehouseCode).ToList();

    }

    return items;

}


public class ItemUsage
{
    public DateTime Date { get; set; }
    public decimal Quantity { get; set; }
    internal string ItemCode { get; set; } // for matching
    internal string WarehouseCode { get; set; } // for matching
}
public class ROPSetting
{
    public string ItemCode { get; set; }
    public string PartNumber { get; set; }
    public decimal AverageCost { get; set; }
    public string SupplierCode { get; set; }
    public string SupplierName { get; set; }
    public int TotalLeadTime { get; set; }
    public int IntervalTime { get; set; }
    public int ReorderPoint { get; set; }
    public string WarehouseCode { get; set; }
    public IList<ItemUsage> Usage { get; set; }
}
\$\endgroup\$
5
  • \$\begingroup\$ if you edit the store procedure, and add the filtering part on the store procedure (from the DBMS), it'll perform faster and would also short your code as well. \$\endgroup\$
    – iSR5
    Jan 9, 2020 at 21:22
  • \$\begingroup\$ @iSR5 how? I need the 2 result sets in a separate table, i don't want to go down the track of returning a single table and then grouping. \$\endgroup\$ Jan 9, 2020 at 21:25
  • \$\begingroup\$ you'll only do it once, and get the results. You can create new or modify GetSupplyChainROP_C procedure. You just want to join the two tables, and select the columns you want from both, then just call the procedure, and get its result. \$\endgroup\$
    – iSR5
    Jan 9, 2020 at 21:42
  • \$\begingroup\$ I assume the child table is already filtered down to just the rows that have parents in the first table. Otherwise it's best to do that \$\endgroup\$ Jan 9, 2020 at 22:06
  • \$\begingroup\$ @CharlesNRice yes that's correct. First table has approx 6500 parents, second table about 25000 children \$\endgroup\$ Jan 9, 2020 at 22:24

1 Answer 1

3
\$\begingroup\$

Option 1) You can create a DataRelations between the two Data Tables and change the loop to fill in both classes at the same time. This would require you do change the code that converts the results set into C# classes. Microsoft has an example of GetChildRows and navigation here

Option 2) Otherwise you could also create a DataView and set the Sort property on the child Data Table and use FindRows method. Again will require you to change how you convert from datatable to C# classes. I'd lean toward the first option instead of creating another set of data objects just for sorting.

Option 3) I'm guessing it will not be much better performance but you can try is using the Linq GroupJoin

foreach (var data in items.GroupJoin(usageItems,
    x => new {x.ItemCode, x.WarehouseCode},
    x => new {x.ItemCode, x.WarehouseCode},
    (itm, usg) => new
    {
        item = itm,
        usage = usg.ToList()
    }))
{
    data.item.Usage = data.usage;
} 

Update #1 - Couple more options.

Option 4) Parallel the finding of data. Take Option 3 and add the AsParallel()

foreach (var data in items.AsParallel().GroupJoin(usageItems.AsParallel(),
    x => new {x.ItemCode, x.WarehouseCode},
    x => new {x.ItemCode, x.WarehouseCode},
    (itm, usg) => new
    {
        item = itm,
        usage = usg.ToList()
    }))
{
    data.item.Usage = data.usage;
} 

I still think option 1 would be the best option as it will create a binary search tree for the relationship and give best performance.

Option 5) if you know the usageItems are sorted by the ItemCode and WarehouseCode you could just iterate over the list once and fill in the Item as it scans over the list. This would be the most efficient but would require more complex code and also required that sort order is always correct. While having this sort order would help Option 1 it wouldn't be required.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.