1

This code is running quite slowly:

public static class DB
    {
        readonly static InlineSql sql = new InlineSql();

        public static IEnumerable<LabItem> GetLabItemsFromLabLineItems(IEnumerable<LabLineItem> items)
        {
            /*
             * Business Rule : 
             * The sproc used to retrieve lab line items has no way of "grouping" lab items out of line items.
             * To compensate, the sproc orders its results by IDCode, we group everything and use the first of each grouping to represent that ID Code.
             * That very same item is also the first item in its Items property.
             * */
            return items
                .GroupBy(c => c.IDCode , c => c, (c, d) => d.First())
                .Select(c => new LabItem(c.IDCode, c.OrderGuid, c.Name, c.SignificantDtm, c.Status, c.Description, items.Where(d => string.Compare(d.IDCode, c.IDCode, true) == 0 )));
        }        
    }

Particularly the select statement where we compare d.IDCode to c.IDCode appears to be the problem. This line reports a hit count of 90million from ANTS with a %Time of 14.8. items.count is around 9 thousand.

I know my breakpoint is not hit 90 million times. What does hit count mean here?

Other useful code:

LabItem has List<LabLineItem> which is what we compare here. LabLineItem.Equals:

    public override bool Equals(object obj)
    {
        LabLineItem otherItem = obj as LabLineItem;

        if (otherItem == null) return false;

        return
            OrderGuid == otherItem.OrderGuid
            && IDCode == otherItem.IDCode
            && SignificantDtm == otherItem.SignificantDtm
            && ObservationGuid == otherItem.ObservationGuid
            && string.Compare(Value, otherItem.Value, true) == 0;
    }
    public override int GetHashCode()
    {
        return
            OrderGuid.GetHashCode()
            ^ IDCode.GetHashCode()
            ^ SignificantDtm.GetHashCode()
            ^ ObservationGuid.GetHashCode();
    }
P.Brian.Mackey
  • 43,228
  • 68
  • 238
  • 348
  • Hmmm, potentially nested enumerators can create severe performance issues...http://stackoverflow.com/questions/1043050/c-sharp-performance-of-nested-yield-in-a-tree. On the ANTS fron I always though the hit count was the 'actual' hit count. This looks helpful: http://stackoverflow.com/questions/8454084/what-is-time-and-hit-count-in-ants-profiler – SpruceMoose Nov 27 '12 at 17:26
  • 1
    @Cal279 - I suspect it's stepping into the delegates. I'm not stepping into the delegates. I step over that line. Maybe that's the difference. – P.Brian.Mackey Nov 27 '12 at 17:32
  • @P.Brian.Mackey Invoked delegates do indeed add to the hit-count. – Dene B Dec 21 '12 at 15:42

1 Answers1

4

ANTS is saying that the Select with the string.Compare call is hit 90 million times because for each item in the main list, it is searching the entire list again.

Each of the primary 9000 iterations causes 9000 additional iterations, so the string.Compare has to be called at least 81,000,000 times.

I would suggest building a cache of the grouping, and then use that to construct the LabItem.

Maybe something like this:

var groupedItems = items.GroupBy(c => c.IDCode);

return items.Select(c => 
                new LabItem(c.IDCode, c.OrderGuid, c.Name, c.SignificantDtm, c.Status, c.Description, 
                groupedItems.Where(d => string.Compare(d.Key, c.IDCode, true) == 0 ).SelectMany(group => group)));
P.Brian.Mackey
  • 43,228
  • 68
  • 238
  • 348
davisoa
  • 5,407
  • 1
  • 28
  • 34
  • +1 This looks promising. I just need to figure out how to convert `IEnumerable>` back to `IEnumerable` or I'll have to change the LabItem constructor which I don't think I should be doing here. – P.Brian.Mackey Nov 27 '12 at 18:04
  • What type does the constructor expect? `IGrouping` implements `IEnumerable`, so if the constructor expects `IEnumerable`, it should compile. – davisoa Nov 27 '12 at 18:09
  • Just needed to add `SelectMany`. Excellent answer. Thank you very much. – P.Brian.Mackey Nov 27 '12 at 18:13
  • Great! Thanks for getting that last bit. – davisoa Nov 27 '12 at 18:14