12

I have a list of Vector2's Generated I have to check against a dictionary to see if they exist, this function gets executed every tick.

which would run fastest/ be better to do it this way?

 public static bool exists(Vector2 Position, Dictionary<Vector2, object> ToCheck) { try { object Test = ToCheck[Position]; return (true); } catch { return (false); } }

Or should I stick with The norm ?

 public static bool exists(Vector2 Position, Dictionary<Vector2, object> ToCheck) { if (ToCheck.ContainsKey(Position)) { return (true); } return (false); }

Thanks for the input :)

Side Note: (The Value for the key doesn't matter at this point or i would use TryGetValue instead of ContainsKey)

Improve this question
6
  • 2
    Why would you ever write the second method? You're literally wrapping a function call with another function call and doing nothing more. Rather than calling that function the caller can just call ContainsKey explicitly Commented Sep 14, 2012 at 1:28
  • 1
    Yes, just return ToCheck.ContainsKey(Position). Commented Apr 11, 2013 at 12:44
  • possible duplicate of Why is it faster to check if dictionary contains the key, rather than catch the exception in case it doesn't? Commented Jun 19, 2014 at 13:45
  • @nawfal - BTW, Servy is saying something deeper. Instead of simplifying that function (which you do an excellent job of saying what the simple one-line contents would be), don't write that method at all. Wherever someone would do exists(myPosition, myDictionary) they could simply make a standard call myDictionary.ContainsKey(myPosition. So that anyone reading the code doesn't have to go look up this mysterious exists, which doesn't add anything useful (it is not any simpler to call). Commented Jan 22, 2018 at 3:33
  • Actually, this question begs the question: what is @Dusty going to do with the result? This is an example of "optimization at too low a level". Usually unsuccessful. Instead, look at where exists or ContainsKey is used ("the callers" of exists). If any of those "callers" are performance-critical, then are they making multiple method calls on ToCheck, which could be replaced with fewer calls? The classic example is replacing ToCheck.ContainsKey( key ) + ... = ToCheck[key] with TryGetValue. That is where there is some hope of a performance gain. Commented Jan 22, 2018 at 3:43

4 Answers 4

Reset to default
29

I know it's an old question, but just to add a bit of empirical data...

Running 50,000,000 look-ups on a dictionary with 10,000 entries and comparing relative times to complete:

..if every look-up is successful:

  • a straight (unchecked) run takes 1.2 seconds
  • a guarded (ContainsKey) run takes 2 seconds
  • a handled (try-catch) run takes 1.21 seconds

..if 1 out of every 10,000 look-ups fail:

  • a guarded (ContainsKey) run takes 2 seconds
  • a handled (try-catch) run takes 1.37 seconds

..if 16 out of every 10,000 look-ups fail:

  • a guarded (ContainsKey) run takes 2 seconds
  • a handled (try-catch) run takes 3.27 seconds

..if 250 out of every 10,000 look-ups fail:

  • a guarded (ContainsKey) run takes 2 seconds
  • a handled (try-catch) run takes 32 seconds

..so a guarded test will add a constant overhead and nothing more, and try-catch test will operate almost as fast as no test if it never fails, but kills performance proportionally to the number of failures.

Code I used to run tests:

using System; using System.Collections.Generic; namespace ConsoleApplication1 { class Program { static void Main(string[] args) { Test(0); Test(1); Test(16); Test(250); } private static void Test(int failsPerSet) { Dictionary<int, bool> items = new Dictionary<int,bool>(); for(int i = 0; i < 10000; i++) if(i >= failsPerSet) items[i] = true; if(failsPerSet == 0) RawLookup(items, failsPerSet); GuardedLookup(items, failsPerSet); CaughtLookup(items, failsPerSet); } private static void RawLookup ( Dictionary<int, bool> items , int failsPerSet ){ int found = 0; DateTime start ; Console.Write("Raw ("); Console.Write(failsPerSet); Console.Write("): "); start = DateTime.Now; for(int i = 0; i < 50000000; i++) { int pick = i % 10000; if(items[pick]) found++; } Console.WriteLine(DateTime.Now - start); } private static void GuardedLookup ( Dictionary<int, bool> items , int failsPerSet ){ int found = 0; DateTime start ; Console.Write("Guarded ("); Console.Write(failsPerSet); Console.Write("): "); start = DateTime.Now; for(int i = 0; i < 50000000; i++) { int pick = i % 10000; if(items.ContainsKey(pick)) if(items[pick]) found++; } Console.WriteLine(DateTime.Now - start); } private static void CaughtLookup ( Dictionary<int, bool> items , int failsPerSet ){ int found = 0; DateTime start ; Console.Write("Caught ("); Console.Write(failsPerSet); Console.Write("): "); start = DateTime.Now; for(int i = 0; i < 50000000; i++) { int pick = i % 10000; try { if(items[pick]) found++; } catch { } } Console.WriteLine(DateTime.Now - start); } } }
Improve this answer
Sign up to request clarification or add additional context in comments.

2 Comments

In realistic situations, ContainsKey is always better.
+1 As I thought. If you want raw performance and you are confident that the lookup will very rarely fail, it's far better to use try-catch rather than ContainsKey.
18

Definitely use the ContainsKey check; exception handling can add a large overhead.

Throwing exceptions can negatively impact performance. For code that routinely fails, you can use design patterns to minimize performance issues.

Exceptions are not meant to be used for conditions you can check for.

I recommend reading the MSDN documentation on exceptions generally, and on exception handling in particular.

Improve this answer

4 Comments

Calling a method that you know throws an exception under certain circumstances is like driving your car with you eyes closed. There are better ways of discovering where the road leads than waiting until you hit or drive over something.
+1. In addition to being wrong code flow construct and huge runtime cost, exceptions make debugging much harder (assuming you have "break when thrown" checked for exceptions... which is a good idea to avoid exceptions in frequently executed pieces of code).
Thanks For the info, Helps a lot, I was wondering because i read in another post (I can't recall the main idea), some one responded saying a try catch would barely effect performance.
@Dusty, I've chronically had to correct that misconception. It's probably due to other languages (for example, Python) where using exceptions as control flow is strongly encouraged. But as far as .NET (and the JVM, for that matter) is concerned, you should never throw an exception unless it represents a scenario for which you cannot recover.
0

Never use try/catch as a part of your regular program path. It is really expensive and should only catch errors that you cannot prevent. ContainsKey is the way to go here.

Side Note: No. You would not. If the value matters you check with ContainsKey if it exists and retrieve it, if it does. Not try/catch.

Improve this answer

Comments

0

Side Note: (The Value for the key doesn't matter at this point or i would use TryGetValue instead of ContainsKey)

The answer you accepted is correct, but just to add, if you only care about the key and not the value, maybe you're looking for a HashSet rather than a Dictionary?

In addition, your second code snippet is a method which literally adds zero value. Just use ToCheck.ContainsKey(Position), don't make a method which just calls that method and returns its value but does nothing else.

Improve this answer

1 Comment

I do need the values, It's used for generating a map. The Side note was made Because I see a lot of people asking "Why arn't you using TryGetValue" When its not relevant to the question. But I will keep HashSet's in mind if the need ever arises thank you :)

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.