Improve IncludeXmlComments performance (2)

[mus65]

Pull Request

The issue or feature being addressed

Fixes #2164

Details on the issue fix or feature implementation

This is a rebase and enhancement of #2443 (thanks to @stevendarby, added you as co-author).

This significantly improves the performance of the XML comment filters. The original fix #2443 created a dictionary of the members to avoid iterating the whole XPath for every single item.

I made some further improvements by replacing all usages of Select and SelectSingleNode with SelectChildren (added some helper extensions for this). This completely avoids XPath expressions and the documentation even mentions SelectChildren being faster.

I made the new constructors internal for now (which is also why I changed ParameterFilter to AddParameterFilterInstance etc.. DI fails otherwise). It felt wrong to me to make these public, but I can make these public if you want.

Tests

The existing tests imho already cover this code pretty well. I only added a test case for a bug I noticed in #2443 (there was a return instead of a continue in XmlCommentsDocumentFilter).

Performance

#2443 already mentions significant improvements (25s -> 4s). With another local test case I have with a 1.5MB XML file, generation time decreases from ~1900ms to ~200ms.

#2443 also mentions some concern about memory consumption, so I made some manual tests with a very big (30MB) XML document file: the XPathDocument itself already loads the document completely into memory which uses about 60 MB. Creating the dictionary creates an additional 40MB. So there is some additional memory consumption, but considering that 30MB is pretty extreme and most files should be < 1MB, the memory increase should be negligible for most applications. In my local test case with a 1.5 MB File, memory consumption after GC.Collect increased by about 1MB.

[martincostello]

With another local test case I have with a 1.5MB XML file, generation time decreases from ~1900ms to ~200ms.

Could you create a BenchmarkDotNet benchmark that replicates what you did and share it as a comment so we can re-run it ourselves?

[mus65]

With another local test case I have with a 1.5MB XML file, generation time decreases from ~1900ms to ~200ms.

Could you create a BenchmarkDotNet benchmark that replicates what you did and share it as a comment so we can re-run it ourselves?

I'll look into it. Could take some time though. I'm not really familiar with BenchmarkDotNet yet and I can't share my local test case.

[mus65]

I created a benchmark in a separate branch here. As expected, the new implementation is a lot
faster, even with a small member count. The performance of the new implementation stays the same,
regardless of member count. The old implementation gets significantly slower the more members
exist. addMemberCount can be changed in the benchmark to try different member counts.

Old implementation with small member count (n = 79):

Method	Mean	Error	StdDev	Median	Gen0	Gen1	Allocated
Document	4.074 us	0.0811 us	0.1780 us	4.002 us	2.2278	0.5569	20.53 KB
Operation	12.692 us	0.0828 us	0.0775 us	12.712 us	5.0659	0.0610	47.06 KB
Parameter	6.878 us	0.1368 us	0.2360 us	6.786 us	2.6321	0.0076	24.29 KB
RequestBody	4.122 us	0.0185 us	0.0164 us	4.125 us	2.2278	0.0610	20.64 KB

Old implementation with big member count (n = 10.000):

Method	Mean	Error	StdDev	Median	Gen0	Gen1	Allocated
Document	541.3 us	10.56 us	13.73 us	542.8 us	78.1250	1.9531	723.66 KB
Operation	1,061.1 us	20.70 us	31.61 us	1,074.7 us	156.2500	-	1453.34 KB
Parameter	489.3 us	6.42 us	5.01 us	489.4 us	79.1016	-	727.42 KB
RequestBody	476.8 us	9.43 us	16.52 us	467.4 us	78.1250	-	723.79 KB

New implementation with any member count:

Method	Mean	Error	StdDev	Median	Gen0	Gen1	Allocated
Document	2.675 us	0.0530 us	0.0955 us	2.726 us	1.8959	0.6294	17.45 KB
Operation	4.851 us	0.0907 us	0.0848 us	4.869 us	3.7842	0.1526	34.84 KB
Parameter	2.421 us	0.0313 us	0.0293 us	2.411 us	1.8959	0.0687	17.46 KB
RequestBody	2.545 us	0.0136 us	0.0127 us	2.545 us	1.8158	-	16.73 KB

[stevendarby]

@mus65 thank you for picking this up and improving it, great stuff seeing the benchmarks too

[martincostello]

Thanks @mus65 for doing those benchmarks - I'll take a closer look at them tomorrow 👍

[martincostello]

Thanks again for adding these benchmarks. I've pushed up a version of them in #3050 for us to use going forwards.

These are the results I get on my laptop - TL;DR - better by every measure 🥳

N=79

Base

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.4037/22H2/2022Update/SunValley2)
12th Gen Intel Core i7-1270P, 1 CPU, 16 logical and 12 physical cores
.NET SDK 8.0.401
  [Host]     : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Gen0	Gen1	Allocated
Document	10.468 μs	0.4412 μs	1.2517 μs	2.8534	0.7172	26.29 KB
Operation	23.157 μs	0.4611 μs	0.5663 μs	6.3477	-	58.5 KB
Parameter	12.081 μs	0.2327 μs	0.2680 μs	3.2501	0.0916	29.98 KB
RequestBody	9.833 μs	0.3246 μs	0.9366 μs	2.8381	-	26.2 KB

This PR

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.4037/22H2/2022Update/SunValley2)
12th Gen Intel Core i7-1270P, 1 CPU, 16 logical and 12 physical cores
.NET SDK 8.0.401
  [Host]     : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Gen0	Gen1	Allocated
Document	3.864 μs	0.2210 μs	0.6341 μs	1.9188	0.4807	17.66 KB
Operation	6.812 μs	0.3186 μs	0.9294 μs	3.8147	-	35.17 KB
Parameter	3.410 μs	0.1073 μs	0.2991 μs	1.9112	0.1030	17.6 KB
RequestBody	3.656 μs	0.1797 μs	0.5271 μs	1.8158	-	16.73 KB

N=10,000

Base

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.4037/22H2/2022Update/SunValley2)
12th Gen Intel Core i7-1270P, 1 CPU, 16 logical and 12 physical cores
.NET SDK 8.0.401
  [Host]     : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Gen0	Gen1	Allocated
Document	598.6 μs	8.41 μs	7.02 μs	78.1250	1.9531	724.36 KB
Operation	1,105.6 μs	19.72 μs	17.48 μs	156.2500	-	1453.67 KB
Parameter	516.7 μs	9.92 μs	8.28 μs	79.1016	-	727.55 KB
RequestBody	487.9 μs	4.22 μs	3.52 μs	78.1250	-	723.79 KB

This PR

BenchmarkDotNet v0.14.0, Windows 11 (10.0.22621.4037/22H2/2022Update/SunValley2)
12th Gen Intel Core i7-1270P, 1 CPU, 16 logical and 12 physical cores
.NET SDK 8.0.401
  [Host]     : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2
  DefaultJob : .NET 8.0.8 (8.0.824.36612), X64 RyuJIT AVX2

Method	Mean	Error	StdDev	Median	Gen0	Gen1	Allocated
Document	3.872 μs	0.1670 μs	0.4655 μs	3.857 μs	1.9150	0.4807	17.66 KB
Operation	12.341 μs	1.2843 μs	3.7869 μs	14.136 μs	3.8147	-	35.17 KB
Parameter	7.158 μs	0.3641 μs	1.0679 μs	7.320 μs	1.8921	0.0916	17.6 KB
RequestBody	7.953 μs	0.3605 μs	1.0629 μs	8.024 μs	1.8005	-	16.73 KB

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 98.83721% with 1 line in your changes missing coverage. Please review.

Project coverage is 90.53%. Comparing base (16e2a94) to head (45a97e9).
Report is 4 commits behind head on master.

Files	Patch %	Lines
...waggerGen/XmlComments/XmlCommentsDocumentHelper.cs	87.50%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3044      +/-   ##
==========================================
+ Coverage   90.46%   90.53%   +0.06%     
==========================================
  Files          74       76       +2     
  Lines        2980     3002      +22     
  Branches      470      479       +9     
==========================================
+ Hits         2696     2718      +22     
  Misses        284      284              

Flag	Coverage Δ
Linux	90.53% <98.83%> (+0.06%)	⬆️
Windows	90.53% <98.83%> (+0.06%)	⬆️
macOS	90.53% <98.83%> (+0.06%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[martincostello]

I have some benchmarks I've been using to compare the benchmarks of Swashbuckle, NSwag and the new .NET 9 ASP.NET Core OpenAPI library, and there's a noticeable really improvement from the ingesting changes from this PR.

The first major improvement is from updating to .NET 9 RC1 (martincostello/aspnetcore-openapi@7e5e365), then the second improvement with the big reduction in memory usage is from updating to 6.8.0 (martincostello/aspnetcore-openapi@b4aba65)

Thanks again!