diff --git a/BitFaster.Caching.Benchmarks/Lru/LruJustGetOrAddGuid.cs b/BitFaster.Caching.Benchmarks/Lru/LruJustGetOrAddGuid.cs new file mode 100644 index 00000000..78602e6c --- /dev/null +++ b/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 dictionary = new ConcurrentDictionary(8, 9, EqualityComparer.Default); + + private static readonly ConcurrentLru concurrentLru = new ConcurrentLru(8, 9, EqualityComparer.Default); + private static readonly FastConcurrentLru fastConcurrentLru = new FastConcurrentLru(8, 9, EqualityComparer.Default); + + + private static readonly BackgroundThreadScheduler background = new BackgroundThreadScheduler(); + private static readonly ConcurrentLfu concurrentLfu = new ConcurrentLfu(1, 9, background, EqualityComparer.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 func = x => new Guid(x, 0, 0, b); + return dictionary.GetOrAdd(1, func); + } + + [Benchmark()] + public Guid FastConcurrentLru() + { + Func func = x => new Guid(x, 0, 0, b); + return fastConcurrentLru.GetOrAdd(1, func); + } + + [Benchmark()] + public Guid ConcurrentLru() + { + Func func = x => new Guid(x, 0, 0, b); + return concurrentLru.GetOrAdd(1, func); + } + + [Benchmark()] + public Guid ConcurrentLfu() + { + Func 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 + { + private readonly MemoryCacheOptions options = new MemoryCacheOptions(); + + public MemoryCacheOptions Value => this.options; + + } + } +} diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs index d78262c9..fc473381 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruSoakTests.cs +++ b/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(1, capacity, EqualityComparer.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, LruPolicy, TelemetryPolicy>(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(); + var cache = new ConcurrentLru(1, capacity, EqualityComparer.Default); + + var setTask = Task.Run(() => Setter(cache, source.Token, started)); + await started.Task; + Checker(cache, source); + + await setTask; + } + + private void Setter(ICache cache, CancellationToken cancelToken, TaskCompletionSource started) + { + started.SetResult(true); + + while (true) + { + cache.AddOrUpdate(1, Guid.NewGuid()); + cache.AddOrUpdate(1, Guid.NewGuid()); + + if (cancelToken.IsCancellationRequested) + { + return; + } + } + } + + private void Checker(ICache 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, LruPolicy, TelemetryPolicy>(this.lru).Validate(); diff --git a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs b/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs index 1a9b32f1..e52015d3 100644 --- a/BitFaster.Caching.UnitTests/Lru/ConcurrentLruTests.cs +++ b/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(1, capacity, EqualityComparer.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() { diff --git a/BitFaster.Caching.UnitTests/Lru/LruItemMemoryLayoutDumps.cs b/BitFaster.Caching.UnitTests/Lru/LruItemMemoryLayoutDumps.cs index b80ddc45..a2ea5a43 100644 --- a/BitFaster.Caching.UnitTests/Lru/LruItemMemoryLayoutDumps.cs +++ b/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 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 k__BackingField(8 bytes) | - //|--------------------------------------------------| - //| 16: Boolean wasAccessed(1 byte) | - //|--------------------------------------------------| - //| 17: Boolean wasRemoved(1 byte) | - //|--------------------------------------------------| - //| 18-23: padding(6 bytes) | - //|--------------------------------------------------| - //| 24-31: Int64 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 k__BackingField (8 bytes) | + //|===================================================| [Fact] public void DumpLongTickCountLruItem() { diff --git a/BitFaster.Caching.UnitTests/Lru/LruItemSoakTests.cs b/BitFaster.Caching.UnitTests/Lru/LruItemSoakTests.cs new file mode 100644 index 00000000..4bc949a6 --- /dev/null +++ b/BitFaster.Caching.UnitTests/Lru/LruItemSoakTests.cs @@ -0,0 +1,116 @@ +using System; +using System.Threading; +using System.Threading.Tasks; +using BitFaster.Caching.Lru; +using Xunit; + +namespace BitFaster.Caching.UnitTests.Lru +{ + [Collection("Soak")] + public class LruItemSoakTests + { + private const int soakIterations = 3; + private readonly LruItem item = new(1, MassiveStruct.A); + + // Adapted from + // https://stackoverflow.com/questions/23262513/reproduce-torn-reads-of-decimal-in-c-sharp + [Theory] + [Repeat(soakIterations)] + public async Task DetectTornStruct(int _) + { + using var source = new CancellationTokenSource(); + var started = new TaskCompletionSource(); + + var setTask = Task.Run(() => Setter(source.Token, started)); + await started.Task; + Checker(source); + + await setTask; + } + + private void Setter(CancellationToken cancelToken, TaskCompletionSource started) + { + started.SetResult(true); + + while (true) + { + item.SeqLockWrite(MassiveStruct.A); + item.SeqLockWrite(MassiveStruct.B); + + if (cancelToken.IsCancellationRequested) + { + return; + } + } + } + + private void Checker(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 < 10_000; ++count) + { + var t = item.SeqLockRead(); + + if (t != MassiveStruct.A && t != MassiveStruct.B) + { + throw new Exception($"Value is torn after {count} iterations"); + } + } + + source.Cancel(); + } + + #pragma warning disable CS0659 // Object.Equals but no GetHashCode + #pragma warning disable CS0661 // operator== but no GetHashCode + public struct MassiveStruct : IEquatable + { + // To repro on x64, struct should be larger than a cache line (64 bytes). + public long a; + public long b; + public long c; + public long d; + + public long e; + public long f; + public long g; + public long h; + + public long i; + + public static readonly MassiveStruct A = new MassiveStruct(); + public static readonly MassiveStruct B = new MassiveStruct() + { a = long.MaxValue, b = long.MaxValue, c = long.MaxValue, d = long.MaxValue, e = long.MaxValue, f= long.MaxValue, g = long.MaxValue, h = long.MaxValue, i = long.MaxValue }; + + public override bool Equals(object obj) + { + return obj is MassiveStruct @struct && Equals(@struct); + } + + public bool Equals(MassiveStruct other) + { + return a == other.a && + b == other.b && + c == other.c && + d == other.d && + e == other.e && + f == other.f && + g == other.g && + h == other.h && + i == other.i; + } + + public static bool operator ==(MassiveStruct left, MassiveStruct right) + { + return left.Equals(right); + } + + public static bool operator !=(MassiveStruct left, MassiveStruct right) + { + return !(left == right); + } + } + #pragma warning restore CS0659 + #pragma warning restore CS0661 + } +} diff --git a/BitFaster.Caching.UnitTests/TypePropsTests.cs b/BitFaster.Caching.UnitTests/TypePropsTests.cs new file mode 100644 index 00000000..a02d9c80 --- /dev/null +++ b/BitFaster.Caching.UnitTests/TypePropsTests.cs @@ -0,0 +1,31 @@ +using System; +using System.Reflection; +using FluentAssertions; +using Xunit; + +namespace BitFaster.Caching.UnitTests +{ + public class TypePropsTests + { + private static readonly MethodInfo method = typeof(TypePropsTests).GetMethod(nameof(TypePropsTests.IsWriteAtomic), BindingFlags.NonPublic | BindingFlags.Static); + + [Theory] + [InlineData(typeof(object), true)] + [InlineData(typeof(IntPtr), true)] + [InlineData(typeof(UIntPtr), true)] + [InlineData(typeof(int), true)] + [InlineData(typeof(long), true)] // this is only expected to pass on 64bit platforms + [InlineData(typeof(Guid), false)] + public void Test(Type argType, bool expected) + { + var isWriteAtomic = method.MakeGenericMethod(argType); + + isWriteAtomic.Invoke(null, null).Should().BeOfType().Which.Should().Be(expected); + } + + private static bool IsWriteAtomic() + { + return TypeProps.IsWriteAtomic; + } + } +} diff --git a/BitFaster.Caching/Lru/ConcurrentLruCore.cs b/BitFaster.Caching/Lru/ConcurrentLruCore.cs index 0438ffd1..adbca475 100644 --- a/BitFaster.Caching/Lru/ConcurrentLruCore.cs +++ b/BitFaster.Caching/Lru/ConcurrentLruCore.cs @@ -180,6 +180,7 @@ private bool GetOrDiscard(I item, [MaybeNullWhen(false)] out V value) } value = item.Value; + this.itemPolicy.Touch(item); this.telemetryPolicy.IncrementHit(); return true; @@ -382,7 +383,9 @@ public bool TryUpdate(K key, V value) if (!existing.WasRemoved) { V oldValue = existing.Value; + existing.Value = value; + this.itemPolicy.Update(existing); // backcompat: remove conditional compile #if NETCOREAPP3_0_OR_GREATER diff --git a/BitFaster.Caching/Lru/LruItem.cs b/BitFaster.Caching/Lru/LruItem.cs index 44e48939..f2e865c9 100644 --- a/BitFaster.Caching/Lru/LruItem.cs +++ b/BitFaster.Caching/Lru/LruItem.cs @@ -1,4 +1,7 @@  +using System.Runtime.CompilerServices; +using System.Threading; + namespace BitFaster.Caching.Lru { /// @@ -9,9 +12,14 @@ namespace BitFaster.Caching.Lru public class LruItem where K : notnull { + private V data; + private volatile bool wasAccessed; private volatile bool wasRemoved; + // only used when V is a non-atomic value type to prevent torn reads + private int sequence; + /// /// Initializes a new instance of the LruItem class with the specified key and value. /// @@ -20,7 +28,7 @@ public class LruItem public LruItem(K k, V v) { this.Key = k; - this.Value = v; + this.data = v; } /// @@ -31,7 +39,33 @@ public LruItem(K k, V v) /// /// Gets or sets the value. /// - public V Value { get; set; } + public V Value + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get + { + if (TypeProps.IsWriteAtomic) + { + return data; + } + else + { + return SeqLockRead(); + } + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + set + { + if (TypeProps.IsWriteAtomic) + { + data = value; + } + else + { + SeqLockWrite(value); + } + } + } /// /// Gets or sets a value indicating whether the item was accessed. @@ -50,5 +84,39 @@ public bool WasRemoved get => this.wasRemoved; set => this.wasRemoved = value; } + + internal V SeqLockRead() + { + var spin = new SpinWait(); + while (true) + { + var start = Volatile.Read(ref this.sequence); + + if ((start & 1) == 1) + { + // A write is in progress, spin. + spin.SpinOnce(); + continue; + } + + V copy = this.data; + + var end = Volatile.Read(ref this.sequence); + if (start == end) + { + return copy; + } + } + } + + // Note: LruItem should be locked while invoking this method. Multiple writer threads are not supported. + internal void SeqLockWrite(V value) + { + Interlocked.Increment(ref sequence); + + this.data = value; + + Interlocked.Increment(ref sequence); + } } } diff --git a/BitFaster.Caching/TypeProps.cs b/BitFaster.Caching/TypeProps.cs new file mode 100644 index 00000000..0303149c --- /dev/null +++ b/BitFaster.Caching/TypeProps.cs @@ -0,0 +1,46 @@ +using System; + +namespace BitFaster.Caching +{ + // https://source.dot.net/#System.Collections.Concurrent/System/Collections/Concurrent/ConcurrentDictionary.cs,2293 + internal static class TypeProps + { + /// Whether T's type can be written atomically (i.e., with no danger of torn reads). + internal static readonly bool IsWriteAtomic = IsWriteAtomicPrivate(); + + private static bool IsWriteAtomicPrivate() + { + // Section 12.6.6 of ECMA CLI explains which types can be read and written atomically without + // the risk of tearing. See https://www.ecma-international.org/publications/files/ECMA-ST/ECMA-335.pdf + + if (!typeof(T).IsValueType || + typeof(T) == typeof(IntPtr) || + typeof(T) == typeof(UIntPtr)) + { + return true; + } + + switch (Type.GetTypeCode(typeof(T))) + { + case TypeCode.Boolean: + case TypeCode.Byte: + case TypeCode.Char: + case TypeCode.Int16: + case TypeCode.Int32: + case TypeCode.SByte: + case TypeCode.Single: + case TypeCode.UInt16: + case TypeCode.UInt32: + return true; + + case TypeCode.Double: + case TypeCode.Int64: + case TypeCode.UInt64: + return IntPtr.Size == 8; + + default: + return false; + } + } + } +}