update metric (#186)

bitfaster · web-flow · commit 9435f9adeb8d · 2022-08-18T21:17:58.000-07:00
* outline

* tests
diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentLfuTests.cs
@@ -233,7 +233,7 @@ public void WhenWriteBufferIsFullUpdatesAreDropped()
             var scheduler = new TestScheduler();
             cache = new ConcurrentLfu<int, int>(1, 20, scheduler);
 
-            cache.GetOrAdd(-1, k => k);
+            cache.GetOrAdd(1, k => k);
             scheduler.RunCount.Should().Be(1);
             cache.PendingMaintenance();
 
@@ -244,7 +244,7 @@ public void WhenWriteBufferIsFullUpdatesAreDropped()
 
             cache.PendingMaintenance();
 
-            // TODO: how to verify this? There is no counter for updates.
+            cache.Metrics.Value.Updated.Should().Be(bufferSize);
         }
 
         [Fact]
diff --git a/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ClassicLruTests.cs
@@ -372,6 +372,16 @@ public void WhenKeyExistsTryUpdateUpdatesValueAndReturnsTrue()
             value.Should().Be("2");
         }
 
+        [Fact]
+        public void WhenKeyExistsTryUpdateIncrementsUpdateCount()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryUpdate(1, "2").Should().BeTrue();
+
+            lru.Metrics.Value.Updated.Should().Be(1);
+        }
+
         [Fact]
         public void WhenKeyDoesNotExistTryUpdateReturnsFalse()
         {
@@ -380,6 +390,16 @@ public void WhenKeyDoesNotExistTryUpdateReturnsFalse()
             lru.TryUpdate(2, "3").Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenKeyDoesNotExistTryUpdateDoesNotIncrementCounter()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryUpdate(2, "3").Should().BeFalse();
+
+            lru.Metrics.Value.Updated.Should().Be(0);
+        }
+
         [Fact]
         public void WhenKeyDoesNotExistAddOrUpdateAddsNewItem()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs
@@ -636,6 +636,16 @@ public void WhenKeyExistsTryUpdateDisposesOldValue()
             disposableValueFactory.Items[1].IsDisposed.Should().BeTrue();
         }
 
+        [Fact]
+        public void WhenKeyExistsTryUpdateIncrementsUpdateCount()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryUpdate(1, "2").Should().BeTrue();
+
+            lru.Metrics.Value.Updated.Should().Be(1);
+        }
+
         [Fact]
         public void WhenKeyDoesNotExistTryUpdateReturnsFalse()
         {
@@ -644,6 +654,16 @@ public void WhenKeyDoesNotExistTryUpdateReturnsFalse()
             lru.TryUpdate(2, "3").Should().BeFalse();
         }
 
+        [Fact]
+        public void WhenKeyDoesNotExistTryUpdateDoesNotIncrementCounter()
+        {
+            lru.GetOrAdd(1, valueFactory.Create);
+
+            lru.TryUpdate(2, "3").Should().BeFalse();
+
+            lru.Metrics.Value.Updated.Should().Be(0);
+        }
+
         [Fact]
         public void WhenKeyDoesNotExistAddOrUpdateAddsNewItem()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/NoTelemetryPolicyTests.cs b/BitFaster.Caching.UnitTests/Lru/NoTelemetryPolicyTests.cs
@@ -35,6 +35,12 @@ public void MissesIsZero()
             counter.Misses.Should().Be(0);
         }
 
+        [Fact]
+        public void UpdatedIsZero()
+        {
+            counter.Updated.Should().Be(0);
+        }
+
         [Fact]
         public void EvictedIsZero()
         {
@@ -53,6 +59,12 @@ public void IncrementTotalCountIsNoOp()
             counter.Invoking(c => c.IncrementMiss()).Should().NotThrow();
         }
 
+        [Fact]
+        public void OnItemUpdatedIsNoOp()
+        {
+            counter.Invoking(c => c.OnItemUpdated(1, 2)).Should().NotThrow();
+        }
+
         [Fact]
         public void OnItemRemovedIsNoOp()
         {
diff --git a/BitFaster.Caching.UnitTests/Lru/TelemetryPolicyTests.cs b/BitFaster.Caching.UnitTests/Lru/TelemetryPolicyTests.cs
@@ -64,15 +64,23 @@ public void WhenTotalCountIsZeroRatioReturnsZero()
         }
 
         [Fact]
-        public void WhenItemRemovedIsEvictedIncrementEvictedCount()
+        public void WhenItemUpdatedIncrementUpdatedCount()
+        {
+            telemetryPolicy.OnItemUpdated(1, 2);
+
+            telemetryPolicy.Updated.Should().Be(1);
+        }
+
+        [Fact]
+        public void WhenItemRemovedIncrementEvictedCount()
         {
             telemetryPolicy.OnItemRemoved(1, 2, ItemRemovedReason.Evicted);
 
             telemetryPolicy.Evicted.Should().Be(1);
         }
 
         [Fact]
-        public void WhenItemRemovedIsRemovedDontIncrementEvictedCount()
+        public void WhenItemRemovedDontIncrementEvictedCount()
         {
             telemetryPolicy.OnItemRemoved(1, 2, ItemRemovedReason.Removed);
 
diff --git a/BitFaster.Caching/ICacheMetrics.cs b/BitFaster.Caching/ICacheMetrics.cs
@@ -36,5 +36,10 @@ public interface ICacheMetrics
         /// Gets the total number of evicted items.
         /// </summary>
         long Evicted { get; }
+
+        /// <summary>
+        /// Gets the total number of updated items.
+        /// </summary>
+        long Updated { get; }
     }
 }
diff --git a/BitFaster.Caching/Lfu/ConcurrentLfu.cs b/BitFaster.Caching/Lfu/ConcurrentLfu.cs
@@ -464,13 +464,16 @@ private void OnWrite(LinkedListNode<LfuNode<K, V>> node)
                     else
                     {
                         this.windowLru.MoveToEnd(node);
+                        this.metrics.updatedCount++;
                     }
                     break;
                 case Position.Probation:
                     PromoteProbation(node);
+                    this.metrics.updatedCount++;
                     break;
                 case Position.Protected:
                     this.protectedLru.MoveToEnd(node);
+                    this.metrics.updatedCount++;
                     break;
             }
         }
@@ -592,6 +595,7 @@ private class CacheMetrics : ICacheMetrics
         {
             public long requestHitCount;
             public long requestMissCount;
+            public long updatedCount;
             public long evictedCount;
 
             public double HitRatio => (double)requestHitCount / (double)Total;
@@ -602,6 +606,8 @@ private class CacheMetrics : ICacheMetrics
 
             public long Misses => requestMissCount;
 
+            public long Updated => updatedCount;
+
             public long Evicted => evictedCount;
         }
 
diff --git a/BitFaster.Caching/Lru/ClassicLru.cs b/BitFaster.Caching/Lru/ClassicLru.cs
@@ -224,6 +224,7 @@ public bool TryUpdate(K key, V value)
             if (this.dictionary.TryGetValue(key, out var node))
             {
                 node.Value.Value = value;
+                Interlocked.Increment(ref this.metrics.updatedCount);
                 return true;
             }
 
@@ -238,6 +239,7 @@ public void AddOrUpdate(K key, V value)
             if (this.dictionary.TryGetValue(key, out var existingNode))
             {
                 existingNode.Value.Value = value;
+                Interlocked.Increment(ref this.metrics.updatedCount);
                 return;
             }
 
@@ -377,6 +379,7 @@ private class CacheMetrics : ICacheMetrics
         {
             public long requestHitCount;
             public long requestTotalCount;
+            public long updatedCount;
             public long evictedCount;
 
             public double HitRatio => (double)requestHitCount / (double)requestTotalCount;
@@ -388,6 +391,8 @@ private class CacheMetrics : ICacheMetrics
             public long Misses => requestTotalCount - requestHitCount;
 
             public long Evicted => evictedCount;
+
+            public long Updated => updatedCount;
         }
     }
 }
diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs
@@ -263,6 +263,7 @@ public bool TryUpdate(K key, V value)
                         V oldValue = existing.Value;
                         existing.Value = value;
                         this.itemPolicy.Update(existing);
+                        this.telemetryPolicy.OnItemUpdated(existing.Key, existing.Value);
                         Disposer<V>.Dispose(oldValue);
 
                         return true;
@@ -655,6 +656,8 @@ public Proxy(ConcurrentLruCore<K, V, I, P, T> lru)
 
             public long Evicted => lru.telemetryPolicy.Evicted;
 
+            public long Updated => lru.telemetryPolicy.Updated;
+
             public int Capacity => lru.Capacity;
 
             public TimeSpan TimeToLive => lru.itemPolicy.TimeToLive;
diff --git a/BitFaster.Caching/Lru/ITelemetryPolicy.cs b/BitFaster.Caching/Lru/ITelemetryPolicy.cs
@@ -14,6 +14,8 @@ public interface ITelemetryPolicy<K, V> : ICacheMetrics, ICacheEvents<K, V>
 
         void OnItemRemoved(K key, V value, ItemRemovedReason reason);
 
+        void OnItemUpdated(K key, V value);
+
         void SetEventSource(object source);
     }
 }
diff --git a/BitFaster.Caching/Lru/NoTelemetryPolicy.cs b/BitFaster.Caching/Lru/NoTelemetryPolicy.cs
@@ -17,6 +17,8 @@ public struct NoTelemetryPolicy<K, V> : ITelemetryPolicy<K, V>
 
         public long Misses => 0;
 
+        public long Updated => 0;
+
         public long Evicted => 0;
 
         public event EventHandler<ItemRemovedEventArgs<K, V>> ItemRemoved
@@ -41,6 +43,11 @@ public void OnItemRemoved(K key, V value, ItemRemovedReason reason)
         {
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        public void OnItemUpdated(K key, V value)
+        {
+        }
+
         [MethodImpl(MethodImplOptions.AggressiveInlining)]
         public void SetEventSource(object source)
         {
diff --git a/BitFaster.Caching/Lru/TelemetryPolicy.cs b/BitFaster.Caching/Lru/TelemetryPolicy.cs
@@ -13,6 +13,7 @@ public struct TelemetryPolicy<K, V> : ITelemetryPolicy<K, V>
         private long hitCount;
         private long missCount;
         private long evictedCount;
+        private long updatedCount;
         private object eventSource;
 
         public event EventHandler<ItemRemovedEventArgs<K, V>> ItemRemoved;
@@ -27,6 +28,8 @@ public struct TelemetryPolicy<K, V> : ITelemetryPolicy<K, V>
 
         public long Evicted => this.evictedCount;
 
+        public long Updated => this.updatedCount;
+
         public void IncrementMiss()
         {
             Interlocked.Increment(ref this.missCount);
@@ -48,6 +51,11 @@ public void OnItemRemoved(K key, V value, ItemRemovedReason reason)
             this.ItemRemoved?.Invoke(this.eventSource, new ItemRemovedEventArgs<K, V>(key, value, reason));
         }
 
+        public void OnItemUpdated(K key, V value)
+        {
+            Interlocked.Increment(ref this.updatedCount);
+        }
+
         public void SetEventSource(object source)
         {
             this.eventSource = source;

Original file line number	Diff line number	Diff line change
`@@ -35,6 +35,12 @@ public void MissesIsZero()`
`35`	`35`	`counter.Misses.Should().Be(0);`
`36`	`36`	`}`
`37`	`37`
	`38`	`+ [Fact]`
	`39`	`+ public void UpdatedIsZero()`
	`40`	`+ {`
	`41`	`+ counter.Updated.Should().Be(0);`
	`42`	`+ }`
	`43`	`+`
`38`	`44`	`[Fact]`
`39`	`45`	`public void EvictedIsZero()`
`40`	`46`	`{`
`@@ -53,6 +59,12 @@ public void IncrementTotalCountIsNoOp()`
`53`	`59`	`counter.Invoking(c => c.IncrementMiss()).Should().NotThrow();`
`54`	`60`	`}`
`55`	`61`
	`62`	`+ [Fact]`
	`63`	`+ public void OnItemUpdatedIsNoOp()`
	`64`	`+ {`
	`65`	`+ counter.Invoking(c => c.OnItemUpdated(1, 2)).Should().NotThrow();`
	`66`	`+ }`
	`67`	`+`
`56`	`68`	`[Fact]`
`57`	`69`	`public void OnItemRemovedIsNoOp()`
`58`	`70`	`{`
Original file line number	Diff line number	Diff line change
`@@ -64,15 +64,23 @@ public void WhenTotalCountIsZeroRatioReturnsZero()`
`64`	`64`	`}`
`65`	`65`
`66`	`66`	`[Fact]`
`67`		`- public void WhenItemRemovedIsEvictedIncrementEvictedCount()`
	`67`	`+ public void WhenItemUpdatedIncrementUpdatedCount()`
	`68`	`+ {`
	`69`	`+ telemetryPolicy.OnItemUpdated(1, 2);`
	`70`	`+`
	`71`	`+ telemetryPolicy.Updated.Should().Be(1);`
	`72`	`+ }`
	`73`	`+`
	`74`	`+ [Fact]`
	`75`	`+ public void WhenItemRemovedIncrementEvictedCount()`
`68`	`76`	`{`
`69`	`77`	`telemetryPolicy.OnItemRemoved(1, 2, ItemRemovedReason.Evicted);`
`70`	`78`
`71`	`79`	`telemetryPolicy.Evicted.Should().Be(1);`
`72`	`80`	`}`
`73`	`81`
`74`	`82`	`[Fact]`
`75`		`- public void WhenItemRemovedIsRemovedDontIncrementEvictedCount()`
	`83`	`+ public void WhenItemRemovedDontIncrementEvictedCount()`
`76`	`84`	`{`
`77`	`85`	`telemetryPolicy.OnItemRemoved(1, 2, ItemRemovedReason.Removed);`
`78`	`86`
Original file line number	Diff line number	Diff line change
`@@ -36,5 +36,10 @@ public interface ICacheMetrics`
`36`	`36`	`/// Gets the total number of evicted items.`
`37`	`37`	`/// </summary>`
`38`	`38`	`long Evicted { get; }`
	`39`	`+`
	`40`	`+ /// <summary>`
	`41`	`+ /// Gets the total number of updated items.`
	`42`	`+ /// </summary>`
	`43`	`+ long Updated { get; }`
`39`	`44`	`}`
`40`	`45`	`}`
Original file line number	Diff line number	Diff line change
`@@ -464,13 +464,16 @@ private void OnWrite(LinkedListNode<LfuNode<K, V>> node)`
`464`	`464`	`else`
`465`	`465`	`{`
`466`	`466`	`this.windowLru.MoveToEnd(node);`
	`467`	`+ this.metrics.updatedCount++;`
`467`	`468`	`}`
`468`	`469`	`break;`
`469`	`470`	`case Position.Probation:`
`470`	`471`	`PromoteProbation(node);`
	`472`	`+ this.metrics.updatedCount++;`
`471`	`473`	`break;`
`472`	`474`	`case Position.Protected:`
`473`	`475`	`this.protectedLru.MoveToEnd(node);`
	`476`	`+ this.metrics.updatedCount++;`
`474`	`477`	`break;`
`475`	`478`	`}`
`476`	`479`	`}`
`@@ -592,6 +595,7 @@ private class CacheMetrics : ICacheMetrics`
`592`	`595`	`{`
`593`	`596`	`public long requestHitCount;`
`594`	`597`	`public long requestMissCount;`
	`598`	`+ public long updatedCount;`
`595`	`599`	`public long evictedCount;`
`596`	`600`
`597`	`601`	`public double HitRatio => (double)requestHitCount / (double)Total;`
`@@ -602,6 +606,8 @@ private class CacheMetrics : ICacheMetrics`
`602`	`606`
`603`	`607`	`public long Misses => requestMissCount;`
`604`	`608`
	`609`	`+ public long Updated => updatedCount;`
	`610`	`+`
`605`	`611`	`public long Evicted => evictedCount;`
`606`	`612`	`}`
`607`	`613`
Original file line number	Diff line number	Diff line change
`@@ -224,6 +224,7 @@ public bool TryUpdate(K key, V value)`
`224`	`224`	`if (this.dictionary.TryGetValue(key, out var node))`
`225`	`225`	`{`
`226`	`226`	`node.Value.Value = value;`
	`227`	`+ Interlocked.Increment(ref this.metrics.updatedCount);`
`227`	`228`	`return true;`
`228`	`229`	`}`
`229`	`230`
`@@ -238,6 +239,7 @@ public void AddOrUpdate(K key, V value)`
`238`	`239`	`if (this.dictionary.TryGetValue(key, out var existingNode))`
`239`	`240`	`{`
`240`	`241`	`existingNode.Value.Value = value;`
	`242`	`+ Interlocked.Increment(ref this.metrics.updatedCount);`
`241`	`243`	`return;`
`242`	`244`	`}`
`243`	`245`
`@@ -377,6 +379,7 @@ private class CacheMetrics : ICacheMetrics`
`377`	`379`	`{`
`378`	`380`	`public long requestHitCount;`
`379`	`381`	`public long requestTotalCount;`
	`382`	`+ public long updatedCount;`
`380`	`383`	`public long evictedCount;`
`381`	`384`
`382`	`385`	`public double HitRatio => (double)requestHitCount / (double)requestTotalCount;`
`@@ -388,6 +391,8 @@ private class CacheMetrics : ICacheMetrics`
`388`	`391`	`public long Misses => requestTotalCount - requestHitCount;`
`389`	`392`
`390`	`393`	`public long Evicted => evictedCount;`
	`394`	`+`
	`395`	`+ public long Updated => updatedCount;`
`391`	`396`	`}`
`392`	`397`	`}`
`393`	`398`	`}`
Original file line number	Diff line number	Diff line change
`@@ -14,6 +14,8 @@ public interface ITelemetryPolicy<K, V> : ICacheMetrics, ICacheEvents<K, V>`
`14`	`14`
`15`	`15`	`void OnItemRemoved(K key, V value, ItemRemovedReason reason);`
`16`	`16`
	`17`	`+ void OnItemUpdated(K key, V value);`
	`18`	`+`
`17`	`19`	`void SetEventSource(object source);`
`18`	`20`	`}`
`19`	`21`	`}`