LRU enumeration should not return expired/removed items (#534)

Author: bitfaster

BitFaster.Caching.UnitTests/Lru/ConcurrentTLruTests.cs

@@ -7,11 +7,11 @@
 
 namespace BitFaster.Caching.UnitTests.Lru
 {
-    public abstract class ConcurrentTLruTests
+    public class ConcurrentTLruTests
     {
         private readonly TimeSpan timeToLive = TimeSpan.FromMilliseconds(10);
         private readonly ICapacityPartition capacity = new EqualCapacityPartition(9);
-        private ICache<int, string> lru;
+        private ConcurrentTLru<int, string> lru;
 
         private ValueFactory valueFactory = new ValueFactory();
 
@@ -25,7 +25,10 @@ private void OnLruItemRemoved(object sender, ItemRemovedEventArgs<int, int> e)
             removedItems.Add(e);
         }
 
-        protected abstract ICache<K, V> CreateTLru<K, V>(ICapacityPartition capacity, TimeSpan timeToLive);
+        public ConcurrentTLru<K, V> CreateTLru<K, V>(ICapacityPartition capacity, TimeSpan timeToLive)
+        {
+            return new ConcurrentTLru<K, V>(1, capacity, EqualityComparer<K>.Default, timeToLive);
+        }
 
         public ConcurrentTLruTests()
         {
@@ -159,7 +162,9 @@ public void WhenItemsAreExpiredExpireRemovesExpiredItems()
                 {
                     lru.Policy.ExpireAfterWrite.Value.TrimExpired();
 
-                    lru.Count.Should().Be(0);
+                    lru.HotCount.Should().Be(0);
+                    lru.WarmCount.Should().Be(0);
+                    lru.ColdCount.Should().Be(0);
                 }
             );
         }
@@ -199,6 +204,9 @@ public void WhenExpiredItemsAreTrimmedCacheMarkedCold()
                     }
 
                     lru.Count.Should().Be(lru.Policy.Eviction.Value.Capacity);
+
+                    var total = lru.HotCount + lru.WarmCount + lru.ColdCount;
+                    total.Should().Be(lru.Policy.Eviction.Value.Capacity);
                 }
             );
         }
@@ -230,6 +238,9 @@ public void WhenCacheHasExpiredAndFreshItemsExpireRemovesOnlyExpiredItems()
                   lru.Policy.ExpireAfterWrite.Value.TrimExpired();
 
                   lru.Count.Should().Be(3);
+
+                  var total = lru.HotCount + lru.WarmCount + lru.ColdCount;
+                  total.Should().Be(3);
               }
           );
         }
@@ -253,17 +264,54 @@ public void WhenItemsAreExpiredTrimRemovesExpiredItems()
                     lru.Policy.Eviction.Value.Trim(1);
 
                     lru.Count.Should().Be(0);
+
+                    lru.HotCount.Should().Be(0);
+                    lru.WarmCount.Should().Be(0);
+                    lru.ColdCount.Should().Be(0);
                 }
             );
         }
-    }
 
-    public class ConcurrentTLruDefaultClockTests : ConcurrentTLruTests
-    {
-        protected override ICache<K, V> CreateTLru<K, V>(ICapacityPartition capacity, TimeSpan timeToLive)
+        [Fact]
+        public void WhenItemsAreExpiredCountFiltersExpiredItems()
         {
-            // backcompat: use TLruTickCount64Policy
-            return new ConcurrentTLru<K, V>(1, capacity, EqualityComparer<K>.Default, timeToLive);
+            Timed.Execute(
+                lru,
+                lru =>
+                {
+                    lru.AddOrUpdate(1, "1");
+                    lru.AddOrUpdate(2, "2");
+                    lru.AddOrUpdate(3, "3");
+
+                    return lru;
+                },
+                timeToLive.MultiplyBy(ttlWaitMlutiplier),
+                lru =>
+                {
+                    lru.Count.Should().Be(0);
+                }
+            );
+        }
+
+        [Fact]
+        public void WhenItemsAreExpiredEnumerateFiltersExpiredItems()
+        {
+            Timed.Execute(
+                lru,
+                lru =>
+                {
+                    lru.AddOrUpdate(1, "1");
+                    lru.AddOrUpdate(2, "2");
+                    lru.AddOrUpdate(3, "3");
+
+                    return lru;
+                },
+                timeToLive.MultiplyBy(ttlWaitMlutiplier),
+                lru =>
+                {
+                    lru.Should().BeEquivalentTo(Array.Empty<KeyValuePair<int, string>>());
+                }
+            );
         }
 
         [Fact]
@@ -290,13 +338,4 @@ public void ConstructPartitionCtorReturnsCapacity()
             x.Capacity.Should().Be(3);
         }
     }
-
-    public class ConcurrentTLruHighResClockTests : ConcurrentTLruTests
-    {
-        protected override ICache<K, V> CreateTLru<K, V>(ICapacityPartition capacity, TimeSpan timeToLive)
-        {
-            // backcompat: use TlruStopwatchPolicy
-            return new ConcurrentLruCore<K, V, LongTickCountLruItem<K, V>, TLruLongTicksPolicy<K, V>, TelemetryPolicy<K, V>>(1, capacity, EqualityComparer<K>.Default, new TLruLongTicksPolicy<K, V>(timeToLive), default);
-        }
-    }
 }

BitFaster.Caching/Lru/ConcurrentLruCore.cs

@@ -100,7 +100,7 @@ public ConcurrentLruCore(
 
         // No lock count: https://arbel.net/2013/02/03/best-practices-for-using-concurrentdictionary/
         ///<inheritdoc/>
-        public int Count => this.dictionary.Skip(0).Count();
+        public int Count => this.dictionary.Where(i => !itemPolicy.ShouldDiscard(i.Value)).Count();
 
         ///<inheritdoc/>
         public int Capacity => this.capacity.Hot + this.capacity.Warm + this.capacity.Cold;
@@ -146,7 +146,10 @@ public IEnumerator<KeyValuePair<K, V>> GetEnumerator()
         {
             foreach (var kvp in this.dictionary)
             {
-                yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.Value);
+                if (!itemPolicy.ShouldDiscard(kvp.Value))
+                { 
+                    yield return new KeyValuePair<K, V>(kvp.Key, kvp.Value.Value); 
+                }
             }
         }