diff --git a/src/libraries/System.Runtime.Caching/src/System.Runtime.Caching.csproj b/src/libraries/System.Runtime.Caching/src/System.Runtime.Caching.csproj index 5502ad3fd33fce..f63c97cdecfb49 100644 --- a/src/libraries/System.Runtime.Caching/src/System.Runtime.Caching.csproj +++ b/src/libraries/System.Runtime.Caching/src/System.Runtime.Caching.csproj @@ -72,6 +72,7 @@ System.Runtime.Caching.ObjectCache + diff --git a/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs b/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs index 8e8d65117bf950..6da5d4675e8849 100644 --- a/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs +++ b/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs @@ -3,7 +3,10 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Diagnostics.Tracing; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; using System.Runtime.Versioning; using System.Threading; @@ -19,7 +22,42 @@ internal sealed class Counters : EventSource private const int NUM_COUNTERS = 7; private DiagnosticCounter[] _counters; - private long[] _counterValues; + + // Backing storage for the raw counter values. + // + // These are updated with Interlocked ops on every cache Get/Add/Remove, so the layout matters: + // + // 1. Named fields instead of a long[]. Indexing an array forces the JIT to emit a bounds check, + // which loads the array's length field. That length lives in the same cache line as the + // elements of a small array, so every increment performed a plain load of a line that is + // simultaneously the target of a contended atomic RMW. That defeats the "far atomic" handling + // LSE-capable hardware uses to keep contended counters resident at the shared cache, and turns + // each increment into a cache-line migration. Struct fields have no length to load. + // + // 2. Counters are grouped onto cache lines by how MemoryCacheStore actually updates them, so that + // unrelated operations running concurrently do not falsely share a line. Counters that are + // always bumped by the *same* operation stay together, because splitting those would force one + // operation to acquire several contended lines with fully-ordered atomics back to back: + // - Entries + Turnover: always bumped together by Add()/RemoveFromCache() + // - Hits: bumped by a Get() that hits + // - Misses: bumped by a Get() that misses + // - Trims: bumped in batches by the (rare) trim path + private CounterValues _counterValues; + + private const int CacheLineSize = Internal.PaddingHelpers.CACHE_LINE_SIZE; + + [StructLayout(LayoutKind.Explicit, Size = CacheLineSize * 4)] + private struct CounterValues + { + [FieldOffset(CacheLineSize * 0)] public long Entries; + [FieldOffset(CacheLineSize * 0 + 8)] public long Turnover; + + [FieldOffset(CacheLineSize * 1)] public long Hits; + + [FieldOffset(CacheLineSize * 2)] public long Misses; + + [FieldOffset(CacheLineSize * 3)] public long Trims; + } internal Counters(string cacheName) : base(EVENT_SOURCE_NAME_ROOT + (cacheName ?? throw new ArgumentNullException(nameof(cacheName)))) { @@ -33,23 +71,31 @@ private void InitDisposableMembers() try { _counters = new DiagnosticCounter[NUM_COUNTERS]; - _counterValues = new long[NUM_COUNTERS]; - _counters[(int)CounterName.Entries] = CreatePollingCounter("entries", "Cache Entries", (int)CounterName.Entries); - _counters[(int)CounterName.Hits] = CreatePollingCounter("hits", "Cache Hits", (int)CounterName.Hits); - _counters[(int)CounterName.Misses] = CreatePollingCounter("misses", "Cache Misses", (int)CounterName.Misses); - _counters[(int)CounterName.Trims] = CreatePollingCounter("trims", "Cache Trims", (int)CounterName.Trims); + _counters[(int)CounterName.Entries] = CreatePollingCounter("entries", "Cache Entries", () => Interlocked.Read(ref _counterValues.Entries)); + _counters[(int)CounterName.Hits] = CreatePollingCounter("hits", "Cache Hits", () => Interlocked.Read(ref _counterValues.Hits)); + _counters[(int)CounterName.Misses] = CreatePollingCounter("misses", "Cache Misses", () => Interlocked.Read(ref _counterValues.Misses)); + _counters[(int)CounterName.Trims] = CreatePollingCounter("trims", "Cache Trims", () => Interlocked.Read(ref _counterValues.Trims)); _counters[(int)CounterName.Turnover] = new IncrementingPollingCounter("turnover", this, - () => (double)_counterValues[(int)CounterName.Turnover]) + () => Interlocked.Read(ref _counterValues.Turnover)) { DisplayName = "Cache Turnover Rate", }; - // This two-step dance with hit-ratio was an old perf-counter artifact. There only needs - // to be one polling counter here, rather than the two-part perf counter. Still keeping array - // indexes and raw counter values consistent between NetFx and Core code though. + // This two-step dance with hit-ratio was an old perf-counter artifact: the ratio used to be + // tracked as a pair of raw counters (HitRatio, incremented on every hit, and HitRatioBase, + // incremented on every hit and every miss). Neither raw value is observable - only the + // percentage computed below is - and they are exactly redundant with Hits and Hits + Misses. + // Deriving the ratio lets the Get() hot path do a single Interlocked op instead of three. + // 0 hits and 0 misses still yields NaN, as it did with the raw counters. Hits is read once + // and reused for both the numerator and the denominator, so unlike the separate + // HitRatio/HitRatioBase reads the result can never transiently exceed 100%. _counters[(int)CounterName.HitRatio] = new PollingCounter("hit-ratio", this, - () => ((double)_counterValues[(int)CounterName.HitRatio] / (double)_counterValues[(int)CounterName.HitRatioBase]) * 100d) + () => + { + double hits = Interlocked.Read(ref _counterValues.Hits); + return (hits / (hits + Interlocked.Read(ref _counterValues.Misses))) * 100d; + }) { DisplayName = "Cache Hit Ratio", }; @@ -64,9 +110,9 @@ private void InitDisposableMembers() } } - private PollingCounter CreatePollingCounter(string name, string displayName, int counterIndex) + private PollingCounter CreatePollingCounter(string name, string displayName, Func getValue) { - return new PollingCounter(name, this, () => (double)_counterValues[counterIndex]) + return new PollingCounter(name, this, getValue) { DisplayName = displayName, }; @@ -86,21 +132,25 @@ private PollingCounter CreatePollingCounter(string name, string displayName, int } } - internal void Increment(CounterName name) - { - int idx = (int)name; - Interlocked.Increment(ref _counterValues[idx]); - } - internal void IncrementBy(CounterName name, long value) - { - int idx = (int)name; - Interlocked.Add(ref _counterValues[idx], value); - } - internal void Decrement(CounterName name) + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private ref long GetCounterRef(CounterName name) { - int idx = (int)name; - Interlocked.Decrement(ref _counterValues[idx]); + switch (name) + { + case CounterName.Entries: return ref _counterValues.Entries; + case CounterName.Hits: return ref _counterValues.Hits; + case CounterName.Misses: return ref _counterValues.Misses; + case CounterName.Trims: return ref _counterValues.Trims; + case CounterName.Turnover: return ref _counterValues.Turnover; + default: throw new UnreachableException(); + } } + + internal void Increment(CounterName name) => Interlocked.Increment(ref GetCounterRef(name)); + + internal void IncrementBy(CounterName name, long value) => Interlocked.Add(ref GetCounterRef(name), value); + + internal void Decrement(CounterName name) => Interlocked.Decrement(ref GetCounterRef(name)); #else #pragma warning disable CA1822, IDE0060 internal Counters(string cacheName) diff --git a/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStore.cs b/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStore.cs index eb5ccb6bdedadb..8f91939a32d959 100644 --- a/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStore.cs +++ b/src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStore.cs @@ -147,8 +147,6 @@ internal void UpdateExpAndUsage(MemoryCacheEntry entry, bool updatePerfCounters if (updatePerfCounters && _perfCounters != null && _countersSupported) { _perfCounters.Increment(CounterName.Hits); - _perfCounters.Increment(CounterName.HitRatio); - _perfCounters.Increment(CounterName.HitRatioBase); } } else @@ -156,7 +154,6 @@ internal void UpdateExpAndUsage(MemoryCacheEntry entry, bool updatePerfCounters if (updatePerfCounters && _perfCounters != null && _countersSupported) { _perfCounters.Increment(CounterName.Misses); - _perfCounters.Increment(CounterName.HitRatioBase); } } }