1

How would i improve this?

the profile is suggesting that fetching data from a list is slow

public List<RowDataDto> Rows { get; set; }

public RowDataDto GetRow(int rowNdx)
{
      var row = Rows.SingleOrDefault(x => x.LineNumber == rowNdx);
      return row;   
}

The list will on average contain around 1000 items im just benchmaching to see how it performance.

its saying the slowest part is fetching from an in memory list.

Rows is populated well before GetRow is called.

LineNumber is an int32

PS - I have just installed dotTrace and im very new to it so I may not be understanding how to use the tool properly. I have simple tests. I start ".Net Process" tell my test to run and at some point hit take snapshot.

If you have used this tool before please guide me.

dot trace profile

Seabizkit
  • 2,417
  • 2
  • 15
  • 32
  • 1
    Well, searching through 1000 items in a list is not a big deal, but if performance is an important factor for you, I would change a couple of things: Firstly, you should limit the usage of lambda expressions in the more frequent methods. Secondly, instead of using Linq, use for or at least foreach – Vahid K. Jun 22 '17 at 09:10
  • Lambda expressions with heap closures are evil in the terms of GC. I have had a lot of problems for performance optimizations in the past – Vahid K. Jun 22 '17 at 09:11
  • @VahidK. could you show me an example of what you mean. – Seabizkit Jun 22 '17 at 09:17
  • 3
    If you will identify this as a real bottleneck - use `Dictionary` instead of list (keyed by LineNumber). – Evk Jun 22 '17 at 09:22
  • SingleOrDefault is slower than FirstOrDefault (the whole collection is explored even after the item is found). Indexing your data as per @Evk's solution is the sensible one though, provides exponential performance improvement over the list (search is O(log n)), and also ensures your items are unique per key (which is what SingleOrDefault does that FirstOrDefault doesn't). – Evren Kuzucuoglu Jun 22 '17 at 09:29
  • @Evk if its right which i have no reason to doubt at this point the difference is 00h:00m:04s:0246ms from 00h:05m:41s:0167ms just changing to a dictionary, that's insane. – Seabizkit Jun 22 '17 at 09:51
  • Well that means you make really A LOT of calls to that `GetRow` method :) – Evk Jun 22 '17 at 09:54
  • @Evk benchmarking hehe ;-) and goes to show that if you do you can make quite a difference with relatively small change. My app is quite complicated and yes will be calling this function on avg about 1000 x[20 between 60] but the 1000 could change to 10 000 and i want it to be able to preform this in seconds. – Seabizkit Jun 22 '17 at 09:58

1 Answers1

0

If you have in-memory list and you want to perform certain lookup a lot of times on it - it's better to use dictionary instead of list. For ease of use you can expose readonly collection of that dictionary values if you need to (it's usually not good to expose dictionary as a public member):

private Dictionary<int, RowDataDto> _rows;
public IReadOnlyCollection<RowDataDto> Rows {get {return _rows.Values;}}

public RowDataDto GetRow(int rowNdx)
{
    var row = _rows.ContainsKey(rowNdx) ? _rows[rowNdx] : null;
    return row;   
}

Also note that using SingleOrDefault is rarely good. If your list is not expected to contain duplicates - just use FirstOrDefault. SingleOrDefault will have to go through the whole list to ensure that there are no duplicates, which in such case means it will always go through the whole list.

Evk
  • 98,527
  • 8
  • 141
  • 191