-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Add Clear() to MemoryCache #57631
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Clear() to MemoryCache #57631
Changes from 4 commits
01d20e8
40df3a3
378bfbe
40cbe82
9cdd21e
1f5d5a5
2507c65
50bdfb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -23,8 +23,8 @@ public class MemoryCache : IMemoryCache | |||||||||||||||||||||||||||||
| internal readonly ILogger _logger; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private readonly MemoryCacheOptions _options; | ||||||||||||||||||||||||||||||
| private readonly ConcurrentDictionary<object, CacheEntry> _entries; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private ConcurrentDictionary<object, CacheEntry> _entries; | ||||||||||||||||||||||||||||||
| private long _cacheSize; | ||||||||||||||||||||||||||||||
| private bool _disposed; | ||||||||||||||||||||||||||||||
| private DateTimeOffset _lastExpirationScan; | ||||||||||||||||||||||||||||||
|
|
@@ -260,8 +260,8 @@ public bool TryGetValue(object key, out object result) | |||||||||||||||||||||||||||||
| public void Remove(object key) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| ValidateCacheKey(key); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| CheckDisposed(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| if (_entries.TryRemove(key, out CacheEntry entry)) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (_options.SizeLimit.HasValue) | ||||||||||||||||||||||||||||||
|
|
@@ -276,6 +276,24 @@ public void Remove(object key) | |||||||||||||||||||||||||||||
| StartScanForExpiredItemsIfNeeded(_options.Clock.UtcNow); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| /// <summary> | ||||||||||||||||||||||||||||||
| /// Removes all keys and values from the cache. | ||||||||||||||||||||||||||||||
| /// </summary> | ||||||||||||||||||||||||||||||
| public void Clear() | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| CheckDisposed(); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // the following two operations are not atomic change as a whole, but an alternative would be to introduce a global lock for every access to _entries and _cacheSize | ||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could we get away with this by introducing a new class that contained both the dictionary and the size? Then we can atomically replace the pointer with a new instance?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this particular case, yes. In other, where we add/remove item from the cache and update the size afterwards, not. runtime/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs Lines 265 to 270 in 40cbe82
runtime/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs Lines 151 to 158 in 40cbe82
I am not sure if it's worth it.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In those other cases you can atomically read the pointer, store it in a local variable and modify that. If Clear gets called concurrently, it will either update the pointer before or after the other mutation. Thus, we should always have a coherent set of entries and cache size.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @eerhardt well, it took me a while to get back to this PR. PTAL at my recent commit. |
||||||||||||||||||||||||||||||
| ConcurrentDictionary<object, CacheEntry> oldEntries = Interlocked.Exchange(ref _entries, new ConcurrentDictionary<object, CacheEntry>()); | ||||||||||||||||||||||||||||||
| Interlocked.Exchange(ref _cacheSize, 0); | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| foreach (var entry in oldEntries) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| entry.Value.SetExpired(EvictionReason.Removed); | ||||||||||||||||||||||||||||||
| entry.Value.InvokeEvictionCallbacks(); | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| private void RemoveEntry(CacheEntry entry) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| if (EntriesCollection.Remove(new KeyValuePair<object, CacheEntry>(entry.Key, entry))) | ||||||||||||||||||||||||||||||
|
|
@@ -317,7 +335,8 @@ private static void ScanForExpiredItems(MemoryCache cache) | |||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock.UtcNow; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| foreach (KeyValuePair<object, CacheEntry> item in cache._entries) | ||||||||||||||||||||||||||||||
| ConcurrentDictionary<object, CacheEntry> entries = cache._entries; // Clear() can update the reference in the meantime | ||||||||||||||||||||||||||||||
| foreach (KeyValuePair<object, CacheEntry> item in entries) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| CacheEntry entry = item.Value; | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
|
|
@@ -402,7 +421,8 @@ private void Compact(long removalSizeTarget, Func<CacheEntry, long> computeEntry | |||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
| // Sort items by expired & priority status | ||||||||||||||||||||||||||||||
| DateTimeOffset now = _options.Clock.UtcNow; | ||||||||||||||||||||||||||||||
| foreach (KeyValuePair<object, CacheEntry> item in _entries) | ||||||||||||||||||||||||||||||
| ConcurrentDictionary<object, CacheEntry> entries = _entries; // Clear() can update the reference in the meantime | ||||||||||||||||||||||||||||||
| foreach (KeyValuePair<object, CacheEntry> item in entries) | ||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||
| CacheEntry entry = item.Value; | ||||||||||||||||||||||||||||||
| if (entry.CheckExpired(now)) | ||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.