2
\$\begingroup\$

I have the following class which is used in a WebApi AspNetCore application as a singleton instance to store Strategy objects (reference type). The class is

/// <summary>
/// Class to allow singleton access to a common thread safe registry of registered strategies.
/// </summary>
public class StrategyRegister : IStrategyRegister
{
    private ConcurrentDictionary<long, Strategy> _registery;

    public StrategyRegister()
    {
        _registery = new ConcurrentDictionary<long, Strategy>();
    }

    public bool TryAdd(Strategy strategy)
    {
        if (_registery.ContainsKey(strategy.Id))
            throw new SystemException("The registry can never contain two identical strategy ids");
        return _registery.TryAdd(strategy.Id, strategy);
    }

    public bool TryRemove(long id)
    {
        if (!_registery.ContainsKey(id))
            return true;

        //GC.Collect();
        return _registery.TryRemove(id, out _);
    }

    public Strategy TryGet(long id)
    {
        if (_registery.ContainsKey(id))
            return _registery[id];
        return null;
    }

    public IEnumerable<Strategy> GetRegisteredStrategies()
    {
        return _registery.Select(kvp => kvp.Value);
    }
}

The problem is, as the ConcurrentDictionary grows large and items are removed, they are not being collected by the GC. I confirm this by adding the crude and nasty line GC.Collect() after every removal. When this is done, memory does not grow... I could make this "smarter" (still crude), by adding a timer to force collection every n seconds, but is there a better way, or should I be doing something obvious here?

\$\endgroup\$
  • \$\begingroup\$ and you are sure that nothing else is using strategies when you remove them? \$\endgroup\$ – t3chb0t Jun 28 at 15:54
  • 1
    \$\begingroup\$ Yes, got it. It is indeed the ConcurrentDictionary. \$\endgroup\$ – MoonKnight Jun 28 at 16:08
  • 2
    \$\begingroup\$ I have checked the source myself. Some strange stuff occurring... "TryRemove does not really remove an element. All it does is changing linked list pointers to skip never assigning the value of an array element to null. This prevent GC from collecting old evicted objects." from here. Forced GC not really helping - just slowing the problem. \$\endgroup\$ – MoonKnight Jun 28 at 16:41
  • 1
    \$\begingroup\$ @dfhwze your suggestion seems to work. I tested it with a netcore2.2 console (1mln strings) and checked the results before and after with dotMemory and the old one is completely released and the memory usage drops by the expected amount (I removed every other element - or copied every other). \$\endgroup\$ – t3chb0t Jun 28 at 17:37
  • 2
    \$\begingroup\$ Interesting. Thanks very much for your time guys. \$\endgroup\$ – MoonKnight Jun 28 at 17:55
2
\$\begingroup\$

Apparently, collections are optimized for performance over memory management. So I'm afraid it is up to us, consumers of the .NET Framework, to implement memory management on collections that grow big and require lots of manipulations.

I would use a scheduled task to periodically make a new instance of the queue, with the values of the previous instance, and clearing the old instance.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.