Improve IncludeXmlComments performance (2)#3044
Merged
martincostello merged 10 commits intodomaindrivendev:masterfrom Aug 27, 2024
Merged
Improve IncludeXmlComments performance (2)#3044martincostello merged 10 commits intodomaindrivendev:masterfrom
martincostello merged 10 commits intodomaindrivendev:masterfrom
Conversation
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
SelectandSelectSingleNodewithSelectChildren(added some helper extensions for this). This completely avoids XPath expressions and the documentation even mentionsSelectChildrenbeing faster.I made the new constructors internal for now (which is also why I changed
ParameterFiltertoAddParameterFilterInstanceetc.. 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
returninstead of acontinueinXmlCommentsDocumentFilter).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.