Mitigate LRU struct tearing using SeqLock (#593)

* lock on read

* bench

* fix title

* seqlock

* add repro program

* soak test

* upd cmts

* matching repro

* soak test

* rem repro

* tests

* encapsulate

* correct obj dump

---------

BitFaster.Caching.Benchmarks/Lru/LruJustGetOrAddGuid.cs

@@ -0,0 +1,108 @@
+using Benchly;
+using BenchmarkDotNet.Attributes;
+using BenchmarkDotNet.Diagnosers;
+using BenchmarkDotNet.Jobs;
+using BitFaster.Caching.Lfu;
+using BitFaster.Caching.Lru;
+using BitFaster.Caching.Scheduler;
+using Microsoft.Extensions.Caching.Memory;
+using System;
+using System.Collections.Concurrent;
+using System.Collections.Generic;
+using System.Threading;
+
+namespace BitFaster.Caching.Benchmarks
+{
+
+#if Windows
+    [DisassemblyDiagnoser(printSource: true, maxDepth: 5)]
+    [SimpleJob(RuntimeMoniker.Net48)]
+#endif
+    [SimpleJob(RuntimeMoniker.Net60)]
+    [MemoryDiagnoser(displayGenColumns: false)]
+    // [HardwareCounters(HardwareCounter.LlcMisses, HardwareCounter.CacheMisses)] // Requires Admin https://adamsitnik.com/Hardware-Counters-Diagnoser/
+    // [ThreadingDiagnoser] // Requires .NET Core
+    [HideColumns("Job", "Median", "RatioSD", "Alloc Ratio")]
+    [ColumnChart(Title= "Guid Lookup Latency ({JOB})", Output = OutputMode.PerJob, Colors = "darkslategray,royalblue,royalblue,#ffbf00,indianred,indianred")]
+    public class LruJustGetOrAddGuid
+    {
+        private static readonly ConcurrentDictionary<int, Guid> dictionary = new ConcurrentDictionary<int, Guid>(8, 9, EqualityComparer<int>.Default);
+
+        private static readonly ConcurrentLru<int, Guid> concurrentLru = new ConcurrentLru<int, Guid>(8, 9, EqualityComparer<int>.Default);
+        private static readonly FastConcurrentLru<int, Guid> fastConcurrentLru = new FastConcurrentLru<int, Guid>(8, 9, EqualityComparer<int>.Default);
+
+
+        private static readonly BackgroundThreadScheduler background = new BackgroundThreadScheduler();
+        private static readonly ConcurrentLfu<int, Guid> concurrentLfu = new ConcurrentLfu<int, Guid>(1, 9, background, EqualityComparer<int>.Default);
+
+        private static readonly int key = 1;
+        private static System.Runtime.Caching.MemoryCache memoryCache = System.Runtime.Caching.MemoryCache.Default;
+
+        Microsoft.Extensions.Caching.Memory.MemoryCache exMemoryCache 
+            = new Microsoft.Extensions.Caching.Memory.MemoryCache(new MemoryCacheOptionsAccessor());
+
+        private static readonly byte[] b = new byte[8];
+
+        [GlobalSetup]
+        public void GlobalSetup()
+        {
+            memoryCache.Set(key.ToString(), new Guid(key, 0, 0, b), new System.Runtime.Caching.CacheItemPolicy());
+            exMemoryCache.Set(key, new Guid(key, 0, 0, b));
+        }
+
+        [GlobalCleanup]
+        public void GlobalCleanup()
+        {
+           background.Dispose();
+        }
+
+        [Benchmark(Baseline = true)]
+        public Guid ConcurrentDictionary()
+        {
+            Func<int, Guid> func = x => new Guid(x, 0, 0, b);
+            return dictionary.GetOrAdd(1, func);
+        }
+
+        [Benchmark()]
+        public Guid FastConcurrentLru()
+        {
+            Func<int, Guid> func = x => new Guid(x, 0, 0, b);
+            return fastConcurrentLru.GetOrAdd(1, func);
+        }
+
+        [Benchmark()]
+        public Guid ConcurrentLru()
+        {
+            Func<int, Guid> func = x => new Guid(x, 0, 0, b);
+            return concurrentLru.GetOrAdd(1, func);
+        }
+
+        [Benchmark()]
+        public Guid ConcurrentLfu()
+        {
+            Func<int, Guid> func = x => new Guid(x, 0, 0, b);
+            return concurrentLfu.GetOrAdd(1, func);
+        }
+
+        [Benchmark()]
+        public Guid RuntimeMemoryCacheGet()
+        {
+            return (Guid)memoryCache.Get("1");
+        }
+
+        [Benchmark()]
+        public Guid ExtensionsMemoryCacheGet()
+        {
+            return (Guid)exMemoryCache.Get(1);
+        }
+
+        public class MemoryCacheOptionsAccessor
+            : Microsoft.Extensions.Options.IOptions<MemoryCacheOptions>
+        {
+            private readonly MemoryCacheOptions options = new MemoryCacheOptions();
+
+            public MemoryCacheOptions Value => this.options;
+
+        }
+    }
+}

BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs

@@ -1,10 +1,13 @@
-using System.Collections.Generic;
+using System;
+using System.Collections.Generic;
 using System.Linq;
+using System.Threading;
 using System.Threading.Tasks;
 using BitFaster.Caching.Lru;
 using FluentAssertions;
 using Xunit;
 using Xunit.Abstractions;
+using static BitFaster.Caching.UnitTests.Lru.LruItemSoakTests;
 
 namespace BitFaster.Caching.UnitTests.Lru
 {
@@ -190,6 +193,29 @@ await Threaded.Run(4, () => {
             }
         }
 
+        [Fact]
+        public async Task WhenSoakConcurrentGetAndUpdateValueTypeCacheEndsInConsistentState()
+        {
+            var lruVT = new ConcurrentLru<int, Guid>(1, capacity, EqualityComparer<int>.Default);
+
+            for (int i = 0; i < 10; i++)
+            {
+                await Threaded.Run(4, () => {
+                    var b = new byte[8];
+                    for (int i = 0; i < 100000; i++)
+                    {
+                        lruVT.TryUpdate(i + 1, new Guid(i, 0, 0, b));
+                        lruVT.GetOrAdd(i + 1, x => new Guid(x, 0, 0, b));
+                    }
+                });
+
+                this.testOutputHelper.WriteLine($"{lruVT.HotCount} {lruVT.WarmCount} {lruVT.ColdCount}");
+                this.testOutputHelper.WriteLine(string.Join(" ", lruVT.Keys));
+
+                new ConcurrentLruIntegrityChecker<int, Guid, LruItem<int, Guid>, LruPolicy<int, Guid>, TelemetryPolicy<int, Guid>>(lruVT).Validate();
+            }
+        }
+
         [Fact]
         public async Task WhenAddingCacheSizeItemsNothingIsEvicted()
         {
@@ -258,6 +284,52 @@ await Threaded.Run(4, r => {
             RunIntegrityCheck();
         }
 
+        // This test will run forever if there is a live lock.
+        // Since the cache bookkeeping has some overhead, it is harder to provoke
+        // spinning inside the reader thread compared to LruItemSoakTests.DetectTornStruct.
+        [Theory]
+        [Repeat(10)]
+        public async Task WhenValueIsBigStructNoLiveLock(int _)
+        { 
+            using var source = new CancellationTokenSource();
+            var started = new TaskCompletionSource<bool>();
+            var cache = new ConcurrentLru<int, Guid>(1, capacity, EqualityComparer<int>.Default);
+
+            var setTask = Task.Run(() => Setter(cache, source.Token, started));
+            await started.Task;
+            Checker(cache, source);
+
+            await setTask;
+        }
+
+        private void Setter(ICache<int, Guid> cache, CancellationToken cancelToken, TaskCompletionSource<bool> started)
+        {
+            started.SetResult(true);
+
+            while (true)
+            {
+                cache.AddOrUpdate(1, Guid.NewGuid());
+                cache.AddOrUpdate(1, Guid.NewGuid());
+
+                if (cancelToken.IsCancellationRequested)
+                { 
+                    return; 
+                }
+            }
+        }
+
+        private void Checker(ICache<int, Guid> cache,CancellationTokenSource source)
+        {
+            // On my machine, without SeqLock, this consistently fails below 100 iterations
+            // on debug build, and below 1000 on release build
+            for (int count = 0; count < 100_000; ++count)
+            {
+                cache.TryGet(1, out _);
+            }
+
+            source.Cancel();
+        }
+
         private void RunIntegrityCheck()
         {
             new ConcurrentLruIntegrityChecker<int, string, LruItem<int, string>, LruPolicy<int, string>, TelemetryPolicy<int, string>>(this.lru).Validate();

BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs

@@ -773,6 +773,19 @@ public void WhenKeyExistsAddOrUpdateUpdatesExistingItem()
             value.Should().Be("2");
         }
 
+        [Fact]
+        public void WhenKeyExistsAddOrUpdateGuidUpdatesExistingItem()
+        { 
+            var lru2 = new ConcurrentLru<int, Guid>(1, capacity, EqualityComparer<int>.Default);    
+
+            var b = new byte[8];
+            lru2.AddOrUpdate(1, new Guid(1, 0, 0, b));
+            lru2.AddOrUpdate(1, new Guid(2, 0, 0, b));
+
+            lru2.TryGet(1, out var value).Should().BeTrue();
+            value.Should().Be(new Guid(2, 0, 0, b));
+        }
+
         [Fact]
         public void WhenKeyExistsAddOrUpdateDisposesOldValue()
         {

BitFaster.Caching.UnitTests/Lru/LruItemMemoryLayoutDumps.cs

@@ -16,22 +16,24 @@ public LruItemMemoryLayoutDumps(ITestOutputHelper testOutputHelper)
         }
 
         //Type layout for 'LruItem`2'
-        //Size: 24 bytes.Paddings: 6 bytes(%25 of empty space)
-        //|===============================================|
-        //| Object Header(8 bytes)                        |
-        //|-----------------------------------------------|
-        //| Method Table Ptr(8 bytes)                     |
-        //|===============================================|
-        //|   0-7: Object Key(8 bytes)                    |
-        //|-----------------------------------------------|
-        //|  8-15: Object<Value> k__BackingField(8 bytes) |
-        //|-----------------------------------------------|
-        //|    16: Boolean wasAccessed(1 byte)            |
-        //|-----------------------------------------------|
-        //|    17: Boolean wasRemoved(1 byte)             |
-        //|-----------------------------------------------|
-        //| 18-23: padding(6 bytes)                       |
-        //|===============================================|
+        //Size: 24 bytes. Paddings: 2 bytes (%8 of empty space)
+        //|=====================================|
+        //| Object Header (8 bytes)             |
+        //|-------------------------------------|
+        //| Method Table Ptr (8 bytes)          |
+        //|=====================================|
+        //|   0-7: Object data (8 bytes)        |
+        //|-------------------------------------|
+        //|  8-15: Object Key (8 bytes)         |
+        //|-------------------------------------|
+        //| 16-19: Int32 sequence (4 bytes)     |
+        //|-------------------------------------|
+        //|    20: Boolean wasAccessed (1 byte) |
+        //|-------------------------------------|
+        //|    21: Boolean wasRemoved (1 byte)  |
+        //|-------------------------------------|
+        //| 22-23: padding (2 bytes)            |
+        //|=====================================|
         [Fact]
         public void DumpLruItem()
         { 
@@ -40,24 +42,26 @@ public void DumpLruItem()
         }
 
         //Type layout for 'LongTickCountLruItem`2'
-        //Size: 32 bytes.Paddings: 6 bytes(%18 of empty space)
-        //|==================================================|
-        //| Object Header(8 bytes)                           |
-        //|--------------------------------------------------|
-        //| Method Table Ptr(8 bytes)                        |
-        //|==================================================|
-        //|   0-7: Object Key(8 bytes)                       |
-        //|--------------------------------------------------|
-        //|  8-15: Object<Value> k__BackingField(8 bytes)    |
-        //|--------------------------------------------------|
-        //|    16: Boolean wasAccessed(1 byte)               |
-        //|--------------------------------------------------|
-        //|    17: Boolean wasRemoved(1 byte)                |
-        //|--------------------------------------------------|
-        //| 18-23: padding(6 bytes)                          |
-        //|--------------------------------------------------|
-        //| 24-31: Int64<TickCount> k__BackingField(8 bytes) |
-        //|==================================================|
+        //Size: 32 bytes. Paddings: 2 bytes (%6 of empty space)
+        //|===================================================|
+        //| Object Header (8 bytes)                           |
+        //|---------------------------------------------------|
+        //| Method Table Ptr (8 bytes)                        |
+        //|===================================================|
+        //|   0-7: Object data (8 bytes)                      |
+        //|---------------------------------------------------|
+        //|  8-15: Object Key (8 bytes)                       |
+        //|---------------------------------------------------|
+        //| 16-19: Int32 sequence (4 bytes)                   |
+        //|---------------------------------------------------|
+        //|    20: Boolean wasAccessed (1 byte)               |
+        //|---------------------------------------------------|
+        //|    21: Boolean wasRemoved (1 byte)                |
+        //|---------------------------------------------------|
+        //| 22-23: padding (2 bytes)                          |
+        //|---------------------------------------------------|
+        //| 24-31: Int64 <TickCount>k__BackingField (8 bytes) |
+        //|===================================================|
         [Fact]
         public void DumpLongTickCountLruItem()
         {