0

I have a custom Dictionary<T> that has a backing collection of KeyedCollection. While trying to optimize performance issues that I'm seeing while running in a profiler, I noticed that the IndexOf(T key) is one of my problem areas. The code is currently implemented as the following:

public int IndexOf(TKey key)
{
     if (keyedCollection.Contains(key))
     {
         return keyedCollection.IndexOf(keyedCollection[key]);
     }
     else
     {
         return -1;
     }
}

I know that both Contains(T key) and IndexOf(T key) have big-O runtimes of O(n) and have confirmed this on the MSDN site. (https://msdn.microsoft.com/en-us/library/ms132438(v=vs.110).aspx)
I thought a good way to optimize this code would be to take out one of the O(n) operations, so I changed the code to this:

public int IndexOf(TKey key)
{
     try
     {
         return keyedCollection.IndexOf(keyedCollection[key]);
     }
     catch (KeyNotFoundException)
     {
         return -1;
     }
}

When I compared the runtimes between the 2 over about 500,000 operations, the code with the Contains(T key) out performed the try-catch scenario by a factor of nearly 2.

My question is, is there a huge amount of overhead when using a try-catch block that would greatly decrease performance?

M.kazem Akhgary
  • 18,645
  • 8
  • 57
  • 118
Middas
  • 1,862
  • 1
  • 29
  • 44
  • Why are you not using the .NET `Dictionary`? If you're implementing your own using a linear search, it's *going* to be a *lot* slower. Don't do that. – Servy Nov 25 '15 at 18:50
  • index on dictionary does not make sense. do you mean its `List`? also dictionary type is `Dictionary` – M.kazem Akhgary Nov 25 '15 at 18:50
  • It's an OrderedDictionary, I know there are performance hits in doing that by itself, but I'm trying to optimize it as much as possible. – Middas Nov 25 '15 at 18:52
  • 1
    @Middas Using a custom implementation that's dramatically slower isn't a good way to improve performance. Just use the .NET implementation. – Servy Nov 25 '15 at 18:52
  • @Servy I agree with you; however, in my scenario, I needed the flexibility that this custom implementation provides. I tried switching it out with a List of objects that had a Name and the Value in the object, but there was no performance gain. – Middas Nov 25 '15 at 18:56
  • If you have additional behavior to provide then encapsulate a `Dictionary`, don't rewrite one. Using a `List` will of course be just as slow, that's the same (flawed) design as what you've shown here. You need to avoid a linear search to improve performance. – Servy Nov 25 '15 at 19:02
  • So KeyedCollection has a protected dictionary. Add a method `TryGetItem` to your KeyedCollection that calls `TryGetValue` on the dictionary. Then you could have something like the first method except you would only need 1 lookup by key and there would be no exception to handle. – Mike Zboray Nov 25 '15 at 19:02
  • @mikez Wouldn't that have the same overhead as my try-catch implementation because the exception is still thrown in the underlying class? – Middas Nov 25 '15 at 19:05
  • @Middas No. That is the point of `TryGetValue`. It is not a wrapper that catches an internally generated exception for you. It directly does the lookup and returns a boolean indicating success or failure. The indexer does the same lookup but throws if the item is not found. – Mike Zboray Nov 25 '15 at 19:10
  • @mikez Thanks, I'll give that a try as well to see if there are performance gains. – Middas Nov 25 '15 at 19:23

1 Answers1

3

Throwing an exception here is going to be O(1), because the cost of throwing and catching the exception is in no way dependent on the size of the collection; it's going to be a fixed cost. That fixed cost may be high, but it won't grow.

Servy
  • 202,030
  • 26
  • 332
  • 449
  • Based on this answer, what is the better implementation? The fixed O(1) of the try-catch implementation knowing that while the size is under a certain amount it will be slower, but the runtime will never increase? – Middas Nov 25 '15 at 18:59
  • @Middas The better implementation is to fix the core algorithm to one that has a better runtime, namely a hash based lookup or a tree based structure, rather than doing linear searches. – Servy Nov 25 '15 at 19:01
  • Thanks for your input, I'll look into that. – Middas Nov 25 '15 at 19:03