diff --git a/src/Caching/Hybrid/src/HybridCacheBuilderExtensions.cs b/src/Caching/Hybrid/src/HybridCacheBuilderExtensions.cs index a27240f66418..4c7dc27c72e2 100644 --- a/src/Caching/Hybrid/src/HybridCacheBuilderExtensions.cs +++ b/src/Caching/Hybrid/src/HybridCacheBuilderExtensions.cs @@ -1,12 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.Extensions.Caching.Hybrid; diff --git a/src/Caching/Hybrid/src/HybridCacheOptions.cs b/src/Caching/Hybrid/src/HybridCacheOptions.cs index 66793746a5dd..65c0709764fc 100644 --- a/src/Caching/Hybrid/src/HybridCacheOptions.cs +++ b/src/Caching/Hybrid/src/HybridCacheOptions.cs @@ -1,13 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; -using Microsoft.Extensions.Options; - namespace Microsoft.Extensions.Caching.Hybrid; /// diff --git a/src/Caching/Hybrid/src/HybridCacheServiceExtensions.cs b/src/Caching/Hybrid/src/HybridCacheServiceExtensions.cs index fcb7e4ae4d57..87bdfd1cda1a 100644 --- a/src/Caching/Hybrid/src/HybridCacheServiceExtensions.cs +++ b/src/Caching/Hybrid/src/HybridCacheServiceExtensions.cs @@ -2,15 +2,9 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.Collections.Generic; -using System.Diagnostics.CodeAnalysis; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Microsoft.Extensions.Caching.Hybrid.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using Microsoft.Extensions.Internal; namespace Microsoft.Extensions.Caching.Hybrid; diff --git a/src/Caching/Hybrid/src/IHybridCacheBuilder.cs b/src/Caching/Hybrid/src/IHybridCacheBuilder.cs index fae49c030fc3..c46c8b650858 100644 --- a/src/Caching/Hybrid/src/IHybridCacheBuilder.cs +++ b/src/Caching/Hybrid/src/IHybridCacheBuilder.cs @@ -1,11 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Text; -using System.Threading.Tasks; using Microsoft.Extensions.DependencyInjection; namespace Microsoft.Extensions.Caching.Hybrid; diff --git a/src/Caching/Hybrid/src/Internal/BufferChunk.cs b/src/Caching/Hybrid/src/Internal/BufferChunk.cs index c783810fcf30..73b89776c287 100644 --- a/src/Caching/Hybrid/src/Internal/BufferChunk.cs +++ b/src/Caching/Hybrid/src/Internal/BufferChunk.cs @@ -9,7 +9,8 @@ namespace Microsoft.Extensions.Caching.Hybrid.Internal; // used to convey buffer status; like ArraySegment, but Offset is always -// zero, and we use the MSB of the length to track whether or not to recycle this value +// zero, and we use the most significant bit (MSB, the sign flag) of the length +// to track whether or not to recycle this value internal readonly struct BufferChunk { private const int MSB = (1 << 31); @@ -40,6 +41,10 @@ public BufferChunk(byte[] array) Array = array; _lengthAndPoolFlag = array.Length; // assume not pooled, if exact-sized + // (we don't expect array.Length to be negative; we're really just saying + // "we expect the result of assigning array.Length to _lengthAndPoolFlag + // to give the expected Length *and* not have the MSB set; we're just + // checking that we haven't fat-fingered our MSB logic) Debug.Assert(!ReturnToPool, "do not return right-sized arrays"); Debug.Assert(Length == array.Length, "array length not respected"); } diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.CacheItem.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.CacheItem.cs index 7dba0f76d6be..bb3ddee3ef11 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.CacheItem.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.CacheItem.cs @@ -2,6 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; +using System.Diagnostics; +using System.Threading; using Microsoft.Extensions.Caching.Memory; namespace Microsoft.Extensions.Caching.Hybrid.Internal; @@ -10,37 +12,85 @@ partial class DefaultHybridCache { internal abstract class CacheItem { + private int _refCount = 1; // the number of pending operations against this cache item + // note: the ref count is the number of callers anticipating this value at any given time; initially, + // it is one for a simple "get the value" flow, but if another call joins with us, it'll be incremented; + // if either cancels, it will get decremented, with the entire flow being cancelled if it ever becomes + // zero + // this counter also drives cache lifetime, with the cache itself incrementing the count by one; in the + // case of mutable data, cache eviction may reduce this to zero (in cooperation with any concurrent readers, + // who incr/decr around their fetch), allowing safe buffer recycling + + internal int RefCount => Volatile.Read(ref _refCount); + internal static readonly PostEvictionDelegate _sharedOnEviction = static (key, value, reason, state) => { if (value is CacheItem item) { - // perform per-item clean-up; this could be buffer recycling (if defensive copies needed), - // or could be disposal - item.OnEviction(); + item.Release(); } }; - public virtual void Release() { } // for recycling purposes + public virtual bool NeedsEvictionCallback => false; // do we need to call Release when evicted? - public abstract bool NeedsEvictionCallback { get; } // do we need to call Release when evicted? - - public virtual void OnEviction() { } // only invoked if NeedsEvictionCallback reported true + protected virtual void OnFinalRelease() { } // any required release semantics public abstract bool TryReserveBuffer(out BufferChunk buffer); public abstract bool DebugIsImmutable { get; } + + public bool Release() // returns true ONLY for the final release step + { + var newCount = Interlocked.Decrement(ref _refCount); + Debug.Assert(newCount >= 0, "over-release detected"); + if (newCount == 0) + { + // perform per-item clean-up, i.e. buffer recycling (if defensive copies needed) + OnFinalRelease(); + return true; + } + return false; + } + + public bool TryReserve() + { + // this is basically interlocked increment, but with a check against: + // a) incrementing upwards from zero + // b) overflowing *back* to zero + var oldValue = Volatile.Read(ref _refCount); + do + { + if (oldValue is 0 or -1) + { + return false; // already burned, or about to roll around back to zero + } + + var updated = Interlocked.CompareExchange(ref _refCount, oldValue + 1, oldValue); + if (updated == oldValue) + { + return true; // we exchanged + } + oldValue = updated; // we failed, but we have an updated state + } while (true); + } + } internal abstract class CacheItem : CacheItem { + internal static CacheItem Create() => ImmutableTypeCache.IsImmutable ? new ImmutableCacheItem() : new MutableCacheItem(); + + // attempt to get a value that was *not* previously reserved public abstract bool TryGetValue(out T value); - public T GetValue() + // get a value that *was* reserved, countermanding our reservation in the process + public T GetReservedValue() { if (!TryGetValue(out var value)) { Throw(); } + Release(); return value; static void Throw() => throw new ObjectDisposedException("The cache item has been recycled before the value was obtained"); diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Debug.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Debug.cs index 0be6f768e5f4..9aaf3da4ed49 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Debug.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Debug.cs @@ -24,17 +24,17 @@ internal bool DebugTryGetCacheItem(string key, [NotNullWhen(true)] out CacheItem private int _outstandingBufferCount; - internal int DebugGetOutstandingBuffers(bool flush = false) + internal int DebugOnlyGetOutstandingBuffers(bool flush = false) => flush ? Interlocked.Exchange(ref _outstandingBufferCount, 0) : Volatile.Read(ref _outstandingBufferCount); [Conditional("DEBUG")] - internal void DebugDecrementOutstandingBuffers() + internal void DebugOnlyDecrementOutstandingBuffers() { Interlocked.Decrement(ref _outstandingBufferCount); } [Conditional("DEBUG")] - internal void DebugIncrementOutstandingBuffers() + internal void DebugOnlyIncrementOutstandingBuffers() { Interlocked.Increment(ref _outstandingBufferCount); } @@ -42,27 +42,27 @@ internal void DebugIncrementOutstandingBuffers() partial class MutableCacheItem { - partial void DebugDecrementOutstandingBuffers(); - partial void DebugTrackBufferCore(DefaultHybridCache cache); + partial void DebugOnlyDecrementOutstandingBuffers(); + partial void DebugOnlyTrackBufferCore(DefaultHybridCache cache); [Conditional("DEBUG")] - internal void DebugTrackBuffer(DefaultHybridCache cache) => DebugTrackBufferCore(cache); + internal void DebugOnlyTrackBuffer(DefaultHybridCache cache) => DebugOnlyTrackBufferCore(cache); #if DEBUG private DefaultHybridCache? _cache; // for buffer-tracking - only enabled in DEBUG - partial void DebugDecrementOutstandingBuffers() + partial void DebugOnlyDecrementOutstandingBuffers() { if (_buffer.ReturnToPool) { - _cache?.DebugDecrementOutstandingBuffers(); + _cache?.DebugOnlyDecrementOutstandingBuffers(); } } - partial void DebugTrackBufferCore(DefaultHybridCache cache) + partial void DebugOnlyTrackBufferCore(DefaultHybridCache cache) { _cache = cache; if (_buffer.ReturnToPool) { - _cache?.DebugIncrementOutstandingBuffers(); + _cache?.DebugOnlyIncrementOutstandingBuffers(); } } #endif diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.ImmutableCacheItem.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.ImmutableCacheItem.cs index f87335674b95..dd245912f6ee 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.ImmutableCacheItem.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.ImmutableCacheItem.cs @@ -1,9 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; +using System.Threading; namespace Microsoft.Extensions.Caching.Hybrid.Internal; @@ -11,23 +9,26 @@ partial class DefaultHybridCache { private sealed class ImmutableCacheItem : CacheItem // used to hold types that do not require defensive copies { - private readonly T _value; - public ImmutableCacheItem(T value) => _value = value; + private T _value = default!; // deferred until SetValue - private static ImmutableCacheItem? _sharedDefault; + public void SetValue(T value) => _value = value; - // this is only used when the underlying store is disabled; we don't need 100% singleton; "good enough is" - public static ImmutableCacheItem Default => _sharedDefault ??= new(default!); + private static ImmutableCacheItem? _sharedDefault; - public override void OnEviction() + // get a shared instance that passes as "reserved"; doesn't need to be 100% singleton, + // but we don't want to break the reservation rules either; if we can't reserve: create new + public static ImmutableCacheItem GetReservedShared() { - var obj = _value as IDisposable; - Debug.Assert(obj is not null, "shouldn't be here for non-disposable types"); - obj?.Dispose(); + var obj = Volatile.Read(ref _sharedDefault); + if (obj is null || !obj.TryReserve()) + { + obj = new(); + obj.TryReserve(); // this is reliable on a new instance + Volatile.Write(ref _sharedDefault, obj); + } + return obj; } - public override bool NeedsEvictionCallback => ImmutableTypeCache.IsDisposable; - public override bool TryGetValue(out T value) { value = _value; diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.L2.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.L2.cs index aef445d44e29..578528e9d4be 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.L2.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.L2.cs @@ -113,13 +113,16 @@ private DistributedCacheEntryOptions GetOptions(HybridCacheEntryOptions? options internal void SetL1(string key, CacheItem value, HybridCacheEntryOptions? options) { - // based on CacheExtensions.Set, but with post-eviction recycling - using var cacheEntry = _localCache.CreateEntry(key); - cacheEntry.AbsoluteExpirationRelativeToNow = options?.LocalCacheExpiration ?? _defaultLocalCacheExpiration; - cacheEntry.Value = value; - if (value.NeedsEvictionCallback) + if (value.TryReserve()) // incr ref-count for the the cache itself; this *may* be released via the NeedsEvictionCallback path { - cacheEntry.RegisterPostEvictionCallback(CacheItem._sharedOnEviction); + // based on CacheExtensions.Set, but with post-eviction recycling + using var cacheEntry = _localCache.CreateEntry(key); + cacheEntry.AbsoluteExpirationRelativeToNow = options?.LocalCacheExpiration ?? _defaultLocalCacheExpiration; + cacheEntry.Value = value; + if (value.NeedsEvictionCallback) + { + cacheEntry.RegisterPostEvictionCallback(CacheItem._sharedOnEviction); + } } } } diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.MutableCacheItem.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.MutableCacheItem.cs index ef99a5738091..892883d4a639 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.MutableCacheItem.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.MutableCacheItem.cs @@ -1,28 +1,23 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Diagnostics; -using System.Threading; - namespace Microsoft.Extensions.Caching.Hybrid.Internal; partial class DefaultHybridCache { private sealed partial class MutableCacheItem : CacheItem // used to hold types that require defensive copies { - private readonly IHybridCacheSerializer _serializer; - private readonly BufferChunk _buffer; - private int _refCount = 1; // buffer released when this becomes zero + private IHybridCacheSerializer _serializer = null!; // deferred until SetValue + private BufferChunk _buffer; - public MutableCacheItem(ref BufferChunk buffer, IHybridCacheSerializer serializer) + public void SetValue(ref BufferChunk buffer, IHybridCacheSerializer serializer) { _serializer = serializer; _buffer = buffer; buffer = default; // we're taking over the lifetime; the caller no longer has it! } - public MutableCacheItem(T value, IHybridCacheSerializer serializer, int maxLength) + public void SetValue(T value, IHybridCacheSerializer serializer, int maxLength) { _serializer = serializer; var writer = RecyclableArrayBufferWriter.Create(maxLength); @@ -34,35 +29,10 @@ public MutableCacheItem(T value, IHybridCacheSerializer serializer, int maxLe public override bool NeedsEvictionCallback => _buffer.ReturnToPool; - public override void OnEviction() => Release(); - - public override void Release() + protected override void OnFinalRelease() { - var newCount = Interlocked.Decrement(ref _refCount); - if (newCount == 0) - { - DebugDecrementOutstandingBuffers(); - _buffer.RecycleIfAppropriate(); - } - } - - public bool TryReserve() - { - var oldValue = Volatile.Read(ref _refCount); - do - { - if (oldValue is 0 or -1) - { - return false; // already burned, or about to roll around back to zero - } - - var updated = Interlocked.CompareExchange(ref _refCount, oldValue + 1, oldValue); - if (updated == oldValue) - { - return true; // we exchanged - } - oldValue = updated; // we failed, but we have an updated state - } while (true); + DebugOnlyDecrementOutstandingBuffers(); + _buffer.RecycleIfAppropriate(); } public override bool TryGetValue(out T value) diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Serialization.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Serialization.cs index 42b5789e4b86..0adf22b793be 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Serialization.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Serialization.cs @@ -57,8 +57,6 @@ internal static class ImmutableTypeCache // lazy memoize; T doesn't change pe { // note for blittable types: a pure struct will be a full copy every time - nothing shared to mutate public static readonly bool IsImmutable = (typeof(T).IsValueType && IsBlittable()) || IsImmutable(typeof(T)); - - public static bool IsDisposable => typeof(IDisposable).IsAssignableFrom(typeof(T)); } private static bool IsBlittable() // minimize the generic portion diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Stampede.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Stampede.cs index 01d797a08194..009d95f0bb08 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Stampede.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.Stampede.cs @@ -43,7 +43,7 @@ public bool GetOrCreateStampedeState(string key, HybridCacheEntryFlag // hmm; failed to add - there's concurrent activity on the same key; we're now // in very rare race condition territory; go ahead and take a lock while we // collect our thoughts - lock (_currentOperations) + lock (GetPartitionedSyncLock(in stampedeKey)) // see notes in SyncLock.cs { // check again while we hold the lock if (TryJoinExistingSession(this, stampedeKey, out existing)) @@ -57,6 +57,16 @@ public bool GetOrCreateStampedeState(string key, HybridCacheEntryFlag // the floor; in the grand scheme of things, that's OK; this is a rare outcome } + // and check whether the value was L1-cached by an outgoing operation (for *us* to check needs local-cache-read, + // and for *them* to have updated needs local-cache-write, but since the shared us/them key includes flags, + // we can skip this if *either* flag is set) + if ((flags & HybridCacheEntryFlags.DisableLocalCache) == 0 && _localCache.TryGetValue(key, out var untyped) + && untyped is CacheItem typed && typed.TryReserve()) + { + stampedeState.SetResultDirect(typed); + return false; // the work has ALREADY been done + } + // otherwise, either nothing existed - or the thing that already exists can't be joined; // in that case, go ahead and use the state that we invented a moment ago (outside of the lock) _currentOperations[stampedeKey] = stampedeState; diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeKey.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeKey.cs index bf5001360eb3..eeca9e589b13 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeKey.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeKey.cs @@ -15,10 +15,18 @@ partial class DefaultHybridCache private readonly int _hashCode; // we know we'll need it; compute it once only public StampedeKey(string key, HybridCacheEntryFlags flags) { + // We'll use both the key *and* the flags as combined flag; in reality, we *expect* + // the flags to be consistent between calls on the same operation, and it must be + // noted that the *cache items* only use the key (not the flags), but: it gets + // very hard to grok what the correct behaviour should be if combining two calls + // with different flags, since they could have mutually exclusive behaviours! + + // As such, we'll treat conflicting calls entirely separately from a stampede + // perspective. _key = key; _flags = flags; #if NETCOREAPP2_1_OR_GREATER || NETSTANDARD2_1_OR_GREATER - _hashCode = HashCode.Combine(key, flags); + _hashCode = System.HashCode.Combine(key, flags); #else _hashCode = key.GetHashCode() ^ (int)flags; #endif @@ -27,6 +35,10 @@ public StampedeKey(string key, HybridCacheEntryFlags flags) public string Key => _key; public HybridCacheEntryFlags Flags => _flags; + // allow direct access to the pre-computed hash-code, semantically emphasizing that + // this is a constant-time operation against a known value + internal int HashCode => _hashCode; + public bool Equals(StampedeKey other) => _flags == other._flags & _key == other._key; public override bool Equals([NotNullWhen(true)] object? obj) diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeState.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeState.cs index f0a983093ac1..c59378c3df0d 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeState.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeState.cs @@ -15,22 +15,27 @@ internal abstract class StampedeState #endif { private readonly DefaultHybridCache _cache; - private int _activeCallers = 1; + private readonly CacheItem _cacheItem; // because multiple callers can enlist, we need to track when the *last* caller cancels // (and keep going until then); that means we need to run with custom cancellation private readonly CancellationTokenSource? _sharedCancellation; internal readonly CancellationToken SharedToken; // this might have a value even when _sharedCancellation is null - public StampedeKey Key { get; } + // we expose the key as a by-ref readonly; this minimizes the stack work involved in passing the key around + // (both in terms of width and copy-semantics) + private readonly StampedeKey _key; + public ref readonly StampedeKey Key => ref _key; + protected CacheItem CacheItem => _cacheItem; /// /// Create a stamped token optionally with shared cancellation support /// - protected StampedeState(DefaultHybridCache cache, in StampedeKey key, bool canBeCanceled) + protected StampedeState(DefaultHybridCache cache, in StampedeKey key, CacheItem cacheItem, bool canBeCanceled) { _cache = cache; - Key = key; + _key = key; + _cacheItem = cacheItem; if (canBeCanceled) { // if the first (or any) caller can't be cancelled; we'll never get to zero; no point tracking @@ -47,10 +52,11 @@ protected StampedeState(DefaultHybridCache cache, in StampedeKey key, bool canBe /// /// Create a stamped token using a fixed cancellation token /// - protected StampedeState(DefaultHybridCache cache, in StampedeKey key, CancellationToken token) + protected StampedeState(DefaultHybridCache cache, in StampedeKey key, CacheItem cacheItem, CancellationToken token) { _cache = cache; - Key = key; + _key = key; + _cacheItem = cacheItem; SharedToken = token; } @@ -68,14 +74,14 @@ protected StampedeState(DefaultHybridCache cache, in StampedeKey key, Cancellati public abstract void SetCanceled(); - public int DebugCallerCount => Volatile.Read(ref _activeCallers); + public int DebugCallerCount => _cacheItem.RefCount; public abstract Type Type { get; } - public void RemoveCaller() + public void CancelCaller() { // note that TryAddCaller has protections to avoid getting back from zero - if (Interlocked.Decrement(ref _activeCallers) == 0) + if (_cacheItem.Release()) { // we're the last to leave; turn off the lights _sharedCancellation?.Cancel(); @@ -83,25 +89,14 @@ public void RemoveCaller() } } - public bool TryAddCaller() // essentially just interlocked-increment, but with a leading zero check and overflow detection + public bool TryAddCaller() => _cacheItem.TryReserve(); + } + + private void RemoveStampedeState(in StampedeKey key) + { + lock (GetPartitionedSyncLock(in key)) // see notes in SyncLock.cs { - var oldValue = Volatile.Read(ref _activeCallers); - do - { - if (oldValue is 0 or -1) - { - return false; // already burned or about to roll around back to zero - } - - var updated = Interlocked.CompareExchange(ref _activeCallers, oldValue + 1, oldValue); - if (updated == oldValue) - { - return true; // we exchanged - } - oldValue = updated; // we failed, but we have an updated state - } while (true); + _currentOperations.TryRemove(key, out _); } } - - private void RemoveStampede(StampedeKey key) => _currentOperations.TryRemove(key, out _); } diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeStateT.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeStateT.cs index 7a76cdcdaf78..b88ed5f236b7 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeStateT.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.StampedeStateT.cs @@ -3,9 +3,9 @@ using System; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Caching.Memory; namespace Microsoft.Extensions.Caching.Hybrid.Internal; @@ -20,7 +20,7 @@ internal sealed class StampedeState : StampedeState private Task? _sharedUnwrap; // allows multiple non-cancellable callers to share a single task (when no defensive copy needed) public StampedeState(DefaultHybridCache cache, in StampedeKey key, bool canBeCanceled) - : base(cache, key, canBeCanceled) + : base(cache, key, CacheItem.Create(), canBeCanceled) { _result = new(TaskCreationOptions.RunContinuationsAsynchronously); } @@ -28,7 +28,7 @@ public StampedeState(DefaultHybridCache cache, in StampedeKey key, bool canBeCan public override Type Type => typeof(T); public StampedeState(DefaultHybridCache cache, in StampedeKey key, CancellationToken token) - : base(cache, key, token) { } // no TCS in this case - this is for SetValue only + : base(cache, key, CacheItem.Create(), token) { } // no TCS in this case - this is for SetValue only public void QueueUserWorkItem(in TState state, Func> underlying, HybridCacheEntryOptions? options) { @@ -134,11 +134,15 @@ private void SetException(Exception ex) { if (_result is not null) { - Cache.RemoveStampede(Key); + Cache.RemoveStampedeState(in Key); _result.TrySetException(ex); } } + // ONLY set the result, without any other side-effects + internal void SetResultDirect(CacheItem value) + => _result?.TrySetResult(value); + private void SetResult(CacheItem value) { if ((Key.Flags & HybridCacheEntryFlags.DisableLocalCacheWrite) == 0) @@ -148,7 +152,7 @@ private void SetResult(CacheItem value) if (_result is not null) { - Cache.RemoveStampede(Key); + Cache.RemoveStampedeState(in Key); _result.TrySetResult(value); } } @@ -158,8 +162,8 @@ private void SetDefaultResult() // note we don't store this dummy result in L1 or L2 if (_result is not null) { - Cache.RemoveStampede(Key); - _result.TrySetResult(ImmutableCacheItem.Default); + Cache.RemoveStampedeState(in Key); + _result.TrySetResult(ImmutableCacheItem.GetReservedShared()); } } @@ -170,36 +174,50 @@ private void SetResultAndRecycleIfAppropriate(ref BufferChunk value) var serializer = Cache.GetSerializer(); CacheItem cacheItem; - if (ImmutableTypeCache.IsImmutable) - { - // deserialize; and store object; buffer can be recycled now - cacheItem = new ImmutableCacheItem(serializer.Deserialize(new(value.Array!, 0, value.Length))); - value.RecycleIfAppropriate(); - } - else + switch (CacheItem) { - // use the buffer directly as the backing in the cache-item; do *not* recycle now - var tmp = new MutableCacheItem(ref value, serializer); - tmp.DebugTrackBuffer(Cache); // conditional: DEBUG - cacheItem = tmp; + case ImmutableCacheItem immutable: + // deserialize; and store object; buffer can be recycled now + immutable.SetValue(serializer.Deserialize(new(value.Array!, 0, value.Length))); + value.RecycleIfAppropriate(); + cacheItem = immutable; + break; + case MutableCacheItem mutable: + // use the buffer directly as the backing in the cache-item; do *not* recycle now + mutable.SetValue(ref value, serializer); + mutable.DebugOnlyTrackBuffer(Cache); + cacheItem = mutable; + break; + default: + cacheItem = ThrowUnexpectedCacheItem(); + break; } - SetResult(cacheItem); } + [DoesNotReturn] + private static CacheItem ThrowUnexpectedCacheItem() => throw new InvalidOperationException("Unexpected cache item"); + private CacheItem SetResult(T value) { // set a result from a value we calculated directly CacheItem cacheItem; - if (ImmutableTypeCache.IsImmutable) - { - cacheItem = new ImmutableCacheItem(value); // no serialize needed - } - else + switch (CacheItem) { - var tmp = new MutableCacheItem(value, Cache.GetSerializer(), MaximumPayloadBytes); // serialization happens here - tmp.DebugTrackBuffer(Cache); // conditional: DEBUG - cacheItem = tmp; + case ImmutableCacheItem immutable: + // no serialize needed + immutable.SetValue(value); + cacheItem = immutable; + break; + case MutableCacheItem mutable: + // serialization happens here + mutable.SetValue(value, Cache.GetSerializer(), MaximumPayloadBytes); + mutable.DebugOnlyTrackBuffer(Cache); + cacheItem = mutable; + break; + default: + cacheItem = ThrowUnexpectedCacheItem(); + break; } SetResult(cacheItem); return cacheItem; @@ -207,7 +225,7 @@ private CacheItem SetResult(T value) public override void SetCanceled() => _result?.TrySetCanceled(SharedToken); - internal ValueTask UnwrapAsync() + internal ValueTask UnwrapReservedAsync() { var task = Task; #if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER @@ -216,22 +234,23 @@ internal ValueTask UnwrapAsync() if (task.Status == TaskStatus.RanToCompletion) #endif { - return new(task.Result.GetValue()); + return new(task.Result.GetReservedValue()); } - // if the type is immutable, callers can share the final step too + // if the type is immutable, callers can share the final step too (this may leave dangling + // reservation counters, but that's OK) var result = ImmutableTypeCache.IsImmutable ? (_sharedUnwrap ??= Awaited(Task)) : Awaited(Task); return new(result); static async Task Awaited(Task> task) - => (await task.ConfigureAwait(false)).GetValue(); + => (await task.ConfigureAwait(false)).GetReservedValue(); } public ValueTask JoinAsync(CancellationToken token) { // if the underlying has already completed, and/or our local token can't cancel: we // can simply wrap the shared task; otherwise, we need our own cancellation state - return token.CanBeCanceled && !Task.IsCompleted ? WithCancellation(this, token) : UnwrapAsync(); + return token.CanBeCanceled && !Task.IsCompleted ? WithCancellation(this, token) : UnwrapReservedAsync(); static async ValueTask WithCancellation(StampedeState stampede, CancellationToken token) { @@ -241,6 +260,7 @@ static async ValueTask WithCancellation(StampedeState stampede, Ca ((TaskCompletionSource)obj!).TrySetResult(true); }, cancelStub); + CacheItem result; try { var first = await System.Threading.Tasks.Task.WhenAny(stampede.Task, cancelStub.Task).ConfigureAwait(false); @@ -252,12 +272,15 @@ static async ValueTask WithCancellation(StampedeState stampede, Ca Debug.Assert(ReferenceEquals(first, stampede.Task)); // this has already completed, but we'll get the stack nicely - return (await stampede.Task.ConfigureAwait(false)).GetValue(); + result = await stampede.Task.ConfigureAwait(false); } - finally + catch { - stampede.RemoveCaller(); + stampede.CancelCaller(); + throw; } + // outside the catch, so we know we only decrement one way or the other + return result.GetReservedValue(); } } } diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.SyncLock.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.SyncLock.cs new file mode 100644 index 000000000000..c702f7e9b3f9 --- /dev/null +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.SyncLock.cs @@ -0,0 +1,34 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +namespace Microsoft.Extensions.Caching.Hybrid.Internal; + +partial class DefaultHybridCache +{ + // HybridCache's stampede protection requires some level of synchronization to avoid unnecessary runs + // of the underlying data fetch; this is *minimized* by the use of double-checked locking and + // interlocked join (adding a new request to an existing execution), but: that would leave a race + // condition where the *remove* step of the stampede would be in a race with the *add new* step; the + // *add new* step is inside a lock, but we need to *remove* step to share that lock, to avoid + // the race. We deal with that by taking the same lock during remove, but *that* means we're locking + // on all executions. + // + // To minimize lock contention, we will therefore use partitioning of the lock-token, by using the + // low 3 bits of the hash-code (which we calculate eagerly only once, so: already known). This gives + // us a fast way to split contention by 8, almost an order-of-magnitude, which is sufficient. We *could* + // use an array for this, but: for directness, let's inline it instead (avoiding bounds-checks, + // an extra layer of dereferencing, and the allocation; I will acknowledge these are miniscule, but: + // it costs us nothing to do) + + private readonly object _syncLock0 = new(), _syncLock1 = new(), _syncLock2 = new(), _syncLock3 = new(), + _syncLock4 = new(), _syncLock5 = new(), _syncLock6 = new(), _syncLock7 = new(); + + internal object GetPartitionedSyncLock(in StampedeKey key) + => (key.HashCode & 0b111) switch // generate 8 partitions using the low 3 bits + { + 0 => _syncLock0, 1 => _syncLock1, + 2 => _syncLock2, 3 => _syncLock3, + 4 => _syncLock4, 5 => _syncLock5, + 6 => _syncLock6, _ => _syncLock7, + }; +} diff --git a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.cs b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.cs index 947648051cf4..171140c4b722 100644 --- a/src/Caching/Hybrid/src/Internal/DefaultHybridCache.cs +++ b/src/Caching/Hybrid/src/Internal/DefaultHybridCache.cs @@ -137,7 +137,7 @@ public override ValueTask GetOrCreateAsync(string key, TState stat { // we're going to run to completion; no need to get complicated _ = stampede.ExecuteDirectAsync(in state, underlyingDataCallback, options); // this larger task includes L2 write etc - return stampede.UnwrapAsync(); + return stampede.UnwrapReservedAsync(); } } diff --git a/src/Caching/Hybrid/src/Internal/RecyclableArrayBufferWriter.cs b/src/Caching/Hybrid/src/Internal/RecyclableArrayBufferWriter.cs index 5ff6d4699db8..9c65a33863a5 100644 --- a/src/Caching/Hybrid/src/Internal/RecyclableArrayBufferWriter.cs +++ b/src/Caching/Hybrid/src/Internal/RecyclableArrayBufferWriter.cs @@ -13,6 +13,22 @@ namespace Microsoft.Extensions.Caching.Hybrid.Internal; // except it uses the array pool for allocations internal sealed class RecyclableArrayBufferWriter : IBufferWriter, IDisposable { + // Usage note: *normally* you might want to use "using" for this, and that is fine; + // however, caution should be exercised in exception scenarios where we don't 100% + // know that the caller has stopped touching the buffer; in particular, this means + // scenarios involving a combination of external code and (for example) "async". + // In those cases, it may be preferable to manually dispose in the success case, + // and just drop the buffers in the failure case, i.e. instead of: + // + // using (writer) + // { DoStuff(); } + // + // simply: + // + // DoStuff(); + // writer.Dispose(); + // + // This does not represent a problem, and is consistent with many ArrayPool use-cases. // Copy of Array.MaxLength. // Used by projects targeting .NET Framework. diff --git a/src/Caching/Hybrid/test/BufferReleaseTests.cs b/src/Caching/Hybrid/test/BufferReleaseTests.cs index f83ce760c3df..5a8c0a8029c7 100644 --- a/src/Caching/Hybrid/test/BufferReleaseTests.cs +++ b/src/Caching/Hybrid/test/BufferReleaseTests.cs @@ -29,17 +29,17 @@ public async Task BufferGetsReleased_NoL2() { using var provider = GetDefaultCache(out var cache); #if DEBUG - cache.DebugGetOutstandingBuffers(flush: true); + cache.DebugOnlyGetOutstandingBuffers(flush: true); #endif var key = Me(); #if DEBUG - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif var first = await cache.GetOrCreateAsync(key, _ => GetAsync()); Assert.NotNull(first); #if DEBUG - Assert.Equal(1, cache.DebugGetOutstandingBuffers()); + Assert.Equal(1, cache.DebugOnlyGetOutstandingBuffers()); #endif Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); @@ -52,6 +52,7 @@ public async Task BufferGetsReleased_NoL2() Assert.NotNull(second); Assert.NotSame(first, second); + Assert.Equal(1, cacheItem.RefCount); await cache.RemoveKeyAsync(key); var third = await cache.GetOrCreateAsync(key, _ => GetAsync(), _noUnderlying); Assert.Null(third); @@ -62,10 +63,11 @@ public async Task BufferGetsReleased_NoL2() await Task.Delay(250); } #if DEBUG - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif // assert that we can *no longer* reserve this buffer, because we've already recycled it Assert.False(cacheItem.TryReserveBuffer(out _)); + Assert.Equal(0, cacheItem.RefCount); Assert.False(cacheItem.NeedsEvictionCallback, "should be recycled now"); static ValueTask GetAsync() => new(new Customer { Id = 42, Name = "Fred" }); } @@ -116,13 +118,13 @@ static bool Write(IBufferWriter destination, byte[]? buffer) cache.BackendCache.Set(key, writer.ToArray()); } #if DEBUG - cache.DebugGetOutstandingBuffers(flush: true); - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + cache.DebugOnlyGetOutstandingBuffers(flush: true); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif var first = await cache.GetOrCreateAsync(key, _ => GetAsync(), _noUnderlying); // we expect this to come from L2, hence NoUnderlying Assert.NotNull(first); #if DEBUG - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); @@ -135,6 +137,7 @@ static bool Write(IBufferWriter destination, byte[]? buffer) Assert.NotNull(second); Assert.NotSame(first, second); + Assert.Equal(1, cacheItem.RefCount); await cache.RemoveKeyAsync(key); var third = await cache.GetOrCreateAsync(key, _ => GetAsync(), _noUnderlying); Assert.Null(third); @@ -146,11 +149,12 @@ static bool Write(IBufferWriter destination, byte[]? buffer) await Task.Delay(250); } #if DEBUG - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif // assert that we can *no longer* reserve this buffer, because we've already recycled it Assert.True(cacheItem.TryReserveBuffer(out _)); // always readable cacheItem.Release(); + Assert.Equal(1, cacheItem.RefCount); // not decremented because there was no need to add the hook Assert.False(cacheItem.NeedsEvictionCallback, "should still not need recycling"); static ValueTask GetAsync() => new(new Customer { Id = 42, Name = "Fred" }); @@ -172,13 +176,13 @@ static bool Write(IBufferWriter destination, byte[]? buffer) cache.BackendCache.Set(key, writer.ToArray()); } #if DEBUG - cache.DebugGetOutstandingBuffers(flush: true); - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + cache.DebugOnlyGetOutstandingBuffers(flush: true); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif var first = await cache.GetOrCreateAsync(key, _ => GetAsync(), _noUnderlying); // we expect this to come from L2, hence NoUnderlying Assert.NotNull(first); #if DEBUG - Assert.Equal(1, cache.DebugGetOutstandingBuffers()); + Assert.Equal(1, cache.DebugOnlyGetOutstandingBuffers()); #endif Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); @@ -191,6 +195,7 @@ static bool Write(IBufferWriter destination, byte[]? buffer) Assert.NotNull(second); Assert.NotSame(first, second); + Assert.Equal(1, cacheItem.RefCount); await cache.RemoveKeyAsync(key); var third = await cache.GetOrCreateAsync(key, _ => GetAsync(), _noUnderlying); Assert.Null(third); @@ -202,10 +207,11 @@ static bool Write(IBufferWriter destination, byte[]? buffer) await Task.Delay(250); } #if DEBUG - Assert.Equal(0, cache.DebugGetOutstandingBuffers()); + Assert.Equal(0, cache.DebugOnlyGetOutstandingBuffers()); #endif // assert that we can *no longer* reserve this buffer, because we've already recycled it Assert.False(cacheItem.TryReserveBuffer(out _)); // released now + Assert.Equal(0, cacheItem.RefCount); Assert.False(cacheItem.NeedsEvictionCallback, "should be recycled by now"); static ValueTask GetAsync() => new(new Customer { Id = 42, Name = "Fred" }); diff --git a/src/Caching/Hybrid/test/DisposableValueTests.cs b/src/Caching/Hybrid/test/DisposableValueTests.cs deleted file mode 100644 index 57b2e950485b..000000000000 --- a/src/Caching/Hybrid/test/DisposableValueTests.cs +++ /dev/null @@ -1,188 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. - -using System; -using System.Collections.Generic; -using System.ComponentModel; -using System.Linq; -using System.Runtime.CompilerServices; -using System.Text; -using System.Threading.Tasks; -using Microsoft.Extensions.Caching.Hybrid.Internal; -using Microsoft.Extensions.DependencyInjection; - -namespace Microsoft.Extensions.Caching.Hybrid.Tests; -public class DisposableValueTests -{ - // We can only reasonable be expected to be responsible for disposal (IDisposable) of objects - // if we're keeping hold of references, which means: things we consider immutable. - // It is noted that this creates an oddity whereby for *mutable* types, the caller needs to dispose - // per fetch (GetOrCreateAsync), and for *immutable* types: they're not - but that is unavoidable. - // In reality, it is expected to be pretty rare to hold disposable types here. - - private static ServiceProvider GetCache(out DefaultHybridCache cache) - { - var services = new ServiceCollection(); - services.AddHybridCache(); - var provider = services.BuildServiceProvider(); - cache = Assert.IsType(provider.GetRequiredService()); - return provider; - } - - [Fact] - public async void NonDisposableImmutableTypeDoesNotNeedEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - var s = await cache.GetOrCreateAsync(key, _ => GetSomeString()); - Assert.NotNull(s); - Assert.False(string.IsNullOrWhiteSpace(s)); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.False(cacheItem.NeedsEvictionCallback); - - static ValueTask GetSomeString() => new(Guid.NewGuid().ToString()); - } - - [Fact] - public async void NonDisposableBlittableTypeDoesNotNeedEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - var g = await cache.GetOrCreateAsync(key, _ => GetSomeGuid()); - Assert.NotEqual(Guid.Empty, g); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.False(cacheItem.NeedsEvictionCallback); - - static ValueTask GetSomeGuid() => new(Guid.NewGuid()); - } - - [Fact] - public async Task DispsableRefTypeNeedsEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - var inst = new DisposableTestClass(); - var obj = await cache.GetOrCreateAsync(key, _ => new ValueTask(inst)); - Assert.Same(inst, obj); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.True(cacheItem.NeedsEvictionCallback); - - Assert.Equal(0, inst.DisposeCount); - - // now remove it - await cache.RemoveKeyAsync(key); - - // give it a moment for the eviction callback to kick in - for (var i = 0; i < 10; i++) - { - await Task.Delay(250); - if (inst.DisposeCount != 0) - { - break; - } - } - Assert.Equal(1, inst.DisposeCount); - } - - [Fact] - public async Task DisposableValueTypeNeedsEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - // disposal of value-type - var inst = new DisposableTestClass(); - var v = await cache.GetOrCreateAsync(key, _ => new ValueTask(new DisposableTestValue(inst))); - Assert.Same(inst, v.Wrapped); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.True(cacheItem.NeedsEvictionCallback); - - Assert.Equal(0, inst.DisposeCount); - - // now remove it - await cache.RemoveKeyAsync(key); - - // give it a moment for the eviction callback to kick in - for (var i = 0; i < 10; i++) - { - await Task.Delay(250); - if (inst.DisposeCount != 0) - { - break; - } - } - Assert.Equal(1, inst.DisposeCount); - } - - [Fact] - public async Task NonDispsableRefTypeDoesNotNeedEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - var inst = new NonDisposableTestClass(); - var obj = await cache.GetOrCreateAsync(key, _ => new ValueTask(inst)); - Assert.Same(inst, obj); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.False(cacheItem.NeedsEvictionCallback); - } - - [Fact] - public async Task NonDisposableValueTypeDoesNotNeedEvictionCallback() - { - using var provider = GetCache(out var cache); - var key = Me(); - - // disposal of value-type - var inst = new DisposableTestClass(); - var v = await cache.GetOrCreateAsync(key, _ => new ValueTask(new NonDisposableTestValue(inst))); - Assert.Same(inst, v.Wrapped); - Assert.True(cache.DebugTryGetCacheItem(key, out var cacheItem)); - Assert.True(cacheItem.DebugIsImmutable); - Assert.False(cacheItem.NeedsEvictionCallback); - } - - [ImmutableObject(true)] - public sealed class DisposableTestClass : IDisposable - { - private int _disposeCount; - public void Dispose() => Interlocked.Increment(ref _disposeCount); - - public int DisposeCount => Volatile.Read(ref _disposeCount); - } - - [ImmutableObject(true)] - public readonly struct DisposableTestValue : IDisposable - { - public DisposableTestClass Wrapped { get; } - public DisposableTestValue(DisposableTestClass inner) => Wrapped = inner; - public void Dispose() => Wrapped.Dispose(); - } - - [ImmutableObject(true)] - public sealed class NonDisposableTestClass - { - private int _disposeCount; - public void Dispose() => Interlocked.Increment(ref _disposeCount); - - public int DisposeCount => Volatile.Read(ref _disposeCount); - } - - [ImmutableObject(true)] - public readonly struct NonDisposableTestValue - { - public DisposableTestClass Wrapped { get; } - public NonDisposableTestValue(DisposableTestClass inner) => Wrapped = inner; - public void Dispose() => Wrapped.Dispose(); - } - - private static string Me([CallerMemberName] string caller = "") => caller; -} diff --git a/src/Caching/Hybrid/test/SampleUsage.cs b/src/Caching/Hybrid/test/SampleUsage.cs new file mode 100644 index 000000000000..f2bcc19de6cd --- /dev/null +++ b/src/Caching/Hybrid/test/SampleUsage.cs @@ -0,0 +1,200 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System; +using System.Collections.Generic; +using System.ComponentModel; +using System.Linq; +using System.Text; +using System.Text.Json; +using System.Threading.Tasks; +using Microsoft.Extensions.Caching.Distributed; +using Microsoft.Extensions.DependencyInjection; + +namespace Microsoft.Extensions.Caching.Hybrid.Tests; + +public class SampleUsage +{ + [Fact] + public async void DistributedCacheWorks() + { + var services = new ServiceCollection(); + services.AddDistributedMemoryCache(); + services.AddTransient(); + using var provider = services.BuildServiceProvider(); + + var obj = provider.GetRequiredService(); + string name = "abc"; + int id = 42; + var x = await obj.GetSomeInformationAsync(name, id); + var y = await obj.GetSomeInformationAsync(name, id); + Assert.NotSame(x, y); + Assert.Equal(id, x.Id); + Assert.Equal(name, x.Name); + Assert.Equal(id, y.Id); + Assert.Equal(name, y.Name); + } + + [Fact] + public async void HybridCacheWorks() + { + var services = new ServiceCollection(); + services.AddHybridCache(); + services.AddTransient(); + using var provider = services.BuildServiceProvider(); + + var obj = provider.GetRequiredService(); + string name = "abc"; + int id = 42; + var x = await obj.GetSomeInformationAsync(name, id); + var y = await obj.GetSomeInformationAsync(name, id); + Assert.NotSame(x, y); + Assert.Equal(id, x.Id); + Assert.Equal(name, x.Name); + Assert.Equal(id, y.Id); + Assert.Equal(name, y.Name); + } + + [Fact] + public async void HybridCacheNoCaptureWorks() + { + var services = new ServiceCollection(); + services.AddHybridCache(); + services.AddTransient(); + using var provider = services.BuildServiceProvider(); + + var obj = provider.GetRequiredService(); + string name = "abc"; + int id = 42; + var x = await obj.GetSomeInformationAsync(name, id); + var y = await obj.GetSomeInformationAsync(name, id); + Assert.NotSame(x, y); + Assert.Equal(id, x.Id); + Assert.Equal(name, x.Name); + Assert.Equal(id, y.Id); + Assert.Equal(name, y.Name); + } + + [Fact] + public async void HybridCacheNoCaptureObjReuseWorks() + { + var services = new ServiceCollection(); + services.AddHybridCache(); + services.AddTransient(); + using var provider = services.BuildServiceProvider(); + + var obj = provider.GetRequiredService(); + string name = "abc"; + int id = 42; + var x = await obj.GetSomeInformationAsync(name, id); + var y = await obj.GetSomeInformationAsync(name, id); + Assert.Same(x, y); + Assert.Equal(id, x.Id); + Assert.Equal(name, x.Name); + } + + public class SomeDCService(IDistributedCache cache) + { + public async Task GetSomeInformationAsync(string name, int id, CancellationToken token = default) + { + var key = $"someinfo:{name}:{id}"; // unique key for this combination + + var bytes = await cache.GetAsync(key, token); // try to get from cache + SomeInformation info; + if (bytes is null) + { + // cache miss; get the data from the real source + info = await SomeExpensiveOperationAsync(name, id, token); + + // serialize and cache it + bytes = SomeSerializer.Serialize(info); + await cache.SetAsync(key, bytes, token); + } + else + { + // cache hit; deserialize it + info = SomeSerializer.Deserialize(bytes); + } + return info; + } + } + + public class SomeHCService(HybridCache cache) + { + public async Task GetSomeInformationAsync(string name, int id, CancellationToken token = default) + { + return await cache.GetOrCreateAsync( + $"someinfo:{name}:{id}", // unique key for this combination + async ct => await SomeExpensiveOperationAsync(name, id, ct), + token: token + ); + } + } + + // this is the work we're trying to cache + private static Task SomeExpensiveOperationAsync(string name, int id, + CancellationToken token = default) + { + return Task.FromResult(new SomeInformation { Id = id, Name = name }); + } + private static Task SomeExpensiveOperationReuseAsync(string name, int id, + CancellationToken token = default) + { + return Task.FromResult(new SomeInformationReuse { Id = id, Name = name }); + } + + public class SomeHCServiceNoCapture(HybridCache cache) + { + public async Task GetSomeInformationAsync(string name, int id, CancellationToken token = default) + { + return await cache.GetOrCreateAsync( + $"someinfo:{name}:{id}", // unique key for this combination + (name, id), // all of the state we need for the final call, if needed + static async (state, token) => + await SomeExpensiveOperationAsync(state.name, state.id, token), + token: token + ); + } + } + + public class SomeHCServiceNoCaptureObjReuse(HybridCache cache, CancellationToken token = default) + { + public async Task GetSomeInformationAsync(string name, int id) + { + return await cache.GetOrCreateAsync( + $"someinfo:{name}:{id}", // unique key for this combination + (name, id), // all of the state we need for the final call, if needed + static async (state, token) => + await SomeExpensiveOperationReuseAsync(state.name, state.id, token), + token: token + ); + } + } + + static class SomeSerializer + { + internal static T Deserialize(byte[] bytes) + { + return JsonSerializer.Deserialize(bytes)!; + } + + internal static byte[] Serialize(T info) + { + using var ms = new MemoryStream(); + JsonSerializer.Serialize(ms, info); + return ms.ToArray(); + } + } + public class SomeInformation + { + public int Id { get; set; } + public string? Name { get; set; } + } + + [ImmutableObject(true)] + public sealed class SomeInformationReuse + { + public int Id { get; set; } + public string? Name { get; set; } + } +} diff --git a/src/Caching/Hybrid/test/StampedeTests.cs b/src/Caching/Hybrid/test/StampedeTests.cs index b6536655a5e3..833ddd7e4a67 100644 --- a/src/Caching/Hybrid/test/StampedeTests.cs +++ b/src/Caching/Hybrid/test/StampedeTests.cs @@ -365,6 +365,38 @@ public async Task MutableTypesNeverShareFinalTask(bool withCancelation) Assert.NotSame(x, y); } + [Fact] + public void ValidatePartitioning() + { + // we just want to validate that key-level partitioning is + // happening to some degree, i.e. it isn't fundamentally broken + using var scope = GetDefaultCache(out var cache); + Dictionary counts = []; + for(int i = 0; i < 1024; i++) + { + var key = new DefaultHybridCache.StampedeKey(Guid.NewGuid().ToString(), default); + var obj = cache.GetPartitionedSyncLock(in key); + if (!counts.TryGetValue(obj, out var count)) + { + count = 0; + } + counts[obj] = count + 1; + } + + // We just want to prove that we got 8 non-empty partitions. + // This is *technically* non-deterministic, but: we'd + // need to be having a very bad day for the math gods + // to conspire against us that badly - if this test + // starts failing, maybe buy a lottery ticket? + Assert.Equal(8, counts.Count); + foreach (var pair in counts) + { + // the *median* should be 128 here; let's + // not be aggressive about it, though + Assert.True(pair.Value > 16); + } + } + class Mutable(Guid value) { public Guid Value => value; diff --git a/src/Caching/SqlServer/src/DatabaseOperations.cs b/src/Caching/SqlServer/src/DatabaseOperations.cs index 0adbd9d54128..cd6f7f2aa404 100644 --- a/src/Caching/SqlServer/src/DatabaseOperations.cs +++ b/src/Caching/SqlServer/src/DatabaseOperations.cs @@ -308,7 +308,7 @@ public void SetCacheItem(string key, ArraySegment value, DistributedCacheE return value; } - static long StreamOut(SqlDataReader source, int ordinal, IBufferWriter destination) + private static long StreamOut(SqlDataReader source, int ordinal, IBufferWriter destination) { long dataIndex = 0; int read = 0;