From 4a5aad9c582a76341180e77709a51882bcc21c64 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Fri, 18 Aug 2023 15:40:01 -0700 Subject: [PATCH 1/8] cov --- .../Lru/ConcurrentLruTests.cs | 46 ++++++++++++++----- 1 file changed, 35 insertions(+), 11 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index 76603e23..a6729a9e 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1090,17 +1090,41 @@ public void WhenItemsAreTrimmedAnEventIsFired() [Fact] public async Task WhenItemsAreScannedInParallelCapacityIsNotExceeded() { - await Threaded.Run(4, () => { - for (int i = 0; i < 100000; i++) - { - lru.GetOrAdd(i + 1, i =>i.ToString()); - } - }); - - this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); - - // allow +/- 1 variance for capacity - lru.Count.Should().BeCloseTo(9, 1); + for (int i = 0; i < 10; i++) + { + await Threaded.Run(4, () => { + for (int i = 0; i < 100000; i++) + { + lru.GetOrAdd(i + 1, i =>i.ToString()); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + + // allow +/- 1 variance for capacity + lru.Count.Should().BeCloseTo(9, 1); + } + } + + [Fact] + public async Task WhenItemsAreScannedInParallel2() + { + for (int i = 0; i < 10; i++) + { + await Threaded.Run(4, () => { + for (int i = 0; i < 100000; i++) + { + lru.TryRemove(i + 1); + lru.GetOrAdd(i + 1, i => i.ToString()); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + + lru.Count.Should().BeLessThanOrEqualTo(9); + } } private void Warmup() From 24e00649b55e175db46e721e55f3f79946aeffc5 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Fri, 18 Aug 2023 15:50:43 -0700 Subject: [PATCH 2/8] cov --- .../Lru/ConcurrentLruTests.cs | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index a6729a9e..f0053660 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1123,6 +1123,26 @@ await Threaded.Run(4, () => { this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + lru.Count.Should().BeLessThanOrEqualTo(9); + } + } + + [Fact] + public async Task WhenItemsAreScannedInParallel3() + { + for (int i = 0; i < 10; i++) + { + await Threaded.Run(4, () => { + for (int i = 0; i < 100000; i++) + { + lru.TryUpdate(i + 1, i.ToString()); + lru.GetOrAdd(i + 1, i => i.ToString()); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + lru.Count.Should().BeLessThanOrEqualTo(9); } } From ea737132b958d09f6355fa00a2d73360165a0580 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Fri, 18 Aug 2023 17:31:00 -0700 Subject: [PATCH 3/8] more --- .../Lru/ConcurrentLruTests.cs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index f0053660..faf7246e 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1107,6 +1107,27 @@ await Threaded.Run(4, () => { } } + [Fact] + public async Task WhenItemsAreScannedInParallelCapacityIsNotExceeded2() + { + for (int i = 0; i < 10; i++) + { + await Threaded.Run(4, () => { + for (int i = 0; i < 100000; i++) + { + // use the arg overload + lru.GetOrAdd(i + 1, (i, s) => i.ToString(), "Foo"); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + + // allow +/- 1 variance for capacity + lru.Count.Should().BeCloseTo(9, 1); + } + } + [Fact] public async Task WhenItemsAreScannedInParallel2() { @@ -1143,6 +1164,26 @@ await Threaded.Run(4, () => { this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + lru.Count.Should().BeLessThanOrEqualTo(9); + } + } + + [Fact] + public async Task WhenItemsAreScannedInParallel4() + { + for (int i = 0; i < 10; i++) + { + await Threaded.Run(4, () => { + for (int i = 0; i < 100000; i++) + { + lru.AddOrUpdate(i + 1, i.ToString()); + lru.GetOrAdd(i + 1, i => i.ToString()); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + lru.Count.Should().BeLessThanOrEqualTo(9); } } From 33a387bdac5006cfa158f7db252ba73739047f00 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Mon, 21 Aug 2023 17:57:32 -0700 Subject: [PATCH 4/8] integrity check --- .../Lru/ConcurrentLruTests.cs | 77 +++++++++++++++++-- 1 file changed, 72 insertions(+), 5 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index faf7246e..3d8eeb8a 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -7,7 +7,10 @@ using System.Threading.Tasks; using Xunit; using Xunit.Abstractions; - +using System.Collections.Concurrent; +using System.Reflection; +using System.Runtime.CompilerServices; + namespace BitFaster.Caching.UnitTests.Lru { public class ConcurrentLruTests @@ -1104,6 +1107,7 @@ await Threaded.Run(4, () => { // allow +/- 1 variance for capacity lru.Count.Should().BeCloseTo(9, 1); + RunIntegrityCheck(); } } @@ -1125,6 +1129,7 @@ await Threaded.Run(4, () => { // allow +/- 1 variance for capacity lru.Count.Should().BeCloseTo(9, 1); + RunIntegrityCheck(); } } @@ -1144,7 +1149,7 @@ await Threaded.Run(4, () => { this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); - lru.Count.Should().BeLessThanOrEqualTo(9); + RunIntegrityCheck(); } } @@ -1164,7 +1169,7 @@ await Threaded.Run(4, () => { this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); - lru.Count.Should().BeLessThanOrEqualTo(9); + RunIntegrityCheck(); } } @@ -1183,8 +1188,8 @@ await Threaded.Run(4, () => { this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); - - lru.Count.Should().BeLessThanOrEqualTo(9); + + RunIntegrityCheck(); } } @@ -1200,5 +1205,67 @@ private void Warmup() lru.GetOrAdd(-8, valueFactory.Create); lru.GetOrAdd(-9, valueFactory.Create); } + + private void RunIntegrityCheck() + { + new ConcurrentLruIntegrityChecker, LruPolicy, TelemetryPolicy>(this.lru).Validate(); + } + } + + public class ConcurrentLruIntegrityChecker + where I : LruItem + where P : struct, IItemPolicy + where T : struct, ITelemetryPolicy + { + private readonly ConcurrentLruCore cache; + + private readonly ConcurrentQueue hotQueue; + private readonly ConcurrentQueue warmQueue; + private readonly ConcurrentQueue coldQueue; + + private static FieldInfo hotQueueField = typeof(ConcurrentLruCore).GetField("hotQueue", BindingFlags.NonPublic | BindingFlags.Instance); + private static FieldInfo warmQueueField = typeof(ConcurrentLruCore).GetField("warmQueue", BindingFlags.NonPublic | BindingFlags.Instance); + private static FieldInfo coldQueueField = typeof(ConcurrentLruCore).GetField("coldQueue", BindingFlags.NonPublic | BindingFlags.Instance); + + public ConcurrentLruIntegrityChecker(ConcurrentLruCore cache) + { + this.cache = cache; + + // get queues via reflection + this.hotQueue = (ConcurrentQueue)hotQueueField.GetValue(cache); + this.warmQueue = (ConcurrentQueue)warmQueueField.GetValue(cache); + this.coldQueue = (ConcurrentQueue)coldQueueField.GetValue(cache); + } + + public void Validate() + { + // queue counters must be consistent with queues + this.hotQueue.Count.Should().Be(cache.HotCount, "hot queue has a corrupted count"); + this.warmQueue.Count.Should().Be(cache.WarmCount, "warm queue has a corrupted count"); + this.coldQueue.Count.Should().Be(cache.ColdCount, "cold queue has a corrupted count"); + + // cache contents must be consistent with queued items + ValidateQueue(cache, this.hotQueue); + ValidateQueue(cache, this.warmQueue); + ValidateQueue(cache, this.coldQueue); + + // cache must be within capacity + cache.Count.Should().BeLessThanOrEqualTo(cache.Capacity + 1, "capacity out of valid range"); + } + + private static void ValidateQueue(ConcurrentLruCore cache, ConcurrentQueue queue) + { + foreach (var item in queue) + { + if (item.WasRemoved) + { + cache.TryGet(item.Key, out var value).Should().BeFalse(); + } + else + { + cache.TryGet(item.Key, out var value).Should().BeTrue(); + } + } + } } } From d725a5b87dbb66733b113f20f8126db7890e783c Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Mon, 21 Aug 2023 18:03:51 -0700 Subject: [PATCH 5/8] better log --- .../Lru/ConcurrentLruTests.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index 3d8eeb8a..d6521f46 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1245,25 +1245,25 @@ public void Validate() this.coldQueue.Count.Should().Be(cache.ColdCount, "cold queue has a corrupted count"); // cache contents must be consistent with queued items - ValidateQueue(cache, this.hotQueue); - ValidateQueue(cache, this.warmQueue); - ValidateQueue(cache, this.coldQueue); + ValidateQueue(cache, this.hotQueue, "hot"); + ValidateQueue(cache, this.warmQueue, "warm"); + ValidateQueue(cache, this.coldQueue, "cold"); // cache must be within capacity cache.Count.Should().BeLessThanOrEqualTo(cache.Capacity + 1, "capacity out of valid range"); } - private static void ValidateQueue(ConcurrentLruCore cache, ConcurrentQueue queue) + private static void ValidateQueue(ConcurrentLruCore cache, ConcurrentQueue queue, string queueName) { foreach (var item in queue) { if (item.WasRemoved) { - cache.TryGet(item.Key, out var value).Should().BeFalse(); + cache.TryGet(item.Key, out var value).Should().BeFalse($"{queueName} removed item {item.Key} was not removed"); } else { - cache.TryGet(item.Key, out var value).Should().BeTrue(); + cache.TryGet(item.Key, out var value).Should().BeTrue($"{queueName} item {item.Key} was not present"); } } } From d27811751b393b7898bd787c0db185f0d303b3c1 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Mon, 21 Aug 2023 18:18:03 -0700 Subject: [PATCH 6/8] todo: handle delete then add --- BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index d6521f46..218ee6d2 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1260,6 +1260,9 @@ private static void ValidateQueue(ConcurrentLruCore cache, Concur if (item.WasRemoved) { cache.TryGet(item.Key, out var value).Should().BeFalse($"{queueName} removed item {item.Key} was not removed"); + + // TODO: it is possible for the queues to contain 2 instances of the same key/item. One that was removed, and one that was added after the other was removed. + // In this case, the dictionary may contain the value. } else { From a742e055a3f37d6b7ac6a9f8f100fd402047927d Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Tue, 22 Aug 2023 12:20:19 -0700 Subject: [PATCH 7/8] handle delete --- .../Lru/ConcurrentLruTests.cs | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index 218ee6d2..b98c4b58 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1091,7 +1091,7 @@ public void WhenItemsAreTrimmedAnEventIsFired() } [Fact] - public async Task WhenItemsAreScannedInParallelCapacityIsNotExceeded() + public async Task WhenSoakConcurrentGetCacheEndsInConsistentState() { for (int i = 0; i < 10; i++) { @@ -1112,7 +1112,7 @@ await Threaded.Run(4, () => { } [Fact] - public async Task WhenItemsAreScannedInParallelCapacityIsNotExceeded2() + public async Task WhenSoakConcurrentGetWithArgCacheEndsInConsistentState() { for (int i = 0; i < 10; i++) { @@ -1134,7 +1134,7 @@ await Threaded.Run(4, () => { } [Fact] - public async Task WhenItemsAreScannedInParallel2() + public async Task WhenSoakConcurrentGetAndRemoveCacheEndsInConsistentState() { for (int i = 0; i < 10; i++) { @@ -1154,7 +1154,7 @@ await Threaded.Run(4, () => { } [Fact] - public async Task WhenItemsAreScannedInParallel3() + public async Task WhenSoakConcurrentGetAndUpdateCacheEndsInConsistentState() { for (int i = 0; i < 10; i++) { @@ -1174,7 +1174,7 @@ await Threaded.Run(4, () => { } [Fact] - public async Task WhenItemsAreScannedInParallel4() + public async Task WhenSoakConcurrentGetAndAddCacheEndsInConsistentState() { for (int i = 0; i < 10; i++) { @@ -1253,16 +1253,22 @@ public void Validate() cache.Count.Should().BeLessThanOrEqualTo(cache.Capacity + 1, "capacity out of valid range"); } - private static void ValidateQueue(ConcurrentLruCore cache, ConcurrentQueue queue, string queueName) + private void ValidateQueue(ConcurrentLruCore cache, ConcurrentQueue queue, string queueName) { foreach (var item in queue) { if (item.WasRemoved) { - cache.TryGet(item.Key, out var value).Should().BeFalse($"{queueName} removed item {item.Key} was not removed"); - // TODO: it is possible for the queues to contain 2 instances of the same key/item. One that was removed, and one that was added after the other was removed. // In this case, the dictionary may contain the value. + if (cache.TryGet(item.Key, out var value)) + { + hotQueue.Union(warmQueue).Union(coldQueue).Any(i => i.Key.Equals(item.Key) && !i.WasRemoved).Should().BeTrue($"{queueName} removed item {item.Key} was not removed"); + } + + //cache.TryGet(item.Key, out var value).Should().BeFalse($"{queueName} removed item {item.Key} was not removed"); + + } else { From e4e5a23b9e264bb579e4fcb4220bacc3179ea743 Mon Sep 17 00:00:00 2001 From: Alex Peck Date: Tue, 22 Aug 2023 12:35:41 -0700 Subject: [PATCH 8/8] soak async --- .../Lru/ConcurrentLruTests.cs | 56 ++++++++++++++++--- BitFaster.Caching.UnitTests/Threaded.cs | 25 +++++++++ 2 files changed, 74 insertions(+), 7 deletions(-) diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index b98c4b58..ed2bf960 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -1111,6 +1111,27 @@ await Threaded.Run(4, () => { } } + [Fact] + public async Task WhenSoakConcurrentGetAsyncCacheEndsInConsistentState() + { + for (int i = 0; i < 10; i++) + { + await Threaded.RunAsync(4, async () => { + for (int i = 0; i < 100000; i++) + { + await lru.GetOrAddAsync(i + 1, i => Task.FromResult(i.ToString())); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + + // allow +/- 1 variance for capacity + lru.Count.Should().BeCloseTo(9, 1); + RunIntegrityCheck(); + } + } + [Fact] public async Task WhenSoakConcurrentGetWithArgCacheEndsInConsistentState() { @@ -1133,6 +1154,28 @@ await Threaded.Run(4, () => { } } + [Fact] + public async Task WhenSoakConcurrentGetAsyncWithArgCacheEndsInConsistentState() + { + for (int i = 0; i < 10; i++) + { + await Threaded.RunAsync(4, async () => { + for (int i = 0; i < 100000; i++) + { + // use the arg overload + await lru.GetOrAddAsync(i + 1, (i, s) => Task.FromResult(i.ToString()), "Foo"); + } + }); + + this.testOutputHelper.WriteLine($"{lru.HotCount} {lru.WarmCount} {lru.ColdCount}"); + this.testOutputHelper.WriteLine(string.Join(" ", lru.Keys)); + + // allow +/- 1 variance for capacity + lru.Count.Should().BeCloseTo(9, 1); + RunIntegrityCheck(); + } + } + [Fact] public async Task WhenSoakConcurrentGetAndRemoveCacheEndsInConsistentState() { @@ -1259,16 +1302,15 @@ private void ValidateQueue(ConcurrentLruCore cache, ConcurrentQue { if (item.WasRemoved) { - // TODO: it is possible for the queues to contain 2 instances of the same key/item. One that was removed, and one that was added after the other was removed. - // In this case, the dictionary may contain the value. + // It is possible for the queues to contain 2 (or more) instances of the same key/item. One that was removed, + // and one that was added after the other was removed. + // In this case, the dictionary may contain the value only if the queues contain an entry for that key marked as WasRemoved == false. if (cache.TryGet(item.Key, out var value)) { - hotQueue.Union(warmQueue).Union(coldQueue).Any(i => i.Key.Equals(item.Key) && !i.WasRemoved).Should().BeTrue($"{queueName} removed item {item.Key} was not removed"); + hotQueue.Union(warmQueue).Union(coldQueue) + .Any(i => i.Key.Equals(item.Key) && !i.WasRemoved) + .Should().BeTrue($"{queueName} removed item {item.Key} was not removed"); } - - //cache.TryGet(item.Key, out var value).Should().BeFalse($"{queueName} removed item {item.Key} was not removed"); - - } else { diff --git a/BitFaster.Caching.UnitTests/Threaded.cs b/BitFaster.Caching.UnitTests/Threaded.cs index 50e3f10b..a3f575f5 100644 --- a/BitFaster.Caching.UnitTests/Threaded.cs +++ b/BitFaster.Caching.UnitTests/Threaded.cs @@ -33,5 +33,30 @@ public static async Task Run(int threadCount, Action action) await Task.WhenAll(tasks); } + + public static Task RunAsync(int threadCount, Func action) + { + return Run(threadCount, i => action()); + } + + public static async Task RunAsync(int threadCount, Func action) + { + var tasks = new Task[threadCount]; + ManualResetEvent mre = new ManualResetEvent(false); + + for (int i = 0; i < threadCount; i++) + { + int run = i; + tasks[i] = Task.Run(async () => + { + mre.WaitOne(); + await action(run); + }); + } + + mre.Set(); + + await Task.WhenAll(tasks); + } } }