Lock on Dictionary's TryGetValue() - Performance issues

Question

I've profiled my application and ran some performance tests which led me to believe that the following if-lock-if arrangement:

private float GetValue(int id)
{
    float value;
    if (!dictionary.TryGetValue(id, out value))
    {
      lock (lockObj)
      {
        if (!dictionary.TryGetValue(id, out value))
        {
          value = ComputeValue(id);
          dictionary.Add(id, value);
        }
      }
    }
}

seems to perform faster than "lock-if" or using ReaderWriterLockSlim. But very rarely, I get the following exception:

1) Exception Information
*********************************************
Exception Type: System.NullReferenceException
Message: Object reference not set to an instance of an object.
Data: System.Collections.ListDictionaryInternal
TargetSite: Int32 FindEntry(TKey)
HelpLink: NULL
Source: mscorlib

StackTrace Information
*********************************************
  at System.Collections.Generic.Dictionary`2.FindEntry(TKey key)
  at System.Collections.Generic.Dictionary`2.TryGetValue(TKey key, TValue& value)
  at MyNamespace.GetValue()
  .....
  .....

What am I doing wrong here ?

Edit: To clarify, this method is called on average more than 50 million times, and the conflict is generally less than 5,000.

Thanks

Answer 1

What you're trying to do here is simply not a supported scenario. The TryGetValue occurs outside of the lock which means is very possible for one thread to be writing to the dictionary while others are simultaneously calling TryGetValue. The only threading scenario inherently supported by Dictionary<TKey, TValue> is reads from multiple threads. Once you start reading and writing from multiple threads all bets are off.

In order to make this safe you should do one of the following

• Use a single lock for all read or write accesses to the Dictionary
• Use a type like ConcurrentDictionary<TKey, TValue> which is designed for multi-threaded scenarios.

Answer 2

Either the usage of that collection by your code is thread-safe, in which case you don't need the lock, or it's not thread-safe, in which case you ALWAYS need the lock.

Try this using ConcurrentDictionary instead, which is thread-safe.

Answer 3

Dictionary isn't thread-safe. If anything's adding to the dictionary when you do a TryGetValue, things can go badly. Your first call to TryGetValue is not protected by a lock. So if thread A is doing an Add and thread B enters that first TryGetValue, it can throw an exception.

Consider using System.Collections.Concurrent.ConcurrentDictionary. Or be sure to lock the dictionary on every access. Probably using ReaderWriterLockSlim.