Swashbuckle.AspNetCore icon indicating copy to clipboard operation
Swashbuckle.AspNetCore copied to clipboard

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

Open DvdKhl opened this issue 2 years ago • 3 comments

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

DvdKhl avatar Feb 10 '24 23:02 DvdKhl

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.

ahoke-cr avatar Feb 23 '24 20:02 ahoke-cr

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.

martincostello avatar Apr 14 '24 08:04 martincostello

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.

DvdKhl avatar Apr 14 '24 11:04 DvdKhl

@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

remcolam avatar Apr 26 '24 11:04 remcolam