diff --git a/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs b/BitFaster.Caching.UnitTests/Lfu/ConcurrentTLfuTests.cs index 34c42293..59b0ad3f 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,36 @@ public ConcurrentTLfuTests() lfu = new ConcurrentTLfu(capacity, new ExpireAfterWrite(timeToLive)); } + // 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(); + + 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() { diff --git a/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs b/BitFaster.Caching/Lfu/ConcurrentLfuCore.cs index 0b56b09d..373a5ebb 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 c0752175..89b1861c 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; } @@ -33,17 +34,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) { } @@ -84,29 +90,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 @@ -116,12 +126,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); } } @@ -134,7 +139,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()); } } }