11

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)

Dusty
  • 413
  • 6
  • 17
  • 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 – Servy Sep 14 '12 at 01:28
  • 1
    Yes, just return `ToCheck.ContainsKey(Position)`. – nawfal Apr 11 '13 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?](http://stackoverflow.com/questions/16101795/why-is-it-faster-to-check-if-dictionary-contains-the-key-rather-than-catch-the) – nawfal Jun 19 '14 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). – ToolmakerSteve Jan 22 '18 at 03: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. – ToolmakerSteve Jan 22 '18 at 03:43
  • @ToolmakerSteve haha, this was so long ago back when I was first learning to program, I feel like I probably should have changed my approach to the problem to avoid needing to check dictionary keys every frame. – Dusty Feb 05 '19 at 02:52

4 Answers4

28

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);
      }

   }
}
Eliott
  • 281
  • 2
  • 2
  • In realistic situations, `ContainsKey` is always better. – nawfal Apr 11 '13 at 12:43
  • 2
    +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`. – SharpC Oct 08 '14 at 15:09
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.

McGarnagle
  • 101,349
  • 31
  • 229
  • 260
  • 4
    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. – Dan Sep 14 '12 at 00:49
  • 1
    +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). – Alexei Levenkov Sep 14 '12 at 00:49
  • 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 Sep 14 '12 at 00:53
  • 2
    @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. – Kirk Woll Sep 14 '12 at 02:09
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.

Jan
  • 2,168
  • 2
  • 19
  • 28
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.

MgSam
  • 12,139
  • 19
  • 64
  • 95
  • 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 :) – Dusty Sep 14 '12 at 03:19