From c05b9b20b08a397d2d8faa5ab309f2c39f555ec0 Mon Sep 17 00:00:00 2001 From: alexpeck Date: Sat, 13 Jun 2020 16:28:43 -0700 Subject: [PATCH 1/5] fix remove --- .../Lru/LruGetOrAddTest.cs | 12 -- .../Lru/MissHitHitRemove.cs | 117 ++++++++++++++++++ ...ntedLruTests.cs => PrimitiveBenchmarks.cs} | 8 +- .../Lru/ConcurrentLruTests.cs | 24 +++- Lightweight.Caching/Lru/LruItem.cs | 11 +- .../Lru/TemplateConcurrentLru.cs | 19 ++- README.md | 10 +- 7 files changed, 174 insertions(+), 27 deletions(-) create mode 100644 Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs rename Lightweight.Caching.Benchmarks/{SegmentedLruTests.cs => PrimitiveBenchmarks.cs} (91%) diff --git a/Lightweight.Caching.Benchmarks/Lru/LruGetOrAddTest.cs b/Lightweight.Caching.Benchmarks/Lru/LruGetOrAddTest.cs index 1a6a667a..d636a6a7 100644 --- a/Lightweight.Caching.Benchmarks/Lru/LruGetOrAddTest.cs +++ b/Lightweight.Caching.Benchmarks/Lru/LruGetOrAddTest.cs @@ -38,12 +38,6 @@ public void ConcurrentDictionaryGetOrAdd() dictionary.GetOrAdd(1, func); } - //[Benchmark()] - //public DateTime DateTimeUtcNow() - //{ - // return DateTime.UtcNow; - //} - [Benchmark()] public void FastConcurrentLruGetOrAdd() { @@ -79,12 +73,6 @@ public void ClassicLruGetOrAdd() classicLru.GetOrAdd(1, func); } - //[Benchmark()] - //public void MemoryCacheGetIntKey() - //{ - // memoryCache.Get(key.ToString()); - //} - [Benchmark()] public void MemoryCacheGetStringKey() { diff --git a/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs b/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs new file mode 100644 index 00000000..364386d6 --- /dev/null +++ b/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs @@ -0,0 +1,117 @@ +using System; +using System.Collections.Concurrent; +using System.Collections.Generic; +using System.Runtime.Caching; +using System.Text; +using BenchmarkDotNet.Attributes; +using Lightweight.Caching.Lru; + +namespace Lightweight.Caching.Benchmarks.Lru +{ + [MemoryDiagnoser] + public class MissHitHitRemove + { + private static readonly ConcurrentDictionary dictionary = new ConcurrentDictionary(8, 9, EqualityComparer.Default); + + private static readonly ClassicLru classicLru = new ClassicLru(8, 9, EqualityComparer.Default); + private static readonly ConcurrentLru concurrentLru = new ConcurrentLru(8, 9, EqualityComparer.Default); + private static readonly ConcurrentTLru concurrentTlru = new ConcurrentTLru(8, 9, EqualityComparer.Default, TimeSpan.FromMinutes(10)); + private static readonly FastConcurrentLru fastConcurrentLru = new FastConcurrentLru(8, 9, EqualityComparer.Default); + private static readonly FastConcurrentTLru fastConcurrentTLru = new FastConcurrentTLru(8, 9, EqualityComparer.Default, TimeSpan.FromMinutes(1)); + + private static MemoryCache memoryCache = System.Runtime.Caching.MemoryCache.Default; + + [Benchmark(Baseline = true)] + public void ConcurrentDictionary() + { + Func func = x => new byte[16]; + dictionary.GetOrAdd(1, func); + + dictionary.GetOrAdd(1, func); + dictionary.GetOrAdd(1, func); + + dictionary.TryRemove(1, out var removed); + } + + [Benchmark()] + public void FastConcurrentLru() + { + Func func = x => new byte[16]; + fastConcurrentLru.GetOrAdd(1, func); + + fastConcurrentLru.GetOrAdd(1, func); + fastConcurrentLru.GetOrAdd(1, func); + + fastConcurrentLru.TryRemove(1); + } + + [Benchmark()] + public void ConcurrentLru() + { + Func func = x => new byte[16]; + concurrentLru.GetOrAdd(1, func); + + concurrentLru.GetOrAdd(1, func); + concurrentLru.GetOrAdd(1, func); + + concurrentLru.TryRemove(1); + } + + [Benchmark()] + public void FastConcurrentTlru() + { + Func func = x => new byte[16]; + fastConcurrentTLru.GetOrAdd(1, func); + + fastConcurrentTLru.GetOrAdd(1, func); + fastConcurrentTLru.GetOrAdd(1, func); + + fastConcurrentTLru.TryRemove(1); + } + + [Benchmark()] + public void ConcurrentTlru() + { + Func func = x => new byte[16]; + concurrentTlru.GetOrAdd(1, func); + + concurrentTlru.GetOrAdd(1, func); + concurrentTlru.GetOrAdd(1, func); + + concurrentTlru.TryRemove(1); + } + + [Benchmark()] + public void ClassicLru() + { + Func func = x => new byte[16]; + classicLru.GetOrAdd(1, func); + + classicLru.GetOrAdd(1, func); + classicLru.GetOrAdd(1, func); + + classicLru.TryRemove(1); + } + + [Benchmark()] + public void MemoryCache() + { + if (memoryCache.Get("1") == null) + { + memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + } + + if (memoryCache.Get("1") == null) + { + memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + } + + if (memoryCache.Get("1") == null) + { + memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + } + + memoryCache.Remove("1"); + } + } +} diff --git a/Lightweight.Caching.Benchmarks/SegmentedLruTests.cs b/Lightweight.Caching.Benchmarks/PrimitiveBenchmarks.cs similarity index 91% rename from Lightweight.Caching.Benchmarks/SegmentedLruTests.cs rename to Lightweight.Caching.Benchmarks/PrimitiveBenchmarks.cs index b8ec653f..7c015eb3 100644 --- a/Lightweight.Caching.Benchmarks/SegmentedLruTests.cs +++ b/Lightweight.Caching.Benchmarks/PrimitiveBenchmarks.cs @@ -10,7 +10,7 @@ namespace Lightweight.Caching.Benchmarks { [MemoryDiagnoser] - public class SegmentedLruTests + public class PrimitiveBenchmarks { private static readonly ConcurrentDictionary dictionary = new ConcurrentDictionary(8, 9, EqualityComparer.Default); LinkedList intList = new LinkedList(new int[] { 1, 2, 3 }); @@ -45,6 +45,12 @@ public void LinkedListLockSwapFirstToLast() } } + [Benchmark()] + public DateTime DateTimeUtcNow() + { + return DateTime.UtcNow; + } + [Benchmark()] public void DictionaryGetOrAdd() { diff --git a/Lightweight.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/Lightweight.Caching.UnitTests/Lru/ConcurrentLruTests.cs index a8a109b2..14b6b60c 100644 --- a/Lightweight.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/Lightweight.Caching.UnitTests/Lru/ConcurrentLruTests.cs @@ -141,14 +141,14 @@ public async Task WhenDifferentKeysAreRequesteValueIsCreatedForEachAsync() [Fact] public void WhenValuesAreNotReadAndMoreKeysRequestedThanCapacityCountDoesNotIncrease() { - int capacity = hotCap + coldCap; - for (int i = 0; i < capacity + 1; i++) + int hotColdCapacity = hotCap + coldCap; + for (int i = 0; i < hotColdCapacity + 1; i++) { lru.GetOrAdd(i, valueFactory.Create); } - lru.Count.Should().Be(capacity); - valueFactory.timesCalled.Should().Be(capacity + 1); + lru.Count.Should().Be(hotColdCapacity); + valueFactory.timesCalled.Should().Be(hotColdCapacity + 1); } [Fact] @@ -352,7 +352,21 @@ public void WhenKeyDoesNotExistTryRemoveReturnsFalse() lru.TryRemove(2).Should().BeFalse(); } - private class DisposableItem : IDisposable + [Fact] + public void WhenRepeatedlyAddingAndRemovingSameValueLruRemainsInConsistentState() + { + int capacity = hotCap + coldCap + warmCap; + for (int i = 0; i < capacity; i++) + { + // Because TryRemove leaves the item in the queue, when it is eventually removed + // from the cold queue, it should not remove the newly created value. + lru.GetOrAdd(1, valueFactory.Create); + lru.TryGet(1, out var value).Should().BeTrue(); + lru.TryRemove(1); + } + } + + private class DisposableItem : IDisposable { public bool IsDisposed { get; private set; } diff --git a/Lightweight.Caching/Lru/LruItem.cs b/Lightweight.Caching/Lru/LruItem.cs index 93cc427f..debae1df 100644 --- a/Lightweight.Caching/Lru/LruItem.cs +++ b/Lightweight.Caching/Lru/LruItem.cs @@ -9,8 +9,9 @@ namespace Lightweight.Caching.Lru public class LruItem { private bool wasAccessed; + private bool wasRemoved; - public LruItem(K k, V v) + public LruItem(K k, V v) { this.Key = k; this.Value = v; @@ -25,5 +26,11 @@ public bool WasAccessed get => this.wasAccessed; set => this.wasAccessed = value; } - } + + public bool WasRemoved + { + get => this.wasRemoved; + set => this.wasRemoved = value; + } + } } diff --git a/Lightweight.Caching/Lru/TemplateConcurrentLru.cs b/Lightweight.Caching/Lru/TemplateConcurrentLru.cs index 8d9e7d83..1d8173ba 100644 --- a/Lightweight.Caching/Lru/TemplateConcurrentLru.cs +++ b/Lightweight.Caching/Lru/TemplateConcurrentLru.cs @@ -169,11 +169,13 @@ public bool TryRemove(K key) { if (this.dictionary.TryRemove(key, out var removedItem)) { + removedItem.WasRemoved = true; + // 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. removedItem.WasAccessed = false; - + if (removedItem.Value is IDisposable d) { d.Dispose(); @@ -290,11 +292,18 @@ private void Move(I item, ItemDestination where) Interlocked.Increment(ref this.coldCount); break; case ItemDestination.Remove: - if (this.dictionary.TryRemove(item.Key, out var removedItem)) - { - if (removedItem.Value is IDisposable d) + if (!item.WasRemoved) + { + // We only mark the item that was actually removed as removed. ConcurrentDictionary.TryRemove + // is thread sage, so only 1 thread can remove each item. If there is a race with this.TryRemove + // only 1 thread will actually remove the item. + if (this.dictionary.TryRemove(item.Key, out var removedItem)) { - d.Dispose(); + item.WasRemoved = true; + if (removedItem.Value is IDisposable d) + { + d.Dispose(); + } } } break; diff --git a/README.md b/README.md index a5671a4e..1856c302 100644 --- a/README.md +++ b/README.md @@ -21,9 +21,15 @@ LRU implementations are intended as an alternative to the System.Runtime.Caching ## Lru Benchmarks -### Lookup speed with queue cycling +### Lookup speed -Cache contains 6 items which are fetched repeatedly, no items are evicted. For LRU caches this measures time spent maintaining item access order. For all other classes (including MemoryCache), it is a pure lookup. FastConcurrentLru does not allocate and is approximately 10x faster than MemoryCache. +Cache contains 6 items which are fetched repeatedly, no items are evicted. + +- ConcurrentLru family does not move items in the queues, it is just marking as accessed for pure cache hits. +- ClassicLru must maintain item order, and is internally splicing the fetched item to the head of the linked list. +- MemoryCache and ConcurrentDictionary represent a pure lookup. This is the best case scenario for MemoryCache, since the lookup key is a string (if the key were a Guid, using MemoryCache adds string conversion overhead). + +FastConcurrentLru does not allocate and is approximately 10x faster than MemoryCache. ~~~ BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.900 (1909/November2018Update/19H2) From 0248d9ed161437771deda575b9026c92f5ad606a Mon Sep 17 00:00:00 2001 From: alexpeck Date: Sat, 13 Jun 2020 16:37:14 -0700 Subject: [PATCH 2/5] update git url --- Lightweight.Caching/Lightweight.Caching.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lightweight.Caching/Lightweight.Caching.csproj b/Lightweight.Caching/Lightweight.Caching.csproj index ca5d12ef..e10934f1 100644 --- a/Lightweight.Caching/Lightweight.Caching.csproj +++ b/Lightweight.Caching/Lightweight.Caching.csproj @@ -11,7 +11,7 @@ 0.9.1 Copyright © Alex Peck 2020 - https://github.com/plexcake/Lightweight.Caching + https://github.com/peck-a/Lightweight.Caching Cache From d5a49b5a8b0153f253b7be2af0a7ebd8ce5baa60 Mon Sep 17 00:00:00 2001 From: alexpeck Date: Sat, 13 Jun 2020 18:40:40 -0700 Subject: [PATCH 3/5] fix race with lock --- .../Lru/LruCycle.cs | 14 ++-- .../Lru/MissHitHitRemove.cs | 33 +++++---- Lightweight.Caching.Benchmarks/Program.cs | 2 +- .../Lightweight.Caching.csproj | 2 +- .../Lru/TemplateConcurrentLru.cs | 73 ++++++++++++++----- README.md | 12 +++ 6 files changed, 92 insertions(+), 44 deletions(-) diff --git a/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs b/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs index b973d63c..5eeba404 100644 --- a/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs +++ b/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs @@ -60,7 +60,7 @@ public void GlobalSetup() } [Benchmark(Baseline = true, OperationsPerInvoke = 24)] - public void ConcurrentDictionaryGetOrAdd() + public void ConcurrentDictionary() { Func func = x => x; dictionary.GetOrAdd(1, func); @@ -93,7 +93,7 @@ public void ConcurrentDictionaryGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void FastConcurrentLruGetOrAdd() + public void FastConcurrentLru() { // size is 9, so segment size is 3. 6 items will cause queue cycling // without eviction. Hot => cold when not accessed. @@ -128,7 +128,7 @@ public void FastConcurrentLruGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void ConcurrentLruGetOrAdd() + public void ConcurrentLru() { // size is 9, so segment size is 3. 6 items will cause queue cycling // without eviction. Hot => cold when not accessed. @@ -163,7 +163,7 @@ public void ConcurrentLruGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void FastConcurrentTLruGetOrAdd() + public void FastConcurrentTLru() { // size is 9, so segment size is 3. 6 items will cause queue cycling // without eviction. Hot => cold when not accessed. @@ -198,7 +198,7 @@ public void FastConcurrentTLruGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void ConcurrentTLruGetOrAdd() + public void ConcurrentTLru() { // size is 9, so segment size is 3. 6 items will cause queue cycling // without eviction. Hot => cold when not accessed. @@ -233,7 +233,7 @@ public void ConcurrentTLruGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void ClassicLruGetOrAdd() + public void ClassicLru() { // size is 9, so segment size is 3. 6 items will cause queue cycling // without eviction. Hot => cold when not accessed. @@ -268,7 +268,7 @@ public void ClassicLruGetOrAdd() } [Benchmark(OperationsPerInvoke = 24)] - public void MemoryCacheGetStringKey() + public void MemoryCache() { memoryCache.Get("1"); memoryCache.Get("2"); diff --git a/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs b/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs index 364386d6..093c2263 100644 --- a/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs +++ b/Lightweight.Caching.Benchmarks/Lru/MissHitHitRemove.cs @@ -11,20 +11,23 @@ namespace Lightweight.Caching.Benchmarks.Lru [MemoryDiagnoser] public class MissHitHitRemove { - private static readonly ConcurrentDictionary dictionary = new ConcurrentDictionary(8, 9, EqualityComparer.Default); + const int capacity = 9; + const int arraySize = 16; - private static readonly ClassicLru classicLru = new ClassicLru(8, 9, EqualityComparer.Default); - private static readonly ConcurrentLru concurrentLru = new ConcurrentLru(8, 9, EqualityComparer.Default); - private static readonly ConcurrentTLru concurrentTlru = new ConcurrentTLru(8, 9, EqualityComparer.Default, TimeSpan.FromMinutes(10)); - private static readonly FastConcurrentLru fastConcurrentLru = new FastConcurrentLru(8, 9, EqualityComparer.Default); - private static readonly FastConcurrentTLru fastConcurrentTLru = new FastConcurrentTLru(8, 9, EqualityComparer.Default, TimeSpan.FromMinutes(1)); + private static readonly ConcurrentDictionary dictionary = new ConcurrentDictionary(8, capacity, EqualityComparer.Default); + + private static readonly ClassicLru classicLru = new ClassicLru(8, capacity, EqualityComparer.Default); + private static readonly ConcurrentLru concurrentLru = new ConcurrentLru(8, capacity, EqualityComparer.Default); + private static readonly ConcurrentTLru concurrentTlru = new ConcurrentTLru(8, capacity, EqualityComparer.Default, TimeSpan.FromMinutes(10)); + private static readonly FastConcurrentLru fastConcurrentLru = new FastConcurrentLru(8, capacity, EqualityComparer.Default); + private static readonly FastConcurrentTLru fastConcurrentTLru = new FastConcurrentTLru(8, capacity, EqualityComparer.Default, TimeSpan.FromMinutes(1)); private static MemoryCache memoryCache = System.Runtime.Caching.MemoryCache.Default; [Benchmark(Baseline = true)] public void ConcurrentDictionary() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; dictionary.GetOrAdd(1, func); dictionary.GetOrAdd(1, func); @@ -36,7 +39,7 @@ public void ConcurrentDictionary() [Benchmark()] public void FastConcurrentLru() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; fastConcurrentLru.GetOrAdd(1, func); fastConcurrentLru.GetOrAdd(1, func); @@ -48,7 +51,7 @@ public void FastConcurrentLru() [Benchmark()] public void ConcurrentLru() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; concurrentLru.GetOrAdd(1, func); concurrentLru.GetOrAdd(1, func); @@ -60,7 +63,7 @@ public void ConcurrentLru() [Benchmark()] public void FastConcurrentTlru() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; fastConcurrentTLru.GetOrAdd(1, func); fastConcurrentTLru.GetOrAdd(1, func); @@ -72,7 +75,7 @@ public void FastConcurrentTlru() [Benchmark()] public void ConcurrentTlru() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; concurrentTlru.GetOrAdd(1, func); concurrentTlru.GetOrAdd(1, func); @@ -84,7 +87,7 @@ public void ConcurrentTlru() [Benchmark()] public void ClassicLru() { - Func func = x => new byte[16]; + Func func = x => new byte[arraySize]; classicLru.GetOrAdd(1, func); classicLru.GetOrAdd(1, func); @@ -98,17 +101,17 @@ public void MemoryCache() { if (memoryCache.Get("1") == null) { - memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + memoryCache.Set("1", new byte[arraySize], new CacheItemPolicy()); } if (memoryCache.Get("1") == null) { - memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + memoryCache.Set("1", new byte[arraySize], new CacheItemPolicy()); } if (memoryCache.Get("1") == null) { - memoryCache.Set("1", new byte[16], new CacheItemPolicy()); + memoryCache.Set("1", new byte[arraySize], new CacheItemPolicy()); } memoryCache.Remove("1"); diff --git a/Lightweight.Caching.Benchmarks/Program.cs b/Lightweight.Caching.Benchmarks/Program.cs index 45a5f364..671ad617 100644 --- a/Lightweight.Caching.Benchmarks/Program.cs +++ b/Lightweight.Caching.Benchmarks/Program.cs @@ -15,7 +15,7 @@ class Program static void Main(string[] args) { var summary = BenchmarkRunner - .Run(ManualConfig.Create(DefaultConfig.Instance) + .Run(ManualConfig.Create(DefaultConfig.Instance) .AddJob(Job.RyuJitX64)); } } diff --git a/Lightweight.Caching/Lightweight.Caching.csproj b/Lightweight.Caching/Lightweight.Caching.csproj index e10934f1..fe1d0c5c 100644 --- a/Lightweight.Caching/Lightweight.Caching.csproj +++ b/Lightweight.Caching/Lightweight.Caching.csproj @@ -11,7 +11,7 @@ 0.9.1 Copyright © Alex Peck 2020 - https://github.com/peck-a/Lightweight.Caching + https://github.com/bitfaster/Lightweight.Caching Cache diff --git a/Lightweight.Caching/Lru/TemplateConcurrentLru.cs b/Lightweight.Caching/Lru/TemplateConcurrentLru.cs index 1d8173ba..c4258b86 100644 --- a/Lightweight.Caching/Lru/TemplateConcurrentLru.cs +++ b/Lightweight.Caching/Lru/TemplateConcurrentLru.cs @@ -167,21 +167,48 @@ public async Task GetOrAddAsync(K key, Func> valueFactory) public bool TryRemove(K key) { - if (this.dictionary.TryRemove(key, out var removedItem)) + // Possible race condition: + // Thread A TryRemove(1), removes LruItem1, has reference to removed item but not yet marked as removed + // Thread B GetOrAdd(1) => Adds LruItem1* + // Thread C GetOrAdd(2), Cycle, Move(LruItem1, Removed) + // + // Thread C can run and remove LruItem1* from this.dictionary before Thread A has marked LruItem1 as removed. + // + // In this situation, a subsequent attempt to fetch 1 will be a miss. The queues will still contain LruItem1*, + // and it will not be marked as removed. If key 1 is fetched while LruItem1* is still in the queue, there will + // be two queue entries for key 1, and neither is marked as removed. Thus when LruItem1 * ages out, it will + // incorrectly remove 1 from the dictionary, and this cycle can repeat. + if (this.dictionary.TryGetValue(key, out var existing)) { - removedItem.WasRemoved = true; - - // 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. - removedItem.WasAccessed = false; - - if (removedItem.Value is IDisposable d) + if (existing.WasRemoved) { - d.Dispose(); + return false; } - return true; + lock (existing) + { + if (existing.WasRemoved) + { + return false; + } + + existing.WasRemoved = true; + } + + if (this.dictionary.TryRemove(key, out var removedItem)) + { + // 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. + removedItem.WasAccessed = false; + + if (removedItem.Value is IDisposable d) + { + d.Dispose(); + } + + return true; + } } return false; @@ -293,16 +320,22 @@ private void Move(I item, ItemDestination where) break; case ItemDestination.Remove: if (!item.WasRemoved) - { - // We only mark the item that was actually removed as removed. ConcurrentDictionary.TryRemove - // is thread sage, so only 1 thread can remove each item. If there is a race with this.TryRemove - // only 1 thread will actually remove the item. - if (this.dictionary.TryRemove(item.Key, out var removedItem)) - { - item.WasRemoved = true; - if (removedItem.Value is IDisposable d) + { + // avoid race where 2 threads could remove the same key - see TryRemove for details. + lock (item) + { + if (item.WasRemoved) + { + break; + } + + if (this.dictionary.TryRemove(item.Key, out var removedItem)) { - d.Dispose(); + item.WasRemoved = true; + if (removedItem.Value is IDisposable d) + { + d.Dispose(); + } } } } diff --git a/README.md b/README.md index 1856c302..fe5b5ba5 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,18 @@ Job=RyuJitX64 Jit=RyuJit Platform=X64 | ClassicLruGetOrAdd | 75.67 ns | 1.513 ns | 1.554 ns | 3.99 | - | - | | MemoryCacheGetStringKey | 309.14 ns | 2.155 ns | 1.910 ns | 16.17 | 0.0153 | 32 B | +MissHitHitRemove + +| Method | Mean | Error | StdDev | Ratio | Gen 0 | Allocated | +|--------------------- |-----------:|---------:|---------:|------:|-------:|----------:| +| ConcurrentDictionary | 175.4 ns | 1.80 ns | 1.50 ns | 1.00 | 0.0381 | 80 B | +| FastConcurrentLru | 370.8 ns | 3.86 ns | 3.02 ns | 2.11 | 0.0534 | 112 B | +| ConcurrentLru | 379.8 ns | 3.50 ns | 2.93 ns | 2.17 | 0.0534 | 112 B | +| FastConcurrentTlru | 891.8 ns | 13.16 ns | 11.67 ns | 5.09 | 0.0572 | 120 B | +| ConcurrentTlru | 917.0 ns | 13.07 ns | 16.05 ns | 5.24 | 0.0572 | 120 B | +| ClassicLru | 356.9 ns | 5.13 ns | 4.80 ns | 2.04 | 0.0763 | 160 B | +| MemoryCache | 2,366.7 ns | 46.05 ns | 47.29 ns | 13.49 | 2.3460 | 4912 B | + ## Meta-programming using structs for JIT dead code removal and inlining TemplateConcurrentLru features injectable policies defined as structs. Since structs are subject to special JIT optimizations, the implementation is much faster than if these policies were defined as classes. Using this technique, 'Fast' versions without hit counting are within 30% of the speed of a ConcurrentDictionary. From 4e24246d2d26385d31dc58c22212d4157aba7546 Mon Sep 17 00:00:00 2001 From: alexpeck Date: Sat, 13 Jun 2020 18:42:59 -0700 Subject: [PATCH 4/5] fix build --- Lightweight.Caching.Benchmarks/Lru/LruCycle.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs b/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs index 5eeba404..41c72be2 100644 --- a/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs +++ b/Lightweight.Caching.Benchmarks/Lru/LruCycle.cs @@ -22,7 +22,7 @@ public class LruCycle private static readonly FastConcurrentLru fastConcurrentLru = new FastConcurrentLru(8, 9, EqualityComparer.Default); private static readonly FastConcurrentTLru fastConcurrentTLru = new FastConcurrentTLru(8, 9, EqualityComparer.Default, TimeSpan.FromMinutes(1)); - private static MemoryCache memoryCache = MemoryCache.Default; + private static MemoryCache memoryCache = System.Runtime.Caching.MemoryCache.Default; [GlobalSetup] public void GlobalSetup() From 50274ce5138d0d20ea018b650a5d09f10bbf73d2 Mon Sep 17 00:00:00 2001 From: alexpeck Date: Sat, 13 Jun 2020 18:55:10 -0700 Subject: [PATCH 5/5] update readme --- README.md | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) diff --git a/README.md b/README.md index fe5b5ba5..e1f38bcc 100644 --- a/README.md +++ b/README.md @@ -21,16 +21,6 @@ LRU implementations are intended as an alternative to the System.Runtime.Caching ## Lru Benchmarks -### Lookup speed - -Cache contains 6 items which are fetched repeatedly, no items are evicted. - -- ConcurrentLru family does not move items in the queues, it is just marking as accessed for pure cache hits. -- ClassicLru must maintain item order, and is internally splicing the fetched item to the head of the linked list. -- MemoryCache and ConcurrentDictionary represent a pure lookup. This is the best case scenario for MemoryCache, since the lookup key is a string (if the key were a Guid, using MemoryCache adds string conversion overhead). - -FastConcurrentLru does not allocate and is approximately 10x faster than MemoryCache. - ~~~ BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.900 (1909/November2018Update/19H2) Intel Core i7-5600U CPU 2.60GHz (Broadwell), 1 CPU, 4 logical and 2 physical cores @@ -41,6 +31,16 @@ Intel Core i7-5600U CPU 2.60GHz (Broadwell), 1 CPU, 4 logical and 2 physical cor Job=RyuJitX64 Jit=RyuJit Platform=X64 ~~~ +### Lookup speed + +Cache contains 6 items which are fetched repeatedly, no items are evicted. Representative of high hit rate scenario. + +- ConcurrentLru family does not move items in the queues, it is just marking as accessed for pure cache hits. +- ClassicLru must maintain item order, and is internally splicing the fetched item to the head of the linked list. +- MemoryCache and ConcurrentDictionary represent a pure lookup. This is the best case scenario for MemoryCache, since the lookup key is a string (if the key were a Guid, using MemoryCache adds string conversion overhead). + +FastConcurrentLru does not allocate and is approximately 10x faster than MemoryCache. + | Method | Mean | Error | StdDev | Ratio | Gen 0 | Allocated | |----------------------------- |----------:|---------:|---------:|------:|-------:|----------:| | ConcurrentDictionaryGetOrAdd | 18.72 ns | 0.289 ns | 0.641 ns | 1.00 | - | - | @@ -51,17 +51,22 @@ Job=RyuJitX64 Jit=RyuJit Platform=X64 | ClassicLruGetOrAdd | 75.67 ns | 1.513 ns | 1.554 ns | 3.99 | - | - | | MemoryCacheGetStringKey | 309.14 ns | 2.155 ns | 1.910 ns | 16.17 | 0.0153 | 32 B | -MissHitHitRemove +### Mixed workload + +Tests 4 operations, 1 miss (adding the item), 2 hits then remove. + +This test needs to be improved to provoke queue cycling. + | Method | Mean | Error | StdDev | Ratio | Gen 0 | Allocated | |--------------------- |-----------:|---------:|---------:|------:|-------:|----------:| -| ConcurrentDictionary | 175.4 ns | 1.80 ns | 1.50 ns | 1.00 | 0.0381 | 80 B | -| FastConcurrentLru | 370.8 ns | 3.86 ns | 3.02 ns | 2.11 | 0.0534 | 112 B | -| ConcurrentLru | 379.8 ns | 3.50 ns | 2.93 ns | 2.17 | 0.0534 | 112 B | -| FastConcurrentTlru | 891.8 ns | 13.16 ns | 11.67 ns | 5.09 | 0.0572 | 120 B | -| ConcurrentTlru | 917.0 ns | 13.07 ns | 16.05 ns | 5.24 | 0.0572 | 120 B | -| ClassicLru | 356.9 ns | 5.13 ns | 4.80 ns | 2.04 | 0.0763 | 160 B | -| MemoryCache | 2,366.7 ns | 46.05 ns | 47.29 ns | 13.49 | 2.3460 | 4912 B | +| ConcurrentDictionary | 178.1 ns | 1.47 ns | 1.23 ns | 1.00 | 0.0381 | 80 B | +| FastConcurrentLru | 420.4 ns | 7.52 ns | 6.67 ns | 2.36 | 0.0534 | 112 B | +| ConcurrentLru | 423.7 ns | 3.17 ns | 2.64 ns | 2.38 | 0.0534 | 112 B | +| FastConcurrentTlru | 941.6 ns | 6.69 ns | 5.93 ns | 5.29 | 0.0572 | 120 B | +| ConcurrentTlru | 960.3 ns | 17.73 ns | 14.80 ns | 5.39 | 0.0572 | 120 B | +| ClassicLru | 363.5 ns | 3.65 ns | 3.23 ns | 2.04 | 0.0763 | 160 B | +| MemoryCache | 2,380.9 ns | 33.22 ns | 27.74 ns | 13.37 | 2.3460 | 4912 B | ## Meta-programming using structs for JIT dead code removal and inlining