Using concurrent dictionary as first-level cache

Question

I am currently implementing a thread-safe in-memory cache mechanism, with the intent of storing objects that are expensive to create or often used by the system.

I also want to include an expiration policy, i.e number of times an object was accessed and when it was last accessed. This will be used to scavenge, so I opted to go with a concurrent dictionary.

Below is my spiked code to get objects in a dictionary. I get an object from memory every time and increment the accessed count and date.

I'm not sure if it is thread-safe or if there are any other patterns out there I could use.

Imports System.Collections.Concurrent
Imports NUnit.Framework
Imports System.Threading
Imports CacheManagement
Imports CacheManagement.Extension
<TestFixture>
Public Class ConcurrentDictionaryTryAndGetTests
 Private _cache As ConcurrentDictionary(Of Integer, CacheItem)
 Private Const Key As Integer = 100
 Private _data As CacheItem = Nothing
 <SetUp()>
 Public Sub Setup()
 _data = New CacheItem With {.NumberOfTimesAccessed = 0, .Priority = CacheItemPriority.Normal}
 _cache = New ConcurrentDictionary(Of Integer, CacheItem)
 Dim added = _cache.TryAdd(Key, _data)
 Console.WriteLine(added)
 End Sub
 <Test>
 Public Sub ConcurrentDictionaryTests()
 Dim parallelOptions = New ParallelOptions
 With parallelOptions
 .MaxDegreeOfParallelism = 2
 End With
 Parallel.For(0, 10, parallelOptions, Sub(index)
 TryAndGet(_cache, Key)
 End Sub)
 End Sub
 Private Shared Function TryAndGet(ByVal dict As ConcurrentDictionary(Of Integer, CacheItem),
 ByVal key As Integer) As WaitCallback
 Dim data As CacheItem = Nothing
 If (dict.TryGetValue(key, data)) Then
 Thread.Sleep(1000)
 Interlocked.Increment(data.NumberOfTimesAccessed)
 Interlocked.Exchange(data.LastUsedDateTicks, DateTime.Now.Ticks)
 'note: interlock are atomic console.write is not atomic
 Console.WriteLine("number of times accessed {0} at ({1})", data.NumberOfTimesAccessed, data.LastUsedDateTicks.ToDate())
 Return Function(o) data
 End If
 Return Nothing
 End Function
End Class

Retrieving data from first and second level cache:

 Public Function GetDataStub(ByVal key As String) As Object
 Dim sw = New Stopwatch
 sw.Start()
 Dim cacheData As Object = realCache.GetDataFromInMemory(key)
 If cacheData Is Nothing Then cacheData = realCache.GetDataFromBackingStore(key)
 logger.InfoFormat("time taken to get data:{0} - {1}", key, sw.ElapsedMilliseconds)
 Return cacheData
 End Function
 Public Function GetDataFromInMemory(ByVal key As String) As Object
 Dim cacheItem As CacheItem = Nothing
 If IsInMemoryCacheEnabled = False Then Return Nothing
 SyncLock inMemoryCache.SyncRoot
 cacheItem = DirectCast(inMemoryCache(key), CacheItem)
 If IsObjectNotInCache(cacheItem) Then Return Nothing
 cacheItem.RetrievalCount += 1
 cacheItem.LastAccessedTime = DateTime.Now
 Return cacheItem.Value
 End SyncLock
 End Function
 Public Function GetDataFromBackingStore(ByVal key As String) As Object
 Dim cacheItem As CacheItem = Nothing
 cacheItem = DirectCast(backingStore.GetData(key), CacheItem)
 If IsObjectNotInCache(cacheItem) Then Return Nothing
 AddToInMemoryStore(key,
 cacheItem.Value,
 cacheItem.ScavengingPriority,
 cacheItem.GetExpirations())
 Return cacheItem.Value
 End Function
 Private Sub AddToInMemoryStore(ByVal key As String,
 ByVal value As Object,
 ByVal scavengingPriority As CacheItemPriority,
 ByVal ParamArray expirations As ICacheItemExpiration())
 Dim cacheItem As CacheItem = Nothing
 SyncLock inMemoryCache.SyncRoot
 If inMemoryCache.Contains(key) Then
 cacheItem = DirectCast(inMemoryCache(key), CacheItem)
 Else
 cacheItem = New CacheItem(key,
 addInProgressFlag,
 CacheItemPriority.Normal,
 Nothing)
 End If
 cacheItem.Replace(value, scavengingPriority, expirations)
 inMemoryCache(key) = cacheItem
 End SyncLock
 End Sub

Adding data to cache:

Public Sub Add(ByVal key As String,
 ByVal value As Object,
 ByVal scavengingPriority As CacheItemPriority,
 ByVal ParamArray expirations As ICacheItemExpiration())
 Dim cacheItem As CacheItem = Nothing
 Dim retrivalCount As Integer
 SyncLock inMemoryCache.SyncRoot
 If inMemoryCache.Contains(key) Then
 cacheItem = DirectCast(inMemoryCache(key), CacheItem)
 Else
 cacheItem = New CacheItem(key, addInProgressFlag, CacheItemPriority.Normal, Nothing)
 End If
 cacheItem.Replace(value, scavengingPriority, expirations)
 inMemoryCache(key) = cacheItem
 retrivalCount = cacheItem.RetrievalCount
 End SyncLock
 'fire and forget
 Dim tmpCacheItem = New CacheItem(key, value, scavengingPriority, expirations)
 tmpCacheItem.RetrievalCount = retrivalCount
 ThreadPool.UnsafeQueueUserWorkItem(AddressOf AddToBackingStore, tmpCacheItem)
 End Sub

• ChrisWue: ConcurrentDictionary suggests .NET 4.0+ and there is a thread-safe MemoryCache built into the framework which comes with all sorts of expiry policy etc. Rolling your own one doesn't seem sensible in that case
• Vijay.Gopalakrishna: stackoverflow.com/questions/21269170/… this link deals with concurrency issues by adding locks, is this not a given in concurrent dictionary!, please let me know
• Vijay.Gopalakrishna: stackoverflow.com/questions/6895956/memorycache-strangeness this mentions issues with growing memory
• Vijay.Gopalakrishna: Thanks, Chris. We have further tested and are implementing MemoryCache.

Answer 1

To follow up on my comment: I would still recommend using the MemoryCache provided by the .NET framework. You linked to two issues but it is not obvious to me how your approach will avoid them or how you are affected by them.

1. The locking. The question you have linked to raises the problem that two threads might by trying to access the same key, get a null result (entry not present in cache) and hence both trying to create the object which is an expensive operation and trying to insert it. You will run into the exact same problem with the ConcurrentDictionary approach. Just because your underlying data structure is thread safe doesn't mean that any combination of operation is thread safe. To give a simple (hypothetical code) example:

   var object = _MyCache.GetOrDefault(key);
   if (object == null)
   {
    object = CreateObjectInVeryExpensiveWay();
    _MyCache.Set(key, object);
   }
   

   If two threads call the above at the same time you will run into trouble independent of the actual underlying cache implementation. To avoid this you need separate synchronization mechanisms in any case.

2. The memory issue with MemoryCache. If you follow the question it will lead to this Connect issue which explains the enforcement limitations. In any case your current implementation has no way of limiting the cache anyway and you will not be able to properly implement an actual memory consumption based limitation in any case for similar reasons.

I have the impression that you are trying to optimize something which you don't even know is a problem yet in a way where do not fully understand all the implications. Thread safety is not just a matter of ensuring that all used data structures are thread safe but also that the access patterns which are composed out of multiple independent operations on the data structure will not leads you into inconsistent states. Writing correctly behaving multi threaded code is hard, writing correctly behaving lock free code is several order of magnitudes harder.

I have rolled my own memory cache implementation before (as .NET 3.5 and earlier did not provide one) and it did take a few iterations to get it right. As you will notice it's using a plain Dictionary and locks. We have never had a performance problem with it as read access is really fast and read contention even with many threads is minimal. You could get fancy and replace the plain lock with ReaderWriterLockSlim if you can prove that it's problematic otherwise.

Premature optimization is the root of all evil. Only optimize something if you can prove it's the bottle neck.

• Vijay.Gopalakrishna: Thanks, Chris. Apologies for not mentioning this earlier, We have a implementation of caching manager, which accepts memory thresholds as percentage and is build around a hash table.And a separate scavenger thread, which removes from hash table when memory reaches a preconfigured threshold. We have implemented locks around reads and writes. On analysis we thought it was a performance bottle neck to use locks around read. Hence opted to re-write using concurrent dictionary. Want to get rid of our customer scavenger and leave framework to manage memory & build lock free cache sys is the intent .
• Vijay.Gopalakrishna: Chris, Thanks for your help. I have updated with existing code for your inputs. Notice we have locks for reads and writes as we are updating retrieval counts and last accessed date.