diff --git a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs index 51b5c3fe523e64..2c21f411b32fdd 100644 --- a/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs +++ b/src/libraries/Microsoft.Extensions.Caching.Memory/src/MemoryCache.cs @@ -186,8 +186,8 @@ internal void SetEntry(CacheEntry entry) // exactly once, tied to the swap we performed. Doing this speculatively // inside UpdateCacheSizeExceedsCapacity (before the swap) races with a // concurrent RemoveEntry of the prior entry and double-counts the decrement, - // drifting _cacheSize negative and permanently blocking all future inserts. - Interlocked.Add(ref coherentState._cacheSize, -priorEntry.Size); + // drifting CacheSize negative and permanently blocking all future inserts. + Interlocked.Add(ref coherentState.CacheSize, -priorEntry.Size); } } else @@ -209,7 +209,7 @@ internal void SetEntry(CacheEntry entry) if (_options.HasSizeLimit) { // Entry could not be added, roll back the size increment for this entry only. - Interlocked.Add(ref coherentState._cacheSize, -entry.Size); + Interlocked.Add(ref coherentState.CacheSize, -entry.Size); } entry.SetExpired(EvictionReason.Replaced); entry.InvokeEvictionCallbacks(); @@ -361,7 +361,7 @@ public void Remove(object key) { if (_options.HasSizeLimit) { - Interlocked.Add(ref coherentState._cacheSize, -entry.Size); + Interlocked.Add(ref coherentState.CacheSize, -entry.Size); } entry.SetExpired(EvictionReason.Removed); @@ -554,11 +554,11 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CacheEntry? priorE { // The capacity decision still accounts for the prior entry being replaced (its size is // freed by the replace), so a same-or-smaller replacement at the size limit is admitted. - // However, only the new entry's size is committed to _cacheSize here. The prior entry's + // However, only the new entry's size is committed to CacheSize here. The prior entry's // size is decremented by the caller, atomically with the dictionary swap that actually // removes it. Decrementing the prior size here (before the swap) races with a concurrent // RemoveEntry of the same prior entry and double-counts the decrement, drifting - // _cacheSize negative and permanently blocking all future inserts. + // CacheSize negative and permanently blocking all future inserts. long sizeAfterReplace = sizeRead + entry.Size - priorSize; if ((ulong)sizeAfterReplace > (ulong)sizeLimit) @@ -568,7 +568,7 @@ private bool UpdateCacheSizeExceedsCapacity(CacheEntry entry, CacheEntry? priorE } long committedSize = sizeRead + entry.Size; - long original = Interlocked.CompareExchange(ref coherentState._cacheSize, committedSize, sizeRead); + long original = Interlocked.CompareExchange(ref coherentState.CacheSize, committedSize, sizeRead); if (sizeRead == original) { return false; @@ -798,7 +798,21 @@ public CoherentState() } #endif - internal long _cacheSize; + // CacheSize is Interlocked-updated on every Set/Remove when SizeLimit is set, while + // _stringEntries/_nonStringEntries are read on every TryGetValue; the padding keeps the + // write-hot atomic off the line holding those read-mostly references. It has to live in + // a struct -- layout attributes on a class only affect marshaling, and the runtime + // reorders class fields freely. 64 is the size of a cache line on many systems; larger + // ones may see a little more false sharing. + private CacheSizePadded _cacheSizePadded; + + [StructLayout(LayoutKind.Explicit, Size = 128)] + private struct CacheSizePadded + { + [FieldOffset(64)] public long Value; + } + + internal ref long CacheSize => ref _cacheSizePadded.Value; internal bool TryGetValue(object key, [NotNullWhen(true)] out CacheEntry? entry) => key is string s ? _stringEntries.TryGetValue(s, out entry) : _nonStringEntries.TryGetValue(key, out entry); @@ -848,7 +862,7 @@ public IEnumerable GetAllKeys() internal int Count => _stringEntries.Count + _nonStringEntries.Count; - internal long Size => Volatile.Read(ref _cacheSize); + internal long Size => Volatile.Read(ref CacheSize); internal bool RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { @@ -862,7 +876,7 @@ internal bool RemoveEntry(CacheEntry entry, MemoryCacheOptions options) { if (options.HasSizeLimit) { - Interlocked.Add(ref _cacheSize, -entry.Size); + Interlocked.Add(ref CacheSize, -entry.Size); } entry.InvokeEvictionCallbacks(); return true;