Skip to content

Improve MemoryCache performance under high contention - #131470

Open
EgorBo wants to merge 4 commits into
dotnet:mainfrom
EgorBo:memorycache-fix
Open

Improve MemoryCache performance under high contention#131470
EgorBo wants to merge 4 commits into
dotnet:mainfrom
EgorBo:memorycache-fix

Conversation

@EgorBo

@EgorBo EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member

Improves perf of a 1P's benchmark by 6x on win-arm64 (their setup): EgorBot/Benchmarks#417 (comment)

Results for windows_azure_cobalt100

BenchmarkDotNet v0.16.0-preview.1, Windows 11 (10.0.26200.8875/25H2/2025Update/HudsonValley2) (Hyper-V)
Cobalt 100 3.40GHz, 1 CPU, 16 logical and 16 physical cores
Memory: 63.99 GB Total, 56.96 GB Available
.NET SDK 11.0.100-preview.7.26366.102
  [Host]     : .NET 11.0.0 (11.0.0-preview.7.26366.102, 11.0.26.36702), Arm64 RyuJIT armv8.0-a

Method Toolchain Mean Error Ratio
Increment_Concurrent_T32 main 60.030 ns 0.2483 ns 6.15
Increment_Concurrent_T32 PR131470 9.766 ns 0.0228 ns 1.00

Alternative benchmark - improvements should be bigger on many-cores systems, I only tested on 8 cores (SMT2 mostly).

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates System.Runtime.Caching’s internal counter implementation to reduce contention in hot cache paths by (1) removing redundant hit-ratio bookkeeping from MemoryCacheStore and (2) changing Counters to use cache-line-separated fields instead of a small long[] backing store.

Changes:

  • Stop incrementing HitRatio / HitRatioBase counters on hits/misses; derive hit ratio from Hits and Misses instead.
  • Replace the long[] counter storage with an explicitly-laid-out, cache-line-padded struct and switch-based Interlocked updates.
  • Add the shared Internal.PaddingHelpers implementation to the project to get the cache-line size constant.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/MemoryCacheStore.cs Removes redundant hit-ratio counter increments from the Get() hot path.
src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs Reworks counter storage/layout to reduce false sharing and derives hit ratio from hits/misses.
src/libraries/System.Runtime.Caching/src/System.Runtime.Caching.csproj Links in Internal/Padding.cs to support cache-line padding constants used by Counters.
Comments suppressed due to low confidence (2)

src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs:80

  • IncrementingPollingCounter for turnover currently reads _counterValues.Turnover with a plain load while updates use Interlocked.Increment/Add. Use Interlocked.Read here as well to avoid torn reads and to match the established EventCounters pattern.
                _counters[(int)CounterName.Turnover] = new IncrementingPollingCounter("turnover", this,
                    () => _counterValues.Turnover)
                {

src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs:96

  • Hit ratio computation reads _counterValues.Hits / .Misses via plain loads. Because these are 64-bit fields updated via Interlocked, use Interlocked.Read to avoid torn reads (especially on 32-bit) and to prevent occasional nonsensical ratios from partially-read values.
                    () =>
                    {
                        double hits = _counterValues.Hits;
                        return (hits / (hits + _counterValues.Misses)) * 100d;
                    })

Comment thread src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs Outdated
@EgorBo

EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@EgorBot -macos_arm -linux_genoa -windows_arm -windows_intel

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.Caching;
using System.Threading.Tasks;

namespace MemoryCacheTests
{
    public class PerfCounterConcurrencyTests
    {
        private const int ThreadCount = 32;
        private const int TotalOperations = 256_000;

        private PerfCounterHarness _harness;
        private Action _increment;

        [GlobalSetup]
        public void Setup()
        {
            _harness = PerfCounterHarness.Create();
            _increment = _harness.Increment;
        }

        [GlobalCleanup]
        public void Cleanup() => _harness?.Dispose();

        [Benchmark(OperationsPerInvoke = TotalOperations)]
        public void Increment_Concurrent_T32()
        {
            const int operationsPerThread = TotalOperations / ThreadCount;
            Parallel.For(0, ThreadCount, new ParallelOptions { MaxDegreeOfParallelism = ThreadCount }, _ =>
            {
                for (int i = 0; i < operationsPerThread; i++)
                    _increment();
            });
        }
    }

    internal sealed class PerfCounterHarness : IDisposable
    {
        private readonly Action _dispose;

        private PerfCounterHarness(Action increment, Action dispose)
        {
            Increment = increment;
            _dispose = dispose;
        }

        public Action Increment { get; }

        public static PerfCounterHarness Create()
        {
            Assembly cachingAssembly = typeof(MemoryCache).Assembly;
            Type countersType = cachingAssembly.GetType("System.Runtime.Caching.Counters")
              ?? throw new InvalidOperationException("Could not locate the internal cache performance-counters type.");
            Type counterNameType = cachingAssembly.GetType("System.Runtime.Caching.CounterName")
              ?? throw new InvalidOperationException("Could not locate the internal counter-name enum type.");
            ConstructorInfo ctor = countersType.GetConstructor(
              BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public,
              binder: null,
              types: new[] { typeof(string) },
              modifiers: null)
              ?? throw new InvalidOperationException("Could not find the performance-counters constructor.");
            object counters = ctor.Invoke(new object[] { "MemoryCacheTests.PerfCounterTests" });
            object counterName = Enum.Parse(counterNameType, "Entries");
            Action increment = BuildCounterAction(counters, countersType, counterNameType, "Increment", counterName);
            Action dispose = BuildDisposeAction(counters, countersType);
            return new PerfCounterHarness(increment, dispose);
        }

        public void Dispose() => _dispose?.Invoke();

        private static Action BuildCounterAction(
          object instance,
          Type countersType,
          Type counterNameType,
          string methodName,
          object counterName)
        {
            MethodInfo method = countersType.GetMethod(
              methodName,
              BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public,
              binder: null,
              types: new[] { counterNameType },
              modifiers: null)
              ?? throw new InvalidOperationException($"Could not find method '{methodName}' on {countersType.FullName}.");
            MethodCallExpression call = Expression.Call(
              Expression.Constant(instance, countersType),
              method,
              Expression.Constant(counterName, counterNameType));
            return Expression.Lambda<Action>(call).Compile();
        }

        private static Action BuildDisposeAction(object instance, Type countersType)
        {
            // Prefer the type's own Dispose() (Counters declares a `new` Dispose that releases the counters).
            MethodInfo dispose = countersType.GetMethod("Dispose",
                BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly,
                binder: null,
                types: Type.EmptyTypes,
                modifiers: null)
              ?? countersType.GetMethod("Dispose",
                BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic,
                binder: null, types: Type.EmptyTypes, modifiers: null);
            if (dispose == null)
                return static () => { };
            MethodCallExpression call = Expression.Call(Expression.Constant(instance, countersType), dispose);
            return Expression.Lambda<Action>(call).Compile();
        }
    }
}

Copilot AI review requested due to automatic review settings July 28, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs:48

  • This PR is a pure performance change on the hot-path counter updates (extra padding + fewer Interlocked ops). Per repo review guidance for optimizations, please include concrete benchmark evidence in the PR description (scenario, hardware/OS/arch, before/after numbers, and whether any regressions were observed). Without that, it’s hard to validate the cost/benefit tradeoff (notably the increased object size due to cache-line padding) and ensure the claimed 2–5× improvement is reproducible.
        // 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)]

@EgorBo

EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

@EgorBot -macos_arm -linux_genoa -windows_arm -windows_intel -profiler

using System;
using System.Runtime.Caching;
using System.Threading;
using System.Threading.Tasks;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

BenchmarkSwitcher.FromAssembly(typeof(MemoryCacheBench).Assembly).Run(args);

public class MemoryCacheBench
{
    // MemoryCache shards its entries into several stores so the per-store locks stay scalable, but all
    // of those stores share a single Counters instance, which makes the counters the only process-wide
    // serialization point on the Get/Set paths. A representative workload therefore needs many keys (so
    // the per-store locks are not themselves the bottleneck) and several threads (so the shared counters
    // are actually contended). Only the public API is used, so this is what an application really pays.
    //
    // WritePercent sweeps from a pure read cache to a write-heavy one, because how much the counters
    // matter depends entirely on how much other work each operation does.

    private const int KeyCount = 4096;
    private const int OpsPerThread = 25_000;

    [Params(0, 5, 25)]
    public int WritePercent { get; set; }

    private const int MissPercent = 7;

    private MemoryCache _cache;
    private string[] _present;
    private string[] _absent;
    private CacheItemPolicy _policy;
    private ParallelOptions _parallel;
    private object _value;
    private int _threads;

    [GlobalSetup]
    public void Setup()
    {
        _threads = Environment.ProcessorCount;
        ThreadPool.SetMinThreads(_threads + 8, _threads + 8);
        _parallel = new ParallelOptions { MaxDegreeOfParallelism = _threads };
        _value = "cached-value";
        _policy = new CacheItemPolicy { AbsoluteExpiration = DateTimeOffset.UtcNow.AddHours(1) };
        _cache = new MemoryCache("bench");
        _present = new string[KeyCount];
        _absent = new string[KeyCount];
        for (int i = 0; i < KeyCount; i++)
        {
            _present[i] = "key-" + i;
            _absent[i] = "missing-" + i;
            _cache.Set(_present[i], _value, _policy);
        }
    }

    [GlobalCleanup]
    public void Cleanup() => _cache.Dispose();

    // Reported time is the per-operation latency seen by one thread while all threads run.
    // A Set over an existing key replaces it, so writes also exercise the eviction counters.
    [Benchmark(OperationsPerInvoke = OpsPerThread)]
    public void CacheOps()
    {
        MemoryCache cache = _cache;
        string[] present = _present, absent = _absent;
        CacheItemPolicy policy = _policy;
        object value = _value;
        int writePct = WritePercent;
        int readMissPct = writePct + MissPercent;

        Parallel.For(0, _threads, _parallel, t =>
        {
            // Per-thread xorshift so threads walk the key space independently instead of
            // marching in lockstep over the same entry.
            uint x = (uint)(t + 1) * 2654435761u | 1u;

            for (int i = 0; i < OpsPerThread; i++)
            {
                x ^= x << 13;
                x ^= x >> 17;
                x ^= x << 5;

                // Take the key from the high bits so it is independent of the op selector.
                int k = (int)((x >> 16) % KeyCount);
                uint op = x % 100;
                if (op < writePct)
                    cache.Set(present[k], value, policy); // evict + insert
                else if (op < readMissPct)
                    cache.Get(absent[k]); // miss
                else
                    cache.Get(present[k]); // hit
            }
        });
    }
}

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@EgorBo

EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Not sure who should I request review from here, perhaps @StephenMolloy ?

@JulieLeeMSFT

Copy link
Copy Markdown
Member

@StephenMolloy, @mangod9, @brianrob, please review with a high priority. It is blocking 1P.

@JulieLeeMSFT JulieLeeMSFT added the Priority:0 Work that we can't release without label Jul 28, 2026
@JulieLeeMSFT JulieLeeMSFT added this to the 11.0.0 milestone Jul 28, 2026

@mangod9 mangod9 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

so this has no measurable improvement on x64?

@EgorBo

EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

so this has no measurable improvement on x64?

20-100% improvement on x64 as well for this benchmark #131470 (comment)

For initial 1P Benchmark the improvement is 10-20% on x64, presumably more on many cores system (in 1P's case that was 32 cores VM). In my case that was 4 physical cores with HP/SMT2 (=8 vcores) - sibling cores on SMT2 suffer less from these cache issues.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: c92365ca-6264-404e-954d-6dcbaf06e698
Copilot AI review requested due to automatic review settings July 28, 2026 23:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

src/libraries/System.Runtime.Caching/src/System/Runtime/Caching/Counters.cs:54

  • Avoid the magic constant 8 in the FieldOffset expression; using sizeof(long) makes the intent clear and remains a compile-time constant usable in attributes.
            [FieldOffset(CacheLineSize * 0)] public long Entries;
            [FieldOffset(CacheLineSize * 0 + 8)] public long Turnover;

Copilot AI review requested due to automatic review settings July 28, 2026 23:44
@EgorBo

EgorBo commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Pushed a small clean up to remove duplicated code.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

@EgorBo

EgorBo commented Jul 29, 2026

Copy link
Copy Markdown
Member Author

@EgorBot -windows_arm -linux_genoa -linux_arm -profiler

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Linq.Expressions;
using System.Reflection;
using System.Runtime.Caching;
using System.Threading.Tasks;

namespace MemoryCacheTests
{
    public class PerfCounterConcurrencyTests
    {
        private const int ThreadCount = 32;
        private const int TotalOperations = 256_000;

        private PerfCounterHarness _harness;
        private Action _increment;

        [GlobalSetup]
        public void Setup()
        {
            _harness = PerfCounterHarness.Create();
            _increment = _harness.Increment;
        }

        [GlobalCleanup]
        public void Cleanup() => _harness?.Dispose();

        [Benchmark(OperationsPerInvoke = TotalOperations)]
        public void Increment_Concurrent_T32()
        {
            const int operationsPerThread = TotalOperations / ThreadCount;
            Parallel.For(0, ThreadCount, new ParallelOptions { MaxDegreeOfParallelism = ThreadCount }, _ =>
            {
                for (int i = 0; i < operationsPerThread; i++)
                    _increment();
            });
        }
    }

    internal sealed class PerfCounterHarness : IDisposable
    {
        private readonly Action _dispose;

        private PerfCounterHarness(Action increment, Action dispose)
        {
            Increment = increment;
            _dispose = dispose;
        }

        public Action Increment { get; }

        public static PerfCounterHarness Create()
        {
            Assembly cachingAssembly = typeof(MemoryCache).Assembly;
            Type countersType = cachingAssembly.GetType("System.Runtime.Caching.Counters")
              ?? throw new InvalidOperationException("Could not locate the internal cache performance-counters type.");
            Type counterNameType = cachingAssembly.GetType("System.Runtime.Caching.CounterName")
              ?? throw new InvalidOperationException("Could not locate the internal counter-name enum type.");
            ConstructorInfo ctor = countersType.GetConstructor(
              BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public,
              binder: null,
              types: new[] { typeof(string) },
              modifiers: null)
              ?? throw new InvalidOperationException("Could not find the performance-counters constructor.");
            object counters = ctor.Invoke(new object[] { "MemoryCacheTests.PerfCounterTests" });
            object counterName = Enum.Parse(counterNameType, "Entries");
            Action increment = BuildCounterAction(counters, countersType, counterNameType, "Increment", counterName);
            Action dispose = BuildDisposeAction(counters, countersType);
            return new PerfCounterHarness(increment, dispose);
        }

        public void Dispose() => _dispose?.Invoke();

        private static Action BuildCounterAction(
          object instance,
          Type countersType,
          Type counterNameType,
          string methodName,
          object counterName)
        {
            MethodInfo method = countersType.GetMethod(
              methodName,
              BindingFlags.Instance | BindingFlags.NonPublic | BindingFlags.Public,
              binder: null,
              types: new[] { counterNameType },
              modifiers: null)
              ?? throw new InvalidOperationException($"Could not find method '{methodName}' on {countersType.FullName}.");
            MethodCallExpression call = Expression.Call(
              Expression.Constant(instance, countersType),
              method,
              Expression.Constant(counterName, counterNameType));
            return Expression.Lambda<Action>(call).Compile();
        }

        private static Action BuildDisposeAction(object instance, Type countersType)
        {
            // Prefer the type's own Dispose() (Counters declares a `new` Dispose that releases the counters).
            MethodInfo dispose = countersType.GetMethod("Dispose",
                BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.DeclaredOnly,
                binder: null,
                types: Type.EmptyTypes,
                modifiers: null)
              ?? countersType.GetMethod("Dispose",
                BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic,
                binder: null, types: Type.EmptyTypes, modifiers: null);
            if (dispose == null)
                return static () => { };
            MethodCallExpression call = Expression.Call(Expression.Constant(instance, countersType), dispose);
            return Expression.Lambda<Action>(call).Compile();
        }
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Runtime.Caching Priority:0 Work that we can't release without

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants