diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs index 6f91cb21f038c..2084360890243 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/CacheEntry.cs @@ -33,6 +33,7 @@ internal CacheEntry(object key, MemoryCache memoryCache) Key = key ?? throw new ArgumentNullException(nameof(key)); _cache = memoryCache ?? throw new ArgumentNullException(nameof(memoryCache)); _previous = CacheEntryHelper.EnterScope(this); + _state = (int)State.CanExpire; } /// @@ -133,9 +134,21 @@ public object Value internal EvictionReason EvictionReason { get; private set; } + internal bool CanExpire + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ((State)_state).HasFlag(State.CanExpire); + set => Set(State.CanExpire, value); + } + private bool IsDisposed { get => ((State)_state).HasFlag(State.IsDisposed); set => Set(State.IsDisposed, value); } - private bool IsExpired { get => ((State)_state).HasFlag(State.IsExpired); set => Set(State.IsExpired, value); } + private bool IsExpired + { + [MethodImpl(MethodImplOptions.AggressiveInlining)] + get => ((State)_state).HasFlag(State.IsExpired); + set => Set(State.IsExpired, value); + } private bool IsValueSet { get => ((State)_state).HasFlag(State.IsValueSet); set => Set(State.IsValueSet, value); } @@ -150,6 +163,9 @@ public void Dispose() // so don't use this entry. if (IsValueSet) { + // set the CanExpire flag before adding the entry to the cache + CanExpire = AbsoluteExpiration.HasValue || SlidingExpiration.HasValue || AbsoluteExpirationRelativeToNow.HasValue || _expirationTokens != null; + _cache.SetEntry(this); if (_previous != null && CanPropagateOptions()) @@ -164,7 +180,7 @@ public void Dispose() } [MethodImpl(MethodImplOptions.AggressiveInlining)] - internal bool CheckExpired(in DateTimeOffset now) => IsExpired || CheckForExpiredTime(now) || CheckForExpiredTokens(); + internal bool CheckExpired(DateTimeOffset now) => IsExpired || (CanExpire && (CheckForExpiredTime(now) || CheckForExpiredTokens())); internal void SetExpired(EvictionReason reason) { @@ -176,32 +192,22 @@ internal void SetExpired(EvictionReason reason) DetachTokens(); } - private bool CheckForExpiredTime(in DateTimeOffset now) + private bool CheckForExpiredTime(DateTimeOffset now) { - if (!AbsoluteExpiration.HasValue && !_slidingExpiration.HasValue) + if (AbsoluteExpiration.HasValue && AbsoluteExpiration.Value <= now) { - return false; + SetExpired(EvictionReason.Expired); + return true; } - return FullCheck(now); - - bool FullCheck(in DateTimeOffset offset) + if (_slidingExpiration.HasValue + && (now - LastAccessed) >= _slidingExpiration) { - if (AbsoluteExpiration.HasValue && AbsoluteExpiration.Value <= offset) - { - SetExpired(EvictionReason.Expired); - return true; - } - - if (_slidingExpiration.HasValue - && (offset - LastAccessed) >= _slidingExpiration) - { - SetExpired(EvictionReason.Expired); - return true; - } - - return false; + SetExpired(EvictionReason.Expired); + return true; } + + return false; } private bool CheckForExpiredTokens() @@ -315,6 +321,7 @@ private static void InvokeCallbacks(CacheEntry entry) } // this simple check very often allows us to avoid expensive call to PropagateOptions(CacheEntryHelper.Current) + [MethodImpl(MethodImplOptions.AggressiveInlining)] internal bool CanPropagateOptions() => _expirationTokens != null || AbsoluteExpiration.HasValue; internal void PropagateOptions(CacheEntry parent) @@ -336,6 +343,7 @@ internal void PropagateOptions(CacheEntry parent) { parent.AddExpirationToken(expirationToken); } + parent.CanExpire = true; } } } @@ -345,6 +353,7 @@ internal void PropagateOptions(CacheEntry parent) if (!parent.AbsoluteExpiration.HasValue || AbsoluteExpiration < parent.AbsoluteExpiration) { parent.AbsoluteExpiration = AbsoluteExpiration; + parent.CanExpire = true; } } } @@ -367,6 +376,7 @@ private enum State IsValueSet = 1 << 0, IsExpired = 1 << 1, IsDisposed = 1 << 2, + CanExpire = 1 << 3, } } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index e863e4d1a221d..aa75cf7892783 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -4,9 +4,7 @@ using System; using System.Collections.Concurrent; using System.Collections.Generic; -using System.Runtime.CompilerServices; using System.Threading; -using System.Threading.Tasks; using Microsoft.Extensions.Internal; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; @@ -24,10 +22,10 @@ public class MemoryCache : IMemoryCache private readonly MemoryCacheOptions _options; private readonly ConcurrentDictionary _entries; + private readonly System.Timers.Timer _timer; private long _cacheSize; private bool _disposed; - private DateTimeOffset _lastExpirationScan; /// /// Creates a new instance. @@ -63,7 +61,9 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory _options.Clock = new SystemClock(); } - _lastExpirationScan = _options.Clock.UtcNow; + _timer = new System.Timers.Timer(Math.Max(1, _options.ExpirationScanFrequency.TotalMilliseconds)); + _timer.Elapsed += OnTimerElapsed; + _timer.Start(); } /// @@ -211,8 +211,6 @@ internal void SetEntry(CacheEntry entry) RemoveEntry(priorEntry); } } - - StartScanForExpiredItemsIfNeeded(utcNow); } /// @@ -221,39 +219,34 @@ public bool TryGetValue(object key, out object result) ValidateCacheKey(key); CheckDisposed(); - DateTimeOffset utcNow = _options.Clock.UtcNow; + if (!_entries.TryGetValue(key, out CacheEntry entry)) + { + result = null; + return false; + } - if (_entries.TryGetValue(key, out CacheEntry entry)) + if (entry.CanExpire) { - // 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) + DateTimeOffset utcNow = _options.Clock.UtcNow; + if (entry.EvictionReason != EvictionReason.Replaced && entry.CheckExpired(utcNow)) { - entry.LastAccessed = utcNow; - result = entry.Value; - - if (entry.CanPropagateOptions()) - { - // 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); - } - - StartScanForExpiredItemsIfNeeded(utcNow); + RemoveEntry(entry); - return true; + result = null; + return false; } - else + + entry.LastAccessed = utcNow; + if (entry.CanPropagateOptions()) { - // TODO: For efficiency queue this up for batch removal - RemoveEntry(entry); + // 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); } } - StartScanForExpiredItemsIfNeeded(utcNow); - - result = null; - return false; + result = entry.Value; + return true; } /// @@ -272,8 +265,6 @@ public void Remove(object key) entry.SetExpired(EvictionReason.Removed); entry.InvokeEvictionCallbacks(); } - - StartScanForExpiredItemsIfNeeded(_options.Clock.UtcNow); } private void RemoveEntry(CacheEntry entry) @@ -292,30 +283,13 @@ internal void EntryExpired(CacheEntry entry) { // TODO: For efficiency consider processing these expirations in batches. RemoveEntry(entry); - StartScanForExpiredItemsIfNeeded(_options.Clock.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) - { - if (_options.ExpirationScanFrequency < utcNow - _lastExpirationScan) - { - ScheduleTask(utcNow); - } - - void ScheduleTask(DateTimeOffset utcNow) - { - _lastExpirationScan = utcNow; - Task.Factory.StartNew(state => ScanForExpiredItems((MemoryCache)state), this, - CancellationToken.None, TaskCreationOptions.DenyChildAttach, TaskScheduler.Default); - } - } + private void OnTimerElapsed(object sender, System.Timers.ElapsedEventArgs e) => ScanForExpiredItems(this); private static void ScanForExpiredItems(MemoryCache cache) { - DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock.UtcNow; + DateTimeOffset now = cache._options.Clock.UtcNow; foreach (CacheEntry entry in cache._entries.Values) { if (entry.CheckExpired(now)) @@ -483,6 +457,10 @@ protected virtual void Dispose(bool disposing) GC.SuppressFinalize(this); } + _timer.Stop(); + _timer.Elapsed -= OnTimerElapsed; + _timer.Dispose(); + _disposed = true; } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs index 3ca46d0c024d6..e9336acd599e1 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CacheEntryScopeExpirationTests.cs @@ -8,23 +8,12 @@ using Microsoft.Extensions.Internal; using Xunit; +using static Microsoft.Extensions.Internal.CacheFactory; + namespace Microsoft.Extensions.Caching.Memory { public class CacheEntryScopeExpirationTests { - private IMemoryCache CreateCache() - { - return CreateCache(new SystemClock()); - } - - private IMemoryCache CreateCache(ISystemClock clock) - { - return new MemoryCache(new MemoryCacheOptions() - { - Clock = clock, - }); - } - [Fact] public void SetPopulates_ExpirationTokens_IntoScopedLink() { diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Infrastructure/CacheFactory.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Infrastructure/CacheFactory.cs new file mode 100644 index 0000000000000..e6d75f1b5a833 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Infrastructure/CacheFactory.cs @@ -0,0 +1,36 @@ +// 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 Microsoft.Extensions.Caching.Memory; + +namespace Microsoft.Extensions.Internal +{ + internal static class CacheFactory + { + internal static IMemoryCache CreateCache() + { + return CreateCache(new SystemClock()); + } + + internal static IMemoryCache CreateCache(ISystemClock clock) + { + return new MemoryCache(new MemoryCacheOptions() + { + Clock = clock, + }); + } + + internal static IMemoryCache CreateCache(ISystemClock clock, TimeSpan expirationScanFrequency) + { + var options = new MemoryCacheOptions() + { + Clock = clock, + ExpirationScanFrequency = expirationScanFrequency, + }; + + return new MemoryCache(options); + } + } +} diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TimeExpirationTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TimeExpirationTests.cs index 4a26ad05a9fd3..2e4a2e172fb3c 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TimeExpirationTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TimeExpirationTests.cs @@ -6,18 +6,12 @@ using Microsoft.Extensions.Internal; using Xunit; +using static Microsoft.Extensions.Internal.CacheFactory; + namespace Microsoft.Extensions.Caching.Memory { public class TimeExpirationTests { - private IMemoryCache CreateCache(ISystemClock clock) - { - return new MemoryCache(new MemoryCacheOptions() - { - Clock = clock, - }); - } - [Fact] public void AbsoluteExpirationInThePastDoesntAddEntry() { @@ -58,7 +52,8 @@ public void AbsoluteExpirationExpires() public void AbsoluteExpirationExpiresInBackground() { var clock = new TestClock(); - var cache = CreateCache(clock); + TimeSpan expirationScanFrequency = TimeSpan.FromMilliseconds(100); + var cache = CreateCache(clock, expirationScanFrequency); var key = "myKey"; var value = new object(); var callbackInvoked = new ManualResetEvent(false); @@ -79,8 +74,6 @@ public void AbsoluteExpirationExpiresInBackground() Assert.Same(value, result); clock.Add(TimeSpan.FromMinutes(2)); - var ignored = cache.Get("otherKey"); // Background expiration checks are triggered by misc cache activity. - Assert.True(callbackInvoked.WaitOne(TimeSpan.FromSeconds(30)), "Callback"); found = cache.TryGetValue(key, out result); diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TokenExpirationTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TokenExpirationTests.cs index 3da2515de8789..fa862f52cff72 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TokenExpirationTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/TokenExpirationTests.cs @@ -9,23 +9,12 @@ using Microsoft.Extensions.Primitives; using Xunit; +using static Microsoft.Extensions.Internal.CacheFactory; + namespace Microsoft.Extensions.Caching.Memory { public class TokenExpirationTests { - private IMemoryCache CreateCache() - { - return CreateCache(new SystemClock()); - } - - private IMemoryCache CreateCache(ISystemClock clock) - { - return new MemoryCache(new MemoryCacheOptions() - { - Clock = clock, - }); - } - [Fact] public void SetWithTokenRegistersForNotification() { @@ -114,7 +103,8 @@ public void ExpiredLazyTokenRemovesItemOnNextAccess() public void ExpiredLazyTokenRemovesItemInBackground() { var clock = new TestClock(); - var cache = CreateCache(clock); + TimeSpan expirationScanFrequency = TimeSpan.FromMilliseconds(100); + var cache = CreateCache(clock, expirationScanFrequency); string key = "myKey"; var value = new object(); var callbackInvoked = new ManualResetEvent(false); @@ -132,7 +122,6 @@ public void ExpiredLazyTokenRemovesItemInBackground() clock.Add(TimeSpan.FromMinutes(2)); expirationToken.HasChanged = true; - var ignored = cache.Get("otherKey"); // Background expiration checks are triggered by misc cache activity. Assert.True(callbackInvoked.WaitOne(TimeSpan.FromSeconds(30)), "Callback"); found = cache.TryGetValue(key, out value);