Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Clear() to MemoryCache #57631

Merged
merged 8 commits into from
Nov 23, 2021
Merged

Add Clear() to MemoryCache #57631

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
01d20e8
add new method to reference assembly
adamsitnik Aug 18, 2021
40df3a3
add tests
adamsitnik Aug 18, 2021
378bfbe
add implementation
adamsitnik Aug 18, 2021
40cbe82
address code review feedback
adamsitnik Aug 19, 2021
9cdd21e
Update src/libraries/Microsoft.Extensions.Caching.Memory/tests/TokenE…
adamsitnik Aug 20, 2021
1f5d5a5
Merge remote-tracking branch 'upstream/main' into issue45593
adamsitnik Nov 19, 2021
2507c65
address code review feedback
adamsitnik Nov 19, 2021
50bdfb9
Apply suggestions from code review
adamsitnik Nov 19, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/Microsoft.Extensions.Caching.Memory/ref/Microsoft.Extensions.Caching.Memory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ protected virtual void Dispose(bool disposing) { }
~MemoryCache() { }
public void Remove(object key) { }
public bool TryGetValue(object key, out object result) { throw null; }
public void Clear() { }
}
public partial class MemoryCacheOptions : Microsoft.Extensions.Options.IOptions<Microsoft.Extensions.Caching.Memory.MemoryCacheOptions>
{
Expand Down
114 changes: 72 additions & 42 deletions src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,8 @@ public class MemoryCache : IMemoryCache
internal readonly ILogger _logger;

private readonly MemoryCacheOptions _options;
private readonly ConcurrentDictionary<object, CacheEntry> _entries;

private long _cacheSize;
private CoherentState _coherentState;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a case where we need volatile? My intuition tells me "no" because we only ever read the field once at the beginning of a method, and then use the local variable. But I wanted to ask the question, since I'm not an expert on when to use volatile.

@stephentoub ?

private bool _disposed;
private DateTimeOffset _lastExpirationScan;

Expand Down Expand Up @@ -56,7 +55,7 @@ public MemoryCache(IOptions<MemoryCacheOptions> optionsAccessor, ILoggerFactory
_options = optionsAccessor.Value;
_logger = loggerFactory.CreateLogger<MemoryCache>();

_entries = new ConcurrentDictionary<object, CacheEntry>();
_coherentState = new();

if (_options.Clock == null)
{
Expand All @@ -75,15 +74,13 @@ public MemoryCache(IOptions<MemoryCacheOptions> optionsAccessor, ILoggerFactory
/// <summary>
/// Gets the count of the current entries for diagnostic purposes.
/// </summary>
public int Count => _entries.Count;
public int Count => _coherentState.Count;

// internal for testing
internal long Size { get => Interlocked.Read(ref _cacheSize); }
internal long Size => _coherentState.Size;

internal bool TrackLinkedCacheEntries { get; }

private ICollection<KeyValuePair<object, CacheEntry>> EntriesCollection => _entries;

/// <inheritdoc />
public ICacheEntry CreateEntry(object key)
{
Expand Down Expand Up @@ -123,7 +120,8 @@ internal void SetEntry(CacheEntry entry)
// Initialize the last access timestamp at the time the entry is added
entry.LastAccessed = utcNow;

if (_entries.TryGetValue(entry.Key, out CacheEntry priorEntry))
CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime
if (coherentState._entries.TryGetValue(entry.Key, out CacheEntry priorEntry))
{
priorEntry.SetExpired(EvictionReason.Replaced);
}
Expand All @@ -133,41 +131,41 @@ internal void SetEntry(CacheEntry entry)
entry.InvokeEvictionCallbacks();
if (priorEntry != null)
{
RemoveEntry(priorEntry);
coherentState.RemoveEntry(priorEntry, _options);
}
StartScanForExpiredItemsIfNeeded(utcNow);
return;
}

bool exceedsCapacity = UpdateCacheSizeExceedsCapacity(entry);
bool exceedsCapacity = UpdateCacheSizeExceedsCapacity(entry, coherentState);
if (!exceedsCapacity)
{
bool entryAdded = false;

if (priorEntry == null)
{
// Try to add the new entry if no previous entries exist.
entryAdded = _entries.TryAdd(entry.Key, entry);
entryAdded = coherentState._entries.TryAdd(entry.Key, entry);
}
else
{
// Try to update with the new entry if a previous entries exist.
entryAdded = _entries.TryUpdate(entry.Key, entry, priorEntry);
entryAdded = coherentState._entries.TryUpdate(entry.Key, entry, priorEntry);

if (entryAdded)
{
if (_options.SizeLimit.HasValue)
{
// The prior entry was removed, decrease the by the prior entry's size
Interlocked.Add(ref _cacheSize, -priorEntry.Size.Value);
Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size.Value);
}
}
else
{
// The update will fail if the previous entry was removed after retrival.
// Adding the new entry will succeed only if no entry has been added since.
// This guarantees removing an old entry does not prevent adding a new entry.
entryAdded = _entries.TryAdd(entry.Key, entry);
entryAdded = coherentState._entries.TryAdd(entry.Key, entry);
}
}

Expand All @@ -180,7 +178,7 @@ internal void SetEntry(CacheEntry entry)
if (_options.SizeLimit.HasValue)
{
// Entry could not be added, reset cache size
Interlocked.Add(ref _cacheSize, -entry.Size.Value);
Interlocked.Add(ref coherentState._cacheSize, -entry.Size.Value);
}
entry.SetExpired(EvictionReason.Replaced);
entry.InvokeEvictionCallbacks();
Expand All @@ -198,7 +196,7 @@ internal void SetEntry(CacheEntry entry)
entry.InvokeEvictionCallbacks();
if (priorEntry != null)
{
RemoveEntry(priorEntry);
coherentState.RemoveEntry(priorEntry, _options);
}
}

Expand All @@ -213,7 +211,8 @@ public bool TryGetValue(object key, out object result)

DateTimeOffset utcNow = _options.Clock.UtcNow;

if (_entries.TryGetValue(key, out CacheEntry entry))
CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime
if (coherentState._entries.TryGetValue(key, out CacheEntry entry))
{
// Check if expired due to expiration tokens, timers, etc. and if so, remove it.
// Allow a stale Replaced value to be returned due to concurrent calls to SetExpired during SetEntry.
Expand All @@ -236,7 +235,7 @@ public bool TryGetValue(object key, out object result)
else
{
// TODO: For efficiency queue this up for batch removal
RemoveEntry(entry);
coherentState.RemoveEntry(entry, _options);
}
}

Expand All @@ -250,13 +249,14 @@ public bool TryGetValue(object key, out object result)
public void Remove(object key)
{
ValidateCacheKey(key);

CheckDisposed();
if (_entries.TryRemove(key, out CacheEntry entry))

CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime
if (coherentState._entries.TryRemove(key, out CacheEntry entry))
{
if (_options.SizeLimit.HasValue)
{
Interlocked.Add(ref _cacheSize, -entry.Size.Value);
Interlocked.Add(ref coherentState._cacheSize, -entry.Size.Value);
}

entry.SetExpired(EvictionReason.Removed);
Expand All @@ -266,22 +266,25 @@ public void Remove(object key)
StartScanForExpiredItemsIfNeeded(_options.Clock.UtcNow);
}

private void RemoveEntry(CacheEntry entry)
/// <summary>
/// Removes all keys and values from the cache.
/// </summary>
public void Clear()
{
if (EntriesCollection.Remove(new KeyValuePair<object, CacheEntry>(entry.Key, entry)))
CheckDisposed();

CoherentState oldState = Interlocked.Exchange(ref _coherentState, new CoherentState());
foreach (var entry in oldState._entries)
{
if (_options.SizeLimit.HasValue)
{
Interlocked.Add(ref _cacheSize, -entry.Size.Value);
}
entry.InvokeEvictionCallbacks();
entry.Value.SetExpired(EvictionReason.Removed);
entry.Value.InvokeEvictionCallbacks();
}
}

internal void EntryExpired(CacheEntry entry)
{
// TODO: For efficiency consider processing these expirations in batches.
RemoveEntry(entry);
_coherentState.RemoveEntry(entry, _options);
StartScanForExpiredItemsIfNeeded(_options.Clock.UtcNow);
}

Expand All @@ -307,18 +310,19 @@ private static void ScanForExpiredItems(MemoryCache cache)
{
DateTimeOffset now = cache._lastExpirationScan = cache._options.Clock.UtcNow;

foreach (KeyValuePair<object, CacheEntry> item in cache._entries)
CoherentState coherentState = cache._coherentState; // Clear() can update the reference in the meantime
foreach (KeyValuePair<object, CacheEntry> item in coherentState._entries)
{
CacheEntry entry = item.Value;

if (entry.CheckExpired(now))
{
cache.RemoveEntry(entry);
coherentState.RemoveEntry(entry, cache._options);
}
}
}

private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry)
private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CoherentState coherentState)
{
if (!_options.SizeLimit.HasValue)
{
Expand All @@ -328,7 +332,7 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry)
long newSize = 0L;
for (int i = 0; i < 100; i++)
{
long sizeRead = Interlocked.Read(ref _cacheSize);
long sizeRead = coherentState.Size;
newSize = sizeRead + entry.Size.Value;

if (newSize < 0 || newSize > _options.SizeLimit)
Expand All @@ -337,7 +341,7 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry)
return true;
}

if (sizeRead == Interlocked.CompareExchange(ref _cacheSize, newSize, sizeRead))
if (sizeRead == Interlocked.CompareExchange(ref coherentState._cacheSize, newSize, sizeRead))
{
return false;
}
Expand All @@ -356,17 +360,18 @@ private void TriggerOvercapacityCompaction()

private static void OvercapacityCompaction(MemoryCache cache)
{
long currentSize = Interlocked.Read(ref cache._cacheSize);
CoherentState coherentState = cache._coherentState; // Clear() can update the reference in the meantime
long currentSize = coherentState.Size;

cache._logger.LogDebug($"Overcapacity compaction executing. Current size {currentSize}");

double? lowWatermark = cache._options.SizeLimit * (1 - cache._options.CompactionPercentage);
if (currentSize > lowWatermark)
{
cache.Compact(currentSize - (long)lowWatermark, entry => entry.Size.Value);
cache.Compact(currentSize - (long)lowWatermark, entry => entry.Size.Value, coherentState);
}

cache._logger.LogDebug($"Overcapacity compaction executed. New size {Interlocked.Read(ref cache._cacheSize)}");
cache._logger.LogDebug($"Overcapacity compaction executed. New size {coherentState.Size}");
}

/// Remove at least the given percentage (0.10 for 10%) of the total entries (or estimated memory?), according to the following policy:
Expand All @@ -378,11 +383,12 @@ private static void OvercapacityCompaction(MemoryCache cache)
/// ?. Larger objects - estimated by object graph size, inaccurate.
public void Compact(double percentage)
{
int removalCountTarget = (int)(_entries.Count * percentage);
Compact(removalCountTarget, _ => 1);
CoherentState coherentState = _coherentState; // Clear() can update the reference in the meantime
int removalCountTarget = (int)(coherentState.Count * percentage);
Compact(removalCountTarget, _ => 1, coherentState);
}

private void Compact(long removalSizeTarget, Func<CacheEntry, long> computeEntrySize)
private void Compact(long removalSizeTarget, Func<CacheEntry, long> computeEntrySize, CoherentState coherentState)
{
var entriesToRemove = new List<CacheEntry>();
var lowPriEntries = new List<CacheEntry>();
Expand All @@ -392,7 +398,7 @@ 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)
foreach (KeyValuePair<object, CacheEntry> item in coherentState._entries)
{
CacheEntry entry = item.Value;
if (entry.CheckExpired(now))
Expand Down Expand Up @@ -427,7 +433,7 @@ private void Compact(long removalSizeTarget, Func<CacheEntry, long> computeEntry

foreach (CacheEntry entry in entriesToRemove)
{
RemoveEntry(entry);
coherentState.RemoveEntry(entry, _options);
}

// Policy:
Expand Down Expand Up @@ -500,5 +506,29 @@ private static void ValidateCacheKey(object key)

static void Throw() => throw new ArgumentNullException(nameof(key));
}

private class CoherentState
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved
{
internal ConcurrentDictionary<object, CacheEntry> _entries = new ConcurrentDictionary<object, CacheEntry>();
internal long _cacheSize;

private ICollection<KeyValuePair<object, CacheEntry>> EntriesCollection => _entries;
adamsitnik marked this conversation as resolved.
Show resolved Hide resolved

internal int Count => _entries.Count;

internal long Size => Interlocked.Read(ref _cacheSize);

internal void RemoveEntry(CacheEntry entry, MemoryCacheOptions options)
{
if (EntriesCollection.Remove(new KeyValuePair<object, CacheEntry>(entry.Key, entry)))
{
if (options.SizeLimit.HasValue)
{
Interlocked.Add(ref _cacheSize, -entry.Size.Value);
}
entry.InvokeEvictionCallbacks();
}
}
}
}
}
14 changes: 14 additions & 0 deletions src/libraries/Microsoft.Extensions.Caching.Memory/tests/CapacityTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,5 +444,19 @@ public void NoCompactionWhenNoMaximumEntriesCountSpecified()
// There should be 6 items in the cache
Assert.Equal(6, cache.Count);
}

[Fact]
public void ClearZeroesTheSize()
{
var cache = new MemoryCache(new MemoryCacheOptions { SizeLimit = 10 });
Assert.Equal(0, cache.Size);

cache.Set("key", "value", new MemoryCacheEntryOptions { Size = 5 });
Assert.Equal(5, cache.Size);

cache.Clear();
Assert.Equal(0, cache.Size);
Assert.Equal(0, cache.Count);
}
}
}
Loading