From 4ee9397d347e619e3ce97269975097c19bffacdf Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Sat, 1 Jun 2024 15:25:34 -0700 Subject: [PATCH 1/4] quick fix --- .../Lfu/ConcurrentTLfuTests.cs | 22 +++++++++++++++++++ BitFaster.Caching/Lfu/ConcurrentLfuCore.cs | 2 +- BitFaster.Caching/Lfu/NodePolicy.cs | 7 ++++++ 3 files changed, 30 insertions(+), 1 deletion(-) diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs index 34c42293..2056fcd3 100644 --- a/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs +++ b/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs @@ -1,5 +1,6 @@ using System; using System.Runtime.InteropServices; +using System.Threading; using BitFaster.Caching.Lfu; using BitFaster.Caching.Scheduler; using BitFaster.Caching.UnitTests.Retry; @@ -25,6 +26,27 @@ public ConcurrentTLfuTests() lfu = new ConcurrentTLfu(capacity, new ExpireAfterWrite(timeToLive)); } + [Fact] + public void Repro() + { + var cache = new ConcurrentLfuBuilder() + .WithCapacity(10) + .WithExpireAfterAccess(TimeSpan.FromSeconds(5)) + .Build(); + + cache.AddOrUpdate(1, 1); + + Thread.Sleep(4000); + + cache.TryGet(1, out var value).Should().BeTrue(); + + Thread.Sleep(2000); + + cache.TryGet(1, out value).Should().BeTrue(); + cache.TryGet(1, out value).Should().BeTrue(); + } + + [Fact] public void ConstructAddAndRetrieveWithCustomComparerReturnsValue() { diff --git a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs index 0b56b09d..af831e4c 100644 --- a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs +++ b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs @@ -272,7 +272,7 @@ private bool TryGetImpl(K key, [MaybeNullWhen(false)] out V value) { bool delayable = this.readBuffer.TryAdd(node) != BufferStatus.Full; - if (this.drainStatus.ShouldDrain(delayable)) + if (this.drainStatus.ShouldDrain(policy.IsReadDrainDelayable() && delayable)) { TryScheduleDrain(); } diff --git a/BitFaster.Caching/Lfu/NodePolicy.cs b/BitFaster.Caching/Lfu/NodePolicy.cs index c0752175..7b51c91d 100644 --- a/BitFaster.Caching/Lfu/NodePolicy.cs +++ b/BitFaster.Caching/Lfu/NodePolicy.cs @@ -15,6 +15,7 @@ internal interface INodePolicy void OnWrite(N node); void OnEvict(N node); void ExpireEntries

(ref ConcurrentLfuCore cache) where P : struct, INodePolicy; + bool IsReadDrainDelayable(); } internal struct AccessOrderPolicy : INodePolicy> @@ -56,6 +57,9 @@ public void OnEvict(AccessOrderNode node) public void ExpireEntries

(ref ConcurrentLfuCore, P> cache) where P : struct, INodePolicy> { } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool IsReadDrainDelayable() => true; } internal struct ExpireAfterPolicy : INodePolicy> @@ -136,5 +140,8 @@ public void ExpireEntries

(ref ConcurrentLfuCore, P> { wheel.Advance(ref cache, current); } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public bool IsReadDrainDelayable() => false; } } From 747e19897a55b71d93343f7997f0ff49492e587e Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Sat, 1 Jun 2024 17:34:36 -0700 Subject: [PATCH 2/4] proper test --- .../Lfu/ConcurrentTLfuTests.cs | 35 ++++++++++++------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs index 2056fcd3..59b0ad3f 100644 --- a/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs +++ b/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs @@ -26,27 +26,36 @@ public ConcurrentTLfuTests() lfu = new ConcurrentTLfu(capacity, new ExpireAfterWrite(timeToLive)); } - [Fact] - public void Repro() + // This is a scenario test to verify maintenance is run promptly after read. + [RetryFact] + public void WhenItemIsAccessedTimeToExpireIsUpdated() { var cache = new ConcurrentLfuBuilder() .WithCapacity(10) .WithExpireAfterAccess(TimeSpan.FromSeconds(5)) .Build(); - cache.AddOrUpdate(1, 1); - - Thread.Sleep(4000); - - cache.TryGet(1, out var value).Should().BeTrue(); - - Thread.Sleep(2000); - - cache.TryGet(1, out value).Should().BeTrue(); - cache.TryGet(1, out value).Should().BeTrue(); + Timed.Execute( + cache, + cache => + { + cache.AddOrUpdate(1, 1); + return cache; + }, + TimeSpan.FromSeconds(4), + cache => + { + cache.TryGet(1, out var value); + }, + TimeSpan.FromSeconds(2), + cache => + { + cache.TryGet(1, out var value).Should().BeTrue(); + cache.TryGet(1, out value).Should().BeTrue(); + } + ); } - [Fact] public void ConstructAddAndRetrieveWithCustomComparerReturnsValue() { From 3b30b74aa7235a426ce95f1e379efd8f17869cde Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Sat, 8 Jun 2024 10:53:34 -0700 Subject: [PATCH 3/4] update time on read --- BitFaster.Caching/Lfu/ConcurrentLfuCore.cs | 8 ++-- BitFaster.Caching/Lfu/NodePolicy.cs | 49 ++++++++++++---------- 2 files changed, 31 insertions(+), 26 deletions(-) diff --git a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs index af831e4c..c94a60dc 100644 --- a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs +++ b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs @@ -276,6 +276,7 @@ private bool TryGetImpl(K key, [MaybeNullWhen(false)] out V value) { TryScheduleDrain(); } + this.policy.OnRead(node); value = node.Value; return true; } @@ -366,6 +367,7 @@ public bool TryUpdate(K key, V value) // and we will just lose ordering/hit count, but not orphan the node. this.writeBuffer.TryAdd(node); TryScheduleDrain(); + this.policy.OnWrite(node); return true; } @@ -525,8 +527,6 @@ private bool Maintenance(N? droppedWrite = null) { this.drainStatus.VolatileWrite(DrainStatus.ProcessingToIdle); - policy.AdvanceTime(); - // Note: this is only Span on .NET Core 3.1+, else this is no-op and it is still an array var buffer = this.drainBuffer.AsSpanOrArray(); @@ -601,7 +601,7 @@ private void OnAccess(N node) break; } - policy.OnRead(node); + policy.AfterRead(node); } private void OnWrite(N node) @@ -650,7 +650,7 @@ private void OnWrite(N node) break; } - policy.OnWrite(node); + policy.AfterWrite(node); } private void PromoteProbation(LfuNode node) diff --git a/BitFaster.Caching/Lfu/NodePolicy.cs b/BitFaster.Caching/Lfu/NodePolicy.cs index 7b51c91d..006563ff 100644 --- a/BitFaster.Caching/Lfu/NodePolicy.cs +++ b/BitFaster.Caching/Lfu/NodePolicy.cs @@ -10,9 +10,10 @@ internal interface INodePolicy { N Create(K key, V value); bool IsExpired(N node); - void AdvanceTime(); void OnRead(N node); void OnWrite(N node); + void AfterRead(N node); + void AfterWrite(N node); void OnEvict(N node); void ExpireEntries

(ref ConcurrentLfuCore cache) where P : struct, INodePolicy; bool IsReadDrainDelayable(); @@ -34,17 +35,22 @@ public bool IsExpired(AccessOrderNode node) } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void AdvanceTime() + public void OnRead(AccessOrderNode node) { } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void OnRead(AccessOrderNode node) + public void OnWrite(AccessOrderNode node) { } [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void OnWrite(AccessOrderNode node) + public void AfterRead(AccessOrderNode node) + { + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void AfterWrite(AccessOrderNode node) { } @@ -88,29 +94,33 @@ public TimeOrderNode Create(K key, V value) [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool IsExpired(TimeOrderNode node) - { - return node.TimeToExpire < Duration.SinceEpoch(); - } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public void AdvanceTime() { current = Duration.SinceEpoch(); + return node.TimeToExpire < current; } [MethodImpl(MethodImplOptions.AggressiveInlining)] public void OnRead(TimeOrderNode node) { - var oldTte = node.TimeToExpire; + // we know IsExpired is always called immediate before OnRead, so piggyback on the current time node.TimeToExpire = current + expiryCalculator.GetExpireAfterRead(node.Key, node.Value, node.TimeToExpire - current); - if (oldTte.raw != node.TimeToExpire.raw) - { - wheel.Reschedule(node); - } } [MethodImpl(MethodImplOptions.AggressiveInlining)] public void OnWrite(TimeOrderNode node) + { + var current = Duration.SinceEpoch(); + node.TimeToExpire = current + expiryCalculator.GetExpireAfterUpdate(node.Key, node.Value, node.TimeToExpire - current); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void AfterRead(TimeOrderNode node) + { + wheel.Reschedule(node); + } + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public void AfterWrite(TimeOrderNode node) { // if the node is not yet scheduled, it is being created // the time is set on create in case it is read before the buffer is processed @@ -120,12 +130,7 @@ public void OnWrite(TimeOrderNode node) } else { - var oldTte = node.TimeToExpire; - node.TimeToExpire = current + expiryCalculator.GetExpireAfterUpdate(node.Key, node.Value, node.TimeToExpire - current); - if (oldTte.raw != node.TimeToExpire.raw) - { - wheel.Reschedule(node); - } + wheel.Reschedule(node); } } @@ -138,7 +143,7 @@ public void OnEvict(TimeOrderNode node) [MethodImpl(MethodImplOptions.AggressiveInlining)] public void ExpireEntries

(ref ConcurrentLfuCore, P> cache) where P : struct, INodePolicy> { - wheel.Advance(ref cache, current); + wheel.Advance(ref cache, Duration.SinceEpoch()); } [MethodImpl(MethodImplOptions.AggressiveInlining)] From ac459e775d0c2cc1664b74f448ddd6c97d7c40e6 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Sat, 8 Jun 2024 11:49:33 -0700 Subject: [PATCH 4/4] rem delayable --- BitFaster.Caching/Lfu/ConcurrentLfuCore.cs | 2 +- BitFaster.Caching/Lfu/NodePolicy.cs | 7 ------- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs index c94a60dc..373a5ebb 100644 --- a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs +++ b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs @@ -272,7 +272,7 @@ private bool TryGetImpl(K key, [MaybeNullWhen(false)] out V value) { bool delayable = this.readBuffer.TryAdd(node) != BufferStatus.Full; - if (this.drainStatus.ShouldDrain(policy.IsReadDrainDelayable() && delayable)) + if (this.drainStatus.ShouldDrain(delayable)) { TryScheduleDrain(); } diff --git a/BitFaster.Caching/Lfu/NodePolicy.cs b/BitFaster.Caching/Lfu/NodePolicy.cs index 006563ff..89b1861c 100644 --- a/BitFaster.Caching/Lfu/NodePolicy.cs +++ b/BitFaster.Caching/Lfu/NodePolicy.cs @@ -16,7 +16,6 @@ internal interface INodePolicy void AfterWrite(N node); void OnEvict(N node); void ExpireEntries

(ref ConcurrentLfuCore cache) where P : struct, INodePolicy; - bool IsReadDrainDelayable(); } internal struct AccessOrderPolicy : INodePolicy> @@ -63,9 +62,6 @@ public void OnEvict(AccessOrderNode node) public void ExpireEntries

(ref ConcurrentLfuCore, P> cache) where P : struct, INodePolicy> { } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool IsReadDrainDelayable() => true; } internal struct ExpireAfterPolicy : INodePolicy> @@ -145,8 +141,5 @@ public void ExpireEntries

(ref ConcurrentLfuCore, P> { wheel.Advance(ref cache, Duration.SinceEpoch()); } - - [MethodImpl(MethodImplOptions.AggressiveInlining)] - public bool IsReadDrainDelayable() => false; } }