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

Custom dictionary for caching assistance

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

Problem

I'm writing a custom dictionary which is to be used as a helper for caching. The reason I use delegates is because it must interface with generated code, so I can't change this. However, you can assume that the delegates are thread-safe. I did some preliminary tests that it's thread-safe, but I can always use an extra pair of eyes to spot possible concurrent issues.

```
//StoreToCache will handle removal, replacement, and addition to the cache
public delegate object StoreToCache(string key, object value, CacheObject info);
public delegate object GetFromCache(string key);

///
/// A super fast concurrent "dictionary" which passes through to the generated Cache class which constructed it.
///
public class CacheDictionary
{
ConcurrentDictionary RealKeys=new ConcurrentDictionary();
readonly string BaseKey;
readonly CacheObject Info;
StoreToCache StoreTo;
GetFromCache GetFrom;
///
/// This returns the amount of keys we are tracking within this CacheDictionary.
/// Note: This does not necessarily indicate how many items are actually still in the cache!
///
///
/// The count.
///
public int TrackedCount
{
get
{
return RealKeys.Count;
}
}
public CacheDictionary(string basekey, CacheObject info, StoreToCache store, GetFromCache get)
{
BaseKey=basekey;
Info=info;
StoreTo=store;
GetFrom=get;
}

void Add (K key, V value)
{
string realkey=RealKeys.GetOrAdd(key, (s) => AddKey(key));
StoreTo(realkey, value, Info);
}
public V Remove (K key)
{
var res=StoreTo(GetKey(key), null, Info);
string trash=null;
RealKeys.TryRemove(key, out trash);
if(res!=null && res is V)
{
return (V)res;
}
else
{
return default(V);
}
}
static long CurrentKey=0;
string AddKey(K key)
{
long tmp=Interlocked.In

Solution

Your code is not safe:

  • simultaneous calls to getter of the indexer and setter for the same key may result in data missing in cache:



  • setter adds the key to RealKeys



  • getter reads it and looks for value in cache while it's not there yet



  • setter add the value to cache



  • getter cleans the cache



  • lock in Clear doesn't prevent other methods from updating RealKeys since other methods don't lock on the RealKeys.



Other issues with code worth noting:

  • naming conventions. Private fields are usually named in camelCase, and often have an underscore prefix



  • single-responsibility. CacheDictionary doesn't need to know about CacheObject. Instead it would be good if "real cache" implements an interface, and it's passed as cache implementation instead of 2 delegates.



  • This cache stores mapping keys->real keys in memory, so real cache most likely is also in-memory. It might be a good idea just to use ConcurrentDictionary or System.Runtime.Caching.MemoryCache instead.

Context

StackExchange Code Review Q#19411, answer score: 7

Revisions (0)

No revisions yet.