Using Concurrent Dictionary and Lazy<T> to cache expensive query results and only run query once in a threadsafe manner

Question

Ok, so I'm querying a webservice. This webservice is slow with multiple o's. I want to cache the results of the query, because I only want to query on a given set of parameters once during my transaction (In addition to the query being slow, it's also rate limited, so I don't want to perform unnecessary queries). Finally, to speed things up, I want to be able to run queries in parallel, as latency is my biggest problem here.

With all of that, I think I need a thread safe way to cache data and only run the queries I actually need to run. I think I've managed to cobble that together with Concurrent Dictionary and Lazy

private static ConcurrentDictionary<KeyObject, Lazy<IEnumerable<ValueObject>>> DataCache =
new ConcurrentDictionary<KeyObject, Lazy<IEnumerable<ValueObject>>>();

    public static IEnumerable<ValueObject> GetData(KeyObject key, QueryObject query)
    {
        var value = new Lazy<IEnumerable<ValueObject>>(() =>
        {
            return query.Run();
        });
        DataCache.TryAdd(key, value);
        return DataCache[key].Value;
    }
}

If I'm correct in understanding how all the moving parts work, it should go something like this.

1. Generate dictionary key and Lazy intializer for value
2. Try and add lazy initializer to dictionary with key. If another thread has already added key, fail and continue.
3. Try and get value of lazy intializer on record in the dictionary. If another thread is already getting value, block until other thread has retrieved value.

Does this sound correct?

Answer 1

What you have will work, but there are a few incremental improvements. Rather than using TryAdd you can use GetOrAdd which returns the value for that key. This will save you a lookup (and also remove a race condition where the key is removed between calls by another thread).

Also note that, currently, you're caching any errors that you get, and continuing to serve them out. If you're okay with that, or aren't worried about it, then okay.

You should do some tests to be sure, but it may be faster to use the overload of GetOrAdd that accepts a function, rather than a value. This will allow you to construct the Lazy only when a new one is needed, rather than on every call. Constructing a Lazy isn't that expensive (it doesn't need to actually compute the value after all) but this is likely still a win in most situations.