0

I've run into an issue that is pretty narrow in scope and would likely be difficult to reproduce in a bare minimum example and I have a workaround in place, but it involves GC.Collect()

I am using TPL Dataflow and have a processing workflow that is running in parallel, but these parallel tasks are using a shared data cache in the form of a ConcurrentDictionary. I have a max size for this cache, and when it gets too large, I am removing items from the cache. The items are very large data wise (it's basically pixel data from some large images used in the processing) so having them sit around too long can be detrimental. Since it's processing in parallel it can pretty aggressively add and remove items from the cache as it's working through the data, and what I have found is that there are scenarios where items are removed from the cache, but they aren't garbage collected until I feel like it is too late. At least once I've taken a memory snapshot, and in the heap there was 2gb of data, but if I check the box "show dead objects" it shows the heap is using up 10gb of memory, so if I'm interpreting things correctly there is about 8gb of memory that is "dead" and just waiting for garbage collection. Before moving on here I'm sure there will be some people with the idea that if the memory is available why not sit around and more efficiently wait to remove the dead objects, but that's why this was a problem in the first place. All available memory is being used which is nearly locking up the computer and the application is switching over to caching items on the C drive which is slowing things down considerably when this happens and sometimes it's unstable enough that things crash, so I'm curious why the garbage collector is waiting so long to collect in a scenario like this.

I tried changing the removal logic to try to help with the thrashing of the cache and removing chunks at a time instead of always adding one and then removing one, and that maybe helped, but there would still be cases where the memory would appear to run away. What seems to fix it better is putting GC.Collect() at the end of the removal from the cache, but is that a reasonable fix? Is there a more proper way of handling this? Below is a stripped down and genericized version of my removal logic.

//entering a lock so that only one thread tries to remove the old entries and calls GC.Collect()
if (_cache.Count > 30 && Monitor.TryEnter(_deleteLock))
{
    var oldestEntries = _cache.OrderBy(item => item.Value.order).Select(item => item.Key).Take(5).ToList();

    foreach(var entry in oldestEntries)
    {
        _cache.TryRemove(entry, out _);
    }
    GC.Collect();
    Monitor.Exit(_deleteLock);
}
TJ Rockefeller
  • 3,178
  • 17
  • 43
  • The [`Count`](https://learn.microsoft.com/en-us/dotnet/api/system.collections.concurrent.concurrentdictionary-2.count) property is expensive. Are you reading it every time you add something on the cache? Also regarding the items in the cache, are these disposable (`IDisposable`)? – Theodor Zoulias May 09 '23 at 19:30
  • I only thought that Count was expensive in the context of LINQ, but this is a `ConcurrentDictionary` so I thought it was a property that was updated as items were added and removed and was as simple as checking an int property (from an snapshot context which maybe this is the overhead you are referring to?). The items in the dictionary are not `IDisposable` they are byte arrays. – TJ Rockefeller May 09 '23 at 19:33
  • 1
    You are between a rock (the large object heap) and a hard place (the cost of Gen2 collections). You can use _Performance Monitor_ (aka _PerfMon_) and the ".NET Memory Counters" to better characterize your problem. Are your large arrays similarly sized? There may be an advantage to creating your own buffer manager (build a bunch of arrays, stick them in a pool, hand them out as needed, and, whe the work is done, hand the previously used buffer back to the manager – Flydog57 May 09 '23 at 19:43
  • By the way, generally `Count` properties are not expensive (they are maintained by the collection, though it probably requires taking a lock in a concurrent collection). You can try decompiling the code to see. LINQ's `Count()` extension method is often expensive; it iterates through the collection – Flydog57 May 09 '23 at 19:47
  • 1
    @Flydog57 For ConcurrentDictionary, it takes *a number of locks* based on the concurrency level (default is # of processors). In this case, it can be expensive, and it's a common gotcha when using this type. Also, the source is [openly available](https://source.dot.net/#System.Collections.Concurrent/System/Collections/Concurrent/ConcurrentDictionary.cs,40c23c8c36011417) so you don't have to decompile. ;) – Jimmy May 10 '23 at 00:10
  • The `Count` property of the `ConcurrentDictionary` acquires and then releases all the internal locks of the collection. It creates contention. If you are reading frequently the `Count`, you might as well switch to a normal `Dictionary` and synchronize it manually. You could also consider switching to a custom [LRU cache](https://stackoverflow.com/questions/754233/is-it-there-any-lru-implementation-of-idictionary) implementation, that enforces strictly the max-30-items policy. – Theodor Zoulias May 10 '23 at 03:07

0 Answers0