aspnetcore icon indicating copy to clipboard operation
aspnetcore copied to clipboard

Excessive memory usage with MapControllerRoute or MapDynamicControllerRoute

Open KyleSkibicki opened this issue 1 year ago • 11 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Describe the bug

We are in the process of converting a huge MVC application from 4.8 to NET8. It currently has 7653 controllers, 354,412 total actions. I created two testing applications to illustrate the problem, which shows a 23429% increase in memory usage.

I did review issue 30092, but this seems different, especially since that was fixed.

dump

Expected Behavior

I would expect memory usage to be similar between net48 and net8.

Steps To Reproduce

Repositories illustrating the problem:

NET8 NET48

Exceptions (if any)

No response

.NET Version

8.0.200

Anything else?

No response

KyleSkibicki avatar Mar 01 '24 17:03 KyleSkibicki

We are in the process of converting a huge MVC application from 4.8 to NET8

"Oh, how big are we talking?"

It currently has 7653 controllers

"Oh, that's quite a lot, I guess they have 1 action per controller then"

354,412 total actions

"Oh. Oh no!"

KennethHoff avatar Mar 01 '24 18:03 KennethHoff

@KyleSkibicki Thanks for sharing these repros. Does the CustomRouteValueTransformer in the repro represent a component in the application you're migrating as well?

I see that this repros with the standard MapControllerRoute but wanted to make sure I wasn't missing a detail here.

captainsafia avatar Mar 02 '24 00:03 captainsafia

@KyleSkibicki Thanks for sharing these repros. Does the CustomRouteValueTransformer in the repro represent a component in the application you're migrating as well?

I see that this repros with the standard MapControllerRoute but wanted to make sure I wasn't missing a detail here.

Using either MapControllerRoute or MapDynamicControllerRoute in the example repository both illustrate the memory issue I believe we are having. It seems to be more of an issue with the number of Actions versus the number of Controllers, that's why I went with 1000 controllers each with 360 actions. We didn't convert CustomRouteValueTransformer but are using it as specified below.

More specifics about our NET8 MVC application:

  • Uses a combination of both MapControllerRoute and MapDynamicControllerRoute<CustomRouteValueTransformer>.
  • Uses a MapDynamicControllerRoute<CustomRouteValueTransformer> and is currently selectively modifying the "action" RouteValue in some cases.
  • Uses a custom EndpointSelector to dynamically fix the AmbiguousMatchException where the same controller name exists in different namespaces, and to not have to add [Area] to all of our controllers
  • Uses a custom IApplicationFeatureProvider<ControllerFeature> and a custom IControllerModelConvention to wire up some data driven controllers on the fly.
  • Uses a custom view engine.
  • Uses a shim to get the invoked controller via a filter.

KyleSkibicki avatar Mar 02 '24 01:03 KyleSkibicki

Got it -- based on your screenshot and local profiling that I've done it looks like the largest contributors to retained size come from the endpoint routing layer.

Absent some of the specifics that you provided, my first instinct would be to use attribute routing for your solution to avoid the costs incurred by endpoint routing given the number of actions in your appliation. Have you experimented with that approach?

captainsafia avatar Mar 05 '24 00:03 captainsafia

I did some testing with attribute routing (here) and I would have to completely disable endpoint routing via EnableEndpointRouting = false for it to not use memory in EndpointDataSource. With the same number of endpoints memory went from 3GB to 2GB, with memory usage now in RouteCollection. I only added the Area attribute to all of the controllers.

attributeRouting

KyleSkibicki avatar Mar 06 '24 01:03 KyleSkibicki

I did some testing with attribute routing (here) and I would have to completely disable endpoint routing via EnableEndpointRouting = false for it to not use memory in EndpointDataSource.

I don't have access to the repro here. Perhaps it's private? The reduction in memory usage is what I'd expect to see. I assume you no longer see things like EndpointMetadataCollection and Endpoint contributing to the retained size.

with memory usage now in RouteCollection.

@javiercn @JamesNK Do you have any thoughts here?

captainsafia avatar Mar 06 '24 20:03 captainsafia

The repo is now public. Correct, with endpoint routing completely disabled there is no EndpointDataSource RefTree. The memory reduction is 1 GB less but is still almost 2 GB over NET48. Also, if we enable endpoint routing for a limited subset of routes or to use middleware, the memory usage jumps back up to 3GB.

KyleSkibicki avatar Mar 07 '24 01:03 KyleSkibicki

@KyleSkibicki Thanks for making the repo public! I'll take another look at this this week.

captainsafia avatar Mar 11 '24 17:03 captainsafia

@KyleSkibicki thanks for contacting us.

As an initial step, have you added route constraints to routes that contain parameters? Especially those that contains parameters in the middle of the route as opposed to the last segment.

See https://learn.microsoft.com/en-us/aspnet/core/fundamentals/routing?view=aspnetcore-8.0#guidance-for-large-route-tables for guidance and let us know if that helps.

javiercn avatar Mar 12 '24 15:03 javiercn

@javiercn I am not seeing "a large number of Microsoft.AspNetCore.Routing.Matching.DfaNode instances" which would suggest we are not running into the same large route table problem.

KyleSkibicki avatar Mar 15 '24 12:03 KyleSkibicki

@javiercn I created another repository BigWebAppMoreMappings

I called MapControllerRoute 1000 times, and memory went from 4GB to 2GB - NET48 uses 144MB.

app.MapControllerRoute(name: "default", pattern: "{action=Index}/{id?}", defaults: new { controller = "Home" });

app.MapControllerRoute(name: "New0001", pattern: "{action=Index}/{id?}", defaults: new { controller = "New0001" });
app.MapControllerRoute(name: "New0002", pattern: "{action=Index}/{id?}", defaults: new { controller = "New0002" });

app.MapControllerRoute(name: "New0999", pattern: "{action=Index}/{id?}", defaults: new { controller = "New0999" });
app.MapControllerRoute(name: "New1000", pattern: "{action=Index}/{id?}", defaults: new { controller = "New1000" });

KyleSkibicki avatar Mar 15 '24 15:03 KyleSkibicki

@KyleSkibicki thanks for the additional details.

I called MapControllerRoute 1000 times, and memory went from 4GB to 2GB - NET48 uses 144MB.

Do you have a similar 4.8 repro/profile that we can look at?

Do you measure this memory just after booting the app or after hitting all the actions in the app with a request. It makes a difference because many of the memory structures in old MVC are computed lazily (this is likely much higher)

It seems most of the cost comes from things that are created during startup like the action descriptors and that the framework needs in order to operate. As such it might be hard to achieve similar results to those that you are seeing in old MVC given how differently they both operate.

Note that having ~360K actions in a single app puts its well beyond the 99% percentile of what we expect for the typical size of an app to be, and the framework is not specifically optimized for those cases.

Reducing the memory usage here might require you to replace large parts of the pipeline with custom implementations that avoid eagerly allocating data structures (even though, you are going to end in a similar situation if all your endpoints are eventually hit).

One place to start might be by replacing the built-in IActionDescriptorProvider with your own. Note that you'll lose access to many of the built-in features provided out of the box like application parts, application model, etc.

javiercn avatar Mar 18 '24 14:03 javiercn

@javiercn thanks for the great response.

The 4.8 testing repo is here. I just tested it by making a request to all 360,000 end-points. The memory went from 144Mb to 1.3GB.

KyleSkibicki avatar Mar 18 '24 16:03 KyleSkibicki

@KyleSkibicki thanks for the additional details.

I suspect based on our conversation that the memory consumption is expected. There are several reasons that point out to the increased memory size.

  • ActionDescriptor in ASP.NET Core is much bigger than in old ASP.NET with fields to store a lot of metadata.
  • EndpointRouting duplicates some of this metadata in the EndpointMetadata collection, which will add to the size.

Taking your numbers as a base, the ratio is (4:1.3) (GB) which is ~3x in memory consumption. I think it is likely that the additional memory consumption is expected, given how much more information ASP.NET Core exposes in the form of metadata (things like the routes, etc.) and the sheer size of the application. For reference, we have benchmarks against a big subset of the entire Azure API here and that it's about 6K endpoints and your app is ~60x that.

What I'm trying to get at is, that in the vast majority of applications, memory consumption is not an issue, but given the size of your app, it starts to become a problem.

I hope it's understandable that there is a balance to be had between memory consumption, developer experience and other performance aspects (like CPU time) and that the team optimizes those aspects to cater the vast majority of apps that we see developer's author.

In cases like this, there are a few things we can do:

  • We can investigate to see if there are any low hanging allocations that we can optimize away, but I don't think they currently are.
  • We can try and give you guidance on alternative approaches or extensibility points that you can leverage to avoid/ incurring in/minimizing the problematic behavior.

You'll have to weight in your options here and choose what you think will be a better path forward for you:

  • Accept the memory size increase and deploy to a bigger machine.
  • Leverage extensibility to provide custom implementations of the areas of the framework that contribute most to the memory consumption. You risk 2 things here:
    • Giving up standard features of the framework.
    • Customizing so many aspects that you end up with "almost a different framework" that you need to support and maintain.

Some additional thoughts based on comments above

Uses a combination of both MapControllerRoute and MapDynamicControllerRoute.

Uses a MapDynamicControllerRoute and is currently selectively modifying the "action" RouteValue in some cases.

Is the action being selected based on information from the request, or something else (like you query a database to get what action needs to handle the result).

Uses a custom EndpointSelector to dynamically fix the AmbiguousMatchException where the same controller name exists in different namespaces, and to not have to add [Area] to all of our controllers

I would suggest you use a convention to apply the [Area] attribute programmatically in this situation.

Uses a custom IApplicationFeatureProvider and a custom IControllerModelConvention to wire up some data driven controllers on the fly.

You don't need the IApplicationFeatureProvider unless you are reflecting over things like types, etc. Otherwise, you can probably wire up things directly through IApplicationModelProvider (or even a custom IActionDescriptorProvider)

Uses a custom view engine.

Uses a shim to get the invoked controller via a filter.

Not clear what this is. Do you need access to the instance or to the type.

javiercn avatar Mar 19 '24 11:03 javiercn

@javiercn thanks for another helpful response.

We will consider your additional thoughts. It is understandable that the framework isn't optimized for our monolithic application. I'm hoping between adding more memory, and refactoring our application to have less end points, we will have an acceptable solution.

I feel we should be nervous about adding custom implementations of the framework - I assume nobody is doing this so getting help will be difficult. I also noticed a lot more sealed classes between NET6 and NET8, so maybe it's possible we will run into that issue too.

Before we close this issue out, can you speculate on the burden or technical debt this would cause us? For someone unfamiliar with the framework would this take hours, days, or months to implement? What about future upgrades to NET10? What about 3rd party nuget packages that touch the pipeline? I suppose a lot of this would depend on how many custom implementations we need to do. In your opinion would this be something we should avoid at all costs?

I know that was a lot of questions, in general I'm looking for your gut feeling on the direction we should take.

KyleSkibicki avatar Mar 20 '24 17:03 KyleSkibicki

@KyleSkibicki I would try and work with the framework as a first step. Reducing the number of actions will definitely play a role here. One way to go about it can be to group similar actions and perform "dispatching" internally within the method to specific methods.

I would avoid going the customization path if possible and focus on adding more memory first and reducing the number of options. It might be worth seeing if you can leverage attribute routing instead of calling MapControllerRoute for each controller, as they work slightly differently and might impact the memory consumption.

I think the customization route will be involved, but it all depends on how much you need to customize. It's hard to provide more pointed guidance without seeing the type of things you are currently customizing and for what reason. There are usually many ways of achieving the same goal within the framework and at your scale the approach might matter.

javiercn avatar Mar 21 '24 16:03 javiercn

@javiercn sounds good. Thanks again.

KyleSkibicki avatar Mar 21 '24 17:03 KyleSkibicki