AsyncLocal Memory Usage in MemoryCache

[Tornhoof]

Description

I recently analyzed a few memory dumps of an .NET 5.0 application which makes heavy use of MemoryCache. Two things, I could not really explain, stood out:

MemoryCacheEntryOptions

For each new cache entry a new MemoryCacheEntryOptions object was created, but at least for the options used by the app it was not necessary to create a new object for each new cache entry, so it was removed and replaced by directly setting the appropriate values on cacheEntry (or using the extension methods). Maybe it would be useful to enhance the docs to point to the extension methods. The examples in https://docs.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-5.0 create MemoryCacheEntryOptions most of the time instead of using the extension methods from https://github.com/dotnet/runtime/blob/a2f81e7258d2963af52d4441e891025aa46a1fc3/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheExtensions.cs. I guess that's the reason why a new options object was created each time.

Memory Usage of AsyncLocal in CacheEntry

This is the more interesting one, the memory usage of System.Threading.AsyncLocalValueMap+ThreeElementAsyncLocalValueMap for the ExecutionContext was actually the third largest memory allocation behind string and byte[]. See Screenshot.
(this is from the real dump, it is not just 1 request, but a batch of them, probably around 400 or 500)

I wrote a short repro which shows the problem:

1. Run the repro
2. Record the memory usage (I used JetBrains dotMemory) after opening the browser at e.g. http://localhost:5000
3. Look at the memory dump, it should look like the example below (depends on the cpu count a bit)

using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace CacheEntryRepro
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var builder = Host.CreateDefaultBuilder().ConfigureWebHostDefaults(whb =>
                whb.ConfigureServices(s => s.AddMemoryCache())
                    .Configure(app =>
                    {
                        app.UseRouting();
                        app.UseEndpoints(erb => { erb.MapGet("/", hc => RequestDelegate(hc)); });
                    }));
            await builder.Build().RunAsync();
        }

        private static async Task RequestDelegate(HttpContext context)
        {
            var cache = context.RequestServices.GetRequiredService<IMemoryCache>();
            int processors = Environment.ProcessorCount;
            var tasks = new Task[processors];
            for (int i = 0; i < 1000; i++)
            {
                for (int j = 0; j < processors; j++)
                {
                    tasks[j] = AddCacheEntry(cache);
                }

                await Task.WhenAll(tasks);
            }

            context.Response.StatusCode = StatusCodes.Status200OK;
            await context.Response.StartAsync();
            await context.Response.WriteAsync("Done");
            await context.Response.CompleteAsync();
        }

        private static Task AddCacheEntry(IMemoryCache cache)
        {
            var guid = Guid.NewGuid();
            using var entry = cache.CreateEntry(guid.ToString());
            entry.Size = 1;
            entry.SlidingExpiration = TimeSpan.FromMinutes(1);
            entry.Value = Guid.NewGuid();
            return Task.CompletedTask;
        }
    }
}

This is partially related to the PR #45280 from @adamsitnik as he specifically lists that AsyncLocal is expensive and he optimized the codepaths related to some of it. I have not tested his PRs if they help with the problem above, he suggested on twitter I should create an issue with a repro.

[ghost]

Tagging subscribers to this area: @eerhardt, @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Description

I recently analyzed a few memory dumps of an .NET 5.0 application which makes heavy use of MemoryCache. Two things, I could not really explain, stood out:

MemoryCacheEntryOptions

For each new cache entry a new MemoryCacheEntryOptions object was created, but at least for the options used by the app it was not necessary to create a new object for each new cache entry, so it was removed and replaced by directly setting the appropriate values on cacheEntry (or using the extension methods). Maybe it would be useful to enhance the docs to point to the extension methods. The examples in https://docs.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-5.0 create MemoryCacheEntryOptions most of the time instead of using the extension methods from https://github.com/dotnet/runtime/blob/a2f81e7258d2963af52d4441e891025aa46a1fc3/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/MemoryCacheExtensions.cs. I guess that's the reason why a new options object was created each time.

Memory Usage of AsyncLocal in CacheEntry

This is the more interesting one, the memory usage of System.Threading.AsyncLocalValueMap+ThreeElementAsyncLocalValueMap for the ExecutionContext was actually the third largest memory allocation behind string and byte[]. See Screenshot.
(this is from the real dump, it is not just 1 request, but a batch of them, probably around 400 or 500)

I wrote a short repro which shows the problem:

1. Run the repro
2. Record the memory usage (I used JetBrains dotMemory) after opening the browser at e.g. http://localhost:5000
3. Look at the memory dump, it should look like the example below (depends on the cpu count a bit)

using System;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Hosting;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Hosting;

namespace CacheEntryRepro
{
    class Program
    {
        static async Task Main(string[] args)
        {
            var builder = Host.CreateDefaultBuilder().ConfigureWebHostDefaults(whb =>
                whb.ConfigureServices(s => s.AddMemoryCache())
                    .Configure(app =>
                    {
                        app.UseRouting();
                        app.UseEndpoints(erb => { erb.MapGet("/", hc => RequestDelegate(hc)); });
                    }));
            await builder.Build().RunAsync();
        }

        private static async Task RequestDelegate(HttpContext context)
        {
            var cache = context.RequestServices.GetRequiredService<IMemoryCache>();
            int processors = Environment.ProcessorCount;
            var tasks = new Task[processors];
            for (int i = 0; i < 1000; i++)
            {
                for (int j = 0; j < processors; j++)
                {
                    tasks[j] = AddCacheEntry(cache);
                }

                await Task.WhenAll(tasks);
            }

            context.Response.StatusCode = StatusCodes.Status200OK;
            await context.Response.StartAsync();
            await context.Response.WriteAsync("Done");
            await context.Response.CompleteAsync();
        }

        private static Task AddCacheEntry(IMemoryCache cache)
        {
            var guid = Guid.NewGuid();
            using var entry = cache.CreateEntry(guid.ToString());
            entry.Size = 1;
            entry.SlidingExpiration = TimeSpan.FromMinutes(1);
            entry.Value = Guid.NewGuid();
            return Task.CompletedTask;
        }
    }
}

This is partially related to the PR #45280 from @adamsitnik as he specifically lists that AsyncLocal is expensive and he optimized the codepaths related to some of it. I have not tested his PRs if they help with the problem above, he suggested on twitter I should create an issue with a repro.

Author:	Tornhoof
Assignees:	-
Labels:	area-Extensions-Caching, tenet-performance, untriaged
Milestone:	-

[adamsitnik]

@Tornhoof thanks for a great repro!

#45280 has not removed this particular allocation, it's caused by the creation of scope when new CacheEntry is being created:

I need to wrap my head around it and see what we can do about it

[adamsitnik]

It seems that the parent scope entry settings are always passed to child scope entries for cases like this:

using (var entry = cache.CreateEntry(key))
{
    entry.SetValue(obj);

    using (var entry1 = cache.CreateEntry(key1))
    {
        entry1.SetValue(obj);
        entry1.AddExpirationToken(expirationToken);
    }
}

Currently there is no way to opt-out of this behavior and avoid these allocations.

I was hoping that I could add a new internal CacheEntry ctor that does not create scopes and change the existing extension methods that were designed to insert a single cache entry at a time, but these methods are using the abstraction IMemoryCache to create entries:

runtime/src/libraries/Microsoft.Extensions.Caching.Abstractions/src/IMemoryCache.cs

Line 26 in 69d6127

ICacheEntry CreateEntry(object key);

I can't find a way to achieve that without introducing a new public API.

@eerhardt @maryamariyan before I write an API proposal, do you believe that avoiding these allocations is worth introducing a new public API?

[adamsitnik]

Maybe it would be useful to enhance the docs to point to the extension methods

@Tornhoof definitely! is there any chance that you would like to contribute to the docs?

[Tornhoof]

@Tornhoof definitely! is there any chance that you would like to contribute to the docs?

Yeah, I'll create a PR for the docs repo in the next few days.

[eerhardt]

@davidfowl @Tratcher - thoughts on making the "linked CacheEntry" feature optional? I think I remember discussing maybe removing it altogether. Given the above analysis, it appears to be a pretty expensive feature even in the case where a user doesn't use it.

[Tratcher]

The "magic" factor of this feature has always been a problem. Letting people opt out of it is a good idea. How you do that without breaking people that are using it is a good question.

Trying to opt out on a per entry bases seems like it's going to be a challenge, the scope is created too early. We don't want to duplicate a bunch of APIs unnecessarily. What about adding one option on MemoryCacheOptions to opt out for the whole cache?

Once there's an API to control the feature we could discuss changing the default value in a future version.

[davidfowl]

I would make a change to turn this feature off by default. It adds overhead for scenarios that don't need it.

[adamsitnik]

I've sent a PR to minimize the cost of it: #45563 and a proposal to add a new API that allows for disabling it: #45592

[adamsitnik]

I am going to close this issue as #45563 has minimized the problem and #45592 tracks the new API for disabling that.