Skip to content

App first request performance with many Minimal APIs #46313

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
1 task done
JamesNK opened this issue Jan 30, 2023 · 11 comments
Open
1 task done

App first request performance with many Minimal APIs #46313

JamesNK opened this issue Jan 30, 2023 · 11 comments
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc Needs: Design This issue requires design work before implementating.
Milestone

Comments

@JamesNK
Copy link
Member

JamesNK commented Jan 30, 2023

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Found while investigating #46154

The app below has 30,000 endpoints. The first request to / takes 0.5 seconds with 280 MB memory usage:

using System.Diagnostics;
using System.Globalization;
using System.Reflection;
using Microsoft.AspNetCore.Http.HttpResults;
using Microsoft.AspNetCore.Http.Metadata;
using Microsoft.AspNetCore.Mvc;

var builder = WebApplication.CreateBuilder(args);
var app = builder.Build();

app.Use(async (HttpContext context, Func<Task> next) =>
{
    Console.WriteLine("Start time");
    Stopwatch stopwatch = Stopwatch.StartNew();

    await next();

    stopwatch.Stop();
    Console.WriteLine(stopwatch.Elapsed.TotalSeconds);
});

app.UseRouting();

Task Plaintext(HttpContext context) => context.Response.WriteAsync("Hello, World!");
for (int i = 0; i < 30_000; i++)
{
    var url = "/plaintext/nested" + i;
    app.MapGet(url, Plaintext);
}

app.MapGet("/", (HttpContext context) =>
{
    return context.Response.WriteAsync("Hello world");
});

Console.WriteLine("Running app");
app.Run();

If I change the Plaintext endpoint to be a minimal API (aka use RequestDelegateFactory) like so:

- Task Plaintext(HttpContext context) => context.Response.WriteAsync("Hello, World!");
+ string Plaintext() => "Hello, World!";

With 30,000 minimal APIs, it now takes 32 seconds to get the first request. And memory usage is 1,065 MB.

Expected Behavior

I expect a fast startup time.

I think the problem is RequestDelegateFactory is building and compiling expressions for all minimal API endpoints when routing starts. Creating a minimal API's expression should be lazy and wait until an endpoint is first visited.

Steps To Reproduce

No response

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

@JamesNK JamesNK added the old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels label Jan 30, 2023
@JamesNK
Copy link
Member Author

JamesNK commented Jan 30, 2023

cc @davidfowl

@davidfowl
Copy link
Member

Yea, this is known, and we could definitely do a better job here. Also the above code isn't super realistic, maybe our benchmark should have different API shapes with argument types to better represent something more realistic. The reason I suggest that is because I can see 2 optimizations:

  1. Making delegate compilation lazy
  2. Caching similar delegates to reduce repeated compilation (would work great in the above scenario).

@DamianEdwards
Copy link
Member

Didn't we have a test set of routes/APIs based on all the public Azure or MS REST APIs at one point? I seem to recall we used it to help determine endpoint routing performance in 3.0.

@JamesNK
Copy link
Member Author

JamesNK commented Jan 30, 2023

I think making delegate compilation lazy is the easiest fix here.

var buildExpressionLazy = new Lazy<RequestDelegate>(Builder, LazyThreadSafetyMode.ExecutionAndPublication);
RequestDelegate d = (context) =>
{
    var realDelegate = buildExpressionLazy.Value;
    return realDelegate(context);
};
return d;

// + Build method which handles calling off to RequestDelegateFactory to generate compiled expression.

Caching similar delegates is useful but could come later.

@davidfowl
Copy link
Member

I agree, it doesn't solve the peak memory problem, but it does make it pay as you go.

@halter73
Copy link
Member

Lazily calling RequestDelegateFactory.Create has the possible issue that this will surface some errors later that used to happen on the first request. It also could introduce subtle ordering issues for filter factories and complicated IEndpointConventionBuilder.Add callbacks if they rely on some sort of global state.

I don't find this super likely to cause issues though, and the fact that the errors were never truly at startup and instead during first request might make the impact of this less significant. More analyzers that highlight these errors at compilation time will also mitigate this.

Caching similar RequestDelegates has some potential. Scenarios like this where endpoints are added programmatically or during source gen seem common enough. A developer could call RequestDelegateFactory.Create() and cache themselves if they start noticing startup issues, but that'd be extremely unobvious.

@captainsafia captainsafia added the Needs: Design This issue requires design work before implementating. label Feb 1, 2023
@captainsafia captainsafia added this to the .NET 8 Planning milestone Feb 1, 2023
@captainsafia
Copy link
Member

Triage: We need to identify what in the startup codepath is causing this (compiling LINQ expressions, something else?)

We'll also want to test how the source generated-endpoints impact this behavior.

@captainsafia
Copy link
Member

Triage: We need to identify what in the startup codepath is causing this (compiling LINQ expressions, something else?)

We'll also want to test how the source generated-endpoints impact this behavior.

Follow up with some numbers from a run I did:

Start time (with RequestDelegate)
0.27807

Start time (with route handler)
1.2106882

Start time (with route handler and generator)
0.7631818

Looks like there's something else producing overhead in the source generator scenario...

@davidfowl
Copy link
Member

The dictionary lookups?

@DamianEdwards
Copy link
Member

DamianEdwards commented Feb 2, 2023

Aren't those per-request? Or are they when the endpoint is built?

@davidfowl
Copy link
Member

davidfowl commented Feb 2, 2023

It’s during startup (the calls the Map*)

@captainsafia captainsafia added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed old-area-web-frameworks-do-not-use *DEPRECATED* This label is deprecated in favor of the area-mvc and area-minimal labels labels Jun 6, 2023
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 6, 2024
@wtgodbe wtgodbe removed the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@dotnet dotnet deleted a comment from dotnet-policy-service bot Feb 13, 2024
@captainsafia captainsafia modified the milestones: .NET 8 Planning, Backlog Mar 1, 2024
@BrennanConroy BrennanConroy changed the title App startup time performance with many Minimal APIs App first request performance with many Minimal APIs Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc Needs: Design This issue requires design work before implementating.
Projects
None yet
Development

No branches or pull requests

6 participants