Let the Learnings Ensue

January 18, 2012

Nerd

Please forgive any lack of clarity, a I am posting this at 1:45am.

I was asked to implement caching on a project today. At my work, it has historically been our lead architect who has implemented caching across projects, so I was happy to get a new task.

So I cracked open his code (like ya do) & copied out something similar to this;

public List<City> GetActiveCities() {

string cacheKey = “GetCities”;
List<City> retVal = CacheManager.Get(cacheKey) as List<City>;
if (retVal == null) {
lock (__cacheLockGetActiveCities) {
retVal = CacheManager.Get(cacheKey) as List<City>;
if (retVal == null) {
retVal = _dbServices.GetActiveCities();
CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
}
}
}
return retVal;
}

After the 3rd time I copied/pasted/modified this, I wanted to pull it out and leave less room for careless copy/paste mistakes.

This turned out to be a bit of a challenge, but I got it working with Generics and a Func param like this:

private T HandleItemCacheRetrieval<T>(string cacheKey, object cacheLock, Func<T> nonCachedMethod) where T : new() {
T retVal = default(T);
try {
retVal = (T)CacheManager.Get(cacheKey);
}
catch (InvalidCastException ex) //it didn’t parse properly
{
LoggerManager.Error(ex);
}
if (retVal == null) {
lock (cacheLock) {
try {
retVal = (T)CacheManager.Get(cacheKey); //make sure it’s still null (i.e. it didn’t get written while lock was being applied)
}
catch (InvalidCastException ex1) //it didn’t parse properly
{
LoggerManager.Error(ex1);
}
retVal = nonCachedMethod.Invoke();
if (retVal != null) {
CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
}
}
}
return retVal;
}

Which enabled the first method (and all methods that returned void) to call it this easily:
public List<City> GetActiveCities() {
return HandleItemCacheRetrieval<List<City>>(“GetActiveCities”, _cacheLockGetActiveCities, _dbServices.GetActiveCities);

But I’m not just returning collections from parameterless methods. So I tried to extend it to something like this with no success:

private T HandleItemCacheRetrieval<T>(string cacheKey, object cacheLock, Func<T> nonCachedMethod, params object[] args) where T : new() {
T retVal = default(T);
try {
retVal = (T)CacheManager.Get(cacheKey);
}
catch (InvalidCastException ex) //it didn’t parse properly
{
LoggerManager.Error(ex);
}
if (retVal == null) {
lock (cacheLock) {
try {
retVal = (T)CacheManager.Get(cacheKey); //make sure it’s still null (i.e. it didn’t get written while lock was being applied)
}
catch (InvalidCastException ex1) //it didn’t parse properly
{
LoggerManager.Error(ex1);
}
if (args.Length < 1) {
retVal = nonCachedMethod.Invoke();
}
else {
retVal = (T)nonCachedMethod.DynamicInvoke(args);
}
if (retVal != null) {
CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
}
}
}
return retVal;
}

That still worked for parametherless Funcs but not for Metods with params.

I ended up writing these two methods for my workaround, but still want to know if I can get them all into the same method (or if I should even try)

private T HandleItemCacheRetrieval<T>(string cacheKey, object cacheLock, Func<int, T> nonCachedMethod, int arg) where T : new() {
T retVal = default(T);
try {
retVal = (T)CacheManager.Get(cacheKey);
}
catch (InvalidCastException ex) //it didn’t parse properly
{
LoggerManager.Error(ex);
}
if (retVal == null) {
lock (cacheLock) {
try {
retVal = (T)CacheManager.Get(cacheKey); //make sure it’s still null (i.e. it didn’t get written while lock was being applied)
}
catch (InvalidCastException ex1) //it didn’t parse properly
{
LoggerManager.Error(ex1);
}
retVal = nonCachedMethod.Invoke(arg);

if (retVal != null) {
CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
}
}
}
return retVal;
}

private T HandleItemCacheRetrieval<T>(string cacheKey, object cacheLock, Func<string, T> nonCachedMethod, string arg) where T : new() {
T retVal = default(T);
try {
retVal = (T)CacheManager.Get(cacheKey);
}
catch (InvalidCastException ex) //it didn’t parse properly
{
LoggerManager.Error(ex);
}
if (retVal == null) {
lock (cacheLock) {
try {
retVal = (T)CacheManager.Get(cacheKey); //make sure it’s still null (i.e. it didn’t get written while lock was being applied)
}
catch (InvalidCastException ex1) //it didn’t parse properly
{
LoggerManager.Error(ex1);
}
retVal = nonCachedMethod.Invoke(arg);

if (retVal != null) {
CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
}
}
}
return retVal;
}



UPDATE:

BIG Thanks to Hugo Dahl & Peter Ritchie, who helped me implement these using a single method (with and/or without params, no longer matters) using Lambdas.

Check it out. This is SLICK:

 

8 Comments on “Let the Learnings Ensue”

  1. jessitron Says:

    Solving a problem as common as caching by hand always feels wrong. In Java I’d use Guava’s CacheBuilder – it’s perfect for exactly this problem, when you have a function that knows how to retrieve the value if it is not in the cache already.

    Reply

  2. TruncatedCoDr Says:

    Yes, it is a very common problem, but the implementation itself was via our architect’s directive. My intention in this post was to see if my use of generics & funcs was acceptable :-)

    It was great meeting you at Codemash, Jess! Thank you for your help with this today on Twitter. Everyone’s feedback was very appreciated!

    Reply

  3. Pedro Reys Says:

    If you refactor the CacheManager, that I’m assuming uses a Dictionary behind the scenes, to use C# 4’s ConcurrentDictionary, you will be able to simplify your code removing all the locking and checking code.

    http://msdn.microsoft.com/en-us/library/dd287191.aspx

    Reply

  4. TruncatedCoDr Says:

    My inital refactor, per recommendations by Peter Ritchie & Bill Wagner (thank you!) was to combine the 2 methods whose Func takes a single in param into one method by changing the param to a generic, like this:

    private T HandleItemCacheRetrieval(string cacheKey, object cacheLock, Func nonCachedMethod, TIn arg) where T : new() {
    T retVal = default(T);
    try {
    retVal = (T)CacheManager.Get(cacheKey);
    }
    catch (InvalidCastException ex) //it didn’t parse properly
    {
    LoggerManager.Error(ex);
    }
    if (retVal == null) {
    lock (cacheLock) {
    try {
    retVal = (T)CacheManager.Get(cacheKey); //make sure it’s still null (i.e. it didn’t get written while lock was being applied)
    }
    catch (InvalidCastException ex1) //it didn’t parse properly
    {
    LoggerManager.Error(ex1);
    }
    retVal = nonCachedMethod.Invoke(arg);

    if (retVal != null) {
    CacheManager.Insert(cacheKey, retVal, _cacheLengthInSeconds);
    }
    }
    }
    return retVal;
    }

    Works like a charm! I am about to experiment with some additional suggestions / recommendations & will post as I make more modifications.

    Code reviews are FUN :-D

    Reply

  5. TruncatedCoDr Says:

    Pedro, I thought the ConcurrentDictionary was to enable thread-safe, multithreaded concurrent reads? The locking only happens when the cached value is being written (checks for value; locks if not in cache; re-checks for value in case it was written while the object was being locked; if still no value, calls db method & writes to cache; releases lock;returns value)

    Does the ConcurrentDictionary implement protection from concurrent write scenarios as well as concurrent reads?

    Reply

    • Pedro Reys Says:

      Cori, it does, through the GetOrAdd method.

      http://msdn.microsoft.com/en-us/library/ee378677.aspx

      GetOrAdd has an overload GetOrAdd(TKey key, Func valueFactory) that allows you to pass in a func that makes the db call and returns the value. Doing exaclty what you described.

      Checks for Key, if not found executes the func to get the value and add it to the Dictionary. All that done in a Thread safe mode, thus no need for manually locking.

      Reply

  6. TruncatedCoDr Says:

    Ok THAT is very cool. I was unaware of that.

    My only problem then, in this particular implementation, might be that I have to go through a predefined caching provider (whose base class does not define an abstract GetOrAdd method) rather than doing the cache implementation directly.

    I am definitely stoked to add ConcurrentDictionary to my toolbelt (but I want to implement it now!)

    Hrm… I coudl probably write an extension method to translate the GetorAdd functionality… should I try to create a ConcurrentDictionaryCacheProvider?

    Reply

  7. TruncatedCoDr Says:

    BIG Thanks to Hugo Dahl & Peter Ritchie, who helped me implement these using a single method (with and without params) using Lambdas.

    I’ve updated my the end of my original post with that implementation.

    Reply

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

%d bloggers like this: