2
\$\begingroup\$

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?

\$\endgroup\$
3
\$\begingroup\$

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.

\$\endgroup\$
-1
\$\begingroup\$

If you've showed the environment the function is used in, the review could be more useful, but I think your reasoning is right, except that Lazy is used to defer instantiation and you actually use the lazy-created object (IEnumerable<dynamic>) the first time it is called for by returning DataCache[key].Value. Therefore Lazy is of little use in this context as its object is instantiated at the same time as the Lazy.

IMO your DataCache would give more meaning if you up front filled it with all possible queries without actually using them and then call them as needed. Alternatively you could just cache the queries directly in the DataCache without Lazy.

\$\endgroup\$
  • \$\begingroup\$ This approach is useful if you consider the possibility of calls to this method, to get a value for a key while another thread is currently in the process of computing that value. With the OP's approach, the computation is done once, if you only cached the values after you computed them, you'd compute it twice. \$\endgroup\$ – Servy Nov 30 '16 at 22:26

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.