IncludeXmlComments performance issues

[coronabytes]

Version 6.1.4 on .NET 5

We were using the following code to include multiple XMLs for comments (model dlls, app parts etc).
This was working for years until a dev referenced an external commercial library with it's own XML comment file (130k lines / 7 MB) and the generation time for the swagger.json went from ~600ms to 40s+ for each request.

Of course filtering out this file fixed the issue, however I don't think performance degradation should be that bad for a mere 130k lines of additional xml and even if it takes 40s why the swagger.json is it taking so long for subsequent requests.

I havn't looked at your code, yet it feels like you're not using a dictionary lookup and just search the xml nodes over and over again and the end result is not cached at all.

c.IncludeXmlComments(() =>
{
    try
    {
        var directory = Path.GetDirectoryName(typeof(Startup).Assembly.Location);

        var xdocs = Directory.EnumerateFiles(directory, "*.xml")
            .Select(x => XDocument.Load(x).Element("doc")?.Descendants()).ToList();

        var doc = new XDocument(new XElement("doc", xdocs));
        return new XPathDocument(doc.CreateReader());
    }
    catch (Exception)
    {
        var doc = new XDocument(new XElement("doc"));
        return new XPathDocument(doc.CreateReader());
    }
}, true);

Update:

var paramNode = _xmlNavigator.SelectSingleNode(
                $"/doc/members/member[@name='{methodMemberName}']/param[@name='{parameterInfo.Name}']");

eww xpath.. that explains it

[domaindrivendev]

If you have ideas around improving the performance of this particular feature, I would suggest submitting a PR for it. If you think the changes are going to be significant, I would start with a 1 pager refactor proposal so we can align on the changes before you make them.

[coronabytes]

Yeah thats a rather significant change.
I will consider writing a proposal in the next few weeks or so

Just take a note that the current implementation is O(N^2) and it could be O(N) parse and O(1) lookup

[stevendarby]

If you have ideas around improving the performance of this particular feature, I would suggest submitting a PR for it.

@domaindrivendev I've raised a PR for it (#2443), please review when you get a chance- thanks!

[domaindrivendev]

@stevendarby at a glance your PR approach makes sense to me. I’ll try do a full review and either merge or provide feedback later today.

[stevendarby]

@domaindrivendev still hoping for a review if you get a chance - thanks!

[stevendarby]

Hi @domaindrivendev, just checking in once more- I'm keen to hear from you regarding the PR, if you have any feedback and/or if it's likely to get merged any time soon. Thanks

[martincostello]

#2443 (comment)

[martincostello]

The changes in #2839 might also contribute to improving this.