Struggling with high memory usage

[rjpowers10]

Describe the bug

My scenario: We have a site built on Orchard Core 1.6. We have a pipeline that periodically deploys an instance to a test server and runs a suite of UI tests against it (the tests are using MSTest and Playwright, if it makes any difference). The site is running in IIS. The test suite runs for about 1 hour, and by the end of that run the w3wp process is consuming over 8.5 GB. ASPNETCORE_ENVIRONMENT is set to "Development".

Here's a quick look at the memory usage over time. This is the memory of the entire machine, not just OC, but looking in Task Manager, I see w3wp using over 8.5 GB.

To Reproduce

I've been struggling for quite a while on pinpointing the root issue, with no success thus far. I've tried using the Visual Studio memory profiler but I've been struggling to make any sense of it.

I next tried a much simpler example. This next example is a plain old OC site (latest from the main branch) using the blog recipe. That's it. ASPNETCORE_ENVIRONMENT is set to "Development". I then wrote a small PowerShell script to ping the site over and over.

function Invoke-Path($path) {
    Invoke-WebRequest -Uri "https://localhost:8001/${path}"
}

$Paths = @(
    '', 
    'categories', 
    'tags', 
    'Contents/ContentItems/40rg5j34w43er1a5dwg0j14cnr', 
    'login'
)

while (1) {
    $Paths | ForEach-Object -Process {
        Invoke-Path $_
    }
}

I ran this script for a little while. Looking at the diagnostic tools in Visual Studio, I can see multiple garbage collector invocations (the yellow arrows), but the memory never seems to decrease. I was hoping to see a more jagged pattern in the memory usage.

At this point I'm looking for any ideas or advice. Thanks.

[MikeAlhayek]

@rjpowers10 have you tried to disable cache from the general site settings? I am guessing there is a cache issue of maybe too many cached object in memory?

[rjpowers10]

@MikeAlhayek thanks for the suggestion but in both my examples (my application built on OC and a plain old OC blog) the cache setting is set to "from environment". ASPNETCORE_ENVIRONMENT is set to 'Development' for both, so the dynamic cache should be disabled. I tried forcing the setting to 'Disabled' as a sanity check and the results are the same.

[MikeAlhayek]

Have you tried to disable all sites except for the default one? This may help trying to understand what is causing the high usage. If the default site has normal memory usage, try to enable one of your blog sites and see if you can pin point the site. Then maybe start disabling one feature at a time to see if you can pin point the problem to a single feature. This may give us more clues.

Also, check your log file for any silent errors that would be causing memory consumption.

Not sure if this will help, but something for now. Hopefully, @jtkech or @sebastienros have more ideas here.

[sebastienros]

How much memory is on the system in total. Take a memory snapshot between two jumps and check the diff. Run in Release, the fact that I see VS might mean it is a debug session.

[rjpowers10]

How much memory is on the system in total. Take a memory snapshot between two jumps and check the diff. Run in Release, the fact that I see VS might mean it is a debug session.

My first example (my app built on OC) is a release build. My second example, which was a plain old OC site with the blog recipe, was in debug. The plain old OC site might just be a red herring, I just found it curious that memory didn't seem to go down after a GC collect. May not be relevant at all to my actual scenario.

The machine has 16GB of memory. SQL Server is also running which takes about 2GB itself. With my monitor I see the memory getting down to 0 and then as expected, the system starts paging and my tests start timing out.

I have a memory diff but I'm having trouble interpreting it. Wondering if anything jumps out to anyone.

Sorted by count diff

Sorted by size diff

Sorted by inclusive size diff

[rjpowers10]

Have you tried to disable all sites except for the default one? This may help trying to understand what is causing the high usage. If the default site has normal memory usage, try to enable one of your blog sites and see if you can pin point the site. Then maybe start disabling one feature at a time to see if you can pin point the problem to a single feature. This may give us more clues.

Also, check your log file for any silent errors that would be causing memory consumption.

Not sure if this will help, but something for now. Hopefully, @jtkech or @sebastienros have more ideas here.

I'm going to try limiting my test suite. Maybe there is one test in particular that exemplifies the problem.

[sebastienros]

RouteEndpoints in the diff would be explained by a tenant being loaded.

Coming back to your original description, the issue is that your test is taking 8.5Gb, can you try to do the same thing with ASPNETCORE_ENVIRONMENT in Production, not "Development", just to see if that has an impact.

Can you take a memory dump when the app is close to the end to see what is still kept in memory?

[rjpowers10]

RouteEndpoints in the diff would be explained by a tenant being loaded.

Coming back to your original description, the issue is that your test is taking 8.5Gb, can you try to do the same thing with ASPNETCORE_ENVIRONMENT in Production, not "Development", just to see if that has an impact.

Can you take a memory dump when the app is close to the end to see what is still kept in memory?

I ran the test suite again, this time with ASPNETCORE_ENVIRONMENT set to 'Production'. It didn't seem to make a difference. The tests finished in 1 hour, 15 minutes. This is what it looked like at the end of the test run.

This is the memory dump at the conclusion of the run.

Sorted by count

Sorted by size

Sorted by inclusive size

[ShaneCourtrille]

Our memory issues are consistent with this as well. These are our worst offenders in terms of memory usage from our latest performance testing while trying (and failing miserably) to run with 1000 tenants.

000071c5d1578080   449841     25195716 System.Int32[]
000071c5d15de7f0   101131     25791704 System.Byte[]
000071c5daff7f50   585044     28082112 System.Collections.Concurrent.ConcurrentDictionary`2+Node[[System.Type, System.Private.CoreLib],[System.Object[], System.Private.CoreLib]]
000071c5d7d8fbd0   728944     29157760 Microsoft.AspNetCore.Routing.RouteValueDictionary
000071c5d56e17c0   572454     32057424 System.Collections.Generic.Dictionary`2+Enumerator[[System.String, System.Private.CoreLib],[System.String, System.Private.CoreLib]]
000071c5daff8000   138733     37735376 System.Collections.Concurrent.ConcurrentDictionary`2+Node[[System.Type, System.Private.CoreLib],[System.Object[], System.Private.CoreLib]][]
000071c5d14ab110   499016     45688856 System.Object[]
000071c5d1e0d050    31347     47360136 System.Reflection.CustomAttributeRecord[]
000071c5d3fc6db8   963130     78553616 System.Collections.Generic.KeyValuePair`2[[System.String, System.Private.CoreLib],[System.Object, System.Private.CoreLib]][]
000071c5d157d2e0  1208078    108278276 System.String
Total 16274388 objects

[sebastienros]

Time to do another pass at it!

I will try with my own "script" but I'd like to be able to repro something that matches your experience. @rjpowers10 how many tenants are you loading and what features do they use? Ideally if you could do the same experiment and confirm it with the smallest possible list of features that would help. @ShaneCourtrille thanks for the confirmation. I'd love to achieve a density of 1000 on 8GB of RAM, we were there at the beginning, but obviously more features and complexity have been added since then.

[rjpowers10]

@sebastienros I'll try to help as much as I can though it may be difficult to repro my situation. We built an eCommerce platform on top of OC and wrote many custom features. Because I'm having trouble figuring out the offending code it's hard for me to say if it's an issue in my code or OC. My tests do a lot of the typical eCommerce things (sign in, register, view product, add to cart, checkout, order history, etc.).

I wonder if something is sticking around after each ViewResult so it's simply the fact that my tests are making a lot of requests. That's why I tried a simpler test with a plain old OC site and the blog recipe, but it's hard to say if that really replicated the issue.

List of OC features we have enabled:

1. OrchardCore.Admin
2. OrchardCore.Alias
3. OrchardCore.Autoroute
4. OrchardCore.BackgroundTasks
5. OrchardCore.ContentFields
6. OrchardCore.ContentPreview
7. OrchardCore.Contents
8. OrchardCore.ContentTypes
9. OrchardCore.CustomSettings
10. OrchardCore.Deployment
11. OrchardCore.Diagnostics
12. OrchardCore.DynamicCache
13. OrchardCore.Email
14. OrchardCore.Features
15. OrchardCore.Flows
16. OrchardCore.Forms
17. OrchardCore.Google.Analytics
18. OrchardCore.HomeRoute
19. OrchardCore.Html
20. OrchardCore.Https
21. OrchardCore.Indexing
22. OrchardCore.Layers
23. OrchardCore.Liquid
24. OrchardCore.Lists
25. OrchardCore.Localization
26. OrchardCore.Markdown
27. OrchardCore.Media
28. OrchardCore.Media.Slugify
29. OrchardCore.Menu
30. OrchardCore.Navigation
31. OrchardCore.Placements
32. OrchardCore.Recipes
33. OrchardCore.Resources
34. OrchardCore.Roles
35. OrchardCore.Rules
36. OrchardCore.Scripting
37. OrchardCore.Search.Lucene
38. OrchardCore.Search.Lucene.Worker
39. OrchardCore.Settings
40. OrchardCore.Shortcodes
41. OrchardCore.Taxonomies
42. OrchardCore.Themes
43. OrchardCore.Title
44. OrchardCore.Users
45. OrchardCore.Users.ChangeEmail
46. OrchardCore.Users.Registration
47. OrchardCore.Users.ResetPassword
48. OrchardCore.Widgets
49. OrchardCore.Workflows
50. TheAdmin

[sebastienros]

@rjpowers10 no tenants?

[rjpowers10]

@sebastienros Oh sorry, we do have two tenants, one for humans and one for automated tests. All the tests are hitting the same tenant. The test are also running serially. The tenant for humans usually isn't doing much at this time though, since the tests run overnight.

[ShaneCourtrille]

@rjpowers10 Out of curiosity.. have you taken a look at your strings? I'm doing some analysis and found an oddity where an estimated 80% of strings that are 46 bytes long are the value "mvc.1.0.view" being repeated over and over. What's really painful is that these strings (or at least the ones I've been able to sample since gcroot is such an expensive operation) don't have a gcroot but are just sort of sitting around.

[rjpowers10]

@rjpowers10 Out of curiosity.. have you taken a look at your strings? I'm doing some analysis and found an oddity where an estimated 80% of strings that are 46 bytes long are the value "mvc.1.0.view" being repeated over and over. What's really painful is that these strings (or at least the ones I've been able to sample since gcroot is such an expensive operation) don't have a gcroot but are just sort of sitting around.

There are a ton of strings in my memory dump, although only four instances of "mvc.1.0.view".

EDIT: Whoops, read the memory dump wrong. I have over 260,000 occurrences of "mvc.1.0.view".

[wAsnk]

This might be a total red herring, but we have experienced increasing memory usage which can be reproduced by the following steps in vanilla OC, so I'd like to share my results. The main idea behind this was to create an idle tenant shutdown handler that would "shut down" tenants/free up resources for tenants that are not used for a predefined time period.

Test controller for the repro:

using Microsoft.AspNetCore.Mvc;
using OrchardCore.Environment.Shell;

namespace OrchardCore.Cms.Web.Controllers;

[Route("test")]
public class TestController : Controller
{
    private readonly IShellHost _shellHost;
    private readonly ShellSettings _shellSettings;

    private static readonly HttpClient _httpClient = new();

    public TestController(
        IShellHost shellHost,
        ShellSettings shellSettings)
    {
        _shellHost = shellHost;
        _shellSettings = shellSettings;
    }

    public async Task<string> Index()
    {
        _httpClient.BaseAddress ??= new Uri(HttpContext.Request.Scheme + "://" + HttpContext.Request.Host);

        for (int i = 0; i < 1000; i++)
        {
            await _shellHost.ReleaseShellContextAsync(_shellSettings);

            await _httpClient.GetAsync(_shellSettings.RequestUrlPrefix, HttpContext.RequestAborted);
        }

        return "OK";
    }
}

Repro steps:

1. Use the OC repository.
2. Setup default tenant with Software as service recipe
3. Add a controller like the one above which uses the IShellHost.ReleaseShellContextAsync function which summary says: Releases a shell so that a new one will be built for subsequent requests. Note: Can be used to free up resources after a given time of inactivity. So we want to do this exactly.
4. Call /test URL, notice that after a while GC kicks in and the memory usage will maximize around ~500MB and never go above.
   
5. Now enable ReCaptcha Users feature.
6. Call /test, notice how the memory usage goes up until the call is aborted (if not aborted it will use all memory), GC can't control the memory usage even after 10 minutes:
   

Before and after the ReCaptcha Users feature memory snapshot results:

1. Count Diff
   
2. Size Diff
   
3. Inclusive Size Diff
   
4. Insights
   

[jtkech]

When you are in Development (there is an env variable) and/or in debug moder under Visual Studio, the GC may not work as expected, I already saw this kind of memory increase. Try to build in Release (maybe not necessary), at least start the app from the command line and then look at the Task Manager.

Then it may depend on your machine to decide when you are under memory pressure or not.

[sebastienros]

@jtkech is right, the debugger will keep some variables "rooted" so you can inspect them and hence prevent them from being collected.

Use a tool like PerfView or dotnetMemory profiler to take snapshots of the memory from a release run that you can then analyze in these tools.

[wAsnk]

I could repro the same result with release mode and "ASPNETCORE_ENVIRONMENT": "Production"

[ShaneCourtrille]

@wAsnk Can you repro in a deployed environment? You can use dotnet dump collect to get a memory dump and then look at it locally in VS.

[jtkech]

@wAsnk

When I will have time I will re-install PerfView and look at it more in depth, for now my Visual Studio doesn't allow me to do 2 concecutive memory snapshots.

but in the meantime I could repro by just looking at the memory usage, when doing a lot of tenant release the GC works as expected, in my case it stabilizes around 700 Mb (but in debug mode). But yes after having enabled the ReCaptcha.Users the memory increases much more with the same test.

I will look at this asap.

Did you see the same behavior but by enabling another feature?

---

@ShaneCourtrille and others

About the "mvc.1.0.view" strings, under the debugger by using the Make object ID command I could see that all our RazorViewCompiledItem that we add (e.g. one for each Liquid file) are all using the same string instance.

But yes, all DefaultRazorCompiledItem compiled at build time are all using a different instance, but I'm afraid that here we can't do anything.

[jtkech]

@wAsnk

ReCaptchaLoginFilter is activated on each request and resolves transitively ReCaptchaClient for which a typed HttpClient has been registered. So on each action request a typed HttpClient is built.

Resolving ReCaptchaClient only on demand in ReCaptchaService fixed the issue.

Note: Maybe still a memory problem with typed HttpClient but less critical if only built as needed.

[wAsnk]

Did you see the same behavior but by enabling another feature?

No, I couldn't repro with every other feature on.

Resolving ReCaptchaClient only on demand in ReCaptchaService fixed the issue.

Sounds good!

[BenedekFarkas]

Note: Maybe still a memory problem with typed HttpClient but less critical if only built as needed.

This reminds me of this PR: dotnet/corefx#19082, might have some useful insights there.

[sebastienros]

We need to use IHttpClientFactory in ReCaptchaClient instead of injecting the http client. And we might be able to make ReCaptchaClient a singleton (and ReacptchaService by resolving _robotDetectors lazily?)

[jtkech]

IHttpClientFactory is already used indirectly, a typed HttpClient is registered meaning that each time ReCaptchaClient is resolved an HttpClient is build using the IHttpClientFactory.

https://github.com/dotnet/runtime/blob/1ff7fbb4c61f08d36afe0e113a676a0fb332ffbc/src/libraries/Microsoft.Extensions.Http/src/DependencyInjection/HttpClientBuilderExtensions.cs#L545C17-L545C35

services.AddHttpClient<ReCaptchaClient>()
    .AddTransientHttpErrorPolicy(policy =>
        policy.WaitAndRetryAsync(3, attempt => TimeSpan.FromSeconds(0.5 * attempt)));

So the problem is because of an mvc action filter that resolves on each request ReCaptchaService, then ReCaptchaClient which triggers an HttpClent building by the IHttpClientFactory.

To fix the issue I only needed in ReCaptchaService to resolve lazily ReCaptchaClient as needed, I already did the change locally, but didn't commit it yet because maybe some other updates may be worth to do, for example maybe better to make ReCaptchaService a scoped service, I will check.

For now the only thing I needed to update is only one line, I will commit it to show what I did.

[jtkech]

Here what I did for now #14335

[ShaneCourtrille]

@rjpowers10 I'm still not done looking but my initial checks look good for the mvc.1.0.view fix and in our case that saves us 500MB of memory alone. /cc @jtkech (FYI)

[sebastienros]

[jtkech]

@sebastienros Not related ;)

@rjpowers10 @ShaneCourtrille

I made good progress in #14348 for the Reloading part, not fully tweaked but can be already tested.

Note: I was wrong, the tenant is not reloaded on Features change, it is only released and then rebuilt based on the updated shell descriptor. The tenant is reloaded from the Admin Tenants UI or by code.

But maybe you are reloading tenants in your tests, anyway worth to re-try as I did other changes.

---

Didn't have time to focus on the razor strings, I will do, didn't see yet the meeting where it was discussed.

Note: Just saw a razor hot relaod that clears caches on application events (so not tenant events), maybe a good path to follow.

[MikeAlhayek]

@jtkech could the actual issue with HttpClient is the usage by not disposing it? Meaning doing this

var client = _httpClientFactory.CreateClient();

// never dispose the client

instead of

using var client = _httpClientFactory.CreateClient();

or

var client = _httpClientFactory.CreateClient();

client.Dispose();

[rjpowers10]

@MikeAlhayek

https://learn.microsoft.com/en-us/aspnet/core/fundamentals/http-requests?view=aspnetcore-7.0#httpclient-and-lifetime-management

[jtkech]

@rjpowers10 @ShaneCourtrille and others.

Here a little summary of what has been done done in #14348, if you can give it a try.

• So in Memory Leaks #14348 we override the IHttpClientFactory, mainly to make it IDisposable so that when releasing / reloading a Tenant we release clients Handlers and cleanup Timers.

• On Tenant Reloading, we now also Release the 2 inner IConfiguration of ShellSettings.

• Finally we now always use our static shared SharedViewCompilerProvider (see below).

About razor view compiled items, we already have a static shared compiler provider but that we only use if razor runtime compilation is enabled, now in #14348 we always register it.

        // Shares across tenants a static compiler even if there is no runtime compilation
        // because the compiler still uses its internal cache to retrieve compiled items.
        services.AddSingleton<IViewCompilerProvider, SharedViewCompilerProvider>();

We still load the CompiledDescriptor from assemblies but not to be hold by another compiler instance, the cache of the same static compiler is populated once.

Doing so it seems that the GC behavior is better, here after many Tenant Release / Reload.

[rjpowers10]

@jtkech Awesome! I will test what I can, but I can only really test the stuff that can be overridden in DI. I've just been copy-pasting your PR changes into my code, since I will need these fixes before the next OC release.

[wAsnk]

@jtkech I tested it in our own solution with local NuGet files and it looks really good. I will do further testing in the upcoming days.

[jtkech]

@rjpowers10 No problem, no pressure, when you will have time.

@wAsnk Okay cool, thanks for testing, I cross my fingers.

[jtkech]

@wAsnk

Just for info in #14348 Recaptcha.Users has not been changed, but still better to create HttpClient on demand as done in #14335. So you may see more memory usage but because we override DefaultHttpClientFactory normally, at an higher value, but it should stabilize.

[jtkech]

@rjpowers10 @ShaneCourtrille @MikeAlhayek @wAsnk @sebastienros

I saw one of the last meeting related to string allocations, interesting. For info by using instance ids under the debugger we can see that compiled items loaded from assemblies hold different instances of the "mvc.1.0.view" string.

In #14348 we now override how compiled items are loaded and we can see that the same instance is used.

Also I could reduce the number of times the compiled items are loaded, before 3 times per tenant building, for example the view descriptors are populated on demand by models providers (one from aspnetcore, one from orchardcore), now only once by caching the descriptors built in a given shell scope.

We can still have duplicate strings but they are no longer rooted and can be reclaimed, the only one holding these strings being our static compiler that we now always use, and now it only references one instance of "mvc.1.0.view".

[wAsnk]

I did some testing with Azure Web App.

TLDR: LGTM

I did some mild testing yesterday, no real surprises the memory usage went up, then stayed with some sawtooth shapes:

But there were no real cleanups like I would expect to GC down to the initial 80-120MB of usage. So I did some heavy testing today to really kick in GC on heavy memory usage. Around 1.4GB GC pressure was really high and did its major cleaning, the memory usage went back to 90MB, just like it happened locally for the same solution around 1GB.

[rjpowers10]

Sorry, I've been meaning to run some more tests on my solution but I've been swamped this week. The PR has also grown considerably, to the point where it would be easier if I had NuGet packages. Maybe I will pull the branch and make some local packages to test with.

[jtkech]

@wAsnk Thanks a lot for your tests.

Yes, I've made other changes, for example to not call CreateChildContainer() twice to build a shell, and as said, in #14348 I haven't intentionally updated the "RecaptchaClient on demand" yet, so perhaps less memory increase with these changes.

But I think we are okay the main goal being to keep the memory reclaimable by the GC.

What's your own conclusion?

@rjpowers10 No problem, only when you will have time.

[wAsnk]

But I think we are okay the main goal being to keep the memory reclaimable by the GC.

Yeah, I agree.

[jtkech]

Okay cool then, thank's again for your tests.

[wAsnk]

@jtkech When Database Shells Configuration Provider is used there is an exception on startup:

System.InvalidOperationException: StreamConfigurationProviders cannot be loaded more than once.
   at Microsoft.Extensions.Configuration.StreamConfigurationProvider.Load()
   at Microsoft.Extensions.Configuration.ConfigurationRoot.Reload()
   at OrchardCore.Environment.Shell.ShellSettingsManager.LoadSettingsAsync(String tenant)
   at OrchardCore.Environment.Shell.Distributed.DistributedShellHostedService.LoadingAsync()
   at OrchardCore.Environment.Shell.ShellHost.PreCreateAndRegisterShellsAsync()
   at OrchardCore.Environment.Shell.ShellHost.InitializeAsync()
   at OrchardCore.Modules.ModularTenantContainerMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)

[jtkech]

Oop, will look at it this night, thanks.

[jtkech]

Okay, we try to reload to prevent to rebuild some configs, I will fix it this night.

[jtkech]

Okay, fixed by #14499

[rjpowers10]

For posterity, I updated my app to Orchard Core 1.8, the first release containing the fixes from #14348.

In this graph you can see a big improvement in memory consumption between OC 1.7 and 1.8.

[hyzx86]

I created a background task to collect garbage every minute

    [BackgroundTask(Schedule = "* * * * *", Description = "Release Memory. GC Collect")]
    public class JintBackgroundTask : IBackgroundTask
    {
        public Task DoWorkAsync(IServiceProvider serviceProvider, CancellationToken cancellationToken)
        {
            GC.Collect();
            return Task.CompletedTask;
        }
    }

[Piedone]

This seems unnecessary and potentially degrading for the overall performance, since it's blocking and while may reclaim memory, uses CPU. Outside of debugging and otherwise very special scenarios, you shouldn't (have to) run the GC manually.

Note that on servers the point is not to keep memory usage low, but to serve the most users with good performance. This may mean keeping memory usage high, but steady. This in itself is not a problem.

[hyzx86]

This is a long time ago test, I'm going to try to disable it now and see what happens with automatic memory reclamation

[rjpowers10]

Agreed with @Piedone that we shouldn't be calling GC.Collect() manually, but for completeness I'll also add that it wouldn't have resolved this particular issue anyway, since the memory was still rooted and therefore couldn't be reclaimed.