HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Locking during cache population

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
duringcachelockingpopulation

Problem

Here I want to lock when populating a particular cache object, without blocking other calls to the same method requesting Foos for other barIds. I realise the MemoryCache will be thread safe, but if two or more concurrent calls come in it seems like it would be better if only one of them populates the cache and locks the others out while this is done.

using System.Runtime.Caching;
using Microsoft.Practices.Unity;

public class FooCacheService : IFooService
{
        private static readonly ConcurrentDictionary FooServiceCacheLocks = new ConcurrentDictionary();

        [Dependency("Explicit")]
        public IFooService ExplicitService { get; set; }

        public IEnumerable GetFoosForBar(int barId)
        {
            string key = barId.ToString();

            object lockObject = FooServiceCacheLocks.GetOrAdd(key, new object());

            object cached = MemoryCache.Default[key];

            if (cached == null)
            {
                lock (lockObject)
                {
                    cached = MemoryCache.Default[key];

                    if (cached == null)
                    {
                        IEnumerable foosForBar = this.ExplicitService.GetFoosForBar(barId);

                        MemoryCache.Default.Add(key, foosForBar, DateTime.Now.AddMinutes(5));

                        return foosForBar;
                    }
                }
            }

            return (IEnumerable)cached;
        }
}


Assume GetFoosForBar is a CPU/IO intensive process and takes a short while to complete. Does this seem like a reasonable way to achieve this? I'm surprised that System.Runtime.Caching does not provide any support for this as default - or have I overlooked something?

Solution

What I don't like about your approach is that the lock objects are never removed from FooServiceCacheLocks, even when the object is removed from the cache.

One way to simplify your code would be to combine MemoryCache with Lazy: instances of Lazy are cheap (as long as you don't access its Value) so you can create more of them than needed. With that your code would look something like this:

public IEnumerable GetFoosForBar(int barId)
{
    var newLazy = new Lazy>(
        () => this.ExplicitService.GetFoosForBar(barId));

    var lazyFromCache = (Lazy>)MemoryCache.Default.AddOrGetExisting(
        barId.ToString(), newLazy, DateTime.Now.AddMinutes(5));

    return (lazyFromCache ?? newLazy).Value;
}

Code Snippets

public IEnumerable<Foo> GetFoosForBar(int barId)
{
    var newLazy = new Lazy<IEnumerable<Foo>>(
        () => this.ExplicitService.GetFoosForBar(barId));

    var lazyFromCache = (Lazy<IEnumerable<Foo>>)MemoryCache.Default.AddOrGetExisting(
        barId.ToString(), newLazy, DateTime.Now.AddMinutes(5));

    return (lazyFromCache ?? newLazy).Value;
}

Context

StackExchange Code Review Q#32026, answer score: 8

Revisions (0)

No revisions yet.