From 68478c3eeed11c671a0f7e26b493eb46f412a9e7 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 11 Mar 2022 07:45:11 -0800 Subject: [PATCH 01/30] Adds API to retrieve caching metrics - Adds MemoryCacheStatistics struct and GetCurrentStatistics API - Ignore MatchingRefApiCompat failure on struct properties - Implement MemoryCacheStatistics props with init --- ...ng.Abstractions.Typeforwards.netcoreapp.cs | 6 +++ ...crosoft.Extensions.Caching.Abstractions.cs | 7 +++ ...oft.Extensions.Caching.Abstractions.csproj | 3 ++ .../src/MatchingRefApiCompatBaseline.txt | 4 ++ .../src/MemoryCacheStatistics.cs | 34 ++++++++++++++ ...ng.Abstractions.Typeforwards.netcoreapp.cs | 6 +++ ...oft.Extensions.Caching.Abstractions.csproj | 5 ++ .../Microsoft.Extensions.Caching.Memory.cs | 1 + .../src/MemoryCache.cs | 35 ++++++++++++++ .../tests/MemoryCacheHasStatisticsTests.cs | 46 +++++++++++++++++++ ...oft.Extensions.Caching.Memory.Tests.csproj | 3 +- 11 files changed, 149 insertions(+), 1 deletion(-) create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs create mode 100644 src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs new file mode 100644 index 00000000000000..fcc1284ccccdf7 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs @@ -0,0 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// The compiler emits a reference to the internal copy of this type in our non-NETCoreApp assembly +// so we must include a forward to be compatible with libraries compiled against non-NETCoreApp Microsoft.Extensions.Caching.Abstractions +[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.IsExternalInit))] \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs index 2805ac7644c9d0..b6152b739883a5 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs @@ -126,6 +126,13 @@ public MemoryCacheEntryOptions() { } public long? Size { get { throw null; } set { } } public System.TimeSpan? SlidingExpiration { get { throw null; } set { } } } + public partial struct MemoryCacheStatistics + { + public long CurrentEntryCount { get { throw null; } init { throw null; } } + public long? CurrentSize { get { throw null; } init { throw null; } } + public long TotalHits { get { throw null; } init { throw null; } } + public long TotalRequests { get { throw null; } init { throw null; } } + } public partial class PostEvictionCallbackRegistration { public PostEvictionCallbackRegistration() { } diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index 9d967d4f3adb20..30fdf3d4aea94a 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -6,6 +6,9 @@ + + diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt new file mode 100644 index 00000000000000..69b77ac1ff9326 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt @@ -0,0 +1,4 @@ +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentEntryCount.get()' in the implementation but not the reference. +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentSize.get()' in the implementation but not the reference. +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalHits.get()' in the implementation but not the reference. +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalRequests.get()' in the implementation but not the reference. \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs new file mode 100644 index 00000000000000..5bbc97c42637d1 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.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. + +using System; +using System.ComponentModel; + +namespace Microsoft.Extensions.Caching.Memory +{ + /// + /// Holds a snapshot of statistics for a memory cache. + /// + public struct MemoryCacheStatistics + { + /// + /// A snapshot of entry count at the current state + /// + public long CurrentEntryCount { get; init; } + + /// + /// A snapshot of size at the current state + /// + public long? CurrentSize { get; init; } + + /// + /// Total number of requests + /// + public long TotalRequests { get; init; } + + /// + /// Total number of hits + /// + public long TotalHits { get; init; } + } +} diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs new file mode 100644 index 00000000000000..b6510c56d117a2 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.Typeforwards.netcoreapp.cs @@ -0,0 +1,6 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// The compiler emits a reference to the internal copy of this type in our non-NETCoreApp assembly +// so we must include a forward to be compatible with libraries compiled against non-NETCoreApp Microsoft.Extensions.Caching.Abstractions +[assembly: System.Runtime.CompilerServices.TypeForwardedTo(typeof(System.Runtime.CompilerServices.IsExternalInit))] diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj index ddd5093c1436a0..31c12819ef7bdd 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj @@ -18,4 +18,9 @@ Microsoft.Extensions.Caching.Memory.IMemoryCache + + + + + diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs index e61a5b0be20ba9..8baef9ea0923c0 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs @@ -33,6 +33,7 @@ public void Compact(double percentage) { } public void Dispose() { } protected virtual void Dispose(bool disposing) { } ~MemoryCache() { } + public Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics GetCurrentStatistics() { throw null; } public void Remove(object key) { } public bool TryGetValue(object key, out object? result) { throw null; } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 7383ecf6324b4d..fae19ea9bf7ccf 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -27,6 +27,9 @@ public class MemoryCache : IMemoryCache private CoherentState _coherentState; private bool _disposed; + private readonly object _sync = new object(); + private long _totalHits; + private long _totalRequests; private DateTimeOffset _lastExpirationScan; /// @@ -196,6 +199,22 @@ internal void SetEntry(CacheEntry entry) /// public bool TryGetValue(object key!!, out object? result) + { + bool gotValue = TryGetValueInternal(key, out result); + lock (_sync) + { + _totalRequests++; + if (gotValue) + { + _totalHits++; + return true; + } + } + + return false; + } + + private bool TryGetValueInternal(object key!!, out object? result) { CheckDisposed(); @@ -270,6 +289,22 @@ public void Clear() } } + public MemoryCacheStatistics GetCurrentStatistics() + { + lock (_sync) + { + var currentStatistics = new MemoryCacheStatistics() + { + TotalRequests = _totalRequests, + TotalHits = _totalHits, + CurrentEntryCount = Count, + CurrentSize = Size + }; + return currentStatistics; + } + + } + internal void EntryExpired(CacheEntry entry) { // TODO: For efficiency consider processing these expirations in batches. diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs new file mode 100644 index 00000000000000..b1ba10502723f5 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs @@ -0,0 +1,46 @@ +// 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.Reflection; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Microsoft.Extensions.Caching.Memory +{ + public class MemoryCacheHasStatisticsTests + { + [Fact] + public void GetCurrentStatistics_Basic() + { + var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + + Assert.Equal(0, stats.CurrentSize); + Assert.Equal(0, stats.CurrentEntryCount); + Assert.Equal(0, stats.TotalRequests); + Assert.Equal(0, stats.TotalHits); + + cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); + Assert.Equal("value", cache.Get("key")); + stats = cache.GetCurrentStatistics(); + + Assert.Equal(2, stats.CurrentSize); + Assert.Equal(1, stats.CurrentEntryCount); + Assert.Equal(1, stats.TotalRequests); + Assert.Equal(1, stats.TotalHits); + + cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 3 }); + Assert.Equal("value1", cache.Get("key")); + stats = cache.GetCurrentStatistics(); + + Assert.Equal(3, stats.CurrentSize); + Assert.Equal(1, stats.CurrentEntryCount); + Assert.Equal(2, stats.TotalRequests); + Assert.Equal(2, stats.TotalHits); + } + + // TODO: add more tests + } +} diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Microsoft.Extensions.Caching.Memory.Tests.csproj b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Microsoft.Extensions.Caching.Memory.Tests.csproj index 9773db13ad7711..429cde4c0db438 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Microsoft.Extensions.Caching.Memory.Tests.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/Microsoft.Extensions.Caching.Memory.Tests.csproj @@ -11,7 +11,8 @@ - + + From 1c4d8b56b55b028f8516f0db4022fac564d27b69 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 14 Mar 2022 22:31:29 -0700 Subject: [PATCH 02/30] Make CurrentSize and CurrentEntryCount return coherent values --- .../src/MemoryCache.cs | 53 +++++++++-- .../tests/MemoryCacheHasStatisticsTests.cs | 95 +++++++++++++++++-- 2 files changed, 129 insertions(+), 19 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index fae19ea9bf7ccf..5ad1d8f47eb150 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -151,7 +151,11 @@ internal void SetEntry(CacheEntry entry) if (_options.SizeLimit.HasValue) { // The prior entry was removed, decrease the by the prior entry's size - Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size!.Value); + lock (coherentState._sync) + { + coherentState._cacheCount--; + coherentState._cacheSize -= priorEntry.Size!.Value; + } } } else @@ -172,7 +176,11 @@ internal void SetEntry(CacheEntry entry) if (_options.SizeLimit.HasValue) { // Entry could not be added, reset cache size - Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); + lock (coherentState._sync) + { + coherentState._cacheCount--; + coherentState._cacheSize -= entry.Size!.Value; + } } entry.SetExpired(EvictionReason.Replaced); entry.InvokeEvictionCallbacks(); @@ -264,7 +272,11 @@ public void Remove(object key!!) { if (_options.SizeLimit.HasValue) { - Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); + lock (coherentState._sync) + { + coherentState._cacheCount--; + coherentState._cacheSize -= entry.Size!.Value; + } } entry.SetExpired(EvictionReason.Removed); @@ -293,14 +305,27 @@ public MemoryCacheStatistics GetCurrentStatistics() { lock (_sync) { - var currentStatistics = new MemoryCacheStatistics() + if (_options.SizeLimit.HasValue) + { + lock (_coherentState._sync) + { + return new MemoryCacheStatistics() + { + TotalRequests = _totalRequests, + TotalHits = _totalHits, + CurrentEntryCount = _coherentState._cacheCount, + CurrentSize = Size + }; + } + } + + return new MemoryCacheStatistics() { TotalRequests = _totalRequests, TotalHits = _totalHits, CurrentEntryCount = Count, - CurrentSize = Size + CurrentSize = null }; - return currentStatistics; } } @@ -364,9 +389,13 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState cohe return true; } - if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead)) + lock (coherentState._sync) { - return false; + if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead)) + { + coherentState._cacheCount++; + return false; + } } } @@ -530,6 +559,8 @@ private sealed class CoherentState { internal ConcurrentDictionary _entries = new ConcurrentDictionary(); internal long _cacheSize; + internal long _cacheCount; + internal readonly object _sync = new object(); private ICollection> EntriesCollection => _entries; @@ -543,7 +574,11 @@ internal void RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { if (options.SizeLimit.HasValue) { - Interlocked.Add(ref _cacheSize, -entry.Size!.Value); + lock (_sync) + { + _cacheCount--; + _cacheSize -= entry.Size!.Value; + } } entry.InvokeEvictionCallbacks(); } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs index b1ba10502723f5..a9e7e8c1e843d5 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs @@ -5,42 +5,117 @@ using System.Reflection; using System.Threading; using System.Threading.Tasks; +using Microsoft.Extensions.Internal; +using Microsoft.Extensions.Caching.Memory.Infrastructure; using Xunit; namespace Microsoft.Extensions.Caching.Memory { public class MemoryCacheHasStatisticsTests { - [Fact] - public void GetCurrentStatistics_Basic() + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetCurrentStatistics_Basic(bool sizeLimitIsSet) { - var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }); + var cache = sizeLimitIsSet ? + new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : + new MemoryCache(new MemoryCacheOptions { }); + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); - Assert.Equal(0, stats.CurrentSize); Assert.Equal(0, stats.CurrentEntryCount); Assert.Equal(0, stats.TotalRequests); Assert.Equal(0, stats.TotalHits); + VerifyCurrentSize(0, sizeLimitIsSet, stats); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) + { + var cache = sizeLimitIsSet ? + new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : + new MemoryCache(new MemoryCacheOptions { }); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); Assert.Equal("value", cache.Get("key")); - stats = cache.GetCurrentStatistics(); + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); - Assert.Equal(2, stats.CurrentSize); Assert.Equal(1, stats.CurrentEntryCount); Assert.Equal(1, stats.TotalRequests); Assert.Equal(1, stats.TotalHits); + VerifyCurrentSize(2, sizeLimitIsSet, stats); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetCurrentStatistics_UpdateExistingCache_UpdatesStatistics(bool sizeLimitIsSet) + { + var cache = sizeLimitIsSet ? + new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : + new MemoryCache(new MemoryCacheOptions { }); + + cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); + Assert.Equal("value", cache.Get("key")); - cache.Set("key", "value1", new MemoryCacheEntryOptions { Size = 3 }); - Assert.Equal("value1", cache.Get("key")); - stats = cache.GetCurrentStatistics(); + cache.Set("key", "updated value", new MemoryCacheEntryOptions { Size = 3 }); + Assert.Equal("updated value", cache.Get("key")); + + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); - Assert.Equal(3, stats.CurrentSize); Assert.Equal(1, stats.CurrentEntryCount); Assert.Equal(2, stats.TotalRequests); Assert.Equal(2, stats.TotalHits); + VerifyCurrentSize(3, sizeLimitIsSet, stats); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentSizeResets(bool sizeLimitIsSet) + { + const string Key = "myKey"; + + var cache = new MemoryCache(sizeLimitIsSet ? + new MemoryCacheOptions { Clock = new SystemClock(), SizeLimit = 10 } : + new MemoryCacheOptions { Clock = new SystemClock() } + ); + + ICacheEntry entry; + using (entry = cache.CreateEntry(Key)) + { + var expirationToken = new TestExpirationToken() { ActiveChangeCallbacks = true }; + var mc = new MemoryCacheEntryOptions { Size = 5 }; + cache.Set(Key, new object(), mc.AddExpirationToken(expirationToken)); + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + Assert.Equal(1, cache.Count); + Assert.Equal(1, stats.CurrentEntryCount); + VerifyCurrentSize(5, sizeLimitIsSet, stats); + + expirationToken.HasChanged = true; + cache.Set(Key, new object(), mc.AddExpirationToken(expirationToken)); + stats = cache.GetCurrentStatistics(); + Assert.Equal(0, cache.Count); + Assert.Equal(0, stats.CurrentEntryCount); + VerifyCurrentSize(0, sizeLimitIsSet, stats); + } } // TODO: add more tests + + private void VerifyCurrentSize(long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) + { + if (sizeLimitIsSet) + { + Assert.Equal(expected, stats.CurrentSize); + } + else + { + Assert.Null(stats.CurrentSize); + } + } } } From 2a4743a9602d7a3bfacfffd69dbea7f99b6f1a72 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Mar 2022 08:13:47 -0700 Subject: [PATCH 03/30] Add test --- .../tests/MemoryCacheHasStatisticsTests.cs | 82 ++++++++++++++----- 1 file changed, 61 insertions(+), 21 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs index a9e7e8c1e843d5..98dd3a65ea2541 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs @@ -13,23 +13,6 @@ namespace Microsoft.Extensions.Caching.Memory { public class MemoryCacheHasStatisticsTests { - [Theory] - [InlineData(true)] - [InlineData(false)] - public void GetCurrentStatistics_Basic(bool sizeLimitIsSet) - { - var cache = sizeLimitIsSet ? - new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : - new MemoryCache(new MemoryCacheOptions { }); - - MemoryCacheStatistics stats = cache.GetCurrentStatistics(); - - Assert.Equal(0, stats.CurrentEntryCount); - Assert.Equal(0, stats.TotalRequests); - Assert.Equal(0, stats.TotalHits); - VerifyCurrentSize(0, sizeLimitIsSet, stats); - } - [Theory] [InlineData(true)] [InlineData(false)] @@ -40,12 +23,18 @@ public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) new MemoryCache(new MemoryCacheOptions { }); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); - Assert.Equal("value", cache.Get("key")); + for (int i = 0; i < 100; i++) + { + Assert.Equal("value", cache.Get("key")); + Assert.Null(cache.Get("missingKey1")); + Assert.Null(cache.Get("missingKey2")); + } + MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + Assert.Equal(300, stats.TotalRequests); + Assert.Equal(100, stats.TotalHits); Assert.Equal(1, stats.CurrentEntryCount); - Assert.Equal(1, stats.TotalRequests); - Assert.Equal(1, stats.TotalHits); VerifyCurrentSize(2, sizeLimitIsSet, stats); } @@ -104,7 +93,58 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentSizeReset } } - // TODO: add more tests + [Theory] + [InlineData(true)] + [InlineData(false)] + public void GetCurrentStatistics_EntrySizesAreOne_MultithreadedCacheUpdates_CountAndSizeRemainInSync(bool sizeLimitIsSet) + { + const int numEntries = 100; + Random random = new Random(); + + var cache = new MemoryCache(sizeLimitIsSet ? + new MemoryCacheOptions { Clock = new SystemClock(), SizeLimit = 2000 } : + new MemoryCacheOptions { Clock = new SystemClock() } + ); + + void FillCache() + { + for (int i = 0; i < numEntries; i++) + { + cache.Set($"key{i}", $"value{i}", + new MemoryCacheEntryOptions { Size = 1 } + .AddExpirationToken(new TestExpirationToken() + { ActiveChangeCallbacks = true })); + } + } + + // start a few tasks to access entries in the background + Task[] backgroundAccessTasks = new Task[Environment.ProcessorCount]; + bool done = false; + + for (int i = 0; i < backgroundAccessTasks.Length; i++) + { + backgroundAccessTasks[i] = Task.Run(async () => + { + while (!done) + { + cache.TryGetValue($"key{random.Next(numEntries)}", out _); + var stats = cache.GetCurrentStatistics(); + VerifyCurrentSize(stats.CurrentEntryCount, sizeLimitIsSet, stats); + await Task.Yield(); + } + }); + } + + for (int i = 0; i < 1000; i++) + { + FillCache(); + cache.Compact(1); + } + + done = true; + + Task.WaitAll(backgroundAccessTasks); + } private void VerifyCurrentSize(long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) { From 92c187b9725e5e9fd044cc64d0b9e90de629adb6 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Mar 2022 15:03:30 -0700 Subject: [PATCH 04/30] Apply API review feedback - TODO: DIM support --- ...crosoft.Extensions.Caching.Abstractions.cs | 7 +-- .../src/MatchingRefApiCompatBaseline.txt | 4 +- .../src/MemoryCacheStatistics.cs | 15 ++++-- .../src/MemoryCache.cs | 51 ++++--------------- .../tests/MemoryCacheHasStatisticsTests.cs | 51 +++++++++++++------ 5 files changed, 61 insertions(+), 67 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs index b6152b739883a5..e072ab9d407127 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs @@ -126,12 +126,13 @@ public MemoryCacheEntryOptions() { } public long? Size { get { throw null; } set { } } public System.TimeSpan? SlidingExpiration { get { throw null; } set { } } } - public partial struct MemoryCacheStatistics + public partial class MemoryCacheStatistics { + public MemoryCacheStatistics() { } public long CurrentEntryCount { get { throw null; } init { throw null; } } - public long? CurrentSize { get { throw null; } init { throw null; } } + public long? CurrentEstimatedSize { get { throw null; } init { throw null; } } public long TotalHits { get { throw null; } init { throw null; } } - public long TotalRequests { get { throw null; } init { throw null; } } + public long TotalMisses { get { throw null; } init { throw null; } } } public partial class PostEvictionCallbackRegistration { diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt index 69b77ac1ff9326..2a8c70e7743cd9 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt @@ -1,4 +1,4 @@ CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentEntryCount.get()' in the implementation but not the reference. -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentSize.get()' in the implementation but not the reference. +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentEstimatedSize.get()' in the implementation but not the reference. CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalHits.get()' in the implementation but not the reference. -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalRequests.get()' in the implementation but not the reference. \ No newline at end of file +CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalMisses.get()' in the implementation but not the reference. \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs index 5bbc97c42637d1..bac243ecb899a6 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs @@ -9,8 +9,13 @@ namespace Microsoft.Extensions.Caching.Memory /// /// Holds a snapshot of statistics for a memory cache. /// - public struct MemoryCacheStatistics + public class MemoryCacheStatistics { + /// + /// Initializes an instance of MemoryCacheStatistics. + /// + public MemoryCacheStatistics() { } + /// /// A snapshot of entry count at the current state /// @@ -19,15 +24,15 @@ public struct MemoryCacheStatistics /// /// A snapshot of size at the current state /// - public long? CurrentSize { get; init; } + public long? CurrentEstimatedSize { get; init; } /// - /// Total number of requests + /// Total number of cache misses /// - public long TotalRequests { get; init; } + public long TotalMisses { get; init; } /// - /// Total number of hits + /// Total number of cache hits /// public long TotalHits { get; init; } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 5ad1d8f47eb150..61e29b015712d5 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -27,9 +27,8 @@ public class MemoryCache : IMemoryCache private CoherentState _coherentState; private bool _disposed; - private readonly object _sync = new object(); private long _totalHits; - private long _totalRequests; + private long _totalMisses; private DateTimeOffset _lastExpirationScan; /// @@ -207,22 +206,6 @@ internal void SetEntry(CacheEntry entry) /// public bool TryGetValue(object key!!, out object? result) - { - bool gotValue = TryGetValueInternal(key, out result); - lock (_sync) - { - _totalRequests++; - if (gotValue) - { - _totalHits++; - return true; - } - } - - return false; - } - - private bool TryGetValueInternal(object key!!, out object? result) { CheckDisposed(); @@ -247,6 +230,7 @@ private bool TryGetValueInternal(object key!!, out object? result) StartScanForExpiredItemsIfNeeded(utcNow); + Interlocked.Increment(ref _totalHits); return true; } else @@ -259,6 +243,7 @@ private bool TryGetValueInternal(object key!!, out object? result) StartScanForExpiredItemsIfNeeded(utcNow); result = null; + Interlocked.Increment(ref _totalMisses); return false; } @@ -303,31 +288,13 @@ public void Clear() public MemoryCacheStatistics GetCurrentStatistics() { - lock (_sync) + return new MemoryCacheStatistics() { - if (_options.SizeLimit.HasValue) - { - lock (_coherentState._sync) - { - return new MemoryCacheStatistics() - { - TotalRequests = _totalRequests, - TotalHits = _totalHits, - CurrentEntryCount = _coherentState._cacheCount, - CurrentSize = Size - }; - } - } - - return new MemoryCacheStatistics() - { - TotalRequests = _totalRequests, - TotalHits = _totalHits, - CurrentEntryCount = Count, - CurrentSize = null - }; - } - + TotalMisses = _totalMisses, + TotalHits = _totalHits, + CurrentEntryCount = Count, + CurrentEstimatedSize = _options.SizeLimit.HasValue ? Size : null + }; } internal void EntryExpired(CacheEntry entry) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs index 98dd3a65ea2541..27b15773cc7df4 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs @@ -32,10 +32,10 @@ public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) MemoryCacheStatistics stats = cache.GetCurrentStatistics(); - Assert.Equal(300, stats.TotalRequests); + Assert.Equal(200, stats.TotalMisses); Assert.Equal(100, stats.TotalHits); - Assert.Equal(1, stats.CurrentEntryCount); - VerifyCurrentSize(2, sizeLimitIsSet, stats); + // Assert.Equal(1, stats.CurrentEntryCount); + VerifyCurrentEstimatedSize(2, sizeLimitIsSet, stats); } [Theory] @@ -56,15 +56,15 @@ public void GetCurrentStatistics_UpdateExistingCache_UpdatesStatistics(bool size MemoryCacheStatistics stats = cache.GetCurrentStatistics(); Assert.Equal(1, stats.CurrentEntryCount); - Assert.Equal(2, stats.TotalRequests); + Assert.Equal(0, stats.TotalMisses); Assert.Equal(2, stats.TotalHits); - VerifyCurrentSize(3, sizeLimitIsSet, stats); + VerifyCurrentEstimatedSize(3, sizeLimitIsSet, stats); } [Theory] [InlineData(true)] [InlineData(false)] - public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentSizeResets(bool sizeLimitIsSet) + public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimatedSizeResets(bool sizeLimitIsSet) { const string Key = "myKey"; @@ -82,14 +82,14 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentSizeReset MemoryCacheStatistics stats = cache.GetCurrentStatistics(); Assert.Equal(1, cache.Count); Assert.Equal(1, stats.CurrentEntryCount); - VerifyCurrentSize(5, sizeLimitIsSet, stats); + VerifyCurrentEstimatedSize(5, sizeLimitIsSet, stats); expirationToken.HasChanged = true; cache.Set(Key, new object(), mc.AddExpirationToken(expirationToken)); stats = cache.GetCurrentStatistics(); Assert.Equal(0, cache.Count); Assert.Equal(0, stats.CurrentEntryCount); - VerifyCurrentSize(0, sizeLimitIsSet, stats); + VerifyCurrentEstimatedSize(0, sizeLimitIsSet, stats); } } @@ -98,6 +98,7 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentSizeReset [InlineData(false)] public void GetCurrentStatistics_EntrySizesAreOne_MultithreadedCacheUpdates_CountAndSizeRemainInSync(bool sizeLimitIsSet) { + bool statsGetInaccurateDuringHeavyLoad = false; const int numEntries = 100; Random random = new Random(); @@ -110,10 +111,16 @@ void FillCache() { for (int i = 0; i < numEntries; i++) { + var expirationToken = new TestExpirationToken() { ActiveChangeCallbacks = true }; cache.Set($"key{i}", $"value{i}", new MemoryCacheEntryOptions { Size = 1 } - .AddExpirationToken(new TestExpirationToken() - { ActiveChangeCallbacks = true })); + .AddExpirationToken(expirationToken)); + + if (random.Next(numEntries) < numEntries * 0.25) + { + // Set to expired 25% of the time + expirationToken.HasChanged = true; + } } } @@ -129,7 +136,14 @@ void FillCache() { cache.TryGetValue($"key{random.Next(numEntries)}", out _); var stats = cache.GetCurrentStatistics(); - VerifyCurrentSize(stats.CurrentEntryCount, sizeLimitIsSet, stats); + + // set flag when stats go out of sync + if ((stats.CurrentEntryCount != cache.Count) + || (sizeLimitIsSet && stats.CurrentEstimatedSize != stats.CurrentEntryCount)) + { + statsGetInaccurateDuringHeavyLoad = true; + } + await Task.Yield(); } }); @@ -137,24 +151,31 @@ void FillCache() for (int i = 0; i < 1000; i++) { - FillCache(); cache.Compact(1); + FillCache(); } done = true; Task.WaitAll(backgroundAccessTasks); + + // even if the values are not exact during heavy multithreaded operations, + // once done, the count and size eventually become fixed to expected value + var finalStats = cache.GetCurrentStatistics(); + Assert.True(statsGetInaccurateDuringHeavyLoad); + Assert.Equal(cache.Count, finalStats.CurrentEntryCount); + VerifyCurrentEstimatedSize(finalStats.CurrentEntryCount, sizeLimitIsSet, finalStats); } - private void VerifyCurrentSize(long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) + private void VerifyCurrentEstimatedSize (long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) { if (sizeLimitIsSet) { - Assert.Equal(expected, stats.CurrentSize); + Assert.Equal(expected, stats.CurrentEstimatedSize ); } else { - Assert.Null(stats.CurrentSize); + Assert.Null(stats.CurrentEstimatedSize ); } } } From f18b4cf22f3d0a90b0a7f4b7f9345ae4e6e180ce Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Mar 2022 15:17:17 -0700 Subject: [PATCH 05/30] Remove MatchingRefApiCompatBaseline, leftover from init on struct --- .../src/MatchingRefApiCompatBaseline.txt | 4 ---- 1 file changed, 4 deletions(-) delete mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt deleted file mode 100644 index 2a8c70e7743cd9..00000000000000 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MatchingRefApiCompatBaseline.txt +++ /dev/null @@ -1,4 +0,0 @@ -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentEntryCount.get()' in the implementation but not the reference. -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.CurrentEstimatedSize.get()' in the implementation but not the reference. -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalHits.get()' in the implementation but not the reference. -CannotRemoveAttribute : Attribute 'System.Runtime.CompilerServices.IsReadOnlyAttribute' exists on 'Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics.TotalMisses.get()' in the implementation but not the reference. \ No newline at end of file From 8f3c13c4d3912731456ffebfdbb5ea43a17d8de0 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 15 Mar 2022 17:24:00 -0700 Subject: [PATCH 06/30] Undo CoherentState._cacheSize changes and add DIM --- ...crosoft.Extensions.Caching.Abstractions.cs | 6 ---- ...oft.Extensions.Caching.Abstractions.csproj | 2 ++ ...ft.Extensions.Caching.Abstractions.net7.cs | 17 ++++++++++ ...t.Extensions.Caching.Abstractions.other.cs | 15 ++++++++ .../src/IMemoryCache.cs | 8 +++++ .../src/MemoryCache.cs | 34 ++++--------------- ...> MemoryCacheGetCurrentStatisticsTests.cs} | 2 +- 7 files changed, 49 insertions(+), 35 deletions(-) create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs create mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs rename src/libraries/Microsoft.Extensions.Caching.Memory/tests/{MemoryCacheHasStatisticsTests.cs => MemoryCacheGetCurrentStatisticsTests.cs} (99%) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs index e072ab9d407127..9954e137561ac6 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs @@ -98,12 +98,6 @@ public partial interface ICacheEntry : System.IDisposable System.TimeSpan? SlidingExpiration { get; set; } object? Value { get; set; } } - public partial interface IMemoryCache : System.IDisposable - { - Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); - void Remove(object key); - bool TryGetValue(object key, out object? value); - } public static partial class MemoryCacheEntryExtensions { public static Microsoft.Extensions.Caching.Memory.MemoryCacheEntryOptions AddExpirationToken(this Microsoft.Extensions.Caching.Memory.MemoryCacheEntryOptions options, Microsoft.Extensions.Primitives.IChangeToken expirationToken) { throw null; } diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index 30fdf3d4aea94a..f97a90ee39de4b 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -6,6 +6,8 @@ + + diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs new file mode 100644 index 00000000000000..2e25e688f32d1a --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// ------------------------------------------------------------------------------ +// Changes to this file must follow the https://aka.ms/api-review process. +// ------------------------------------------------------------------------------ + +namespace Microsoft.Extensions.Caching.Memory +{ + public partial interface IMemoryCache : System.IDisposable + { + Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); + void Remove(object key); + bool TryGetValue(object key, out object? value); + MemoryCacheStatistics? GetCurrentStatistics() => null; + + } +} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs new file mode 100644 index 00000000000000..84ab455803cca0 --- /dev/null +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs @@ -0,0 +1,15 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// ------------------------------------------------------------------------------ +// Changes to this file must follow the https://aka.ms/api-review process. +// ------------------------------------------------------------------------------ + +namespace Microsoft.Extensions.Caching.Memory +{ + public partial interface IMemoryCache : System.IDisposable + { + Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); + void Remove(object key); + bool TryGetValue(object key, out object? value); + } +} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs index 88576eba04c74e..c41a6fff29cb53 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs @@ -30,5 +30,13 @@ public interface IMemoryCache : IDisposable /// /// An object identifying the entry. void Remove(object key); + +#if NET7_0_OR_GREATER + /// + /// Gets a snapshot of the cache statistics if available. + /// + /// An instance of the instance. + MemoryCacheStatistics? GetCurrentStatistics() => null; +#endif } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 61e29b015712d5..65fc9194d1a104 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -150,11 +150,7 @@ internal void SetEntry(CacheEntry entry) if (_options.SizeLimit.HasValue) { // The prior entry was removed, decrease the by the prior entry's size - lock (coherentState._sync) - { - coherentState._cacheCount--; - coherentState._cacheSize -= priorEntry.Size!.Value; - } + Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size!.Value); } } else @@ -175,11 +171,7 @@ internal void SetEntry(CacheEntry entry) if (_options.SizeLimit.HasValue) { // Entry could not be added, reset cache size - lock (coherentState._sync) - { - coherentState._cacheCount--; - coherentState._cacheSize -= entry.Size!.Value; - } + Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); } entry.SetExpired(EvictionReason.Replaced); entry.InvokeEvictionCallbacks(); @@ -257,11 +249,7 @@ public void Remove(object key!!) { if (_options.SizeLimit.HasValue) { - lock (coherentState._sync) - { - coherentState._cacheCount--; - coherentState._cacheSize -= entry.Size!.Value; - } + Interlocked.Add(ref coherentState._cacheSize, -entry.Size!.Value); } entry.SetExpired(EvictionReason.Removed); @@ -356,13 +344,9 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState cohe return true; } - lock (coherentState._sync) + if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead)) { - if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead)) - { - coherentState._cacheCount++; - return false; - } + return false; } } @@ -526,8 +510,6 @@ private sealed class CoherentState { internal ConcurrentDictionary _entries = new ConcurrentDictionary(); internal long _cacheSize; - internal long _cacheCount; - internal readonly object _sync = new object(); private ICollection> EntriesCollection => _entries; @@ -541,11 +523,7 @@ internal void RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { if (options.SizeLimit.HasValue) { - lock (_sync) - { - _cacheCount--; - _cacheSize -= entry.Size!.Value; - } + Interlocked.Add(ref _cacheSize, -entry.Size!.Value); } entry.InvokeEvictionCallbacks(); } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs similarity index 99% rename from src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs rename to src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index 27b15773cc7df4..7889f91c3aab05 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheHasStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -34,7 +34,7 @@ public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) Assert.Equal(200, stats.TotalMisses); Assert.Equal(100, stats.TotalHits); - // Assert.Equal(1, stats.CurrentEntryCount); + Assert.Equal(1, stats.CurrentEntryCount); VerifyCurrentEstimatedSize(2, sizeLimitIsSet, stats); } From d36af3f6b6841b7927fb30d93ca448a6e4381ccb Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 16 Mar 2022 11:16:19 -0700 Subject: [PATCH 07/30] Add DIM test and fix up csproj condition --- ...icrosoft.Extensions.Caching.Abstractions.cs | 6 ++++++ ...soft.Extensions.Caching.Abstractions.csproj | 3 +-- ...oft.Extensions.Caching.Abstractions.net7.cs | 4 ---- ...ft.Extensions.Caching.Abstractions.other.cs | 15 --------------- .../MemoryCacheGetCurrentStatisticsTests.cs | 18 +++++++++++++++++- 5 files changed, 24 insertions(+), 22 deletions(-) delete mode 100644 src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs index 9954e137561ac6..e072ab9d407127 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs @@ -98,6 +98,12 @@ public partial interface ICacheEntry : System.IDisposable System.TimeSpan? SlidingExpiration { get; set; } object? Value { get; set; } } + public partial interface IMemoryCache : System.IDisposable + { + Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); + void Remove(object key); + bool TryGetValue(object key, out object? value); + } public static partial class MemoryCacheEntryExtensions { public static Microsoft.Extensions.Caching.Memory.MemoryCacheEntryOptions AddExpirationToken(this Microsoft.Extensions.Caching.Memory.MemoryCacheEntryOptions options, Microsoft.Extensions.Primitives.IChangeToken expirationToken) { throw null; } diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index f97a90ee39de4b..a4ff56880fb4a8 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -6,8 +6,7 @@ - - + diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs index 2e25e688f32d1a..d3812319a7e19b 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs @@ -8,10 +8,6 @@ namespace Microsoft.Extensions.Caching.Memory { public partial interface IMemoryCache : System.IDisposable { - Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); - void Remove(object key); - bool TryGetValue(object key, out object? value); MemoryCacheStatistics? GetCurrentStatistics() => null; - } } \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs deleted file mode 100644 index 84ab455803cca0..00000000000000 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.other.cs +++ /dev/null @@ -1,15 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// ------------------------------------------------------------------------------ -// Changes to this file must follow the https://aka.ms/api-review process. -// ------------------------------------------------------------------------------ - -namespace Microsoft.Extensions.Caching.Memory -{ - public partial interface IMemoryCache : System.IDisposable - { - Microsoft.Extensions.Caching.Memory.ICacheEntry CreateEntry(object key); - void Remove(object key); - bool TryGetValue(object key, out object? value); - } -} \ No newline at end of file diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index 7889f91c3aab05..e7aabd0060862f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -167,7 +167,23 @@ void FillCache() VerifyCurrentEstimatedSize(finalStats.CurrentEntryCount, sizeLimitIsSet, finalStats); } - private void VerifyCurrentEstimatedSize (long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) + [Fact] + public void GetCurrentStatistics_DIMReturnsNull() + { +#if NET7_0_OR_GREATER + Assert.Null((new FakeMemoryCache() as IMemoryCache).GetCurrentStatistics()); +#endif + } + + private class FakeMemoryCache : IMemoryCache + { + public ICacheEntry CreateEntry(object key) => throw new NotImplementedException(); + public void Dispose() => throw new NotImplementedException(); + public void Remove(object key) => throw new NotImplementedException(); + public bool TryGetValue(object key, out object? value) => throw new NotImplementedException(); + } + + private void VerifyCurrentEstimatedSize(long expected, bool sizeLimitIsSet, MemoryCacheStatistics stats) { if (sizeLimitIsSet) { From d013412d1127b22e71917b9ca12208bc49ab0467 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 16 Mar 2022 13:29:42 -0700 Subject: [PATCH 08/30] CommonPath -> CoreLibSharedDir --- .../src/Microsoft.Extensions.Caching.Abstractions.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj index 31c12819ef7bdd..25d77c9de8c998 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/Microsoft.Extensions.Caching.Abstractions.csproj @@ -20,7 +20,7 @@ Microsoft.Extensions.Caching.Memory.IMemoryCache - + From da10ccd6a6fb293b0195b52dba758d946f67a5a7 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 16 Mar 2022 13:39:46 -0700 Subject: [PATCH 09/30] ref fix up --- .../ref/Microsoft.Extensions.Caching.Abstractions.cs | 8 ++++---- .../ref/Microsoft.Extensions.Caching.Abstractions.csproj | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs index e072ab9d407127..3bda5a0a27b009 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.cs @@ -129,10 +129,10 @@ public MemoryCacheEntryOptions() { } public partial class MemoryCacheStatistics { public MemoryCacheStatistics() { } - public long CurrentEntryCount { get { throw null; } init { throw null; } } - public long? CurrentEstimatedSize { get { throw null; } init { throw null; } } - public long TotalHits { get { throw null; } init { throw null; } } - public long TotalMisses { get { throw null; } init { throw null; } } + public long CurrentEntryCount { get { throw null; } init { } } + public long? CurrentEstimatedSize { get { throw null; } init { } } + public long TotalHits { get { throw null; } init { } } + public long TotalMisses { get { throw null; } init { } } } public partial class PostEvictionCallbackRegistration { diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index a4ff56880fb4a8..06e3f06d8e09bc 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -8,7 +8,7 @@ - From 86db1b35969b57e5423e21e0fb6dbf00ae3e4a44 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 16 Mar 2022 16:21:55 -0700 Subject: [PATCH 10/30] Applies PR feedback --- .../src/MemoryCacheStatistics.cs | 10 +-- .../src/MemoryCache.cs | 3 + .../MemoryCacheGetCurrentStatisticsTests.cs | 78 +------------------ 3 files changed, 10 insertions(+), 81 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs index bac243ecb899a6..52f0757bc3472e 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System; -using System.ComponentModel; namespace Microsoft.Extensions.Caching.Memory { @@ -17,22 +16,23 @@ public class MemoryCacheStatistics public MemoryCacheStatistics() { } /// - /// A snapshot of entry count at the current state + /// Gets the number of instances currently in the memory cache. /// public long CurrentEntryCount { get; init; } /// - /// A snapshot of size at the current state + /// Gets an estimated sum of all the values currently in the memory cache. /// + /// Returns if no size limit is set for the cache. public long? CurrentEstimatedSize { get; init; } /// - /// Total number of cache misses + /// Gets the total number of cache hits. /// public long TotalMisses { get; init; } /// - /// Total number of cache hits + /// Gets the total number of cache hits. /// public long TotalHits { get; init; } } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 65fc9194d1a104..c707a55279f4ba 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -274,6 +274,9 @@ public void Clear() } } + /// + /// Gets a snapshot of the current statistics for the memory cache. + /// public MemoryCacheStatistics GetCurrentStatistics() { return new MemoryCacheStatistics() diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index e7aabd0060862f..aff69b78e8de3f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -93,87 +93,13 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimated } } - [Theory] - [InlineData(true)] - [InlineData(false)] - public void GetCurrentStatistics_EntrySizesAreOne_MultithreadedCacheUpdates_CountAndSizeRemainInSync(bool sizeLimitIsSet) - { - bool statsGetInaccurateDuringHeavyLoad = false; - const int numEntries = 100; - Random random = new Random(); - - var cache = new MemoryCache(sizeLimitIsSet ? - new MemoryCacheOptions { Clock = new SystemClock(), SizeLimit = 2000 } : - new MemoryCacheOptions { Clock = new SystemClock() } - ); - - void FillCache() - { - for (int i = 0; i < numEntries; i++) - { - var expirationToken = new TestExpirationToken() { ActiveChangeCallbacks = true }; - cache.Set($"key{i}", $"value{i}", - new MemoryCacheEntryOptions { Size = 1 } - .AddExpirationToken(expirationToken)); - - if (random.Next(numEntries) < numEntries * 0.25) - { - // Set to expired 25% of the time - expirationToken.HasChanged = true; - } - } - } - - // start a few tasks to access entries in the background - Task[] backgroundAccessTasks = new Task[Environment.ProcessorCount]; - bool done = false; - - for (int i = 0; i < backgroundAccessTasks.Length; i++) - { - backgroundAccessTasks[i] = Task.Run(async () => - { - while (!done) - { - cache.TryGetValue($"key{random.Next(numEntries)}", out _); - var stats = cache.GetCurrentStatistics(); - - // set flag when stats go out of sync - if ((stats.CurrentEntryCount != cache.Count) - || (sizeLimitIsSet && stats.CurrentEstimatedSize != stats.CurrentEntryCount)) - { - statsGetInaccurateDuringHeavyLoad = true; - } - - await Task.Yield(); - } - }); - } - - for (int i = 0; i < 1000; i++) - { - cache.Compact(1); - FillCache(); - } - - done = true; - - Task.WaitAll(backgroundAccessTasks); - - // even if the values are not exact during heavy multithreaded operations, - // once done, the count and size eventually become fixed to expected value - var finalStats = cache.GetCurrentStatistics(); - Assert.True(statsGetInaccurateDuringHeavyLoad); - Assert.Equal(cache.Count, finalStats.CurrentEntryCount); - VerifyCurrentEstimatedSize(finalStats.CurrentEntryCount, sizeLimitIsSet, finalStats); - } - +#if NET7_0_OR_GREATER [Fact] public void GetCurrentStatistics_DIMReturnsNull() { -#if NET7_0_OR_GREATER Assert.Null((new FakeMemoryCache() as IMemoryCache).GetCurrentStatistics()); -#endif } +#endif private class FakeMemoryCache : IMemoryCache { From e7be224241dd4b095d697d740757fb25330f5504 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 16 Mar 2022 19:58:42 -0400 Subject: [PATCH 11/30] Update src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs Co-authored-by: Eric Erhardt --- .../src/IMemoryCache.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs index c41a6fff29cb53..0a01ddc2ef6d4f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs @@ -35,7 +35,7 @@ public interface IMemoryCache : IDisposable /// /// Gets a snapshot of the cache statistics if available. /// - /// An instance of the instance. + /// An instance of containing a snapshot of the cache statistics. MemoryCacheStatistics? GetCurrentStatistics() => null; #endif } From fede4142b394ece4aa72440db714209e38b62b1c Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 18 Mar 2022 09:11:19 -0700 Subject: [PATCH 12/30] Rename to "*net7.0.cs" from "*net7.cs" --- .../ref/Microsoft.Extensions.Caching.Abstractions.csproj | 2 +- ...7.cs => Microsoft.Extensions.Caching.Abstractions.net7.0.cs} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/{Microsoft.Extensions.Caching.Abstractions.net7.cs => Microsoft.Extensions.Caching.Abstractions.net7.0.cs} (100%) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index 06e3f06d8e09bc..1a9526035e4b2c 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -6,7 +6,7 @@ - + diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.0.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.cs rename to src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.0.cs From 77e390d792ec1cdef4d0ba5642b775f07326f7ec Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 18 Mar 2022 15:51:29 -0700 Subject: [PATCH 13/30] using thread local to get per memory cache stats and better perf --- .../src/MemoryCache.cs | 95 +++++++++++++++++-- 1 file changed, 89 insertions(+), 6 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index c707a55279f4ba..6987c3f1713db2 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -27,8 +27,6 @@ public class MemoryCache : IMemoryCache private CoherentState _coherentState; private bool _disposed; - private long _totalHits; - private long _totalMisses; private DateTimeOffset _lastExpirationScan; /// @@ -55,6 +53,7 @@ public MemoryCache(IOptions optionsAccessor!!, ILoggerFactor _options.Clock = new SystemClock(); } + _accumulatedStats = new Stats(this); _lastExpirationScan = _options.Clock.UtcNow; TrackLinkedCacheEntries = _options.TrackLinkedCacheEntries; // we store the setting now so it's consistent for entire MemoryCache lifetime } @@ -222,7 +221,7 @@ public bool TryGetValue(object key!!, out object? result) StartScanForExpiredItemsIfNeeded(utcNow); - Interlocked.Increment(ref _totalHits); + Hit(); return true; } else @@ -235,7 +234,7 @@ public bool TryGetValue(object key!!, out object? result) StartScanForExpiredItemsIfNeeded(utcNow); result = null; - Interlocked.Increment(ref _totalMisses); + Miss(); return false; } @@ -279,10 +278,11 @@ public void Clear() /// public MemoryCacheStatistics GetCurrentStatistics() { + (int hit, int miss) sumTotal = Sum(); return new MemoryCacheStatistics() { - TotalMisses = _totalMisses, - TotalHits = _totalHits, + TotalMisses = sumTotal.miss, + TotalHits = sumTotal.hit, CurrentEntryCount = Count, CurrentEstimatedSize = _options.SizeLimit.HasValue ? Size : null }; @@ -313,6 +313,88 @@ void ScheduleTask(DateTimeOffset utcNow) } } + internal readonly List?> _allStats = new(); + internal readonly Stats _accumulatedStats; + + private (int, int) Sum() + { + int hits = 0, misses = 0; + lock (_allStats) + { + hits += _accumulatedStats.Hits; + misses += _accumulatedStats.Misses; + foreach (WeakReference? wr in _allStats) + { + if (wr is not null && wr.TryGetTarget(out Stats? stats)) + { + hits += stats.Hits; + misses += stats.Misses; + } + } + } + return (hits, misses); + } + + private ThreadLocal? _stats; + + private void Hit() + { + GetStats().Hits++; + } + + private void Miss() + { + GetStats().Misses++; + } + + private Stats GetStats() + { + if (_stats == null) + { + return Initialize(); + } + return _stats!.Value!; + + Stats Initialize() + { + var s = _stats = new ThreadLocal(() => new Stats(this)); + lock (_allStats) + { + _allStats.Add(new WeakReference(s.Value!)); + } + return s.Value!; + } + } + + internal sealed class Stats + { + public int Hits; + public int Misses; + private MemoryCache _memoryCache; + public Stats(MemoryCache memoryCache) + { + _memoryCache = memoryCache; + } + ~Stats() + { + var allStats = _memoryCache._allStats; + lock (allStats) + { + for (int i = 0; i < allStats.Count; i++) + { + if (allStats[i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == this) + { + allStats[i] = null; + break; + } + } + allStats.RemoveAll(s => s is null); + Interlocked.Add(ref _memoryCache._accumulatedStats.Hits, Hits); + Interlocked.Add(ref _memoryCache._accumulatedStats.Misses, Misses); + } + } + } + private static void ScanForExpiredItems(MemoryCache cache) { DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock!.UtcNow; @@ -487,6 +569,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { + _stats?.Dispose(); GC.SuppressFinalize(this); } From 4aefd1b98f3a671448322ad0af38a2d20b9b15ef Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 18 Mar 2022 17:44:06 -0700 Subject: [PATCH 14/30] Applies PR feedback - with micro perf improvement --- .../src/MemoryCacheStatistics.cs | 2 +- .../src/MemoryCache.cs | 32 ++++++------------- 2 files changed, 10 insertions(+), 24 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs index 52f0757bc3472e..621d800dca9729 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs @@ -23,7 +23,7 @@ public MemoryCacheStatistics() { } /// /// Gets an estimated sum of all the values currently in the memory cache. /// - /// Returns if no size limit is set for the cache. + /// Returns if size isn't being tracked. The common MemoryCache implementation tracks size whenever a SizeLimit is set on the cache. public long? CurrentEstimatedSize { get; init; } /// diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 6987c3f1713db2..d8c8832d2a6ead 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -53,7 +53,8 @@ public MemoryCache(IOptions optionsAccessor!!, ILoggerFactor _options.Clock = new SystemClock(); } - _accumulatedStats = new Stats(this); + _accumulatedStats = new Stats(this, skipAdd: true); + _stats = new ThreadLocal(() => new Stats(this)); _lastExpirationScan = _options.Clock.UtcNow; TrackLinkedCacheEntries = _options.TrackLinkedCacheEntries; // we store the setting now so it's consistent for entire MemoryCache lifetime } @@ -335,7 +336,7 @@ void ScheduleTask(DateTimeOffset utcNow) return (hits, misses); } - private ThreadLocal? _stats; + private ThreadLocal _stats; private void Hit() { @@ -347,33 +348,18 @@ private void Miss() GetStats().Misses++; } - private Stats GetStats() - { - if (_stats == null) - { - return Initialize(); - } - return _stats!.Value!; - - Stats Initialize() - { - var s = _stats = new ThreadLocal(() => new Stats(this)); - lock (_allStats) - { - _allStats.Add(new WeakReference(s.Value!)); - } - return s.Value!; - } - } + private Stats GetStats() => _stats!.Value!; internal sealed class Stats { public int Hits; public int Misses; private MemoryCache _memoryCache; - public Stats(MemoryCache memoryCache) + public Stats(MemoryCache memoryCache, bool skipAdd = false) { _memoryCache = memoryCache; + if (!skipAdd) + _memoryCache._allStats.Add(new WeakReference(this)); } ~Stats() { @@ -389,8 +375,8 @@ public Stats(MemoryCache memoryCache) } } allStats.RemoveAll(s => s is null); - Interlocked.Add(ref _memoryCache._accumulatedStats.Hits, Hits); - Interlocked.Add(ref _memoryCache._accumulatedStats.Misses, Misses); + _memoryCache._accumulatedStats.Hits += Hits; + _memoryCache._accumulatedStats.Misses += Misses; } } } From bdd6fb7e3f3c9059ff191300606cc55119f4690f Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 18 Mar 2022 18:23:32 -0700 Subject: [PATCH 15/30] nit fix/readability improvement --- .../src/MemoryCache.cs | 37 ++++++++++++------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index d8c8832d2a6ead..aa56ad338c88bf 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -354,33 +354,42 @@ internal sealed class Stats { public int Hits; public int Misses; - private MemoryCache _memoryCache; + private readonly MemoryCache _memoryCache; public Stats(MemoryCache memoryCache, bool skipAdd = false) { _memoryCache = memoryCache; if (!skipAdd) - _memoryCache._allStats.Add(new WeakReference(this)); + _memoryCache.AddToStats(this); } ~Stats() { - var allStats = _memoryCache._allStats; - lock (allStats) + _memoryCache.RemoveFromStats(this); + } + } + + private void RemoveFromStats(Stats current) + { + lock (_allStats) + { + for (int i = 0; i < _allStats.Count; i++) { - for (int i = 0; i < allStats.Count; i++) + if (_allStats[i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == current) { - if (allStats[i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == this) - { - allStats[i] = null; - break; - } + _allStats[i] = null; + break; } - allStats.RemoveAll(s => s is null); - _memoryCache._accumulatedStats.Hits += Hits; - _memoryCache._accumulatedStats.Misses += Misses; } + _allStats.RemoveAll(s => s is null); + _accumulatedStats.Hits += current.Hits; + _accumulatedStats.Misses += current.Misses; } } + private void AddToStats(Stats current) + { + _allStats.Add(new WeakReference(current)); + } + private static void ScanForExpiredItems(MemoryCache cache) { DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock!.UtcNow; @@ -555,7 +564,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _stats?.Dispose(); + _stats.Dispose(); GC.SuppressFinalize(this); } From c9004839efcef5bec093ef311887284e62d3bcd4 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 21 Mar 2022 09:03:24 -0700 Subject: [PATCH 16/30] apply nit feedback --- .../Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index aa56ad338c88bf..c7e970059e09be 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -375,11 +375,10 @@ private void RemoveFromStats(Stats current) { if (_allStats[i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == current) { - _allStats[i] = null; + _allStats.RemoveAt(i); break; } } - _allStats.RemoveAll(s => s is null); _accumulatedStats.Hits += current.Hits; _accumulatedStats.Misses += current.Misses; } From 2eef44133fd29b6e87bf448be5e6d2dc31deebb8 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 21 Mar 2022 14:54:28 -0700 Subject: [PATCH 17/30] supporting long hit/miss --- .../src/MemoryCache.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index c7e970059e09be..04680d41c84a33 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -279,7 +279,7 @@ public void Clear() /// public MemoryCacheStatistics GetCurrentStatistics() { - (int hit, int miss) sumTotal = Sum(); + (long hit, long miss) sumTotal = Sum(); return new MemoryCacheStatistics() { TotalMisses = sumTotal.miss, @@ -317,9 +317,9 @@ void ScheduleTask(DateTimeOffset utcNow) internal readonly List?> _allStats = new(); internal readonly Stats _accumulatedStats; - private (int, int) Sum() + private (long, long) Sum() { - int hits = 0, misses = 0; + long hits = 0, misses = 0; lock (_allStats) { hits += _accumulatedStats.Hits; @@ -328,8 +328,8 @@ void ScheduleTask(DateTimeOffset utcNow) { if (wr is not null && wr.TryGetTarget(out Stats? stats)) { - hits += stats.Hits; - misses += stats.Misses; + hits += Interlocked.Read(ref stats.Hits); + misses += Interlocked.Read(ref stats.Misses); } } } @@ -340,20 +340,26 @@ void ScheduleTask(DateTimeOffset utcNow) private void Hit() { - GetStats().Hits++; + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Hits); + else + GetStats().Hits++; } private void Miss() { - GetStats().Misses++; + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Misses); + else + GetStats().Misses++; } private Stats GetStats() => _stats!.Value!; internal sealed class Stats { - public int Hits; - public int Misses; + public long Hits; + public long Misses; private readonly MemoryCache _memoryCache; public Stats(MemoryCache memoryCache, bool skipAdd = false) { @@ -379,8 +385,9 @@ private void RemoveFromStats(Stats current) break; } } - _accumulatedStats.Hits += current.Hits; - _accumulatedStats.Misses += current.Misses; + + _accumulatedStats.Hits += Interlocked.Read(ref current.Hits); + _accumulatedStats.Misses += Interlocked.Read(ref current.Misses); } } From e7b8d8fab9160828ec66708d9d9940e5e9be4c2c Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 21 Mar 2022 16:04:01 -0700 Subject: [PATCH 18/30] Adds MemoryCacheOptions.TrackStatistics options flag --- .../Microsoft.Extensions.Caching.Memory.cs | 3 +- .../src/MemoryCache.cs | 85 +++++++++++-------- .../src/MemoryCacheOptions.cs | 5 ++ .../MemoryCacheGetCurrentStatisticsTests.cs | 21 +++-- 4 files changed, 71 insertions(+), 43 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs index 8baef9ea0923c0..7a13cdf999acb7 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs @@ -33,7 +33,7 @@ public void Compact(double percentage) { } public void Dispose() { } protected virtual void Dispose(bool disposing) { } ~MemoryCache() { } - public Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics GetCurrentStatistics() { throw null; } + public Microsoft.Extensions.Caching.Memory.MemoryCacheStatistics? GetCurrentStatistics() { throw null; } public void Remove(object key) { } public bool TryGetValue(object key, out object? result) { throw null; } } @@ -46,6 +46,7 @@ public MemoryCacheOptions() { } Microsoft.Extensions.Caching.Memory.MemoryCacheOptions Microsoft.Extensions.Options.IOptions.Value { get { throw null; } } public long? SizeLimit { get { throw null; } set { } } public bool TrackLinkedCacheEntries { get { throw null; } set { } } + public bool TrackStatistics { get { throw null; } set { } } } public partial class MemoryDistributedCacheOptions : Microsoft.Extensions.Caching.Memory.MemoryCacheOptions { diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 04680d41c84a33..8c025ec2ba0c44 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -53,8 +53,13 @@ public MemoryCache(IOptions optionsAccessor!!, ILoggerFactor _options.Clock = new SystemClock(); } - _accumulatedStats = new Stats(this, skipAdd: true); - _stats = new ThreadLocal(() => new Stats(this)); + if (_options.TrackStatistics) + { + _allStats = new(); + _accumulatedStats = new Stats(this, skipAdd: true); + _stats = new ThreadLocal(() => new Stats(this)); + } + _lastExpirationScan = _options.Clock.UtcNow; TrackLinkedCacheEntries = _options.TrackLinkedCacheEntries; // we store the setting now so it's consistent for entire MemoryCache lifetime } @@ -221,7 +226,6 @@ public bool TryGetValue(object key!!, out object? result) } StartScanForExpiredItemsIfNeeded(utcNow); - Hit(); return true; } @@ -236,6 +240,7 @@ public bool TryGetValue(object key!!, out object? result) result = null; Miss(); + return false; } @@ -277,16 +282,21 @@ public void Clear() /// /// Gets a snapshot of the current statistics for the memory cache. /// - public MemoryCacheStatistics GetCurrentStatistics() + public MemoryCacheStatistics? GetCurrentStatistics() { - (long hit, long miss) sumTotal = Sum(); - return new MemoryCacheStatistics() + if (_options.TrackStatistics) { - TotalMisses = sumTotal.miss, - TotalHits = sumTotal.hit, - CurrentEntryCount = Count, - CurrentEstimatedSize = _options.SizeLimit.HasValue ? Size : null - }; + (long hit, long miss) sumTotal = Sum(); + return new MemoryCacheStatistics() + { + TotalMisses = sumTotal.miss, + TotalHits = sumTotal.hit, + CurrentEntryCount = Count, + CurrentEstimatedSize = _options.SizeLimit.HasValue ? Size : null + }; + } + + return null; } internal void EntryExpired(CacheEntry entry) @@ -314,17 +324,18 @@ void ScheduleTask(DateTimeOffset utcNow) } } - internal readonly List?> _allStats = new(); - internal readonly Stats _accumulatedStats; + private readonly List?>? _allStats; + private readonly Stats? _accumulatedStats; + private ThreadLocal? _stats; private (long, long) Sum() { long hits = 0, misses = 0; - lock (_allStats) + lock (_allStats!) { - hits += _accumulatedStats.Hits; - misses += _accumulatedStats.Misses; - foreach (WeakReference? wr in _allStats) + hits += _accumulatedStats!.Hits; + misses += _accumulatedStats!.Misses; + foreach (WeakReference? wr in _allStats!) { if (wr is not null && wr.TryGetTarget(out Stats? stats)) { @@ -336,22 +347,26 @@ void ScheduleTask(DateTimeOffset utcNow) return (hits, misses); } - private ThreadLocal _stats; - private void Hit() { - if (IntPtr.Size == 4) - Interlocked.Increment(ref GetStats().Hits); - else - GetStats().Hits++; + if (_options.TrackStatistics) + { + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Hits); + else + GetStats().Hits++; + } } private void Miss() { - if (IntPtr.Size == 4) - Interlocked.Increment(ref GetStats().Misses); - else - GetStats().Misses++; + if (_options.TrackStatistics) + { + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Misses); + else + GetStats().Misses++; + } } private Stats GetStats() => _stats!.Value!; @@ -375,25 +390,25 @@ public Stats(MemoryCache memoryCache, bool skipAdd = false) private void RemoveFromStats(Stats current) { - lock (_allStats) + lock (_allStats!) { - for (int i = 0; i < _allStats.Count; i++) + for (int i = 0; i < _allStats!.Count; i++) { - if (_allStats[i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == current) + if (_allStats![i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == current) { - _allStats.RemoveAt(i); + _allStats!.RemoveAt(i); break; } } - _accumulatedStats.Hits += Interlocked.Read(ref current.Hits); - _accumulatedStats.Misses += Interlocked.Read(ref current.Misses); + _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); + _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); } } private void AddToStats(Stats current) { - _allStats.Add(new WeakReference(current)); + _allStats!.Add(new WeakReference(current)); } private static void ScanForExpiredItems(MemoryCache cache) @@ -570,7 +585,7 @@ protected virtual void Dispose(bool disposing) { if (disposing) { - _stats.Dispose(); + _stats?.Dispose(); GC.SuppressFinalize(this); } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs index 82968cefd33b15..a37111ad60749c 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCacheOptions.cs @@ -59,6 +59,11 @@ public double CompactionPercentage /// Prior to .NET 7 this feature was always enabled. public bool TrackLinkedCacheEntries { get; set; } + /// + /// Gets or sets whether to track memory cache statistics. Disabled by default. + /// + public bool TrackStatistics { get; set; } + MemoryCacheOptions IOptions.Value { get { return this; } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index aff69b78e8de3f..f40e5ab6004c30 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -13,14 +13,21 @@ namespace Microsoft.Extensions.Caching.Memory { public class MemoryCacheHasStatisticsTests { + [Fact] + public void GetCurrentStatistics_TrackStatisticsFalse_ReturnsNull() + { + var cache = new MemoryCache(new MemoryCacheOptions { TrackStatistics = false }); + Assert.Null(cache.GetCurrentStatistics()); + } + [Theory] [InlineData(true)] [InlineData(false)] public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) { var cache = sizeLimitIsSet ? - new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : - new MemoryCache(new MemoryCacheOptions { }); + new MemoryCache(new MemoryCacheOptions { TrackStatistics = true, SizeLimit = 10 }) : + new MemoryCache(new MemoryCacheOptions { TrackStatistics = true }); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); for (int i = 0; i < 100; i++) @@ -44,8 +51,8 @@ public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) public void GetCurrentStatistics_UpdateExistingCache_UpdatesStatistics(bool sizeLimitIsSet) { var cache = sizeLimitIsSet ? - new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 }) : - new MemoryCache(new MemoryCacheOptions { }); + new MemoryCache(new MemoryCacheOptions { TrackStatistics = true, SizeLimit = 10 }) : + new MemoryCache(new MemoryCacheOptions { TrackStatistics = true }); cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 2 }); Assert.Equal("value", cache.Get("key")); @@ -53,7 +60,7 @@ public void GetCurrentStatistics_UpdateExistingCache_UpdatesStatistics(bool size cache.Set("key", "updated value", new MemoryCacheEntryOptions { Size = 3 }); Assert.Equal("updated value", cache.Get("key")); - MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + MemoryCacheStatistics stats = cache.GetCurrentStatistics();// Assert.Equal(1, stats.CurrentEntryCount); Assert.Equal(0, stats.TotalMisses); @@ -69,8 +76,8 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimated const string Key = "myKey"; var cache = new MemoryCache(sizeLimitIsSet ? - new MemoryCacheOptions { Clock = new SystemClock(), SizeLimit = 10 } : - new MemoryCacheOptions { Clock = new SystemClock() } + new MemoryCacheOptions { TrackStatistics = true, Clock = new SystemClock(), SizeLimit = 10 } : + new MemoryCacheOptions { TrackStatistics = true, Clock = new SystemClock() } ); ICacheEntry entry; From e149d8fe566b4cb643261a9db4a94f5c9a9bd6a4 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Mon, 21 Mar 2022 20:08:41 -0400 Subject: [PATCH 19/30] Update src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs Co-authored-by: Stephen Halter --- .../Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 8c025ec2ba0c44..82b15526e1b4c8 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -282,6 +282,7 @@ public void Clear() /// /// Gets a snapshot of the current statistics for the memory cache. /// + /// Returns if statistics are not being tracked because is . public MemoryCacheStatistics? GetCurrentStatistics() { if (_options.TrackStatistics) From f4462930241bdb20b0fff422a8ee863ad49f823e Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 22 Mar 2022 16:15:36 -0700 Subject: [PATCH 20/30] update on lock for 32bit machine --- .../src/MemoryCache.cs | 30 +++++++++++++++---- 1 file changed, 25 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 82b15526e1b4c8..9fef1071089fa6 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -334,8 +334,17 @@ void ScheduleTask(DateTimeOffset utcNow) long hits = 0, misses = 0; lock (_allStats!) { - hits += _accumulatedStats!.Hits; - misses += _accumulatedStats!.Misses; + if (IntPtr.Size == 4) + { + Interlocked.Add(ref hits, Interlocked.Read(ref _accumulatedStats!.Hits)); + Interlocked.Add(ref misses, Interlocked.Read(ref _accumulatedStats!.Misses)); + } + else + { + hits += _accumulatedStats!.Hits; + misses += _accumulatedStats!.Misses; + } + foreach (WeakReference? wr in _allStats!) { if (wr is not null && wr.TryGetTarget(out Stats? stats)) @@ -402,14 +411,25 @@ private void RemoveFromStats(Stats current) } } - _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); - _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); + if (IntPtr.Size == 4) + { + Interlocked.Add(ref _accumulatedStats!.Hits, Interlocked.Read(ref current!.Hits)); + Interlocked.Add(ref _accumulatedStats!.Misses, Interlocked.Read(ref current!.Misses)); + } + else + { + _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); + _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); + } } } private void AddToStats(Stats current) { - _allStats!.Add(new WeakReference(current)); + lock (_allStats!) + { + _allStats!.Add(new WeakReference(current)); + } } private static void ScanForExpiredItems(MemoryCache cache) From 1a5c7f49a45151ae153f7f1f05a7a656bfecee3b Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 22 Mar 2022 16:55:47 -0700 Subject: [PATCH 21/30] undo unnecessary Interlock --- .../src/MemoryCache.cs | 25 +++---------------- 1 file changed, 4 insertions(+), 21 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 9fef1071089fa6..85ef9d5d3dbdc4 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -334,17 +334,8 @@ void ScheduleTask(DateTimeOffset utcNow) long hits = 0, misses = 0; lock (_allStats!) { - if (IntPtr.Size == 4) - { - Interlocked.Add(ref hits, Interlocked.Read(ref _accumulatedStats!.Hits)); - Interlocked.Add(ref misses, Interlocked.Read(ref _accumulatedStats!.Misses)); - } - else - { - hits += _accumulatedStats!.Hits; - misses += _accumulatedStats!.Misses; - } - + hits += _accumulatedStats!.Hits; + misses += _accumulatedStats!.Misses; foreach (WeakReference? wr in _allStats!) { if (wr is not null && wr.TryGetTarget(out Stats? stats)) @@ -411,16 +402,8 @@ private void RemoveFromStats(Stats current) } } - if (IntPtr.Size == 4) - { - Interlocked.Add(ref _accumulatedStats!.Hits, Interlocked.Read(ref current!.Hits)); - Interlocked.Add(ref _accumulatedStats!.Misses, Interlocked.Read(ref current!.Misses)); - } - else - { - _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); - _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); - } + _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); + _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); } } From 3e2ce4fc8c29a9cf54742617b1982ab820499a86 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 22 Mar 2022 17:07:51 -0700 Subject: [PATCH 22/30] Add TrimExcess --- .../Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 85ef9d5d3dbdc4..2252d4d07b49ed 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -344,6 +344,8 @@ void ScheduleTask(DateTimeOffset utcNow) misses += Interlocked.Read(ref stats.Misses); } } + + _allStats.TrimExcess(); } return (hits, misses); } From fc10cc01cea6585db4cd5acbd163e3a383e0cb1a Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Tue, 22 Mar 2022 17:59:30 -0700 Subject: [PATCH 23/30] test cleanup --- .../tests/MemoryCacheGetCurrentStatisticsTests.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index f40e5ab6004c30..19ecd6baab7f86 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -37,8 +37,9 @@ public void GetCurrentStatistics_GetCache_UpdatesStatistics(bool sizeLimitIsSet) Assert.Null(cache.Get("missingKey2")); } - MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + MemoryCacheStatistics? stats = cache.GetCurrentStatistics(); + Assert.NotNull(stats); Assert.Equal(200, stats.TotalMisses); Assert.Equal(100, stats.TotalHits); Assert.Equal(1, stats.CurrentEntryCount); @@ -60,8 +61,9 @@ public void GetCurrentStatistics_UpdateExistingCache_UpdatesStatistics(bool size cache.Set("key", "updated value", new MemoryCacheEntryOptions { Size = 3 }); Assert.Equal("updated value", cache.Get("key")); - MemoryCacheStatistics stats = cache.GetCurrentStatistics();// + MemoryCacheStatistics? stats = cache.GetCurrentStatistics(); + Assert.NotNull(stats); Assert.Equal(1, stats.CurrentEntryCount); Assert.Equal(0, stats.TotalMisses); Assert.Equal(2, stats.TotalHits); @@ -86,7 +88,9 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimated var expirationToken = new TestExpirationToken() { ActiveChangeCallbacks = true }; var mc = new MemoryCacheEntryOptions { Size = 5 }; cache.Set(Key, new object(), mc.AddExpirationToken(expirationToken)); - MemoryCacheStatistics stats = cache.GetCurrentStatistics(); + MemoryCacheStatistics? stats = cache.GetCurrentStatistics(); + + Assert.NotNull(stats); Assert.Equal(1, cache.Count); Assert.Equal(1, stats.CurrentEntryCount); VerifyCurrentEstimatedSize(5, sizeLimitIsSet, stats); @@ -94,6 +98,8 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimated expirationToken.HasChanged = true; cache.Set(Key, new object(), mc.AddExpirationToken(expirationToken)); stats = cache.GetCurrentStatistics(); + + Assert.NotNull(stats); Assert.Equal(0, cache.Count); Assert.Equal(0, stats.CurrentEntryCount); VerifyCurrentEstimatedSize(0, sizeLimitIsSet, stats); From 0cc5d7fee9d75add5e4ad2005f27d80ae280ffa8 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 23 Mar 2022 10:41:10 -0700 Subject: [PATCH 24/30] change flag condition to _allStats not null --- .../Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 2252d4d07b49ed..1ae4788b4665bc 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -285,7 +285,7 @@ public void Clear() /// Returns if statistics are not being tracked because is . public MemoryCacheStatistics? GetCurrentStatistics() { - if (_options.TrackStatistics) + if (_allStats is not null) { (long hit, long miss) sumTotal = Sum(); return new MemoryCacheStatistics() @@ -352,7 +352,7 @@ void ScheduleTask(DateTimeOffset utcNow) private void Hit() { - if (_options.TrackStatistics) + if (_allStats is not null) { if (IntPtr.Size == 4) Interlocked.Increment(ref GetStats().Hits); @@ -363,7 +363,7 @@ private void Hit() private void Miss() { - if (_options.TrackStatistics) + if (_allStats is not null) { if (IntPtr.Size == 4) Interlocked.Increment(ref GetStats().Misses); From 20cc800e122b0c2381163f82d65ca9a018de2912 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 23 Mar 2022 13:42:01 -0700 Subject: [PATCH 25/30] Applies latest PR feedback --- .../src/MemoryCache.cs | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 1ae4788b4665bc..e2ce8b033ede59 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -25,6 +25,9 @@ public class MemoryCache : IMemoryCache private readonly MemoryCacheOptions _options; + private readonly List?>? _allStats; + private readonly Stats? _accumulatedStats; + private ThreadLocal? _stats; private CoherentState _coherentState; private bool _disposed; private DateTimeOffset _lastExpirationScan; @@ -55,8 +58,8 @@ public MemoryCache(IOptions optionsAccessor!!, ILoggerFactor if (_options.TrackStatistics) { - _allStats = new(); - _accumulatedStats = new Stats(this, skipAdd: true); + _allStats = new List?>(); + _accumulatedStats = new Stats(this, isAccumulatedStat: true); _stats = new ThreadLocal(() => new Stats(this)); } @@ -325,17 +328,13 @@ void ScheduleTask(DateTimeOffset utcNow) } } - private readonly List?>? _allStats; - private readonly Stats? _accumulatedStats; - private ThreadLocal? _stats; - private (long, long) Sum() { - long hits = 0, misses = 0; lock (_allStats!) { - hits += _accumulatedStats!.Hits; - misses += _accumulatedStats!.Misses; + long hits = _accumulatedStats!.Hits; + long misses = _accumulatedStats!.Misses; + foreach (WeakReference? wr in _allStats!) { if (wr is not null && wr.TryGetTarget(out Stats? stats)) @@ -345,9 +344,8 @@ void ScheduleTask(DateTimeOffset utcNow) } } - _allStats.TrimExcess(); + return (hits, misses); } - return (hits, misses); } private void Hit() @@ -378,16 +376,18 @@ internal sealed class Stats { public long Hits; public long Misses; - private readonly MemoryCache _memoryCache; - public Stats(MemoryCache memoryCache, bool skipAdd = false) + private readonly MemoryCache? _memoryCache; + public Stats(MemoryCache memoryCache, bool isAccumulatedStat = false) { - _memoryCache = memoryCache; - if (!skipAdd) + if (!isAccumulatedStat) + { + _memoryCache = memoryCache; _memoryCache.AddToStats(this); + } } ~Stats() { - _memoryCache.RemoveFromStats(this); + _memoryCache?.RemoveFromStats(this); } } @@ -406,6 +406,7 @@ private void RemoveFromStats(Stats current) _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); + _allStats.TrimExcess(); } } From b3c565344755ff27132663f0da121bc02572d4bc Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Wed, 23 Mar 2022 17:46:41 -0700 Subject: [PATCH 26/30] Second round feedback --- .../src/MemoryCache.cs | 34 +++++++++---------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index e2ce8b033ede59..a3a9add8795299 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -25,9 +25,9 @@ public class MemoryCache : IMemoryCache private readonly MemoryCacheOptions _options; - private readonly List?>? _allStats; + private readonly List>? _allStats; private readonly Stats? _accumulatedStats; - private ThreadLocal? _stats; + private readonly ThreadLocal? _stats; private CoherentState _coherentState; private bool _disposed; private DateTimeOffset _lastExpirationScan; @@ -58,8 +58,8 @@ public MemoryCache(IOptions optionsAccessor!!, ILoggerFactor if (_options.TrackStatistics) { - _allStats = new List?>(); - _accumulatedStats = new Stats(this, isAccumulatedStat: true); + _allStats = new List>(); + _accumulatedStats = new Stats(); _stats = new ThreadLocal(() => new Stats(this)); } @@ -333,11 +333,11 @@ void ScheduleTask(DateTimeOffset utcNow) lock (_allStats!) { long hits = _accumulatedStats!.Hits; - long misses = _accumulatedStats!.Misses; + long misses = _accumulatedStats.Misses; - foreach (WeakReference? wr in _allStats!) + foreach (WeakReference wr in _allStats) { - if (wr is not null && wr.TryGetTarget(out Stats? stats)) + if (wr.TryGetTarget(out Stats? stats)) { hits += Interlocked.Read(ref stats.Hits); misses += Interlocked.Read(ref stats.Misses); @@ -377,13 +377,11 @@ internal sealed class Stats public long Hits; public long Misses; private readonly MemoryCache? _memoryCache; - public Stats(MemoryCache memoryCache, bool isAccumulatedStat = false) + public Stats() { } + public Stats(MemoryCache memoryCache) { - if (!isAccumulatedStat) - { - _memoryCache = memoryCache; - _memoryCache.AddToStats(this); - } + _memoryCache = memoryCache; + _memoryCache.AddToStats(this); } ~Stats() { @@ -395,17 +393,17 @@ private void RemoveFromStats(Stats current) { lock (_allStats!) { - for (int i = 0; i < _allStats!.Count; i++) + for (int i = 0; i < _allStats.Count; i++) { - if (_allStats![i] is WeakReference wr && wr.TryGetTarget(out Stats? stats) && stats == current) + if (_allStats[i].TryGetTarget(out Stats? stats) && stats == current) { - _allStats!.RemoveAt(i); + _allStats.RemoveAt(i); break; } } _accumulatedStats!.Hits += Interlocked.Read(ref current.Hits); - _accumulatedStats!.Misses += Interlocked.Read(ref current.Misses); + _accumulatedStats.Misses += Interlocked.Read(ref current.Misses); _allStats.TrimExcess(); } } @@ -414,7 +412,7 @@ private void AddToStats(Stats current) { lock (_allStats!) { - _allStats!.Add(new WeakReference(current)); + _allStats.Add(new WeakReference(current)); } } From 663b66f21eba38d178d702e0edfa4468ae68f0c7 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 24 Mar 2022 09:32:35 -0700 Subject: [PATCH 27/30] support DIM on net6 as well --- .../ref/Microsoft.Extensions.Caching.Abstractions.csproj | 2 +- ...0.cs => Microsoft.Extensions.Caching.Abstractions.net6.0.cs} | 0 .../src/IMemoryCache.cs | 2 +- .../tests/MemoryCacheGetCurrentStatisticsTests.cs | 2 +- 4 files changed, 3 insertions(+), 3 deletions(-) rename src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/{Microsoft.Extensions.Caching.Abstractions.net7.0.cs => Microsoft.Extensions.Caching.Abstractions.net6.0.cs} (100%) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj index 1a9526035e4b2c..36666f11889e4f 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.csproj @@ -6,7 +6,7 @@ - + diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.0.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net6.0.cs similarity index 100% rename from src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net7.0.cs rename to src/libraries/Microsoft.Extensions.Caching.Abstractions/ref/Microsoft.Extensions.Caching.Abstractions.net6.0.cs diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs index 0a01ddc2ef6d4f..ccce249b23d958 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs @@ -31,7 +31,7 @@ public interface IMemoryCache : IDisposable /// An object identifying the entry. void Remove(object key); -#if NET7_0_OR_GREATER +#if NET6_0_OR_GREATER /// /// Gets a snapshot of the cache statistics if available. /// diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs index 19ecd6baab7f86..cee6245f3e9c6b 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/tests/MemoryCacheGetCurrentStatisticsTests.cs @@ -106,7 +106,7 @@ public void GetCurrentStatistics_UpdateAfterExistingItemExpired_CurrentEstimated } } -#if NET7_0_OR_GREATER +#if NET6_0_OR_GREATER [Fact] public void GetCurrentStatistics_DIMReturnsNull() { From 2a5b0c6aaa75cf3f6109571ed5f84c37a05e2e72 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 24 Mar 2022 13:32:42 -0400 Subject: [PATCH 28/30] Update src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs Co-authored-by: Stephen Toub --- .../src/MemoryCache.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index a3a9add8795299..58b78bcc392b4a 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -374,19 +374,19 @@ private void Miss() internal sealed class Stats { + private readonly MemoryCache? _memoryCache; public long Hits; public long Misses; - private readonly MemoryCache? _memoryCache; + public Stats() { } + public Stats(MemoryCache memoryCache) { _memoryCache = memoryCache; _memoryCache.AddToStats(this); } - ~Stats() - { - _memoryCache?.RemoveFromStats(this); - } + + ~Stats() => _memoryCache?.RemoveFromStats(this); } private void RemoveFromStats(Stats current) From cc641edcb3cd90df7ba58301934201d243fbb135 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Thu, 24 Mar 2022 15:17:36 -0700 Subject: [PATCH 29/30] Caching inlining --- .../src/MemoryCacheStatistics.cs | 2 +- .../Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs index 621d800dca9729..2040522c0b46b1 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheStatistics.cs @@ -27,7 +27,7 @@ public MemoryCacheStatistics() { } public long? CurrentEstimatedSize { get; init; } /// - /// Gets the total number of cache hits. + /// Gets the total number of cache misses. /// public long TotalMisses { get; init; } diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 58b78bcc392b4a..745150c0aaa372 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -348,6 +348,7 @@ void ScheduleTask(DateTimeOffset utcNow) } } + [MethodImpl(MethodImplOptions.NoInlining)] private void Hit() { if (_allStats is not null) @@ -359,6 +360,7 @@ private void Hit() } } + [MethodImpl(MethodImplOptions.NoInlining)] private void Miss() { if (_allStats is not null) From 9d2c26a9f3c733f97b9dc0212d9ef335c77fe917 Mon Sep 17 00:00:00 2001 From: Maryam Ariyan Date: Fri, 25 Mar 2022 14:36:23 -0700 Subject: [PATCH 30/30] Removing method and writing code inline --- .../src/MemoryCache.cs | 43 ++++++++----------- 1 file changed, 17 insertions(+), 26 deletions(-) diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 745150c0aaa372..3bc50c68bedcc8 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -229,7 +229,15 @@ public bool TryGetValue(object key!!, out object? result) } StartScanForExpiredItemsIfNeeded(utcNow); - Hit(); + // Hit + if (_allStats is not null) + { + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Hits); + else + GetStats().Hits++; + } + return true; } else @@ -242,7 +250,14 @@ public bool TryGetValue(object key!!, out object? result) StartScanForExpiredItemsIfNeeded(utcNow); result = null; - Miss(); + // Miss + if (_allStats is not null) + { + if (IntPtr.Size == 4) + Interlocked.Increment(ref GetStats().Misses); + else + GetStats().Misses++; + } return false; } @@ -348,30 +363,6 @@ void ScheduleTask(DateTimeOffset utcNow) } } - [MethodImpl(MethodImplOptions.NoInlining)] - private void Hit() - { - if (_allStats is not null) - { - if (IntPtr.Size == 4) - Interlocked.Increment(ref GetStats().Hits); - else - GetStats().Hits++; - } - } - - [MethodImpl(MethodImplOptions.NoInlining)] - private void Miss() - { - if (_allStats is not null) - { - if (IntPtr.Size == 4) - Interlocked.Increment(ref GetStats().Misses); - else - GetStats().Misses++; - } - } - private Stats GetStats() => _stats!.Value!; internal sealed class Stats