From 3b2d59293c6ffb599f554a55529fbf4876a4d50f Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Fri, 8 Sep 2023 18:36:43 -0700
Subject: [PATCH 01/14] overloads

---
 BitFaster.Caching/ICache.cs                |  4 +++
 BitFaster.Caching/Lru/ConcurrentLruCore.cs | 41 +++++++++++++++++-----
 2 files changed, 36 insertions(+), 9 deletions(-)

diff --git a/BitFaster.Caching/ICache.cs b/BitFaster.Caching/ICache.cs
index 33ff16de..5f80b253 100644
--- a/BitFaster.Caching/ICache.cs
+++ b/BitFaster.Caching/ICache.cs
@@ -66,6 +66,10 @@ public interface ICache<K, V> : IEnumerable<KeyValuePair<K, V>>
         /// <returns>The value for the key. This will be either the existing value for the key if the key is already 
         /// in the cache, or the new value if the key was not in the cache.</returns>
         V GetOrAdd<TArg>(K key, Func<K, TArg, V> valueFactory, TArg factoryArgument) => this.GetOrAdd(key, k => valueFactory(k, factoryArgument));
+
+        //bool TryRemove(K key, out V value);
+
+        //bool TryRemove(KeyValuePair<K, V> item);
 #endif
 
         /// <summary>
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
index 4ee2eab6..82e1c5e5 100644
--- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs
+++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -293,13 +293,18 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
             }
         }
 
-        ///<inheritdoc/>
-        public bool TryRemove(K key)
+        private bool TryRemoveInternal(K key, V value, bool matchValue, out V oldValue)
         {
             while (true)
-            { 
+            {
                 if (this.dictionary.TryGetValue(key, out var existing))
                 {
+                    if (matchValue & !EqualityComparer<V>.Default.Equals(existing.Value, value))
+                    {
+                        oldValue = default;
+                        return false;
+                    }
+
                     var kvp = new KeyValuePair<K, I>(key, existing);
 
                     // hidden atomic remove
@@ -309,29 +314,47 @@ public bool TryRemove(K key)
                         // Mark as not accessed, it will later be cycled out of the queues because it can never be fetched 
                         // from the dictionary. Note: Hot/Warm/Cold count will reflect the removed item until it is cycled 
                         // from the queue.
-                        existing.WasAccessed = false;
-                        existing.WasRemoved = true;
+                        kvp.Value.WasAccessed = false;
+                        kvp.Value.WasRemoved = true;
 
-                        this.telemetryPolicy.OnItemRemoved(existing.Key, existing.Value, ItemRemovedReason.Removed);
+                        this.telemetryPolicy.OnItemRemoved(kvp.Key, kvp.Value.Value, ItemRemovedReason.Removed);
 
                         // serialize dispose (common case dispose not thread safe)
-                        lock (existing)
+                        lock (kvp.Value)
                         {
-                            Disposer<V>.Dispose(existing.Value);
+                            Disposer<V>.Dispose(kvp.Value.Value);
                         }
 
+                        oldValue = existing.Value;
                         return true;
                     }
 
                     // it existed, but we couldn't remove - this means value was replaced afer the TryGetValue (a race), try again
                 }
                 else
-                { 
+                {
+                    oldValue = default;
                     return false;
                 }
             }
         }
 
+        public bool TryRemove(KeyValuePair<K, V> item)
+        {
+            return TryRemoveInternal(item.Key, item.Value, matchValue: true, out V _);
+        }
+
+        public bool TryRemove(K key, out V value)
+        {
+            return TryRemoveInternal(key, default, matchValue: false, out value);
+        }
+
+        ///<inheritdoc/>
+        public bool TryRemove(K key)
+        {
+            return TryRemoveInternal(key, default, matchValue: false, out _);
+        }
+
         ///<inheritdoc/>
         ///<remarks>Note: Calling this method does not affect LRU order.</remarks>
         public bool TryUpdate(K key, V value)

From 9b843f24d1e6004f2bfc0f542e3a498b2b0a83e7 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Fri, 8 Sep 2023 18:55:44 -0700
Subject: [PATCH 02/14] ICache

---
 .../Atomic/AtomicFactoryCache.cs              | 34 ++++++++++++++++++-
 BitFaster.Caching/ICache.cs                   |  4 +--
 BitFaster.Caching/Lfu/ConcurrentLfu.cs        | 10 ++++++
 BitFaster.Caching/Lru/ClassicLru.cs           | 10 ++++++
 4 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/BitFaster.Caching/Atomic/AtomicFactoryCache.cs b/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
index f814ee80..4e4668cd 100644
--- a/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
+++ b/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
@@ -2,7 +2,8 @@
 using System.Collections;
 using System.Collections.Generic;
 using System.Diagnostics;
-
+using System.Linq.Expressions;
+
 namespace BitFaster.Caching.Atomic
 {
     /// <summary>
@@ -100,9 +101,40 @@ public bool TryGet(K key, out V value)
                 return true;
             }
 
+            value = default;
+            return false;
+        }
+
+        // backcompat: remove conditional compile
+#if NETCOREAPP3_0_OR_GREATER
+        ///<inheritdoc/>
+        ///<remarks>
+        ///If the value factory is still executing, returns false.
+        ///</remarks>
+        public bool TryRemove(KeyValuePair<K, V> item)
+        {
+            // TODO: AtomicFactory must be comparable via EqualityComparer<V>.Default
+            var kvp = new KeyValuePair<K, AtomicFactory<K, V>>(item.Key, new AtomicFactory<K, V>(item.Value));
+
+            return cache.TryRemove(kvp);
+        }
+
+        ///<inheritdoc/>
+        /// <remarks>
+        /// If the value factory is still executing, the default value will be returned.
+        /// </remarks>
+        public bool TryRemove(K key, out V value)
+        {
+            if (cache.TryRemove(key, out var atomic))
+            {
+                value = atomic.ValueIfCreated;
+                return true;
+            }
+
             value = default;
             return false;
         }
+#endif
 
         ///<inheritdoc/>
         public bool TryRemove(K key)
diff --git a/BitFaster.Caching/ICache.cs b/BitFaster.Caching/ICache.cs
index 5f80b253..2841bd06 100644
--- a/BitFaster.Caching/ICache.cs
+++ b/BitFaster.Caching/ICache.cs
@@ -67,9 +67,9 @@ public interface ICache<K, V> : IEnumerable<KeyValuePair<K, V>>
         /// in the cache, or the new value if the key was not in the cache.</returns>
         V GetOrAdd<TArg>(K key, Func<K, TArg, V> valueFactory, TArg factoryArgument) => this.GetOrAdd(key, k => valueFactory(k, factoryArgument));
 
-        //bool TryRemove(K key, out V value);
+        bool TryRemove(K key, out V value);
 
-        //bool TryRemove(KeyValuePair<K, V> item);
+        bool TryRemove(KeyValuePair<K, V> item);
 #endif
 
         /// <summary>
diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
index 223735ba..51c1b497 100644
--- a/BitFaster.Caching/Lfu/ConcurrentLfu.cs
+++ b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -316,6 +316,16 @@ public bool TryGet(K key, out V value)
 
             value = default;
             return false;
+        }
+
+        public bool TryRemove(KeyValuePair<K, V> item)
+        {
+            throw new NotImplementedException();
+        }
+
+        public bool TryRemove(K key, out V value)
+        {
+            throw new NotImplementedException();
         }
 
         ///<inheritdoc/>
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
index 5950bb93..7e8cc18a 100644
--- a/BitFaster.Caching/Lru/ClassicLru.cs
+++ b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -237,6 +237,16 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
             return await this.GetOrAddAsync(key, valueFactory, factoryArgument);
         }
 
+        public bool TryRemove(KeyValuePair<K, V> item)
+        {
+            throw new NotImplementedException();
+        }
+
+        public bool TryRemove(K key, out V value)
+        {
+            throw new NotImplementedException();
+        }
+
         ///<inheritdoc/>
         public bool TryRemove(K key)
         {

From cebdb32e79c7a7f8d62cb5b9016cccfef349cb9a Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sat, 9 Sep 2023 09:59:55 -0700
Subject: [PATCH 03/14] defaults

---
 BitFaster.Caching/Atomic/AtomicFactory.cs | 24 ++++++++++++++++++++---
 BitFaster.Caching/ICache.cs               | 15 ++++++++++++--
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/BitFaster.Caching/Atomic/AtomicFactory.cs b/BitFaster.Caching/Atomic/AtomicFactory.cs
index aeb96a67..ef3a9a44 100644
--- a/BitFaster.Caching/Atomic/AtomicFactory.cs
+++ b/BitFaster.Caching/Atomic/AtomicFactory.cs
@@ -1,4 +1,5 @@
 using System;
+using System.Collections.Generic;
 using System.Diagnostics;
 using System.Threading;
 
@@ -11,7 +12,7 @@ namespace BitFaster.Caching.Atomic
     /// <typeparam name="K">The type of the key.</typeparam>
     /// <typeparam name="V">The type of the value.</typeparam>
     [DebuggerDisplay("IsValueCreated={IsValueCreated}, Value={ValueIfCreated}")]
-    public sealed class AtomicFactory<K, V>
+    public sealed class AtomicFactory<K, V> : IEquatable<AtomicFactory<K, V>>
     {
         private Initializer initializer;
 
@@ -102,8 +103,25 @@ private V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : s
             }
 
             return value;
-        }
-
+        }
+
+        ///<inheritdoc/>
+        public override bool Equals(object obj)
+        {
+            return Equals(obj as AtomicFactory<K, V>);
+        }
+
+        ///<inheritdoc/>
+        public bool Equals(AtomicFactory<K, V> other)
+        {
+            if (other is null || !IsValueCreated || !other.IsValueCreated)
+            {
+                return false;
+            }
+
+            return EqualityComparer<V>.Default.Equals(ValueIfCreated, other.ValueIfCreated);
+        }
+
         private class Initializer
         {
             private readonly object syncLock = new();
diff --git a/BitFaster.Caching/ICache.cs b/BitFaster.Caching/ICache.cs
index 2841bd06..55ca00bf 100644
--- a/BitFaster.Caching/ICache.cs
+++ b/BitFaster.Caching/ICache.cs
@@ -67,9 +67,20 @@ public interface ICache<K, V> : IEnumerable<KeyValuePair<K, V>>
         /// in the cache, or the new value if the key was not in the cache.</returns>
         V GetOrAdd<TArg>(K key, Func<K, TArg, V> valueFactory, TArg factoryArgument) => this.GetOrAdd(key, k => valueFactory(k, factoryArgument));
 
-        bool TryRemove(K key, out V value);
+        /// <summary>
+        /// Attempts to remove and return the value that has the specified key.
+        /// </summary>
+        /// <param name="key">The key of the element to remove.</param>
+        /// <param name="value">When this method returns, contains the object removed, or the default value of the value type if key does not exist.</param>
+        /// <returns>true if the object was removed successfully; otherwise, false.</returns>
+        bool TryRemove(K key, out V value) => throw new NotSupportedException();
 
-        bool TryRemove(KeyValuePair<K, V> item);
+        /// <summary>
+        /// Attempts to remove 
+        /// </summary>
+        /// <param name="item"></param>
+        /// <returns></returns>
+        bool TryRemove(KeyValuePair<K, V> item) => throw new NotSupportedException();
 #endif
 
         /// <summary>

From 0d1d8a98140d4a09a4349ed243074414107912db Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sat, 9 Sep 2023 10:22:34 -0700
Subject: [PATCH 04/14] atomic tests

---
 .../Atomic/AtomicFactoryCacheTests.cs         | 27 +++++++++++++++++--
 BitFaster.Caching/Atomic/AtomicFactory.cs     | 13 ++++++++-
 BitFaster.Caching/ICache.cs                   |  6 ++---
 3 files changed, 40 insertions(+), 6 deletions(-)

diff --git a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
index f4ab92cb..2e22d127 100644
--- a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
+++ b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
@@ -70,10 +70,33 @@ public void WhenRemovedEventHandlerIsRegisteredItIsFired()
             this.cache.TryRemove(1);
 
             this.removedItems.First().Key.Should().Be(1);
+        }
+
+        // backcompat: remove conditional compile
+#if NETCOREAPP3_0_OR_GREATER
+        [Fact]
+        public void WhenRemovedValueIsReturned()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(1, out var value);
+
+            value.Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueDoesntMatchDontRemove()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueDoesMatchRemove()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
         }
 
-// backcompat: remove conditional compile
-#if NETCOREAPP3_0_OR_GREATER
         [Fact]
         public void WhenUpdatedEventHandlerIsRegisteredItIsFired()
         {
diff --git a/BitFaster.Caching/Atomic/AtomicFactory.cs b/BitFaster.Caching/Atomic/AtomicFactory.cs
index ef3a9a44..d2f93677 100644
--- a/BitFaster.Caching/Atomic/AtomicFactory.cs
+++ b/BitFaster.Caching/Atomic/AtomicFactory.cs
@@ -122,6 +122,17 @@ public bool Equals(AtomicFactory<K, V> other)
             return EqualityComparer<V>.Default.Equals(ValueIfCreated, other.ValueIfCreated);
         }
 
+        ///<inheritdoc/>
+        public override int GetHashCode()
+        {
+            if (!IsValueCreated)
+            {
+                return 0;
+            }
+
+            return ValueIfCreated.GetHashCode();
+        }
+
         private class Initializer
         {
             private readonly object syncLock = new();
@@ -147,6 +158,6 @@ public V CreateValue<TFactory>(K key, TFactory valueFactory) where TFactory : st
                     return value;
                 }
             }
-        }
+        }
     }
 }
diff --git a/BitFaster.Caching/ICache.cs b/BitFaster.Caching/ICache.cs
index 55ca00bf..00e16c96 100644
--- a/BitFaster.Caching/ICache.cs
+++ b/BitFaster.Caching/ICache.cs
@@ -76,10 +76,10 @@ public interface ICache<K, V> : IEnumerable<KeyValuePair<K, V>>
         bool TryRemove(K key, out V value) => throw new NotSupportedException();
 
         /// <summary>
-        /// Attempts to remove 
+        /// Attempts to remove the specified key value pair.
         /// </summary>
-        /// <param name="item"></param>
-        /// <returns></returns>
+        /// <param name="item">The item to remove.</param>
+        /// <returns>true if the item was removed successfully; otherwise, false.</returns>
         bool TryRemove(KeyValuePair<K, V> item) => throw new NotSupportedException();
 #endif
 

From 87ab1dec4d4b08cf47d0df84f4d8d4cf1b7d9544 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sat, 9 Sep 2023 10:48:39 -0700
Subject: [PATCH 05/14] atomic tests

---
 .../Atomic/AtomicFactoryCacheTests.cs         | 21 +++++++-
 .../Atomic/AtomicFactoryTests.cs              | 52 +++++++++++++++++++
 BitFaster.Caching.UnitTests/CacheTests.cs     | 23 ++++++++
 3 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
index 2e22d127..272a7a3b 100644
--- a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
+++ b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
@@ -13,11 +13,18 @@ namespace BitFaster.Caching.UnitTests.Atomic
     public class AtomicFactoryCacheTests
     {
         private const int capacity = 6;
-        private readonly AtomicFactoryCache<int, int> cache = new(new ConcurrentLru<int, AtomicFactory<int, int>>(capacity));
+        private readonly ConcurrentLru<int, AtomicFactory<int, int>> innerCache;
+        private readonly AtomicFactoryCache<int, int> cache;
 
         private List<ItemRemovedEventArgs<int, int>> removedItems = new();
         private List<ItemUpdatedEventArgs<int, int>> updatedItems = new();
 
+        public AtomicFactoryCacheTests()
+        {
+            innerCache = new ConcurrentLru<int, AtomicFactory<int, int>>(capacity);
+            cache = new(innerCache);
+        }
+
         [Fact]
         public void WhenInnerCacheIsNullCtorThrows()
         {
@@ -91,10 +98,20 @@ public void WhenRemoveKeyValueAndValueDoesntMatchDontRemove()
         }
 
         [Fact]
-        public void WhenRemoveKeyValueAndValueDoesMatchRemove()
+        public void WhenRemoveKeyValueAndValueDoesMatchThenRemove()
         {
             this.cache.AddOrUpdate(1, 1);
             this.cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
+        }
+
+        [Fact]
+        public void WhenRemoveKeyValueAndValueIsNotCreatedDoesNotRemove()
+        {
+            // seed the inner cache with an not yet created value
+            this.innerCache.AddOrUpdate(1, new AtomicFactory<int, int>());
+
+            // try to remove with the default value (0)
+            this.cache.TryRemove(new KeyValuePair<int, int>(1, 0)).Should().BeFalse();
         }
 
         [Fact]
diff --git a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs
index d3c72caf..85987dfe 100644
--- a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs
+++ b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryTests.cs
@@ -63,6 +63,58 @@ public void WhenValueCreatedArgGetValueReturnsOriginalValue()
             a.GetValue(1, (k, a) => k + a, 9).Should().Be(8);
         }
 
+        [Fact]
+        public void WhenValueNotCreatedHashCodeIsZero()
+        {
+            new AtomicFactory<int, int>()
+                .GetHashCode()
+                .Should().Be(0);
+        }
+
+        [Fact]
+        public void WhenValueCreatedHashCodeIsValueHashCode()
+        {
+            new AtomicFactory<int, int>(1)
+                .GetHashCode()
+                .Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenValueNotCreatedEqualsFalse()
+        {
+            var a = new AtomicFactory<int, int>();
+            var b = new AtomicFactory<int, int>();
+
+            a.Equals(b).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenOtherValueNotCreatedEqualsFalse()
+        {
+            var a = new AtomicFactory<int, int>(1);
+            var b = new AtomicFactory<int, int>();
+
+            a.Equals(b).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenArgNullEqualsFalse()
+        {
+            new AtomicFactory<int, int>(1)
+                .Equals(null)
+                .Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenArgObjectValuesAreSameEqualsTrue()
+        {
+            object other = new AtomicFactory<int, int>(1);
+
+            new AtomicFactory<int, int>(1)
+                .Equals(other)
+                .Should().BeTrue();
+        }
+
         [Fact]
         public async Task WhenCallersRunConcurrentlyResultIsFromWinner()
         {
diff --git a/BitFaster.Caching.UnitTests/CacheTests.cs b/BitFaster.Caching.UnitTests/CacheTests.cs
index b9ae6b5f..1840e301 100644
--- a/BitFaster.Caching.UnitTests/CacheTests.cs
+++ b/BitFaster.Caching.UnitTests/CacheTests.cs
@@ -1,5 +1,6 @@
 
 using System;
+using System.Collections.Generic;
 using System.Threading.Tasks;
 using FluentAssertions;
 using Moq;
@@ -25,6 +26,28 @@ public void WhenCacheInterfaceDefaultGetOrAddFallback()
                 1, 
                 (k, a) => k + a, 
                 2).Should().Be(3);
+        }
+
+        [Fact]
+        public void WhenCacheInterfaceDefaultTryRemoveKeyThrows()
+        {
+            var cache = new Mock<ICache<int, int>>();
+            cache.CallBase = true;
+
+            Action tryRemove = () => { cache.Object.TryRemove(1, out var value); };
+
+            tryRemove.Should().Throw<NotSupportedException>();
+        }
+
+        [Fact]
+        public void WhenCacheInterfaceDefaultTryRemoveKeyValueThrows()
+        {
+            var cache = new Mock<ICache<int, int>>();
+            cache.CallBase = true;
+
+            Action tryRemove = () => { cache.Object.TryRemove(new KeyValuePair<int, int>(1, 1)); };
+
+            tryRemove.Should().Throw<NotSupportedException>();
         }
 
         [Fact]

From a68bcd43a103e34b5eabb4b54ad7188ac4ff4278 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sat, 9 Sep 2023 10:51:09 -0700
Subject: [PATCH 06/14] test not removed

---
 .../Atomic/AtomicFactoryCacheTests.cs                    | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
index 272a7a3b..8c047ec6 100644
--- a/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
+++ b/BitFaster.Caching.UnitTests/Atomic/AtomicFactoryCacheTests.cs
@@ -90,6 +90,15 @@ public void WhenRemovedValueIsReturned()
             value.Should().Be(1);
         }
 
+        [Fact]
+        public void WhenNotRemovedValueIsDefault()
+        {
+            this.cache.AddOrUpdate(1, 1);
+            this.cache.TryRemove(2, out var value);
+
+            value.Should().Be(0);
+        }
+
         [Fact]
         public void WhenRemoveKeyValueAndValueDoesntMatchDontRemove()
         {

From 11fa17cad4a866b93bda5439f431fe24294a4e5d Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sun, 10 Sep 2023 10:57:55 -0700
Subject: [PATCH 07/14] lfu+classic

---
 BitFaster.Caching/Lfu/ConcurrentLfu.cs | 31 +++++++++---
 BitFaster.Caching/Lru/ClassicLru.cs    | 69 +++++++++++++++++---------
 2 files changed, 69 insertions(+), 31 deletions(-)

diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
index 51c1b497..48f151e2 100644
--- a/BitFaster.Caching/Lfu/ConcurrentLfu.cs
+++ b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -320,27 +320,44 @@ public bool TryGet(K key, out V value)
 
         public bool TryRemove(KeyValuePair<K, V> item)
         {
-            throw new NotImplementedException();
-        }
+            if (this.dictionary.TryGetValue(item.Key, out var node))
+            {
+                if (EqualityComparer<V>.Default.Equals(node.Value, item.Value))
+                {
+                    var kvp = new KeyValuePair<K, LfuNode<K,V>>(item.Key, node);
+
+                    if (this.dictionary.TryRemove(kvp))
+                    {
+                        node.WasRemoved = true;
+                        AfterWrite(node);
+                        return true;
+                    }
+                }
+            }
 
-        public bool TryRemove(K key, out V value)
-        {
-            throw new NotImplementedException();
+            return false;
         }
 
-        ///<inheritdoc/>
-        public bool TryRemove(K key)
+        public bool TryRemove(K key, out V value)
         {
             if (this.dictionary.TryRemove(key, out var node))
             {
                 node.WasRemoved = true;
                 AfterWrite(node);
+                value = node.Value;
                 return true;
             }
 
+            value = default;
             return false;
         }
 
+        ///<inheritdoc/>
+        public bool TryRemove(K key)
+        {
+            return this.TryRemove(key, out var _);
+        }
+
         ///<inheritdoc/>
         public bool TryUpdate(K key, V value)
         {
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
index 7e8cc18a..29a52320 100644
--- a/BitFaster.Caching/Lru/ClassicLru.cs
+++ b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -239,39 +239,60 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
 
         public bool TryRemove(KeyValuePair<K, V> item)
         {
-            throw new NotImplementedException();
+            if (this.dictionary.TryGetValue(item.Key, out var node))
+            {
+                if (EqualityComparer<V>.Default.Equals(node.Value.Value, item.Value))
+                {
+                    var kvp = new KeyValuePair<K, LinkedListNode<LruItem>>(item.Key, node);
+
+                    if (this.dictionary.TryRemove(kvp))
+                    {
+                        OnRemove(node);
+                        return true;
+                    }
+                }
+            }
+
+            return false;
         }
 
         public bool TryRemove(K key, out V value)
         {
-            throw new NotImplementedException();
+            if (dictionary.TryRemove(key, out var node))
+            {
+                OnRemove(node);
+                value = node.Value.Value;
+                return true;
+            }
+
+            value = default;
+            return false;
+
         }
 
         ///<inheritdoc/>
         public bool TryRemove(K key)
         {
-            if (dictionary.TryRemove(key, out var node))
-            {
-                // If the node has already been removed from the list, ignore.
-                // E.g. thread A reads x from the dictionary. Thread B adds a new item, removes x from 
-                // the List & Dictionary. Now thread A will try to move x to the end of the list.
-                if (node.List != null)
-                {
-                    lock (this.linkedList)
-                    {
-                        if (node.List != null)
-                        {
-                            linkedList.Remove(node);
-                        }
-                    }
-                }
-
-                Disposer<V>.Dispose(node.Value.Value);
-
-                return true;
-            }
-
-            return false;
+            return TryRemove(key, out var _);
+        }
+
+        private void OnRemove(LinkedListNode<LruItem> node)
+        {
+            // If the node has already been removed from the list, ignore.
+            // E.g. thread A reads x from the dictionary. Thread B adds a new item, removes x from 
+            // the List & Dictionary. Now thread A will try to move x to the end of the list.
+            if (node.List != null)
+            {
+                lock (this.linkedList)
+                {
+                    if (node.List != null)
+                    {
+                        linkedList.Remove(node);
+                    }
+                }
+            }
+
+            Disposer<V>.Dispose(node.Value.Value);
         }
 
         ///<inheritdoc/>

From 99b1ce3eadfe776a9b5bb4600923871561e0d798 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sun, 10 Sep 2023 11:19:44 -0700
Subject: [PATCH 08/14] cleanup logic

---
 .../Atomic/AtomicFactoryCache.cs              |  2 -
 BitFaster.Caching/Lru/ConcurrentLruCore.cs    | 80 +++++++++----------
 2 files changed, 38 insertions(+), 44 deletions(-)

diff --git a/BitFaster.Caching/Atomic/AtomicFactoryCache.cs b/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
index 4e4668cd..6af07f8c 100644
--- a/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
+++ b/BitFaster.Caching/Atomic/AtomicFactoryCache.cs
@@ -113,9 +113,7 @@ public bool TryGet(K key, out V value)
         ///</remarks>
         public bool TryRemove(KeyValuePair<K, V> item)
         {
-            // TODO: AtomicFactory must be comparable via EqualityComparer<V>.Default
             var kvp = new KeyValuePair<K, AtomicFactory<K, V>>(item.Key, new AtomicFactory<K, V>(item.Value));
-
             return cache.TryRemove(kvp);
         }
 
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
index 82e1c5e5..45d046d7 100644
--- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs
+++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -292,69 +292,65 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
                 }
             }
         }
-
-        private bool TryRemoveInternal(K key, V value, bool matchValue, out V oldValue)
+        
+        public bool TryRemove(KeyValuePair<K, V> item)
         {
-            while (true)
+            if (this.dictionary.TryGetValue(item.Key, out var existing))
             {
-                if (this.dictionary.TryGetValue(key, out var existing))
+                if (EqualityComparer<V>.Default.Equals(existing.Value, item.Value))
                 {
-                    if (matchValue & !EqualityComparer<V>.Default.Equals(existing.Value, value))
-                    {
-                        oldValue = default;
-                        return false;
-                    }
+                    var kvp = new KeyValuePair<K, I>(item.Key, existing);
 
-                    var kvp = new KeyValuePair<K, I>(key, existing);
-
-                    // hidden atomic remove
-                    // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
-                    if (((ICollection<KeyValuePair<K, I>>)this.dictionary).Remove(kvp))
+                    if (this.dictionary.TryRemove(kvp))
                     {
-                        // Mark as not accessed, it will later be cycled out of the queues because it can never be fetched 
-                        // from the dictionary. Note: Hot/Warm/Cold count will reflect the removed item until it is cycled 
-                        // from the queue.
-                        kvp.Value.WasAccessed = false;
-                        kvp.Value.WasRemoved = true;
-
-                        this.telemetryPolicy.OnItemRemoved(kvp.Key, kvp.Value.Value, ItemRemovedReason.Removed);
-
-                        // serialize dispose (common case dispose not thread safe)
-                        lock (kvp.Value)
-                        {
-                            Disposer<V>.Dispose(kvp.Value.Value);
-                        }
-
-                        oldValue = existing.Value;
+                        OnRemove(item.Key, kvp.Value);
                         return true;
                     }
-
-                    // it existed, but we couldn't remove - this means value was replaced afer the TryGetValue (a race), try again
-                }
-                else
-                {
-                    oldValue = default;
-                    return false;
                 }
+
+                // it existed, but we couldn't remove - this means value was replaced afer the TryGetValue (a race)
             }
-        }
 
-        public bool TryRemove(KeyValuePair<K, V> item)
-        {
-            return TryRemoveInternal(item.Key, item.Value, matchValue: true, out V _);
+            return false;
         }
 
         public bool TryRemove(K key, out V value)
         {
-            return TryRemoveInternal(key, default, matchValue: false, out value);
+            if (this.dictionary.TryRemove(key, out var item))
+            {
+                OnRemove(key, item);
+                value = item.Value;
+                return true;
+            }
+
+            value = default;
+            return false;
         }
 
         ///<inheritdoc/>
         public bool TryRemove(K key)
         {
-            return TryRemoveInternal(key, default, matchValue: false, out _);
+            return TryRemove(key, out _);
         }
 
+        private void OnRemove(K key, I item)
+        {
+            // Mark as not accessed, it will later be cycled out of the queues because it can never be fetched 
+            // from the dictionary. Note: Hot/Warm/Cold count will reflect the removed item until it is cycled 
+            // from the queue.
+            item.WasAccessed = false;
+            item.WasRemoved = true;
+
+            this.telemetryPolicy.OnItemRemoved(key, item.Value, ItemRemovedReason.Removed);
+
+            // serialize dispose (common case dispose not thread safe)
+            lock (item.Value)
+            {
+                Disposer<V>.Dispose(item.Value);
+            }
+        }
+
+
         ///<inheritdoc/>
         ///<remarks>Note: Calling this method does not affect LRU order.</remarks>
         public bool TryUpdate(K key, V value)

From 3d3ef30ecc4231f2523fb09b7dcdb576b7baea21 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Sun, 10 Sep 2023 21:50:26 -0700
Subject: [PATCH 09/14] 6only

---
 BitFaster.Caching/Lfu/ConcurrentLfu.cs     | 8 ++++++--
 BitFaster.Caching/Lru/ClassicLru.cs        | 4 ++++
 BitFaster.Caching/Lru/ConcurrentLruCore.cs | 5 ++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
index 48f151e2..7d9d5fd2 100644
--- a/BitFaster.Caching/Lfu/ConcurrentLfu.cs
+++ b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -324,9 +324,13 @@ public bool TryRemove(KeyValuePair<K, V> item)
             {
                 if (EqualityComparer<V>.Default.Equals(node.Value, item.Value))
                 {
-                    var kvp = new KeyValuePair<K, LfuNode<K,V>>(item.Key, node);
-
+                    var kvp = new KeyValuePair<K, LfuNode<K,V>>(item.Key, node);
+
+#if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
+#else
+                    if (((ICollection<KeyValuePair<K, LfuNode<K, V>>>)this.dictionary).Remove(kvp))
+#endif
                     {
                         node.WasRemoved = true;
                         AfterWrite(node);
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
index 29a52320..c87bdebc 100644
--- a/BitFaster.Caching/Lru/ClassicLru.cs
+++ b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -245,7 +245,11 @@ public bool TryRemove(KeyValuePair<K, V> item)
                 {
                     var kvp = new KeyValuePair<K, LinkedListNode<LruItem>>(item.Key, node);
 
+#if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
+#else
+                    if (((ICollection<KeyValuePair<K, LinkedListNode<LruItem>>>)this.dictionary).Remove(kvp))
+#endif
                     {
                         OnRemove(node);
                         return true;
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
index 45d046d7..e310339d 100644
--- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs
+++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -300,8 +300,11 @@ public bool TryRemove(KeyValuePair<K, V> item)
                 if (EqualityComparer<V>.Default.Equals(existing.Value, item.Value))
                 {
                     var kvp = new KeyValuePair<K, I>(item.Key, existing);
-
+#if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
+#else
+                    if (((ICollection<KeyValuePair<K, I>>)this.dictionary).Remove(kvp))
+#endif
                     {
                         OnRemove(item.Key, kvp.Value);
                         return true;

From 9491e4795516057c140fc44ee705606e19072074 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Mon, 11 Sep 2023 14:39:12 -0700
Subject: [PATCH 10/14] kvp tests

---
 .../Lfu/ConcurrentLfuTests.cs                 | 20 ++++++++++++++++++-
 .../Lru/ClassicLruTests.cs                    | 18 +++++++++++++++++
 .../Lru/ConcurrentLruTests.cs                 | 20 +++++++++++++++++++
 3 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
index 73e9c762..10325185 100644
--- a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
+++ b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
@@ -582,7 +582,7 @@ public void WhenItemDoesNotExistUpdatedAddsItem()
         }
 
         [Fact]
-        public void WhenItemIsRemovedItIsRemoved()
+        public void WhenKeyIsRemovedItIsRemoved()
         {
             cache.GetOrAdd(1, k => k);
 
@@ -590,6 +590,24 @@ public void WhenItemIsRemovedItIsRemoved()
             cache.TryGet(1, out var value).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenItemIsRemovedItIsRemoved()
+        {
+            cache.GetOrAdd(1, k => k);
+
+            cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
+            cache.TryGet(1, out var value).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenItemDoesntMatchItIsNotRemoved()
+        {
+            cache.GetOrAdd(1, k => k);
+
+            cache.TryRemove(new KeyValuePair<int, int>(1, 2)).Should().BeFalse();
+            cache.TryGet(1, out var value).Should().BeTrue();
+        }
+
         [Fact]
         public void WhenItemIsRemovedItIsDisposed()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
index def8a864..cd9e989f 100644
--- a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
+++ b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
@@ -362,6 +362,24 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
             lru.TryGet(1, out var value).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenItemExistsTryRemovesItemAndReturnsTrue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(new KeyValuePair<int, string>(1, "1")).Should().BeTrue();
+            lru.TryGet(1, out var value).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenTryRemoveKvpDoesntMatchItemNotRemovedAndReturnsFalse()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(new KeyValuePair<int, string>(1, "2")).Should().BeFalse();
+            lru.TryGet(1, out var value).Should().BeTrue();
+        }
+
         [Fact]
         public void WhenItemIsRemovedItIsDisposed()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
index ed2bf960..c22161a8 100644
--- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
+++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
@@ -1196,6 +1196,26 @@ await Threaded.Run(4, () => {
             }
         }
 
+        [Fact]
+        public async Task WhenSoakConcurrentGetAndRemoveKvpCacheEndsInConsistentState()
+        {
+            for (int i = 0; i < 10; i++)
+            {
+                await Threaded.Run(4, () => {
+                    for (int i = 0; i < 100000; i++)
+                    {
+                        lru.TryRemove(new KeyValuePair<int, string>(i + 1, (i + 1).ToString()));
+                        lru.GetOrAdd(i + 1, i => i.ToString());
+                    }
+                });
+
+                this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}");
+                this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys));
+
+                RunIntegrityCheck();
+            }
+        }
+
         [Fact]
         public async Task WhenSoakConcurrentGetAndUpdateCacheEndsInConsistentState()
         {

From b4ff63342efc2954146af07f530b2c52dfeba930 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Mon, 11 Sep 2023 15:09:22 -0700
Subject: [PATCH 11/14] api docs

---
 BitFaster.Caching/Lfu/ConcurrentLfu.cs     | 15 +++++++++++++--
 BitFaster.Caching/Lru/ClassicLru.cs        | 19 +++++++++++++++----
 BitFaster.Caching/Lru/ConcurrentLruCore.cs | 13 ++++++++++++-
 3 files changed, 40 insertions(+), 7 deletions(-)

diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
index 7d9d5fd2..aa1e1964 100644
--- a/BitFaster.Caching/Lfu/ConcurrentLfu.cs
+++ b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -318,6 +318,11 @@ public bool TryGet(K key, out V value)
             return false;
         }
 
+        /// <summary>
+        /// Attempts to remove the specified key value pair.
+        /// </summary>
+        /// <param name="item">The item to remove.</param>
+        /// <returns>true if the item was removed successfully; otherwise, false.</returns>
         public bool TryRemove(KeyValuePair<K, V> item)
         {
             if (this.dictionary.TryGetValue(item.Key, out var node))
@@ -340,8 +345,14 @@ public bool TryRemove(KeyValuePair<K, V> item)
             }
 
             return false;
-        }
-
+        }
+
+        /// <summary>
+        /// Attempts to remove and return the value that has the specified key.
+        /// </summary>
+        /// <param name="key">The key of the element to remove.</param>
+        /// <param name="value">When this method returns, contains the object removed, or the default value of the value type if key does not exist.</param>
+        /// <returns>true if the object was removed successfully; otherwise, false.</returns>
         public bool TryRemove(K key, out V value)
         {
             if (this.dictionary.TryRemove(key, out var node))
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
index c87bdebc..5ad00baa 100644
--- a/BitFaster.Caching/Lru/ClassicLru.cs
+++ b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -235,8 +235,13 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
             }
 
             return await this.GetOrAddAsync(key, valueFactory, factoryArgument);
-        }
-
+        }
+
+        /// <summary>
+        /// Attempts to remove the specified key value pair.
+        /// </summary>
+        /// <param name="item">The item to remove.</param>
+        /// <returns>true if the item was removed successfully; otherwise, false.</returns>
         public bool TryRemove(KeyValuePair<K, V> item)
         {
             if (this.dictionary.TryGetValue(item.Key, out var node))
@@ -258,8 +263,14 @@ public bool TryRemove(KeyValuePair<K, V> item)
             }
 
             return false;
-        }
-
+        }
+
+        /// <summary>
+        /// Attempts to remove and return the value that has the specified key.
+        /// </summary>
+        /// <param name="key">The key of the element to remove.</param>
+        /// <param name="value">When this method returns, contains the object removed, or the default value of the value type if key does not exist.</param>
+        /// <returns>true if the object was removed successfully; otherwise, false.</returns>
         public bool TryRemove(K key, out V value)
         {
             if (dictionary.TryRemove(key, out var node))
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
index e310339d..b442c3f5 100644
--- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs
+++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -292,7 +292,12 @@ public async ValueTask<V> GetOrAddAsync<TArg>(K key, Func<K, TArg, Task<V>> valu
                 }
             }
         }
-        
+
+        /// <summary>
+        /// Attempts to remove the specified key value pair.
+        /// </summary>
+        /// <param name="item">The item to remove.</param>
+        /// <returns>true if the item was removed successfully; otherwise, false.</returns>
         public bool TryRemove(KeyValuePair<K, V> item)
         {
             if (this.dictionary.TryGetValue(item.Key, out var existing))
@@ -317,6 +322,12 @@ public bool TryRemove(KeyValuePair<K, V> item)
             return false;
         }
 
+        /// <summary>
+        /// Attempts to remove and return the value that has the specified key.
+        /// </summary>
+        /// <param name="key">The key of the element to remove.</param>
+        /// <param name="value">When this method returns, contains the object removed, or the default value of the value type if key does not exist.</param>
+        /// <returns>true if the object was removed successfully; otherwise, false.</returns>
         public bool TryRemove(K key, out V value)
         {
             if (this.dictionary.TryRemove(key, out var item))

From 298fb20f2a22a94f131b74e89868f342ee75c785 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Mon, 11 Sep 2023 15:21:22 -0700
Subject: [PATCH 12/14] comment

---
 BitFaster.Caching/Lfu/ConcurrentLfu.cs     | 1 +
 BitFaster.Caching/Lru/ClassicLru.cs        | 1 +
 BitFaster.Caching/Lru/ConcurrentLruCore.cs | 1 +
 3 files changed, 3 insertions(+)

diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
index aa1e1964..5d4a3bc7 100644
--- a/BitFaster.Caching/Lfu/ConcurrentLfu.cs
+++ b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -334,6 +334,7 @@ public bool TryRemove(KeyValuePair<K, V> item)
 #if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
 #else
+                    // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
                     if (((ICollection<KeyValuePair<K, LfuNode<K, V>>>)this.dictionary).Remove(kvp))
 #endif
                     {
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
index 5ad00baa..1b777691 100644
--- a/BitFaster.Caching/Lru/ClassicLru.cs
+++ b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -253,6 +253,7 @@ public bool TryRemove(KeyValuePair<K, V> item)
 #if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
 #else
+                    // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
                     if (((ICollection<KeyValuePair<K, LinkedListNode<LruItem>>>)this.dictionary).Remove(kvp))
 #endif
                     {
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
index b442c3f5..59ec796c 100644
--- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs
+++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -308,6 +308,7 @@ public bool TryRemove(KeyValuePair<K, V> item)
 #if NET6_0_OR_GREATER
                     if (this.dictionary.TryRemove(kvp))
 #else
+                    // https://devblogs.microsoft.com/pfxteam/little-known-gems-atomic-conditional-removals-from-concurrentdictionary/
                     if (((ICollection<KeyValuePair<K, I>>)this.dictionary).Remove(kvp))
 #endif
                     {

From 68ed095b6d43177e10c293e241f22bcdff008de0 Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Mon, 11 Sep 2023 17:57:22 -0700
Subject: [PATCH 13/14] verify value

---
 BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs | 9 +++++++++
 BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs    | 9 +++++++++
 BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs | 9 +++++++++
 3 files changed, 27 insertions(+)

diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
index 10325185..89c5c27a 100644
--- a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
+++ b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
@@ -590,6 +590,15 @@ public void WhenKeyIsRemovedItIsRemoved()
             cache.TryGet(1, out var value).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            cache.GetOrAdd(1, valueFactory.Create);
+
+            cache.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be(1);
+        }
+
         [Fact]
         public void WhenItemIsRemovedItIsRemoved()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
index cd9e989f..9141759b 100644
--- a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
+++ b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
@@ -362,6 +362,15 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
             lru.TryGet(1, out var value).Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be("1");
+        }
+
         [Fact]
         public void WhenItemExistsTryRemovesItemAndReturnsTrue()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
index c22161a8..74f7800f 100644
--- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
+++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
@@ -634,6 +634,15 @@ public void WhenKeyExistsTryRemoveRemovesItemAndReturnsTrue()
 
             lru.TryRemove(1).Should().BeTrue();
             lru.TryGet(1, out var value).Should().BeFalse();
+        }
+
+        [Fact]
+        public void WhenKeyExistsTryRemoveReturnsValue()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryRemove(1, out var value).Should().BeTrue();
+            value.Should().Be("1");
         }
 
         [Fact]

From cc1e338e91b395e7e7b59ae9019243ee6162f01d Mon Sep 17 00:00:00 2001
From: Alex Peck <alexpeck@microsoft.com>
Date: Mon, 11 Sep 2023 18:25:42 -0700
Subject: [PATCH 14/14] test naming

---
 BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
index 89c5c27a..453a77a6 100644
--- a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
+++ b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
@@ -582,12 +582,12 @@ public void WhenItemDoesNotExistUpdatedAddsItem()
         }
 
         [Fact]
-        public void WhenKeyIsRemovedItIsRemoved()
+        public void WhenKeyExistsTryRemoveRemovesItem()
         {
             cache.GetOrAdd(1, k => k);
 
             cache.TryRemove(1).Should().BeTrue();
-            cache.TryGet(1, out var value).Should().BeFalse();
+            cache.TryGet(1, out _).Should().BeFalse();
         }
 
         [Fact]
@@ -600,16 +600,16 @@ public void WhenKeyExistsTryRemoveReturnsValue()
         }
 
         [Fact]
-        public void WhenItemIsRemovedItIsRemoved()
+        public void WhenItemExistsTryRemoveRemovesItem()
         {
             cache.GetOrAdd(1, k => k);
 
             cache.TryRemove(new KeyValuePair<int, int>(1, 1)).Should().BeTrue();
-            cache.TryGet(1, out var value).Should().BeFalse();
+            cache.TryGet(1, out _).Should().BeFalse();
         }
 
         [Fact]
-        public void WhenItemDoesntMatchItIsNotRemoved()
+        public void WhenItemDoesntMatchTryRemoveDoesNotRemove()
         {
             cache.GetOrAdd(1, k => k);