Skip to content
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

Registered IDocumentFilter, IOperationFilter types instantiated very often on first http request #2765

Closed
DvdKhl opened this issue Feb 10, 2024 · 4 comments · Fixed by #2839

Comments

@DvdKhl
Copy link

DvdKhl commented Feb 10, 2024

We do did a bit of heavy lifting in the constructor in classes which implement IDocumentFilter and IOperationFilter.
To our surprise the constructor seems to be called very often on the first http request (any request, I don't mean the swagger.json specifically), causing a rather long delay and a lot of GC pressure.

While both is due to the heavy lifting in the constructor, causing ~30ms delay and some MBs of garbage per constructor call. I think it would be better if the instances were reused, causing less surprises.
In our case it summed to 2244 constructor calls, ~20s delay and jump of 1GB of RAM after all constructor calls are finished.

image

Example:

services.AddSwaggerGen(c => {
	//We add multiple DocumentFilters/OperationFilters
	c.DocumentFilter<DocumentFilter>();
	c.OperationFilter<OperationFilter>();
});

public class DocumentFilter : IDocumentFilter {
	public DocumentFilter() { Thread.Sleep(20); }
	public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context) {}
}
public class OperationFilter : IOperationFilter {
	public DocumentFilter() { Thread.Sleep(20); }
	public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context) {}
}

Nuget Package: Swashbuckle.AspNetCore 6.5.0
Runtime: NET8
OS: Win11

@ahoke-cr
Copy link

Why do you need the first request to be so performant? The OpenAPI document is generated on startup and mostly reused after that for the life of the process. Outside of development, typically a warmup process of the web server would be used where some basic requests are initiated to get first-time startup logic out of the way so consumers can use the web service as expected.

@martincostello
Copy link
Collaborator

As noted above, could you share a bit more information about the scenario where you see this? If it's production, then I'd agree we should look to improve it if there's an issue, but if this is just when starting the application under the debugger and your Swagger UI page is the default configured launch page (so the first request, every time), then I don't think there's anything we need to do here.

@DvdKhl
Copy link
Author

DvdKhl commented Apr 14, 2024

Ah sorry for the late reply, I didn't notice the question.

Yes it also happens in production. I don't have to call the Swagger UI for this to occur. Calling any api is enough.

Minimal Example:

using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddEndpointsApiExplorer();

for(int i = 0; i < 10; i++) {
    var x = i;
    builder.Services.AddSwaggerGen(c => {
        c.SwaggerDoc($"title-{x}", new OpenApiInfo { Title = $"SMARTR ({i})", Version = "1.0" });
        c.DocumentFilter<DocumentFilter>();
        c.OperationFilter<OperationFilter>();
    });
}

var app = builder.Build();
app.UseSwagger();
app.MapGet("/weatherforecast", () => 0).WithName("GetWeatherForecast").WithOpenApi();
app.Run();

public class DocumentFilter : IDocumentFilter {
    static int count;
    public DocumentFilter() {Thread.Sleep(20); Console.WriteLine("DocumentFilter " + count++);  }
    public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context) { }
}
public class OperationFilter : IOperationFilter {
    static int count;
    public OperationFilter() { Thread.Sleep(20); Console.WriteLine("OperationFilter " + count++); }
    public void Apply(OpenApiOperation operation, OperationFilterContext context) { }
}

Notice the delay calling the "weatherforecast" endpoint and see the console for the constructor calls.

martincostello pushed a commit that referenced this issue Apr 26, 2024
- Add methods to support filter instance re-use.
- Fix CI on macOS 14.

Resolves #2765.
@remcolam
Copy link
Contributor

@DvdKhl In Swashbuckle 6.6.0 you can do the following:

using Microsoft.OpenApi.Models;
using Swashbuckle.AspNetCore.SwaggerGen;

var builder = WebApplication.CreateBuilder(args);
builder.Services.AddEndpointsApiExplorer();

var documentFilter = new DocumentFilter();
var operationFilter = new OperationFilter();

for(int i = 0; i < 10; i++) {
    var x = i;
    builder.Services.AddSwaggerGen(c => {
        c.SwaggerDoc($"title-{x}", new OpenApiInfo { Title = $"SMARTR ({i})", Version = "1.0" });
        c.AddDocumentFilterInstance(documentFilter );
        c.AddOperationFilterInstance(operationFilter );
    });
}

var app = builder.Build();
app.UseSwagger();
app.MapGet("/weatherforecast", () => 0).WithName("GetWeatherForecast").WithOpenApi();
app.Run();

public class DocumentFilter : IDocumentFilter {
    static int count;
    public DocumentFilter() {Thread.Sleep(20); Console.WriteLine("DocumentFilter " + count++);  }
    public void Apply(OpenApiDocument swaggerDoc, DocumentFilterContext context) { }
}
public class OperationFilter : IOperationFilter {
    static int count;
    public OperationFilter() { Thread.Sleep(20); Console.WriteLine("OperationFilter " + count++); }
    public void Apply(OpenApiOperation operation, OperationFilterContext context) { }
}

So, instead of using options.DocumentFilter<DocumentFilter>(); you should use options.AddDocumentFilterInstance(documentFilter); and passing in an existing instance.

This pattern has been added for all filters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants