diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/ICacheEntry.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/ICacheEntry.cs index 02ff52ee5942a2..7ea2c8c8595b3b 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/ICacheEntry.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/ICacheEntry.cs @@ -9,6 +9,7 @@ namespace Microsoft.Extensions.Caching.Memory { /// /// Represents an entry in the implementation. + /// When Disposed, is committed to the cache. /// public interface ICacheEntry : IDisposable { diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 0bb304273877a0..88b4af2a9e9c8c 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -76,7 +76,12 @@ public MemoryCache(IOptions optionsAccessor, ILoggerFactory /// public int Count => _coherentState.Count; - // internal for testing + /// + /// Internal accessor for Size for testing only. + /// + /// Note that this is only eventually consistent with the contents of the collection. + /// See comment on . + /// internal long Size => _coherentState.Size; internal bool TrackLinkedCacheEntries { get; } @@ -421,6 +426,10 @@ private void ScanForExpiredItems() } } + /// + /// Returns true if increasing the cache size by the size of entry would + /// cause it to exceed any size limit on the cache, otherwise, returns false. + /// private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState coherentState) { long sizeLimit = _options.SizeLimitValue; @@ -613,6 +622,22 @@ private static void ValidateCacheKey(object key) ThrowHelper.ThrowIfNull(key); } + /// + /// Wrapper for the memory cache entries collection. + /// + /// Entries may have various sizes. If a size limit has been set, the cache keeps track of the aggregate of all the entries' sizes + /// in order to trigger compaction when the size limit is exceeded. + /// + /// For performance reasons, the size is not updated atomically with the collection, but is only made eventually consistent. + /// + /// When the memory cache is cleared, it replaces the backing collection entirely. This may occur in parallel with operations + /// like add, set, remove, and compact which may modify the collection and thus its overall size. + /// + /// To keep the overall size eventually consistent, therefore, the collection and the overall size are wrapped in this CoherentState + /// object. Individual operations take a local reference to this wrapper object while they work, and make size updates to this object. + /// Clearing the cache simply replaces the object, so that any still in progress updates do not affect the overall size value for + /// the new backing collection. + /// private sealed class CoherentState { internal ConcurrentDictionary _entries = new ConcurrentDictionary(); diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CapacityTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CapacityTests.cs index 79e09aaf54449c..8a0ba15ac8fcfd 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CapacityTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/CapacityTests.cs @@ -72,11 +72,11 @@ public void AddingEntryIncreasesCacheSizeWhenEnforcingSizeLimit() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 }); - Assert.Equal(5, cache.Size); + AssertCacheSize(5, cache); } [Fact] @@ -84,11 +84,11 @@ public void AddingEntryDoesNotIncreasesCacheSizeWhenNotEnforcingSizeLimit() { var cache = new MemoryCache(new MemoryCacheOptions()); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] @@ -96,21 +96,20 @@ public void DoNotAddEntryIfItExceedsCapacity() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 4 }); Assert.Equal("value", cache.Get("key")); - Assert.Equal(4, cache.Size); + AssertCacheSize(4, cache); cache.Set("key2", "value2", new MemoryCacheEntryOptions { Size = 7 }); Assert.Null(cache.Get("key2")); - Assert.Equal(4, cache.Size); + AssertCacheSize(4, cache); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public async Task DoNotAddIfSizeOverflows() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = long.MaxValue }); @@ -123,12 +122,12 @@ public async Task DoNotAddIfSizeOverflows() State = null }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", entryOptions); Assert.Equal("value", cache.Get("key")); - Assert.Equal(long.MaxValue, cache.Size); + AssertCacheSize(long.MaxValue, cache); cache.Set("key1", "value1", new MemoryCacheEntryOptions { Size = long.MaxValue }); // Do not add the new item @@ -139,11 +138,10 @@ public async Task DoNotAddIfSizeOverflows() // Compaction removes old item Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public async Task ExceedsCapacityCompacts() { var cache = new MemoryCache(new MemoryCacheOptions @@ -161,12 +159,12 @@ public async Task ExceedsCapacityCompacts() State = null }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", entryOptions); Assert.Equal("value", cache.Get("key")); - Assert.Equal(6, cache.Size); + AssertCacheSize(6, cache); cache.Set("key2", "value2", new MemoryCacheEntryOptions { Size = 5 }); @@ -175,7 +173,7 @@ public async Task ExceedsCapacityCompacts() Assert.Null(cache.Get("key")); Assert.Null(cache.Get("key2")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] @@ -183,17 +181,17 @@ public void AddingReplacementWithSizeIncreaseUpdates() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); Assert.Equal("value", cache.Get("key")); - Assert.Equal(2, cache.Size); + AssertCacheSize(2, cache); cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 3 }); Assert.Equal("value1", cache.Get("key")); - Assert.Equal(3, cache.Size); + AssertCacheSize(3, cache); } [Fact] @@ -201,17 +199,17 @@ public void AddingReplacementWithSizeDecreaseUpdates() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); Assert.Equal("value", cache.Get("key")); - Assert.Equal(2, cache.Size); + AssertCacheSize(2, cache); cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 1 }); Assert.Equal("value1", cache.Get("key")); - Assert.Equal(1, cache.Size); + AssertCacheSize(1, cache); } [Fact] @@ -223,21 +221,20 @@ public void AddingReplacementWhenTotalSizeExceedsCapacityDoesNotUpdateAndRemoves CompactionPercentage = 0.5 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 }); Assert.Equal("value", cache.Get("key")); - Assert.Equal(5, cache.Size); + AssertCacheSize(5, cache); cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 6 }); Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public async Task AddingReplacementWhenTotalSizeExceedsCapacityDoesNotUpdateRemovesOldEntryAndTriggersCompaction() { var cache = new MemoryCache(new MemoryCacheOptions @@ -254,12 +251,12 @@ public async Task AddingReplacementWhenTotalSizeExceedsCapacityDoesNotUpdateRemo State = null }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", entryOptions); Assert.Equal("value", cache.Get("key")); - Assert.Equal(6, cache.Size); + AssertCacheSize(6, cache); cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 5 }); @@ -267,7 +264,7 @@ public async Task AddingReplacementWhenTotalSizeExceedsCapacityDoesNotUpdateRemo Assert.True(await sem.WaitAsync(TimeSpan.FromSeconds(10))); Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] @@ -279,17 +276,18 @@ public void AddingReplacementExceedsCapacityRemovesOldEntry() CompactionPercentage = 0.5 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 6 }); Assert.Equal("value", cache.Get("key")); - Assert.Equal(6, cache.Size); + + AssertCacheSize(6, cache); cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 11 }); Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); // addition was rejected due to size, and previous item with the same key removed } [Fact] @@ -299,15 +297,14 @@ public void RemovingEntryDecreasesCacheSize() cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 }); - Assert.Equal(5, cache.Size); + AssertCacheSize(5, cache); cache.Remove("key"); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public async Task ExpiringEntryDecreasesCacheSize() { var cache = new MemoryCache(new MemoryCacheOptions @@ -328,7 +325,7 @@ public async Task ExpiringEntryDecreasesCacheSize() cache.Set("key", "value", entryOptions); - Assert.Equal(5, cache.Size); + AssertCacheSize(5, cache); // Expire entry changeToken.Fire(); @@ -339,7 +336,7 @@ public async Task ExpiringEntryDecreasesCacheSize() // Wait for compaction to complete Assert.True(await sem.WaitAsync(TimeSpan.FromSeconds(10))); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] @@ -355,9 +352,9 @@ public void TryingToAddExpiredEntryDoesNotIncreaseCacheSize() }; cache.Set("key", "value", entryOptions); - + Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] @@ -372,13 +369,12 @@ public void TryingToAddEntryWithExpiredTokenDoesNotIncreaseCacheSize() }; cache.Set("key", "value", entryOptions); - + Assert.Null(cache.Get("key")); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public async Task CompactsToLessThanLowWatermarkUsingLRUWhenHighWatermarkExceeded() { var testClock = new TestClock(); @@ -449,14 +445,23 @@ public void NoCompactionWhenNoMaximumEntriesCountSpecified() public void ClearZeroesTheSize() { var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 }); - Assert.Equal(5, cache.Size); + AssertCacheSize(5, cache); cache.Clear(); - Assert.Equal(0, cache.Size); + AssertCacheSize(0, cache); Assert.Equal(0, cache.Count); } + + internal static void AssertCacheSize(long size, MemoryCache cache) + { + // Size is only eventually consistent, so retry a few times + RetryHelper.Execute(() => + { + Assert.Equal(size, cache.Size); + }, maxAttempts: 12, (iteration) => (int)Math.Pow(2, iteration)); // 2ms, 4ms.. 4096 ms. In practice, retries are rarely needed. + } } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs index 4ac0e73ceca71b..761fe4c3ab4892 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheSetAndRemoveTests.cs @@ -543,7 +543,6 @@ public void SetGetAndRemoveWorksWithObjectKeysWhenDifferentReferences() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public void GetAndSet_AreThreadSafe_AndUpdatesNeverLeavesNullValues() { var cache = CreateCache(); @@ -597,7 +596,6 @@ public void GetAndSet_AreThreadSafe_AndUpdatesNeverLeavesNullValues() } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public void OvercapacityPurge_AreThreadSafe() { var cache = new MemoryCache(new MemoryCacheOptions @@ -657,13 +655,12 @@ public void OvercapacityPurge_AreThreadSafe() Assert.Equal(TaskStatus.RanToCompletion, task1.Status); Assert.Equal(TaskStatus.RanToCompletion, task2.Status); Assert.Equal(TaskStatus.RanToCompletion, task3.Status); - Assert.Equal(cache.Count, cache.Size); + CapacityTests.AssertCacheSize(cache.Count, cache); Assert.InRange(cache.Count, 0, 10); Assert.False(limitExceeded); } [Fact] - [ActiveIssue("https://github.com/dotnet/runtime/issues/33993")] public void AddAndReplaceEntries_AreThreadSafe() { var cache = new MemoryCache(new MemoryCacheOptions @@ -719,7 +716,7 @@ public void AddAndReplaceEntries_AreThreadSafe() cacheSize += cache.Get(i); } - Assert.Equal(cacheSize, cache.Size); + CapacityTests.AssertCacheSize(cacheSize, cache); Assert.InRange(cache.Count, 0, 20); }