remove coldest when full (#378)

* remove cold

* cleanup

* disable long running test

* rem

* reverse cold/warm

* reverse

Author: bitfaster

BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs

@@ -364,8 +364,6 @@ public class KeysInOrderTestDataGenerator : IEnumerable<object[]>
                 new object[] { new FavorWarmPartition(128, 0.6) },
                 new object[] { new FavorWarmPartition(256, 0.6) },
                 new object[] { new FavorWarmPartition(1024, 0.6) },
-                //new object[] { new FavorWarmPartition(10*1024, 0.6) },
-                //new object[] { new FavorWarmPartition(100*1024, 0.6) },
             };
 
             public IEnumerator<object[]> GetEnumerator() => _data.GetEnumerator();
@@ -392,14 +390,10 @@ public void WhenKeysAreContinuouslyRequestedInTheOrderTheyAreAddedCountIsBounded
                 for (int j = 0; j < i; j++)
                 {
                     lru.GetOrAdd(j, valueFactory.Create);
-                }
-            }
-
-            // For larger cache sizes, I have observed capacity + 5. This is linked to the number of attempts.
-            // This is clearly a bug that needs further investigation, but considered not harmful at this point
-            // since growth is bounded, we just allow 4 more items than we should in the absolute worst case.
-            testOutputHelper.WriteLine($"Total: {lru.Count} Hot: {lru.HotCount} Warm: {lru.WarmCount} Cold: {lru.ColdCount}");
-            lru.Count.Should().BeLessOrEqualTo(capacity + 1);
+                }
+
+                lru.Count.Should().BeLessOrEqualTo(capacity + 1, $"Total: {lru.Count} Hot: {lru.HotCount} Warm: {lru.WarmCount} Cold: {lru.ColdCount}");
+            }
         }
 
         [Fact]

BitFaster.Caching/Lru/ConcurrentLruCore.cs

@@ -557,6 +557,20 @@ private void Cycle(int hotCount)
                         break;
                     }
                 }
+
+                // If we get here, we have cycled the queues multiple times and still have not removed an item.
+                // This can happen if the cache is full of items that are not discardable. In this case, we simply
+                // discard the coldest item to avoid unbounded growth.
+                if (dest != ItemDestination.Remove)
+                {
+                    // if an item was last moved into warm, move the last warm item to cold to prevent enlarging warm
+                    if (dest == ItemDestination.Warm)
+                    {
+                        LastWarmToCold();
+                    }
+
+                    RemoveCold(ItemRemovedReason.Evicted);
+                }
             }
             else
             {
@@ -566,6 +580,20 @@ private void Cycle(int hotCount)
             }
         }
 
+        private void LastWarmToCold()
+        {
+            Interlocked.Decrement(ref this.counter.warm);
+
+            if (this.hotQueue.TryDequeue(out var item))
+            {
+                this.Move(item, ItemDestination.Cold, ItemRemovedReason.Evicted);
+            }
+            else
+            {
+                Interlocked.Increment(ref this.counter.warm);
+            }
+        }
+
         private void CycleDuringWarmup(int hotCount)
         {
             // do nothing until hot is full
@@ -698,6 +726,20 @@ private void CycleDuringWarmup(int hotCount)
             }
         }
 
+        private void RemoveCold(ItemRemovedReason removedReason) 
+        {
+            Interlocked.Decrement(ref this.counter.cold);
+
+            if (this.coldQueue.TryDequeue(out var item))
+            {
+                 this.Move(item, ItemDestination.Remove, removedReason);
+            }
+            else
+            {
+                Interlocked.Increment(ref this.counter.cold);
+            }
+        }
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         private int Move(I item, ItemDestination where, ItemRemovedReason removedReason)
         {