0

I'm developing a thread safe class that I'll use as cache, it should work in .NET and Mono.

The items have a time to live, and every time that a object is retrieved, its time to live is refreshed. Each time that I add a item, the timestamp is added to another collection that holds the same keys. A timer raises the method that looks for out dated items and remove them.

When I try to get and item, I have to provide also a delegate indicating how to obtain it if it wouldn't exist in the cache.

I've testing, and although the items removal should happen every 30 seconds in the test, it's happening very often, almost every second, and I'don't know why.

This is the class:

    public class GenericCache<TId, TItem>:IDisposable where TItem : class
{
    SortedDictionary<TId, TItem> _cache;
    SortedDictionary<TId, DateTime> _timeouts;
    Timer _timer;
    Int32 _cacheTimeout;
    System.Threading.ReaderWriterLockSlim _locker;

    public GenericCache(Int32 minutesTTL)
    {
        _locker = new System.Threading.ReaderWriterLockSlim();
        _cacheTimeout = minutesTTL;
        _cache = new SortedDictionary<TId, TItem>();
        _timeouts = new SortedDictionary<TId, DateTime>();
        _timer = new Timer((minutesTTL * 60) / 2);
        _timer.Elapsed += new ElapsedEventHandler(_timer_Elapsed);
        _timer.AutoReset = true;
        _timer.Enabled = true;
        _timer.Start();
    }

    /// <summary>
    /// Get an item, if it doesn't exist, create it using the delegate
    /// </summary>
    /// <param name="id">Id for the item</param>
    /// <param name="create">A delegate that generates the item</param>
    /// <returns>The item</returns>
    public TItem Get(TId id, Func<TItem> create)
    {
        _locker.EnterUpgradeableReadLock();
        try
        {
            TItem item = _cache.Where(ci => ci.Key.Equals(id)).Select(ci => ci.Value).SingleOrDefault();
            if (item == null)
            {
                _locker.EnterWriteLock();
                // check again, maybe another thread is waiting in EnterWriteLock cos the same item is null
                item = _cache.Where(ci => ci.Key.Equals(id)).Select(ci => ci.Value).SingleOrDefault();
                if (item == null)
                {
                    Debug.Write("_");
                    item = create.Invoke();
                    if (item != null)
                    {
                        _cache.Add(id, item);
                        _timeouts.Add(id, DateTime.Now);
                    }
                }
            }
            else
                _timeouts[id] = DateTime.Now;

            return item;
        }
        finally
        {
            if(_locker.IsWriteLockHeld)
                _locker.ExitWriteLock();
            _locker.ExitUpgradeableReadLock();
        }
    }

    /// <summary>
    /// Execute a delegate in the items, for example clear nested collections.
    /// </summary>
    /// <param name="action">The delegate</param>
    public void ExecuteOnItems(Action<TItem> action)
    {
        _locker.EnterWriteLock();
        try
        {
            foreach (var i in _cache.Values)
                action.Invoke(i);
        }
        finally
        {
            _locker.ExitWriteLock();
        }
    }

    /// <summary>
    /// Clear this cache
    /// </summary>
    public void Clear()
    {
        _locker.EnterWriteLock();
        try
        {
            _cache.Clear();
            _timeouts.Clear();
        }
        finally
        {
            _locker.ExitWriteLock();
        }
    }

    /// <summary>
    /// Remove outdated items
    /// </summary>
    /// <param name="sender"></param>
    /// <param name="e"></param>
    private void _timer_Elapsed(object sender, ElapsedEventArgs e)
    {
        _locker.EnterUpgradeableReadLock();
        try
        {
            var delete = _timeouts.Where(to => DateTime.Now.Subtract(to.Value).TotalMinutes > _cacheTimeout).ToArray();

            if(delete.Any())
            {
                _locker.EnterWriteLock();
                foreach (var timeitem in delete)
                {
                    Debug.Write("-");
                    _cache.Remove(timeitem.Key);
                    _timeouts.Remove(timeitem.Key);
                }
            }
        }
        finally
        {
            if(_locker.IsWriteLockHeld)
                _locker.ExitWriteLock();
            _locker.ExitUpgradeableReadLock();
        }
    }

    #region IDisposable Members
    private volatile Boolean disposed = false;
    protected virtual void Dispose(bool disposing)
    {
        if (!disposed)
        {
            if (disposing)
            try
            {
                this.Clear();
            }
            finally
            {
                _locker.Dispose();
            }

            disposed = true;
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    ~GenericCache()
    {
        Dispose(false);
    }

    #endregion
}

As you can see, in debug mode, when a item is added a "_" symbol is printed, and when and item is removed a "-" symbol is printed. In the tests, after the second minute can see how items are removed and added in the same second, when the items should be removed only every 30 seconds, and I don't know why:

This is how I tests:

        static void Main(string[] args)
    {
        GenericCache<Int32, String> cache = new GenericCache<Int32, String>(1);

        Debug.Listeners.Add(new ConsoleTraceListener());

        Action a = delegate()
        {
            Random r = new Random(DateTime.Now.Millisecond);
            while (true)
            {
                Int32 number = r.Next(0, 9999);
                if (String.IsNullOrEmpty(cache.Get(number, () => number.ToString())))
                    Debug.Write("E");
                Thread.Sleep(number);
            }
        };

        for (int i = 0; i < 150; i++)
        {
            new Thread(new ThreadStart(a)).Start();
            Thread.Sleep(5);
        }

        Console.ReadKey();
    }

Do you see any problem in the GenericCache class?

Thanks in advance, kind regards.

vtortola
  • 34,709
  • 29
  • 161
  • 263
  • 2
    why reinvent the wheel? Whats wrong with EL Caching? – Nix Apr 06 '10 at 14:48
  • 3
    Why not use the Enterprise Library Caching Block? It's threadsafe. http://msdn.microsoft.com/en-us/library/cc309103.aspx – Randolpho Apr 06 '10 at 14:49
  • 1
    @Randolpho exactly..... – Nix Apr 06 '10 at 14:50
  • Huumm... nice :D I'll take a look on it, but I'd like still now what is wrong with that code hehe. Also I have to ensure that EL 4.1 works in Mono :P – vtortola Apr 06 '10 at 15:00
  • Why do you set 1/2 time to live? Why not use what the user passes in? – Nix Apr 06 '10 at 15:09
  • the ttl used is what the user pass in, but I check them out every (ttl/2). What is set to be the half is the timer period, but if you see for delete items is comparing with _cacheTimeout that is the full ttl. – vtortola Apr 06 '10 at 16:49
  • @vtortola: You didn't mention Mono in your post! That said, although much of Enterprise Library is Mono safe, it's not 100% Mono friendly. It's pure .NET, but some portions may rely on libraries not yet implemented in Mono. The Caching block should work in Mono, however; I'm about 80% certain you could just drop that block in and go to town. – Randolpho Apr 06 '10 at 17:53
  • Sorry, my bad. I'll give to the caching block a try if I have time enough. – vtortola Apr 06 '10 at 18:41

1 Answers1

1

First issue i see (assuming you are using System.Timers.Timer accepts milliseconds and you are passing seconds).

  _timer = new Timer((minutesTTL * 60000) / 2); 
Nix
  • 57,072
  • 29
  • 149
  • 198
  • 1 * 60000 is the milliseconds that a minute contains, isn't it? I'm setting the Timer to the half of the time to live. – vtortola Apr 06 '10 at 15:28
  • 1
    right, u could divide that by to... i just was wondering why you would do that? – Nix Apr 06 '10 at 18:11
  • well... may be is not the right approach :P I just wanted to ensure that a out dated item remains in the collection as minimun as possible. – vtortola Apr 06 '10 at 18:42
  • 1
    You did u understand what i am saying about the Timer class? and why it is firing every second? – Nix Apr 06 '10 at 19:27
  • No, I didn't understand why is firing every second. The timer is set to 30 seconds, so I don't get why is firing in that way :S – vtortola Apr 06 '10 at 21:18
  • 1
    im trying to tell you the timer is not being set to fire every thirty seconds. – Nix Apr 06 '10 at 21:27
  • HOLY CRAP!! I was looking that piece of code you put thinking that you made a copy&paste and now I've realised that in my code appears "60" hahaha. Thanks a lot man, sorry about that :D – vtortola Apr 06 '10 at 21:46