diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryState.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryState.cs deleted file mode 100644 index a3774fef395de..0000000000000 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryState.cs +++ /dev/null @@ -1,61 +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; - -namespace Microsoft.Extensions.Caching.Memory -{ - internal sealed partial class CacheEntry - { - // this type exists just to reduce CacheEntry size by replacing many enum & boolean fields with one of a size of Int32 - private struct CacheEntryState - { - private byte _flags; - private byte _evictionReason; - private byte _priority; - - internal CacheEntryState(CacheItemPriority priority) : this() => _priority = (byte)priority; - - internal bool IsDisposed - { - get => ((Flags)_flags & Flags.IsDisposed) != 0; - set => SetFlag(Flags.IsDisposed, value); - } - - internal bool IsExpired - { - get => ((Flags)_flags & Flags.IsExpired) != 0; - set => SetFlag(Flags.IsExpired, value); - } - - internal bool IsValueSet - { - get => ((Flags)_flags & Flags.IsValueSet) != 0; - set => SetFlag(Flags.IsValueSet, value); - } - - internal EvictionReason EvictionReason - { - get => (EvictionReason)_evictionReason; - set => _evictionReason = (byte)value; - } - - internal CacheItemPriority Priority - { - get => (CacheItemPriority)_priority; - set => _priority = (byte)value; - } - - private void SetFlag(Flags option, bool value) => _flags = (byte)(value ? (_flags | (byte)option) : (_flags & ~(byte)option)); - - [Flags] - private enum Flags : byte - { - Default = 0, - IsValueSet = 1 << 0, - IsExpired = 1 << 1, - IsDisposed = 1 << 2, - } - } - } -} diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryTokens.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryTokens.cs index d0276412ab7d9..93e258e56e832 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryTokens.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.CacheEntryTokens.cs @@ -26,13 +26,14 @@ private sealed class CacheEntryTokens internal void AttachTokens(CacheEntry cacheEntry) { - if (_expirationTokens != null) + List? expirationTokens = _expirationTokens; + if (expirationTokens is not null) { lock (this) { - for (int i = 0; i < _expirationTokens.Count; i++) + for (int i = 0; i < expirationTokens.Count; i++) { - IChangeToken expirationToken = _expirationTokens[i]; + IChangeToken expirationToken = expirationTokens[i]; if (expirationToken.ActiveChangeCallbacks) { _expirationTokenRegistrations ??= new List(1); @@ -46,11 +47,12 @@ internal void AttachTokens(CacheEntry cacheEntry) internal bool CheckForExpiredTokens(CacheEntry cacheEntry) { - if (_expirationTokens != null) + List? expirationTokens = _expirationTokens; + if (expirationTokens is not null) { - for (int i = 0; i < _expirationTokens.Count; i++) + for (int i = 0; i < expirationTokens.Count; i++) { - IChangeToken expiredToken = _expirationTokens[i]; + IChangeToken expiredToken = expirationTokens[i]; if (expiredToken.HasChanged) { cacheEntry.SetExpired(EvictionReason.TokenExpired); @@ -69,12 +71,10 @@ internal void PropagateTokens(CacheEntry parentEntry) { lock (this) { - lock (parentEntry.GetOrCreateTokens()) + CacheEntryTokens parentTokens = parentEntry.GetOrCreateTokens(); + lock (parentTokens) { - foreach (IChangeToken expirationToken in _expirationTokens) - { - parentEntry.AddExpirationToken(expirationToken); - } + parentTokens.ExpirationTokens.AddRange(_expirationTokens); } } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index 677786a71bdcc..28eafc51ef5db 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using System.Threading; @@ -14,16 +15,25 @@ namespace Microsoft.Extensions.Caching.Memory internal sealed partial class CacheEntry : ICacheEntry { private static readonly Action ExpirationCallback = ExpirationTokensExpired; + private static readonly AsyncLocal _current = new AsyncLocal(); private readonly MemoryCache _cache; private CacheEntryTokens? _tokens; // might be null if user is not using the tokens or callbacks - private TimeSpan? _absoluteExpirationRelativeToNow; - private TimeSpan? _slidingExpiration; - private long? _size; + private TimeSpan _absoluteExpirationRelativeToNow; + private TimeSpan _slidingExpiration; + private long _size = NotSet; private CacheEntry? _previous; // this field is not null only before the entry is added to the cache and tracking is enabled private object? _value; - private CacheEntryState _state; + private long _absoluteExpirationTicks = NotSet; + private short _absoluteExpirationOffsetMinutes; + private bool _isDisposed; + private bool _isExpired; + private bool _isValueSet; + private byte _evictionReason; + private byte _priority = (byte)CacheItemPriority.Normal; + + private const int NotSet = -1; internal CacheEntry(object key, MemoryCache memoryCache) { @@ -32,27 +42,64 @@ internal CacheEntry(object key, MemoryCache memoryCache) Key = key; _cache = memoryCache; - _previous = memoryCache.TrackLinkedCacheEntries ? CacheEntryHelper.EnterScope(this) : null; - _state = new CacheEntryState(CacheItemPriority.Normal); + if (memoryCache.TrackLinkedCacheEntries) + { + AsyncLocal holder = _current; + _previous = holder.Value; + holder.Value = this; + } } - /// - /// Gets or sets an absolute expiration date for the cache entry. - /// - public DateTimeOffset? AbsoluteExpiration { get; set; } + // internal for testing + internal static CacheEntry? Current => _current.Value; - /// - /// Gets or sets an absolute expiration time, relative to now. - /// - public TimeSpan? AbsoluteExpirationRelativeToNow + internal long AbsoluteExpirationTicks { - get => _absoluteExpirationRelativeToNow; + get => _absoluteExpirationTicks; + set + { + _absoluteExpirationTicks = value; + _absoluteExpirationOffsetMinutes = 0; + } + } + + DateTimeOffset? ICacheEntry.AbsoluteExpiration + { + get + { + if (_absoluteExpirationTicks < 0) + return null; + + var offset = new TimeSpan(_absoluteExpirationOffsetMinutes * TimeSpan.TicksPerMinute); + return new DateTimeOffset(_absoluteExpirationTicks + offset.Ticks, offset); + } + set + { + if (value is null) + { + _absoluteExpirationTicks = NotSet; + _absoluteExpirationOffsetMinutes = default; + } + else + { + DateTimeOffset expiration = value.GetValueOrDefault(); + _absoluteExpirationTicks = expiration.UtcTicks; + _absoluteExpirationOffsetMinutes = (short)(expiration.Offset.Ticks / TimeSpan.TicksPerMinute); + } + } + } + + internal TimeSpan AbsoluteExpirationRelativeToNow => _absoluteExpirationRelativeToNow; + + TimeSpan? ICacheEntry.AbsoluteExpirationRelativeToNow + { + get => _absoluteExpirationRelativeToNow.Ticks == 0 ? null : _absoluteExpirationRelativeToNow; set { // this method does not set AbsoluteExpiration as it would require calling Clock.UtcNow twice: // once here and once in MemoryCache.SetEntry - if (value <= TimeSpan.Zero) + if (value is { Ticks: <= 0 }) { throw new ArgumentOutOfRangeException( nameof(AbsoluteExpirationRelativeToNow), @@ -60,7 +107,7 @@ public TimeSpan? AbsoluteExpirationRelativeToNow "The relative expiration value must be positive."); } - _absoluteExpirationRelativeToNow = value; + _absoluteExpirationRelativeToNow = value.GetValueOrDefault(); } } @@ -70,10 +117,10 @@ public TimeSpan? AbsoluteExpirationRelativeToNow /// public TimeSpan? SlidingExpiration { - get => _slidingExpiration; + get => _slidingExpiration.Ticks == 0 ? null : _slidingExpiration; set { - if (value <= TimeSpan.Zero) + if (value is { Ticks: <= 0 }) { throw new ArgumentOutOfRangeException( nameof(SlidingExpiration), @@ -81,7 +128,7 @@ public TimeSpan? SlidingExpiration "The sliding expiration value must be positive."); } - _slidingExpiration = value; + _slidingExpiration = value.GetValueOrDefault(); } } @@ -101,14 +148,13 @@ public TimeSpan? SlidingExpiration /// Gets or sets the priority for keeping the cache entry in the cache during a /// memory pressure triggered cleanup. The default is . /// - public CacheItemPriority Priority { get => _state.Priority; set => _state.Priority = value; } + public CacheItemPriority Priority { get => (CacheItemPriority)_priority; set => _priority = (byte)value; } - /// - /// Gets or sets the size of the cache entry value. - /// - public long? Size + internal long Size => _size; + + long? ICacheEntry.Size { - get => _size; + get => _size < 0 ? null : _size; set { if (value < 0) @@ -116,11 +162,11 @@ public long? Size throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be non-negative."); } - _size = value; + _size = value ?? NotSet; } } - public object Key { get; private set; } + public object Key { get; } public object? Value { @@ -128,46 +174,62 @@ public object? Value set { _value = value; - _state.IsValueSet = true; + _isValueSet = true; } } - internal DateTimeOffset LastAccessed { get; set; } + internal DateTime LastAccessed { get; set; } - internal EvictionReason EvictionReason { get => _state.EvictionReason; private set => _state.EvictionReason = value; } + internal EvictionReason EvictionReason { get => (EvictionReason)_evictionReason; private set => _evictionReason = (byte)value; } public void Dispose() { - if (!_state.IsDisposed) + if (!_isDisposed) { - _state.IsDisposed = true; + _isDisposed = true; if (_cache.TrackLinkedCacheEntries) { - CacheEntryHelper.ExitScope(this, _previous); + CommitWithTracking(); } - - // Don't commit or propagate options if the CacheEntry Value was never set. - // We assume an exception occurred causing the caller to not set the Value successfully, - // so don't use this entry. - if (_state.IsValueSet) + else if (_isValueSet) { _cache.SetEntry(this); + } + } + } + + private void CommitWithTracking() + { + Debug.Assert(_current.Value == this, "Entries disposed in invalid order"); + _current.Value = _previous; + + // Don't commit or propagate options if the CacheEntry Value was never set. + // We assume an exception occurred causing the caller to not set the Value successfully, + // so don't use this entry. + if (_isValueSet) + { + _cache.SetEntry(this); - if (_previous != null && CanPropagateOptions()) + CacheEntry? parent = _previous; + if (parent != null) + { + if ((ulong)_absoluteExpirationTicks < (ulong)parent._absoluteExpirationTicks) { - PropagateOptions(_previous); + parent._absoluteExpirationTicks = _absoluteExpirationTicks; + parent._absoluteExpirationOffsetMinutes = _absoluteExpirationOffsetMinutes; } + _tokens?.PropagateTokens(parent); } - - _previous = null; // we don't want to root unnecessary objects } + + _previous = null; // we don't want to root unnecessary objects } [MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling - internal bool CheckExpired(in DateTimeOffset now) - => _state.IsExpired - || CheckForExpiredTime(now) + internal bool CheckExpired(DateTime utcNow) + => _isExpired + || CheckForExpiredTime(utcNow) || (_tokens != null && _tokens.CheckForExpiredTokens(this)); internal void SetExpired(EvictionReason reason) @@ -176,30 +238,30 @@ internal void SetExpired(EvictionReason reason) { EvictionReason = reason; } - _state.IsExpired = true; + _isExpired = true; _tokens?.DetachTokens(); } [MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling - private bool CheckForExpiredTime(in DateTimeOffset now) + private bool CheckForExpiredTime(DateTime utcNow) { - if (!AbsoluteExpiration.HasValue && !_slidingExpiration.HasValue) + if (_absoluteExpirationTicks < 0 && _slidingExpiration.Ticks == 0) { return false; } - return FullCheck(now); + return FullCheck(utcNow); - bool FullCheck(in DateTimeOffset offset) + bool FullCheck(DateTime utcNow) { - if (AbsoluteExpiration.HasValue && AbsoluteExpiration.Value <= offset) + if ((ulong)_absoluteExpirationTicks <= (ulong)utcNow.Ticks) { SetExpired(EvictionReason.Expired); return true; } - if (_slidingExpiration.HasValue - && (offset - LastAccessed) >= _slidingExpiration) + if (_slidingExpiration.Ticks > 0 + && (utcNow - LastAccessed) >= _slidingExpiration) { SetExpired(EvictionReason.Expired); return true; @@ -224,28 +286,22 @@ private static void ExpirationTokensExpired(object obj) internal void InvokeEvictionCallbacks() => _tokens?.InvokeEvictionCallbacks(this); - // this simple check very often allows us to avoid expensive call to PropagateOptions(CacheEntryHelper.Current) - [MethodImpl(MethodImplOptions.AggressiveInlining)] // added based on profiling - internal bool CanPropagateOptions() => (_tokens != null && _tokens.CanPropagateTokens()) || AbsoluteExpiration.HasValue; - - internal void PropagateOptions(CacheEntry? parent) + internal void PropagateOptionsToCurrent() { - if (parent == null) + if ((_tokens == null || !_tokens.CanPropagateTokens()) && _absoluteExpirationTicks < 0 || _current.Value is not CacheEntry parent) { return; } // Copy expiration tokens and AbsoluteExpiration to the cache entries hierarchy. // We do this regardless of it gets cached because the tokens are associated with the value we'll return. - _tokens?.PropagateTokens(parent); - - if (AbsoluteExpiration.HasValue) + if ((ulong)_absoluteExpirationTicks < (ulong)parent._absoluteExpirationTicks) { - if (!parent.AbsoluteExpiration.HasValue || AbsoluteExpiration < parent.AbsoluteExpiration) - { - parent.AbsoluteExpiration = AbsoluteExpiration; - } + parent._absoluteExpirationTicks = _absoluteExpirationTicks; + parent._absoluteExpirationOffsetMinutes = _absoluteExpirationOffsetMinutes; } + + _tokens?.PropagateTokens(parent); } [MemberNotNull(nameof(_tokens))] diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs deleted file mode 100644 index cfe33085711a3..0000000000000 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntryHelper.cs +++ /dev/null @@ -1,32 +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.Diagnostics; -using System.Threading; - -namespace Microsoft.Extensions.Caching.Memory -{ - internal static class CacheEntryHelper - { - private static readonly AsyncLocal _current = new AsyncLocal(); - - internal static CacheEntry? Current - { - get => _current.Value; - private set => _current.Value = value; - } - - internal static CacheEntry? EnterScope(CacheEntry? current) - { - CacheEntry? previous = Current; - Current = current; - return previous; - } - - internal static void ExitScope(CacheEntry? current, CacheEntry? previous) - { - Debug.Assert(Current == current, "Entries disposed in invalid order"); - Current = previous; - } - } -} diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index a8ee2410194f5..445a575b371e8 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -8,7 +8,6 @@ using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; -using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; @@ -30,7 +29,7 @@ public class MemoryCache : IMemoryCache private readonly ThreadLocal? _stats; private CoherentState _coherentState; private bool _disposed; - private DateTimeOffset _lastExpirationScan; + private DateTime _lastExpirationScan; /// /// Creates a new instance. @@ -54,11 +53,6 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory _coherentState = new CoherentState(); - if (_options.Clock == null) - { - _options.Clock = new SystemClock(); - } - if (_options.TrackStatistics) { _allStats = new List>(); @@ -66,10 +60,12 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory _stats = new ThreadLocal(() => new Stats(this)); } - _lastExpirationScan = _options.Clock.UtcNow; + _lastExpirationScan = UtcNow; TrackLinkedCacheEntries = _options.TrackLinkedCacheEntries; // we store the setting now so it's consistent for entire MemoryCache lifetime } + private DateTime UtcNow => _options.Clock?.UtcNow.UtcDateTime ?? DateTime.UtcNow; + /// /// Cleans up the background collection events. /// @@ -102,22 +98,22 @@ internal void SetEntry(CacheEntry entry) return; } - if (_options.SizeLimit.HasValue && !entry.Size.HasValue) + if (_options.HasSizeLimit && entry.Size < 0) { throw new InvalidOperationException(SR.Format(SR.CacheEntryHasEmptySize, nameof(entry.Size), nameof(_options.SizeLimit))); } - DateTimeOffset utcNow = _options.Clock!.UtcNow; + DateTime utcNow = UtcNow; // Applying the option's absolute expiration only if it's not already smaller. // This can be the case if a dependent cache entry has a smaller value, and // it was set by cascading it to its parent. - if (entry.AbsoluteExpirationRelativeToNow.HasValue) + if (entry.AbsoluteExpirationRelativeToNow.Ticks > 0) { - var absoluteExpiration = utcNow + entry.AbsoluteExpirationRelativeToNow.Value; - if (!entry.AbsoluteExpiration.HasValue || absoluteExpiration < entry.AbsoluteExpiration.Value) + long absoluteExpiration = (utcNow + entry.AbsoluteExpirationRelativeToNow).Ticks; + if ((ulong)absoluteExpiration < (ulong)entry.AbsoluteExpirationTicks) { - entry.AbsoluteExpiration = absoluteExpiration; + entry.AbsoluteExpirationTicks = absoluteExpiration; } } @@ -137,15 +133,10 @@ internal void SetEntry(CacheEntry entry) { coherentState.RemoveEntry(priorEntry, _options); } - StartScanForExpiredItemsIfNeeded(utcNow); - return; } - - bool exceedsCapacity = UpdateCacheSizeExceedsCapacity(entry, coherentState); - if (!exceedsCapacity) + else if (!UpdateCacheSizeExceedsCapacity(entry, coherentState)) { bool entryAdded; - if (priorEntry == null) { // Try to add the new entry if no previous entries exist. @@ -158,10 +149,10 @@ internal void SetEntry(CacheEntry entry) if (entryAdded) { - if (_options.SizeLimit.HasValue) + if (_options.HasSizeLimit) { // The prior entry was removed, decrease the by the prior entry's size - Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size!.Value); + Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size); } } else @@ -179,19 +170,16 @@ internal void SetEntry(CacheEntry entry) } else { - if (_options.SizeLimit.HasValue) + if (_options.HasSizeLimit) { // Entry could not be added, reset cache size - Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); + Interlocked.Add(ref coherentState._cacheSize, -entry.Size); } entry.SetExpired(EvictionReason.Replaced); entry.InvokeEvictionCallbacks(); } - if (priorEntry != null) - { - priorEntry.InvokeEvictionCallbacks(); - } + priorEntry?.InvokeEvictionCallbacks(); } else { @@ -214,11 +202,12 @@ public bool TryGetValue(object key, out object? result) CheckDisposed(); - DateTimeOffset utcNow = _options.Clock!.UtcNow; + DateTime utcNow = UtcNow; CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime - if (coherentState._entries.TryGetValue(key, out CacheEntry? entry)) + if (coherentState._entries.TryGetValue(key, out CacheEntry? tmp)) { + CacheEntry entry = tmp; // Check if expired due to expiration tokens, timers, etc. and if so, remove it. // Allow a stale Replaced value to be returned due to concurrent calls to SetExpired during SetEntry. if (!entry.CheckExpired(utcNow) || entry.EvictionReason == EvictionReason.Replaced) @@ -226,11 +215,11 @@ public bool TryGetValue(object key, out object? result) entry.LastAccessed = utcNow; result = entry.Value; - if (TrackLinkedCacheEntries && entry.CanPropagateOptions()) + if (TrackLinkedCacheEntries) { // When this entry is retrieved in the scope of creating another entry, // that entry needs a copy of these expiration tokens. - entry.PropagateOptions(CacheEntryHelper.Current); + entry.PropagateOptionsToCurrent(); } StartScanForExpiredItemsIfNeeded(utcNow); @@ -277,16 +266,16 @@ public void Remove(object key) CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime if (coherentState._entries.TryRemove(key, out CacheEntry? entry)) { - if (_options.SizeLimit.HasValue) + if (_options.HasSizeLimit) { - Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); + Interlocked.Add(ref coherentState._cacheSize, -entry.Size); } entry.SetExpired(EvictionReason.Removed); entry.InvokeEvictionCallbacks(); } - StartScanForExpiredItemsIfNeeded(_options.Clock!.UtcNow); + StartScanForExpiredItemsIfNeeded(UtcNow); } /// @@ -297,7 +286,7 @@ public void Clear() CheckDisposed(); CoherentState oldState = Interlocked.Exchange(ref _coherentState, new CoherentState()); - foreach (var entry in oldState._entries) + foreach (KeyValuePair entry in oldState._entries) { entry.Value.SetExpired(EvictionReason.Removed); entry.Value.InvokeEvictionCallbacks(); @@ -329,23 +318,23 @@ internal void EntryExpired(CacheEntry entry) { // TODO: For efficiency consider processing these expirations in batches. _coherentState.RemoveEntry(entry, _options); - StartScanForExpiredItemsIfNeeded(_options.Clock!.UtcNow); + StartScanForExpiredItemsIfNeeded(UtcNow); } // Called by multiple actions to see how long it's been since we last checked for expired items. // If sufficient time has elapsed then a scan is initiated on a background task. [MethodImpl(MethodImplOptions.AggressiveInlining)] - private void StartScanForExpiredItemsIfNeeded(DateTimeOffset utcNow) + private void StartScanForExpiredItemsIfNeeded(DateTime utcNow) { if (_options.ExpirationScanFrequency < utcNow - _lastExpirationScan) { ScheduleTask(utcNow); } - void ScheduleTask(DateTimeOffset utcNow) + void ScheduleTask(DateTime utcNow) { _lastExpirationScan = utcNow; - Task.Factory.StartNew(state => ScanForExpiredItems((MemoryCache)state!), this, + Task.Factory.StartNew(state => ((MemoryCache)state!).ScanForExpiredItems(), this, CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); } } @@ -416,44 +405,47 @@ private void AddToStats(Stats current) } } - private static void ScanForExpiredItems(MemoryCache cache) + private void ScanForExpiredItems() { - DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock!.UtcNow; + DateTime utcNow = _lastExpirationScan = UtcNow; - CoherentState coherentState = cache._coherentState; // Clear() can update the reference in the meantime + CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime foreach (KeyValuePair item in coherentState._entries) { CacheEntry entry = item.Value; - if (entry.CheckExpired(now)) + if (entry.CheckExpired(utcNow)) { - coherentState.RemoveEntry(entry, cache._options); + coherentState.RemoveEntry(entry, _options); } } } private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState coherentState) { - if (!_options.SizeLimit.HasValue) + long sizeLimit = _options.SizeLimitValue; + if (sizeLimit < 0) { return false; } + long sizeRead = coherentState.Size; for (int i = 0; i < 100; i++) { - long sizeRead = coherentState.Size; - long newSize = sizeRead + entry.Size!.Value; + long newSize = sizeRead + entry.Size; - if (newSize < 0 || newSize > _options.SizeLimit) + if ((ulong)newSize > (ulong)sizeLimit) { // Overflow occurred, return true without updating the cache size return true; } - if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead)) + long original = Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead); + if (sizeRead == original) { return false; } + sizeRead = original; } return true; @@ -461,26 +453,33 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState cohe private void TriggerOvercapacityCompaction() { - _logger.LogDebug("Overcapacity compaction triggered"); + if (_logger.IsEnabled(LogLevel.Debug)) + _logger.LogDebug("Overcapacity compaction triggered"); // Spawn background thread for compaction - ThreadPool.QueueUserWorkItem(s => OvercapacityCompaction((MemoryCache)s!), this); + ThreadPool.QueueUserWorkItem(s => ((MemoryCache)s!).OvercapacityCompaction(), this); } - private static void OvercapacityCompaction(MemoryCache cache) + private void OvercapacityCompaction() { - CoherentState coherentState = cache._coherentState; // Clear() can update the reference in the meantime + CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime long currentSize = coherentState.Size; - cache._logger.LogDebug($"Overcapacity compaction executing. Current size {currentSize}"); + if (_logger.IsEnabled(LogLevel.Debug)) + _logger.LogDebug($"Overcapacity compaction executing. Current size {currentSize}"); - double? lowWatermark = cache._options.SizeLimit * (1 - cache._options.CompactionPercentage); - if (currentSize > lowWatermark) + long sizeLimit = _options.SizeLimitValue; + if (sizeLimit >= 0) { - cache.Compact(currentSize - (long)lowWatermark, entry => entry.Size!.Value, coherentState); + long lowWatermark = sizeLimit - (long)(sizeLimit * _options.CompactionPercentage); + if (currentSize > lowWatermark) + { + Compact(currentSize - (long)lowWatermark, entry => entry.Size, coherentState); + } } - cache._logger.LogDebug($"Overcapacity compaction executed. New size {coherentState.Size}"); + if (_logger.IsEnabled(LogLevel.Debug)) + _logger.LogDebug($"Overcapacity compaction executed. New size {coherentState.Size}"); } /// Remove at least the given percentage (0.10 for 10%) of the total entries (or estimated memory?), according to the following policy: @@ -507,11 +506,11 @@ private void Compact(long removalSizeTarget, Func computeEntry long removedSize = 0; // Sort items by expired & priority status - DateTimeOffset now = _options.Clock!.UtcNow; + DateTime utcNow = UtcNow; foreach (KeyValuePair item in coherentState._entries) { CacheEntry entry = item.Value; - if (entry.CheckExpired(now)) + if (entry.CheckExpired(utcNow)) { entriesToRemove.Add(entry); removedSize += computeEntrySize(entry); @@ -623,7 +622,7 @@ private sealed class CoherentState internal int Count => _entries.Count; - internal long Size => Interlocked.Read(ref _cacheSize); + internal long Size => Volatile.Read(ref _cacheSize); internal void RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { @@ -631,7 +630,7 @@ internal void RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { if (options.SizeLimit.HasValue) { - Interlocked.Add(ref _cacheSize, -entry.Size!.Value); + Interlocked.Add(ref _cacheSize, -entry.Size); } entry.InvokeEvictionCallbacks(); } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs index a37111ad60749..efb5b197d9378 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs @@ -9,9 +9,11 @@ namespace Microsoft.Extensions.Caching.Memory { public class MemoryCacheOptions : IOptions { - private long? _sizeLimit; + private long _sizeLimit = NotSet; private double _compactionPercentage = 0.05; + private const int NotSet = -1; + public ISystemClock? Clock { get; set; } /// @@ -19,12 +21,16 @@ public class MemoryCacheOptions : IOptions /// public TimeSpan ExpirationScanFrequency { get; set; } = TimeSpan.FromMinutes(1); + internal bool HasSizeLimit => _sizeLimit >= 0; + + internal long SizeLimitValue => _sizeLimit; + /// /// Gets or sets the maximum size of the cache. /// public long? SizeLimit { - get => _sizeLimit; + get => _sizeLimit < 0 ? null : _sizeLimit; set { if (value < 0) @@ -32,7 +38,7 @@ public long? SizeLimit throw new ArgumentOutOfRangeException(nameof(value), value, $"{nameof(value)} must be non-negative."); } - _sizeLimit = value; + _sizeLimit = value ?? NotSet; } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryDistributedCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryDistributedCache.cs index cb0b241de87c4..f7b1ee34b9387 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryDistributedCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryDistributedCache.cs @@ -13,7 +13,7 @@ namespace Microsoft.Extensions.Caching.Distributed { public class MemoryDistributedCache : IDistributedCache { - private readonly IMemoryCache _memCache; + private readonly MemoryCache _memCache; public MemoryDistributedCache(IOptions optionsAccessor) : this(optionsAccessor, NullLoggerFactory.Instance) { } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs index c8f242e9b46ba..0b7885ba81994 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs @@ -44,8 +44,8 @@ public void SetPopulates_ExpirationTokens_IntoScopedLink(bool trackLinkedCacheEn cache.Set(key, obj, new MemoryCacheEntryOptions().AddExpirationToken(expirationToken)); } - Assert.Equal(trackLinkedCacheEntries ? 1 : 0, ((CacheEntry)entry).ExpirationTokens.Count); - Assert.Null(((CacheEntry)entry).AbsoluteExpiration); + Assert.Equal(trackLinkedCacheEntries ? 1 : 0, entry.ExpirationTokens.Count); + Assert.Null(entry.AbsoluteExpiration); } [Theory] @@ -67,8 +67,8 @@ public void SetPopulates_AbsoluteExpiration_IntoScopeLink(bool trackLinkedCacheE cache.Set(key, obj, new MemoryCacheEntryOptions().SetAbsoluteExpiration(time)); } - Assert.Empty(((CacheEntry)entry).ExpirationTokens); - Assert.Equal(trackLinkedCacheEntries ? time : null, ((CacheEntry)entry).AbsoluteExpiration); + Assert.Empty(entry.ExpirationTokens); + Assert.Equal(trackLinkedCacheEntries ? time : null, entry.AbsoluteExpiration); } [Theory] @@ -340,7 +340,7 @@ public void GetWithImplicitLinkPopulatesExpirationTokens(bool trackLinkedCacheEn string key = "myKey"; string key1 = "myKey1"; - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); ICacheEntry entry; using (entry = cache.CreateEntry(key)) @@ -351,10 +351,10 @@ public void GetWithImplicitLinkPopulatesExpirationTokens(bool trackLinkedCacheEn cache.Set(key1, obj, new MemoryCacheEntryOptions().AddExpirationToken(expirationToken)); } - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); - Assert.Equal(trackLinkedCacheEntries ? 1 : 0, ((CacheEntry)entry).ExpirationTokens.Count); - Assert.Null(((CacheEntry)entry).AbsoluteExpiration); + Assert.Equal(trackLinkedCacheEntries ? 1 : 0, entry.ExpirationTokens.Count); + Assert.Null(entry.AbsoluteExpiration); } [Theory] @@ -367,7 +367,7 @@ public void LinkContextsCanNest(bool trackLinkedCacheEntries) string key = "myKey"; string key1 = "myKey1"; - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); ICacheEntry entry; ICacheEntry entry1; @@ -387,12 +387,12 @@ public void LinkContextsCanNest(bool trackLinkedCacheEntries) VerifyCurrentEntry(trackLinkedCacheEntries, entry); } - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); - Assert.Single(((CacheEntry)entry1).ExpirationTokens); - Assert.Null(((CacheEntry)entry1).AbsoluteExpiration); - Assert.Equal(trackLinkedCacheEntries ? 1 : 0, ((CacheEntry)entry).ExpirationTokens.Count); - Assert.Null(((CacheEntry)entry).AbsoluteExpiration); + Assert.Single(entry1.ExpirationTokens); + Assert.Null(entry1.AbsoluteExpiration); + Assert.Equal(trackLinkedCacheEntries ? 1 : 0, entry.ExpirationTokens.Count); + Assert.Null(entry.AbsoluteExpiration); } [Theory] @@ -428,13 +428,13 @@ public void NestedLinkContextsCanAggregate(bool trackLinkedCacheEntries) } } - Assert.Equal(trackLinkedCacheEntries ? 2 : 1, ((CacheEntry)entry1).ExpirationTokens.Count()); - Assert.NotNull(((CacheEntry)entry1).AbsoluteExpiration); - Assert.Equal(clock.UtcNow + TimeSpan.FromSeconds(10), ((CacheEntry)entry1).AbsoluteExpiration); + Assert.Equal(trackLinkedCacheEntries ? 2 : 1, entry1.ExpirationTokens.Count()); + Assert.NotNull(entry1.AbsoluteExpiration); + Assert.Equal(clock.UtcNow + TimeSpan.FromSeconds(10), entry1.AbsoluteExpiration); - Assert.Single(((CacheEntry)entry2).ExpirationTokens); - Assert.NotNull(((CacheEntry)entry2).AbsoluteExpiration); - Assert.Equal(clock.UtcNow + TimeSpan.FromSeconds(15), ((CacheEntry)entry2).AbsoluteExpiration); + Assert.Single(entry2.ExpirationTokens); + Assert.NotNull(entry2.AbsoluteExpiration); + Assert.Equal(clock.UtcNow + TimeSpan.FromSeconds(15), entry2.AbsoluteExpiration); } [Fact] @@ -519,22 +519,22 @@ await Task.WhenAll( Task.Run(() => SetExpiredManyTimes(entry)), Task.Run(() => SetExpiredManyTimes(entry))); - Assert.True(entry.CheckExpired(DateTimeOffset.UtcNow)); + Assert.True(entry.CheckExpired(DateTime.UtcNow)); static void SetExpiredManyTimes(CacheEntry cacheEntry) { - var now = DateTimeOffset.UtcNow; + var utcNow = DateTime.UtcNow; for (int i = 0; i < 1_000; i++) { cacheEntry.SetExpired(EvictionReason.Expired); // modifies CacheEntry._state - Assert.True(cacheEntry.CheckExpired(now)); + Assert.True(cacheEntry.CheckExpired(utcNow)); cacheEntry.Value = cacheEntry; // modifies CacheEntry._state - Assert.True(cacheEntry.CheckExpired(now)); + Assert.True(cacheEntry.CheckExpired(utcNow)); cacheEntry.SetExpired(EvictionReason.Expired); // modifies CacheEntry._state - Assert.True(cacheEntry.CheckExpired(now)); + Assert.True(cacheEntry.CheckExpired(utcNow)); cacheEntry.Dispose(); // might modify CacheEntry._state - Assert.True(cacheEntry.CheckExpired(now)); + Assert.True(cacheEntry.CheckExpired(utcNow)); } } } @@ -543,11 +543,11 @@ private static void VerifyCurrentEntry(bool trackLinkedCacheEntries, ICacheEntry { if (trackLinkedCacheEntries) { - Assert.Same(entry, CacheEntryHelper.Current); + Assert.Same(entry, CacheEntry.Current); } else { - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); } } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index 28ad9ebebf795..4ac0e73ceca71 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -185,7 +185,7 @@ public void GetOrCreate_WillNotCreateEmptyValue_WhenFactoryThrows(bool trackLink Assert.False(cache.TryGetValue(key, out int obj)); // verify that throwing an exception doesn't leak CacheEntry objects - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); } [Theory] @@ -209,7 +209,7 @@ await cache.GetOrCreateAsync(key, entry => Assert.False(cache.TryGetValue(key, out int obj)); // verify that throwing an exception doesn't leak CacheEntry objects - Assert.Null(CacheEntryHelper.Current); + Assert.Null(CacheEntry.Current); } [Theory]