From 945a503518c70592637788f8b8638a439625c83f Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Tue, 7 Oct 2025 22:58:59 +0200 Subject: [PATCH 1/8] Optimized XmlDocumentationFileResolver-cache --- .../Descriptors/Conventions/XmlDocumentationFileResolver.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs index 58530014338..2d25ebf48d4 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs @@ -12,7 +12,7 @@ public class XmlDocumentationFileResolver : IXmlDocumentationFileResolver private readonly Func? _resolveXmlDocumentationFileName; - private readonly ConcurrentDictionary _cache = + private readonly ConcurrentDictionary _cache = new(StringComparer.OrdinalIgnoreCase); public XmlDocumentationFileResolver() @@ -38,8 +38,9 @@ public bool TryGetXmlDocument( if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) { doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); - _cache[fullName] = doc; } + + _cache[fullName] = doc; } document = doc; From d0232ef95d585ccaece77de92891195a6f5f48fe Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Tue, 7 Oct 2025 23:51:09 +0200 Subject: [PATCH 2/8] Switch to XPathDocument (core changes) --- .../IXmlDocumentationFileResolver.cs | 7 +- .../XmlDocumentationFileResolver.cs | 13 +- .../Conventions/XmlDocumentationProvider.cs | 186 ++++++++++-------- 3 files changed, 111 insertions(+), 95 deletions(-) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs index 0173130f678..a2ce3f29dcc 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs @@ -1,6 +1,6 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; -using System.Xml.Linq; +using System.Xml.XPath; namespace HotChocolate.Types.Descriptors; @@ -12,7 +12,6 @@ public interface IXmlDocumentationFileResolver /// /// Trues to resolve an XML documentation file from the given assembly.. /// - bool TryGetXmlDocument( - Assembly assembly, - [NotNullWhen(true)] out XDocument? document); + bool TryGetXmlDocument(Assembly assembly, + [NotNullWhen(true)] out XPathDocument? document); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs index 2d25ebf48d4..9b87f36cb78 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs @@ -1,7 +1,8 @@ using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Reflection; -using System.Xml.Linq; +using System.Xml; +using System.Xml.XPath; using IOPath = System.IO.Path; namespace HotChocolate.Types.Descriptors; @@ -12,7 +13,7 @@ public class XmlDocumentationFileResolver : IXmlDocumentationFileResolver private readonly Func? _resolveXmlDocumentationFileName; - private readonly ConcurrentDictionary _cache = + private readonly ConcurrentDictionary _cache = new(StringComparer.OrdinalIgnoreCase); public XmlDocumentationFileResolver() @@ -27,7 +28,7 @@ public XmlDocumentationFileResolver(Func? resolveXmlDocumentat public bool TryGetXmlDocument( Assembly assembly, - [NotNullWhen(true)] out XDocument? document) + [NotNullWhen(true)] out XPathDocument? document) { var fullName = assembly.GetName().FullName; @@ -37,7 +38,11 @@ public bool TryGetXmlDocument( if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) { - doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); + var settings = new XmlReaderSettings { IgnoreWhitespace = false }; + + using var xmlFileStream = File.OpenRead(xmlDocumentFileName); + using var xmlFileReader = XmlReader.Create(xmlFileStream, settings); + doc = new XPathDocument(xmlFileReader); } _cache[fullName] = doc; diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs index b4f1df150cf..a5b56bd90e6 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs @@ -69,18 +69,22 @@ public XmlDocumentationProvider( return null; } + var summaryNode = element.SelectSingleNode(SummaryElementName); + var returnsNode = element.SelectSingleNode(ReturnsElementName); + var exceptionNodes = element.Select(ExceptionElementName); + var description = ComposeMemberDescription( - element.Element(SummaryElementName), - element.Element(ReturnsElementName), - element.Elements(ExceptionElementName)); + summaryNode, + returnsNode, + exceptionNodes); return RemoveLineBreakWhiteSpaces(description); } private string? ComposeMemberDescription( - XElement? summary, - XElement? returns, - IEnumerable errors) + XPathNavigator? summary, + XPathNavigator? returns, + XPathNodeIterator errors) { var description = _stringBuilderPool.Get(); @@ -113,17 +117,21 @@ public XmlDocumentationProvider( } private void AppendErrorDescription( - IEnumerable errors, + XPathNodeIterator errors, StringBuilder description, bool needsNewLine) { var errorCount = 0; - foreach (var error in errors) + while (errors.MoveNext()) { - var code = error.Attribute(Code); - if (code is { } - && !string.IsNullOrEmpty(error.Value) - && !string.IsNullOrEmpty(code.Value)) + var error = errors.Current; + if(string.IsNullOrEmpty(error?.Value)) + { + continue; + } + + var code = error.GetAttribute(Code, string.Empty); + if (!string.IsNullOrEmpty(code)) { if (errorCount == 0) { @@ -136,7 +144,7 @@ private void AppendErrorDescription( } description.Append($"{++errorCount}. "); - description.Append($"{code.Value}: "); + description.Append($"{code}: "); AppendText(error, description); } @@ -144,7 +152,7 @@ private void AppendErrorDescription( } private static void AppendText( - XElement? element, + XPathNavigator? element, StringBuilder description) { if (element is null || string.IsNullOrWhiteSpace(element.Value)) @@ -152,63 +160,65 @@ private static void AppendText( return; } - foreach (var node in element.Nodes()) + var children = element.SelectChildren(XPathNodeType.All); + while (children.MoveNext()) { - if (node is not XElement currentElement) + var child = children.Current; + switch (child?.NodeType) { - if (node is XText text) - { - description.Append(text.Value); - } + case XPathNodeType.Text: + case XPathNodeType.SignificantWhitespace: + case XPathNodeType.Whitespace: + description.Append(child.Value); + break; - continue; - } + case XPathNodeType.Element: + var localName = child.LocalName; - if (currentElement.Name == Paramref) - { - var nameAttribute = currentElement.Attribute(Name); + if (localName == Paramref) + { + var nameAttr = child.GetAttribute(Name, string.Empty); + description.Append(nameAttr); + break; + } - if (nameAttribute != null) - { - description.Append(nameAttribute.Value); - continue; - } - } + if (localName != See) + { + description.Append(child.Value); + break; + } - if (currentElement.Name != See) - { - description.Append(currentElement.Value); - continue; - } + // handle + var langword = child.GetAttribute(Langword, string.Empty); + if (!string.IsNullOrEmpty(langword)) + { + description.Append(langword); + break; + } - var attribute = currentElement.Attribute(Langword); - if (attribute != null) - { - description.Append(attribute.Value); - continue; - } + if (!string.IsNullOrEmpty(child.Value)) + { + description.Append(child.Value); + break; + } - if (!string.IsNullOrEmpty(currentElement.Value)) - { - description.Append(currentElement.Value); - } - else - { - attribute = currentElement.Attribute(Cref); - if (attribute != null) - { - description.Append(attribute.Value - .Trim('!', ':').Trim() - .Split('.').Last()); - } - else - { - attribute = currentElement.Attribute(Href); - if (attribute != null) + var cref = child.GetAttribute(Cref, string.Empty); + if (!string.IsNullOrEmpty(cref)) { - description.Append(attribute.Value); + // TODO + description.Append( + cref.Trim('!', ':', ' ') + .Split('.').Last()); + break; } - } + + var href = child.GetAttribute(Href, string.Empty); + if (!string.IsNullOrEmpty(href)) + { + description.Append(href); + } + + break; } } } @@ -224,7 +234,7 @@ private void AppendNewLineIfNeeded( } } - private XElement? GetMemberElement(MemberInfo member) + private XPathNavigator? GetMemberElement(MemberInfo member) { try { @@ -233,8 +243,11 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(member); - var element = document.XPathSelectElements(name.Path) - .FirstOrDefault(); + var element = document.CreateNavigator().SelectSingleNode(name.Path); + if (element == null) + { + return null; + } ReplaceInheritdocElements(member, element); @@ -249,7 +262,7 @@ private void AppendNewLineIfNeeded( } } - private XElement? GetParameterElement(ParameterInfo parameter) + private XPathNavigator? GetParameterElement(ParameterInfo parameter) { try { @@ -258,28 +271,28 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(parameter.Member); - var result = document.XPathSelectElements(name.Path); - var element = result.FirstOrDefault(); + var navigator = document.CreateNavigator(); + var result = navigator.SelectSingleNode(name.Path); - if (element is null) + if (result is null) { return null; } - ReplaceInheritdocElements(parameter.Member, element); + ReplaceInheritdocElements(parameter.Member, result); if (parameter.IsRetval || string.IsNullOrEmpty(parameter.Name)) { - result = document.XPathSelectElements(name.ReturnsPath); + result = navigator.SelectSingleNode(name.ReturnsPath); } else { - result = document.XPathSelectElements( + result = navigator.SelectSingleNode( name.GetParameterPath(parameter.Name)); } - return result.FirstOrDefault(); + return result; } return null; @@ -292,18 +305,19 @@ private void AppendNewLineIfNeeded( private void ReplaceInheritdocElements( MemberInfo member, - XElement? element) + XPathNavigator element) { - if (element is null) + var interitDocChildren = element.SelectChildren(Inheritdoc, string.Empty); + while (interitDocChildren.MoveNext()) { - return; - } + var child = interitDocChildren.Current; + if (child is null) + { + continue; + } - var children = element.Nodes().ToList(); - foreach (var child in children.OfType()) - { - if (string.Equals(child.Name.LocalName, Inheritdoc, - StringComparison.OrdinalIgnoreCase)) + if (string.Equals(child.LocalName, Inheritdoc, + StringComparison.OrdinalIgnoreCase)) { var baseType = member.DeclaringType?.GetTypeInfo().BaseType; @@ -316,9 +330,7 @@ private void ReplaceInheritdocElements( var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - var nodes = - baseDoc.Nodes().OfType().ToArray(); - child.ReplaceWith(nodes); + child.ReplaceSelf(baseDoc.InnerXml); } else { @@ -335,7 +347,7 @@ private void ReplaceInheritdocElements( private void ProcessInheritdocInterfaceElements( MemberInfo member, - XElement child) + XPathNavigator child) { if (member.DeclaringType is { }) { @@ -350,8 +362,8 @@ private void ProcessInheritdocInterfaceElements( var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - child.ReplaceWith( - baseDoc.Nodes().OfType().ToArray()); + child.ReplaceSelf(baseDoc.InnerXml); + return; } } } From 02e4840dde4f384756719713308813898d67ec06 Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Wed, 8 Oct 2025 23:00:22 +0200 Subject: [PATCH 3/8] Green tests + Benchmark + Small improvements Worse for inheritdoc than before #8775 --- .../IXmlDocumentationFileResolver.cs | 2 +- .../XmlDocumentationFileResolver.cs | 34 +- .../Conventions/XmlDocumentationProvider.cs | 183 +-- .../HotChocolate.Types.Tests.csproj | 1 + .../XmlDocumentProviderBenchmarks.cs | 1260 +++++++++++++++++ 5 files changed, 1384 insertions(+), 96 deletions(-) create mode 100644 src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs index a2ce3f29dcc..bc90e521594 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs @@ -13,5 +13,5 @@ public interface IXmlDocumentationFileResolver /// Trues to resolve an XML documentation file from the given assembly.. /// bool TryGetXmlDocument(Assembly assembly, - [NotNullWhen(true)] out XPathDocument? document); + [NotNullWhen(true)] out XPathNavigator? document); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs index 9b87f36cb78..3b3ce8b4df5 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs @@ -13,7 +13,7 @@ public class XmlDocumentationFileResolver : IXmlDocumentationFileResolver private readonly Func? _resolveXmlDocumentationFileName; - private readonly ConcurrentDictionary _cache = + private readonly ConcurrentDictionary _cache = new(StringComparer.OrdinalIgnoreCase); public XmlDocumentationFileResolver() @@ -26,9 +26,8 @@ public XmlDocumentationFileResolver(Func? resolveXmlDocumentat _resolveXmlDocumentationFileName = resolveXmlDocumentationFileName; } - public bool TryGetXmlDocument( - Assembly assembly, - [NotNullWhen(true)] out XPathDocument? document) + public bool TryGetXmlDocument(Assembly assembly, + [NotNullWhen(true)] out XPathNavigator? document) { var fullName = assembly.GetName().FullName; @@ -38,11 +37,10 @@ public bool TryGetXmlDocument( if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) { - var settings = new XmlReaderSettings { IgnoreWhitespace = false }; - - using var xmlFileStream = File.OpenRead(xmlDocumentFileName); - using var xmlFileReader = XmlReader.Create(xmlFileStream, settings); - doc = new XPathDocument(xmlFileReader); + var xml = File.ReadAllText(xmlDocumentFileName); + xml = AddXmlSpacePreserve(xml); + using var reader = XmlReader.Create(new StringReader(xml), new XmlReaderSettings { IgnoreWhitespace = false }); + doc = new XPathDocument(reader).CreateNavigator(); } _cache[fullName] = doc; @@ -136,4 +134,22 @@ public bool TryGetXmlDocument( return null; } } + string AddXmlSpacePreserve(string xml) + { + // Skip metatag + var startIndex = 0; + if (xml.StartsWith("", StringComparison.Ordinal) + 2; + } + + var i = xml.IndexOf('>', startIndex); + if (i < 0) + { + return xml; + } + + var head = xml.Substring(startIndex, i - startIndex); + return head.Trim() != " _stringBuilderPool; @@ -49,15 +54,22 @@ public XmlDocumentationProvider( return null; } - var description = new StringBuilder(); - AppendText(element, description); + var description = _stringBuilderPool.Get(); + try + { + AppendText(element, description); - if (description.Length == 0) + if (description.Length == 0) + { + return null; + } + + return RemoveLineBreakWhiteSpaces(description); + } + finally { - return null; + _stringBuilderPool.Return(description); } - - return RemoveLineBreakWhiteSpaces(description.ToString()); } private string? GetDescriptionInternal(MemberInfo member) @@ -69,16 +81,14 @@ public XmlDocumentationProvider( return null; } - var summaryNode = element.SelectSingleNode(SummaryElementName); - var returnsNode = element.SelectSingleNode(ReturnsElementName); - var exceptionNodes = element.Select(ExceptionElementName); + var summaryNode = element.SelectSingleNode(s_summaryXPath); + var returnsNode = element.SelectSingleNode(s_returnsXPath); + var exceptionNodes = element.Select(s_exceptionXPath); - var description = ComposeMemberDescription( + return ComposeMemberDescription( summaryNode, returnsNode, exceptionNodes); - - return RemoveLineBreakWhiteSpaces(description); } private string? ComposeMemberDescription( @@ -108,7 +118,7 @@ public XmlDocumentationProvider( AppendErrorDescription(errors, description, needsNewLine); - return description.Length == 0 ? null : description.ToString(); + return RemoveLineBreakWhiteSpaces(description); } finally { @@ -174,7 +184,6 @@ private static void AppendText( case XPathNodeType.Element: var localName = child.LocalName; - if (localName == Paramref) { var nameAttr = child.GetAttribute(Name, string.Empty); @@ -243,13 +252,13 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(member); - var element = document.CreateNavigator().SelectSingleNode(name.Path); + var element = document.SelectSingleNode(name.Path); if (element == null) { return null; } - ReplaceInheritdocElements(member, element); + element = ReplaceInheritdocElements(member, element); return element; } @@ -271,25 +280,24 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(parameter.Member); - var navigator = document.CreateNavigator(); - var result = navigator.SelectSingleNode(name.Path); + var result = document.SelectSingleNode(name.Path); if (result is null) { return null; } - ReplaceInheritdocElements(parameter.Member, result); + result = ReplaceInheritdocElements(parameter.Member, result); if (parameter.IsRetval || string.IsNullOrEmpty(parameter.Name)) { - result = navigator.SelectSingleNode(name.ReturnsPath); + result = result.SelectSingleNode(MemberName.RelativeReturnsPath); } else { - result = navigator.SelectSingleNode( - name.GetParameterPath(parameter.Name)); + result = result.SelectSingleNode( + name.GetRelativeParameterPath(parameter.Name)); } return result; @@ -303,89 +311,99 @@ private void AppendNewLineIfNeeded( } } - private void ReplaceInheritdocElements( + private void ProcessInheritdocInterfaceElements( MemberInfo member, - XPathNavigator element) + XElement child) { - var interitDocChildren = element.SelectChildren(Inheritdoc, string.Empty); - while (interitDocChildren.MoveNext()) + if (member.DeclaringType is { }) { - var child = interitDocChildren.Current; - if (child is null) - { - continue; - } - - if (string.Equals(child.LocalName, Inheritdoc, - StringComparison.OrdinalIgnoreCase)) + foreach (var baseInterface in member.DeclaringType + .GetTypeInfo().ImplementedInterfaces) { - var baseType = - member.DeclaringType?.GetTypeInfo().BaseType; - var baseMember = - baseType?.GetTypeInfo().DeclaredMembers - .SingleOrDefault(m => m.Name == member.Name); - + var baseMember = baseInterface.GetTypeInfo() + .DeclaredMembers.SingleOrDefault(m => + m.Name.EqualsOrdinal(member.Name)); if (baseMember != null) { var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - child.ReplaceSelf(baseDoc.InnerXml); - } - else - { - ProcessInheritdocInterfaceElements(member, child); + var nodes = XElement.Parse(baseDoc.OuterXml, LoadOptions.PreserveWhitespace).Nodes().OfType().ToArray(); + child.ReplaceWith(nodes); } } - else - { - ProcessInheritdocInterfaceElements(member, child); - } } } } - private void ProcessInheritdocInterfaceElements( + private XPathNavigator ReplaceInheritdocElements( MemberInfo member, - XPathNavigator child) + XPathNavigator element) { - if (member.DeclaringType is { }) + if (member.DeclaringType?.BaseType is null || !element.InnerXml.Contains(Inheritdoc, StringComparison.Ordinal)) { - foreach (var baseInterface in member.DeclaringType - .GetTypeInfo().ImplementedInterfaces) + return element; + } + + XElement xElement = XElement.Parse(element.OuterXml, LoadOptions.PreserveWhitespace); + var inheritDocNodes = xElement.XPathSelectElements($"//*[local-name()='{Inheritdoc}']"); + + foreach (var child in inheritDocNodes) + { + var baseMember = + member.DeclaringType.BaseType + .GetTypeInfo().DeclaredMembers + .SingleOrDefault(m => m.Name == member.Name); + + if (baseMember != null) { - var baseMember = baseInterface.GetTypeInfo() - .DeclaredMembers.SingleOrDefault(m => - m.Name.EqualsOrdinal(member.Name)); - if (baseMember != null) + var baseDoc = GetMemberElement(baseMember); + if (baseDoc != null) { - var baseDoc = GetMemberElement(baseMember); - if (baseDoc != null) - { - child.ReplaceSelf(baseDoc.InnerXml); - return; - } + var nodes = XElement.Parse(baseDoc.OuterXml, LoadOptions.PreserveWhitespace).Nodes().OfType().ToArray(); + child.ReplaceWith(nodes); + } + else + { + ProcessInheritdocInterfaceElements(member, child); } } + else + { + ProcessInheritdocInterfaceElements(member, child); + } } + + return xElement.CreateNavigator(); } - private static string? RemoveLineBreakWhiteSpaces(string? documentation) + private static string? RemoveLineBreakWhiteSpaces(StringBuilder stringBuilder) { - if (string.IsNullOrWhiteSpace(documentation)) + if (stringBuilder.Length == 0) { return null; } - documentation = - "\n" + documentation.Replace("\r", string.Empty).Trim('\n'); + stringBuilder.Replace("\r", string.Empty); + if (stringBuilder[0] != '\n') + { + stringBuilder.Insert(0, '\n'); + } - var whitespace = - Regex.Match(documentation, "(\\n[ \\t]*)").Value; + if (stringBuilder[^1] == '\n') + { + stringBuilder.Remove(stringBuilder.Length - 1, 1); + } - documentation = documentation.Replace(whitespace, "\n"); + var materializedString = stringBuilder.ToString(); + var whitespace = MyRegex().Match(materializedString).Value; + if (!string.IsNullOrEmpty(whitespace)) + { + stringBuilder.Replace(whitespace, "\n"); + materializedString = stringBuilder.ToString(); + } - return documentation.Trim('\n').Trim(); + return materializedString.Trim('\n', ' '); } private static MemberName GetMemberElementName(MemberInfo member) @@ -459,35 +477,28 @@ private static MemberName GetMemberElementName(MemberInfo member) private ref struct MemberName { private const string GetMemberDocPathFormat = "/doc/members/member[@name='{0}']"; - private const string ReturnsPathFormat = "{0}/returns"; - private const string ParamsPathFormat = "{0}/param[@name='{1}']"; + private const string ParamsPathFormat = "./param[@name='{0}']"; + internal const string RelativeReturnsPath = "./returns"; public MemberName(string name) { - Value = name; Path = string.Format( CultureInfo.InvariantCulture, GetMemberDocPathFormat, name); - ReturnsPath = string.Format( - CultureInfo.InvariantCulture, - ReturnsPathFormat, - Path); } - public string Value { get; } - public string Path { get; } - public string ReturnsPath { get; } - - public string GetParameterPath(string name) + public string GetRelativeParameterPath(string name) { return string.Format( CultureInfo.InvariantCulture, ParamsPathFormat, - Path, name); } } + + [GeneratedRegex("(\\n[ \\t]*)")] + private static partial Regex MyRegex(); } diff --git a/src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj b/src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj index fcfee76f3fe..c934c01195f 100644 --- a/src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj +++ b/src/HotChocolate/Core/test/Types.Tests/HotChocolate.Types.Tests.csproj @@ -6,6 +6,7 @@ + diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs new file mode 100644 index 00000000000..38d82aa1243 --- /dev/null +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -0,0 +1,1260 @@ +using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; +using System.Drawing; +using System.Globalization; +using System.Reflection; +using System.Text; +using System.Text.RegularExpressions; +using System.Xml.Linq; +using System.Xml.XPath; +using BenchmarkDotNet.Attributes; +using BenchmarkDotNet.Configs; +using BenchmarkDotNet.Diagnosers; +using BenchmarkDotNet.Exporters; +using BenchmarkDotNet.Jobs; +using BenchmarkDotNet.Loggers; +using BenchmarkDotNet.Running; +using HotChocolate.Utilities; +using Microsoft.Extensions.ObjectPool; +using Xunit.Abstractions; +using IOPath = System.IO.Path; + +#pragma warning disable + +namespace HotChocolate.Types.Descriptors; + +public class XmlDocumentProviderBenchmarks +{ + private readonly ITestOutputHelper _testOutputHelper; + + public XmlDocumentProviderBenchmarks(ITestOutputHelper testOutputHelper) + { + _testOutputHelper = testOutputHelper; + } + + //[Fact(Skip = "Run manually when performance regression testing is required")] + [Fact] + public void RunBenchmarks() + { + var config = ManualConfig.Create(DefaultConfig.Instance) + .AddDiagnoser(MemoryDiagnoser.Default) + .AddJob(Job.Dry) + .AddExporter(MarkdownExporter.GitHub); + + var summary = BenchmarkRunner.Run(typeof(Bench), config); + + var files = MarkdownExporter.GitHub.ExportToFiles(summary, NullLogger.Instance); + + _testOutputHelper.WriteLine($"Benchmark report saved to: {string.Join(" ,", files)}"); + _testOutputHelper.WriteLine(string.Join("\n", summary.ValidationErrors)); + } + + public class Bench + { + private static XmlDocumentationProvider s_documentationProvider = new XmlDocumentationProvider( + new XmlDocumentationFileResolver(), + new NoOpStringBuilderPool()); + + private static OldXmlDocumentationProvider s_oldDocumentationProvider = new OldXmlDocumentationProvider( + new OldXmlDocumentationFileResolver(), + new NoOpStringBuilderPool()); + + // Example parameterization + [Params(1, 10, 100)] public int N { get; set; } + + [Benchmark] + public void When_xml_doc_is_missing_then_description_is_empty() + { + for (int i = 0; i < N; i++) + { + s_documentationProvider.GetDescription(typeof(Point)); + } + } + + [Benchmark] + public void When_xml_doc_is_missing_then_description_is_empty_old() + { + for (int i = 0; i < N; i++) + { + s_oldDocumentationProvider.GetDescription(typeof(Point)); + } + } + + [Benchmark] + public void When_xml_doc_with_multiple_breaks_is_read_then_they_are_not_stripped_away() + { + for (int i = 0; i < N; i++) + { + s_documentationProvider.GetDescription( + typeof(WithMultilineXmlDoc) + .GetProperty(nameof(WithMultilineXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_xml_doc_with_multiple_breaks_is_read_then_they_are_not_stripped_away_old() + { + for (int i = 0; i < N; i++) + { + s_oldDocumentationProvider.GetDescription( + typeof(WithMultilineXmlDoc) + .GetProperty(nameof(WithMultilineXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_see_tag_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithSeeTagInXmlDoc) + .GetProperty(nameof(WithSeeTagInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_see_tag_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithSeeTagInXmlDoc) + .GetProperty(nameof(WithSeeTagInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_paramref_tag_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + s_documentationProvider.GetDescription( + typeof(WithParamrefTagInXmlDoc) + .GetMethod(nameof(WithParamrefTagInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_paramref_tag_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + s_oldDocumentationProvider.GetDescription( + typeof(WithParamrefTagInXmlDoc) + .GetMethod(nameof(WithParamrefTagInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_generic_tags_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithGenericTagsInXmlDoc) + .GetProperty(nameof(WithGenericTagsInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_description_has_generic_tags_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithGenericTagsInXmlDoc) + .GetProperty(nameof(WithGenericTagsInXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_type_has_description_then_it_it_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(BaseBaseClass)); + } + } + + [Benchmark] + public void When_type_has_description_then_it_it_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(BaseBaseClass)); + } + } + + [Benchmark] + public void When_we_use_custom_documentation_files_they_are_correctly_loaded() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(BaseBaseClass)); + } + } + + [Benchmark] + public void When_we_use_custom_documentation_files_they_are_correctly_loaded_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(BaseBaseClass)); + } + } + + [Benchmark] + public void When_parameter_has_inheritdoc_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + [Benchmark] + public void When_parameter_has_inheritdoc_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + } + } + + [Benchmark] + public void When_type_is_an_interface_then_description_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(IBaseBaseInterface)); + } + } + [Benchmark] + public void When_type_is_an_interface_then_description_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(IBaseBaseInterface)); + } + } + + [Benchmark] + public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + [Benchmark] + public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + + [Benchmark] + public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_on_interface_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_on_interface_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + } + } + + [Benchmark] + public void When_class_implements_interface_and_property_has_description_then_property_description_is_used() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetProperty(nameof(ClassWithInterfaceAndCustomSummaries.Foo))!); + } + } + + [Benchmark] + public void When_class_implements_interface_and_property_has_description_then_property_description_is_used_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetProperty(nameof(ClassWithInterfaceAndCustomSummaries.Foo))!); + } + } + + [Benchmark] + public void When_class_implements_interface_and_method_has_description_then_method_description_is_used() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetMethod(nameof(ClassWithInterfaceAndCustomSummaries.Bar))!); + } + } + + [Benchmark] + public void When_class_implements_interface_and_method_has_description_then_method_description_is_used_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetMethod(nameof(ClassWithInterfaceAndCustomSummaries.Bar))!); + } + } + + [Benchmark] + public void + When_class_implements_interface_and_method_has_description_then_method_parameter_description_is_used() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetMethod(nameof(ClassWithInterfaceAndCustomSummaries.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + [Benchmark] + public void + When_class_implements_interface_and_method_has_description_then_method_parameter_description_is_used_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInterfaceAndCustomSummaries) + .GetMethod(nameof(ClassWithInterfaceAndCustomSummaries.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + [Benchmark] + public void When_class_has_description_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithSummary)); + } + } + + [Benchmark] + public void When_class_has_description_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithSummary)); + } + } + + [Benchmark] + public void When_method_has_exceptions_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_exceptions_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_exceptions_then_exceptions_with_no_code_will_be_ignored() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Bar))!); + } + } + + [Benchmark] + public void When_method_has_exceptions_then_exceptions_with_no_code_will_be_ignored_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Bar))!); + } + } + + [Benchmark] + public void When_method_has_only_exceptions_with_no_code_then_error_section_will_not_be_written() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Baz))!); + } + } + + [Benchmark] + public void When_method_has_only_exceptions_with_no_code_then_error_section_will_not_be_written_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithExceptionsXmlDoc).GetMethod(nameof(WithExceptionsXmlDoc.Baz))!); + } + } + + [Benchmark] + public void When_method_has_no_exceptions_then_it_is_ignored() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithoutExceptionsXmlDoc).GetMethod(nameof(WithoutExceptionsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_no_exceptions_then_it_is_ignored_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithoutExceptionsXmlDoc).GetMethod(nameof(WithoutExceptionsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_returns_then_it_is_converted() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithReturnsXmlDoc).GetMethod(nameof(WithReturnsXmlDoc.Foo))!); + } + } + + + [Benchmark] + public void When_method_has_returns_then_it_is_converted_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithReturnsXmlDoc).GetMethod(nameof(WithReturnsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_no_returns_then_it_is_ignored() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithoutReturnsXmlDoc).GetMethod(nameof(WithoutReturnsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_no_returns_then_it_is_ignored_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithoutReturnsXmlDoc).GetMethod(nameof(WithoutReturnsXmlDoc.Foo))!); + } + } + + [Benchmark] + public void When_method_has_dictionary_args_then_it_is_found() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(WithDictionaryArgs).GetMethod(nameof(WithDictionaryArgs.Method))!); + } + } + + [Benchmark] + public void When_method_has_dictionary_args_then_it_is_found_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(WithDictionaryArgs).GetMethod(nameof(WithDictionaryArgs.Method))!); + } + } + } +} + +/// +/// Resolves an XML documentation file from an assembly. +/// +public interface IOldXmlDocumentationFileResolver +{ + /// + /// Trues to resolve an XML documentation file from the given assembly.. + /// + bool TryGetXmlDocument(Assembly assembly, + [NotNullWhen(true)] out XDocument? document); +} + + +public class OldXmlDocumentationFileResolver : IOldXmlDocumentationFileResolver +{ + private const string Bin = "bin"; + + private readonly Func? _resolveXmlDocumentationFileName; + + private readonly ConcurrentDictionary _cache = + new(StringComparer.OrdinalIgnoreCase); + + public OldXmlDocumentationFileResolver() + { + _resolveXmlDocumentationFileName = null; + } + + public OldXmlDocumentationFileResolver(Func? resolveXmlDocumentationFileName) + { + _resolveXmlDocumentationFileName = resolveXmlDocumentationFileName; + } + + public bool TryGetXmlDocument( + Assembly assembly, + [NotNullWhen(true)] out XDocument? document) + { + var fullName = assembly.GetName().FullName; + + if (!_cache.TryGetValue(fullName, out var doc)) + { + var xmlDocumentFileName = GetXmlDocumentationPath(assembly); + + if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) + { + doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); + _cache[fullName] = doc; + } + } + + document = doc; + return document != null; + } + + private string? GetXmlDocumentationPath(Assembly? assembly) + { + try + { + if (assembly is null) + { + return null; + } + + var assemblyName = assembly.GetName(); + if (string.IsNullOrEmpty(assemblyName.Name)) + { + return null; + } + + if (_cache.ContainsKey(assemblyName.FullName)) + { + return null; + } + + var expectedDocFile = _resolveXmlDocumentationFileName is null + ? $"{assemblyName.Name}.xml" + : _resolveXmlDocumentationFileName(assembly); + + string path; + if (!string.IsNullOrEmpty(assembly.Location)) + { + var assemblyDirectory = IOPath.GetDirectoryName(assembly.Location); + path = IOPath.Combine(assemblyDirectory!, expectedDocFile); + if (File.Exists(path)) + { + return path; + } + } + +#pragma warning disable SYSLIB0012 + var codeBase = assembly.CodeBase; +#pragma warning restore SYSLIB0012 + if (!string.IsNullOrEmpty(codeBase)) + { + path = IOPath.Combine( + IOPath.GetDirectoryName(codeBase.Replace("file:///", string.Empty))!, + expectedDocFile) + .Replace("file:\\", string.Empty); + + if (File.Exists(path)) + { + return path; + } + } + + var baseDirectory = AppDomain.CurrentDomain.BaseDirectory; + if (!string.IsNullOrEmpty(baseDirectory)) + { + path = IOPath.Combine(baseDirectory, expectedDocFile); + if (File.Exists(path)) + { + return path; + } + + return IOPath.Combine(baseDirectory, Bin, expectedDocFile); + } + + var currentDirectory = Directory.GetCurrentDirectory(); + path = IOPath.Combine(currentDirectory, expectedDocFile); + if (File.Exists(path)) + { + return path; + } + + path = IOPath.Combine(currentDirectory, Bin, expectedDocFile); + + if (File.Exists(path)) + { + return path; + } + + return null; + } + catch + { + return null; + } + } +} + + +public class OldXmlDocumentationProvider : IDocumentationProvider +{ + private const string SummaryElementName = "summary"; + private const string ExceptionElementName = "exception"; + private const string ReturnsElementName = "returns"; + private const string Inheritdoc = "inheritdoc"; + private const string See = "see"; + private const string Langword = "langword"; + private const string Cref = "cref"; + private const string Href = "href"; + private const string Code = "code"; + private const string Paramref = "paramref"; + private const string Name = "name"; + + private readonly IOldXmlDocumentationFileResolver _fileResolver; + private readonly ObjectPool _stringBuilderPool; + + public OldXmlDocumentationProvider( + IOldXmlDocumentationFileResolver fileResolver, + ObjectPool stringBuilderPool) + { + _fileResolver = fileResolver ?? throw new ArgumentNullException(nameof(fileResolver)); + _stringBuilderPool = stringBuilderPool; + } + + public string? GetDescription(Type type) => + GetDescriptionInternal(type); + + public string? GetDescription(MemberInfo member) => + GetDescriptionInternal(member); + + public string? GetDescription(ParameterInfo parameter) + { + var element = GetParameterElement(parameter); + + if (element is null) + { + return null; + } + + var description = new StringBuilder(); + AppendText(element, description); + + if (description.Length == 0) + { + return null; + } + + return RemoveLineBreakWhiteSpaces(description.ToString()); + } + + private string? GetDescriptionInternal(MemberInfo member) + { + var element = GetMemberElement(member); + + if (element is null) + { + return null; + } + + var description = ComposeMemberDescription( + element.Element(SummaryElementName), + element.Element(ReturnsElementName), + element.Elements(ExceptionElementName)); + + return RemoveLineBreakWhiteSpaces(description); + } + + private string? ComposeMemberDescription( + XElement? summary, + XElement? returns, + IEnumerable errors) + { + var description = _stringBuilderPool.Get(); + + try + { + var needsNewLine = false; + + if (!string.IsNullOrEmpty(summary?.Value)) + { + AppendText(summary, description); + needsNewLine = true; + } + + if (!string.IsNullOrEmpty(returns?.Value)) + { + AppendNewLineIfNeeded(description, needsNewLine); + description.AppendLine("**Returns:**"); + AppendText(returns, description); + needsNewLine = true; + } + + AppendErrorDescription(errors, description, needsNewLine); + + return description.Length == 0 ? null : description.ToString(); + } + finally + { + _stringBuilderPool.Return(description); + } + } + + private void AppendErrorDescription( + IEnumerable errors, + StringBuilder description, + bool needsNewLine) + { + var errorCount = 0; + foreach (var error in errors) + { + var code = error.Attribute(Code); + if (code is { } + && !string.IsNullOrEmpty(error.Value) + && !string.IsNullOrEmpty(code.Value)) + { + if (errorCount == 0) + { + AppendNewLineIfNeeded(description, needsNewLine); + description.AppendLine("**Errors:**"); + } + else + { + description.AppendLine(); + } + + description.Append($"{++errorCount}. "); + description.Append($"{code.Value}: "); + + AppendText(error, description); + } + } + } + + private static void AppendText( + XElement? element, + StringBuilder description) + { + if (element is null || string.IsNullOrWhiteSpace(element.Value)) + { + return; + } + + foreach (var node in element.Nodes()) + { + if (node is not XElement currentElement) + { + if (node is XText text) + { + description.Append(text.Value); + } + + continue; + } + + if (currentElement.Name == Paramref) + { + var nameAttribute = currentElement.Attribute(Name); + + if (nameAttribute != null) + { + description.Append(nameAttribute.Value); + continue; + } + } + + if (currentElement.Name != See) + { + description.Append(currentElement.Value); + continue; + } + + var attribute = currentElement.Attribute(Langword); + if (attribute != null) + { + description.Append(attribute.Value); + continue; + } + + if (!string.IsNullOrEmpty(currentElement.Value)) + { + description.Append(currentElement.Value); + } + else + { + attribute = currentElement.Attribute(Cref); + if (attribute != null) + { + description.Append(attribute.Value + .Trim('!', ':').Trim() + .Split('.').Last()); + } + else + { + attribute = currentElement.Attribute(Href); + if (attribute != null) + { + description.Append(attribute.Value); + } + } + } + } + } + + private void AppendNewLineIfNeeded( + StringBuilder description, + bool condition) + { + if (condition) + { + description.AppendLine(); + description.AppendLine(); + } + } + + private XElement? GetMemberElement(MemberInfo member) + { + try + { + if (_fileResolver.TryGetXmlDocument( + member.Module.Assembly, + out var document)) + { + var name = GetMemberElementName(member); + var element = document.XPathSelectElements(name.Path) + .FirstOrDefault(); + + ReplaceInheritdocElements(member, element); + + return element; + } + + return null; + } + catch + { + return null; + } + } + + private XElement? GetParameterElement(ParameterInfo parameter) + { + try + { + if (_fileResolver.TryGetXmlDocument( + parameter.Member.Module.Assembly, + out var document)) + { + var name = GetMemberElementName(parameter.Member); + var result = document.XPathSelectElements(name.Path); + var element = result.FirstOrDefault(); + + if (element is null) + { + return null; + } + + ReplaceInheritdocElements(parameter.Member, element); + + if (parameter.IsRetval + || string.IsNullOrEmpty(parameter.Name)) + { + result = document.XPathSelectElements(name.ReturnsPath); + } + else + { + result = document.XPathSelectElements( + name.GetParameterPath(parameter.Name)); + } + + return result.FirstOrDefault(); + } + + return null; + } + catch + { + return null; + } + } + + private void ReplaceInheritdocElements( + MemberInfo member, + XElement? element) + { + if (element is null) + { + return; + } + + var children = element.Nodes().ToList(); + foreach (var child in children.OfType()) + { + if (string.Equals(child.Name.LocalName, Inheritdoc, + StringComparison.OrdinalIgnoreCase)) + { + var baseType = + member.DeclaringType?.GetTypeInfo().BaseType; + var baseMember = + baseType?.GetTypeInfo().DeclaredMembers + .SingleOrDefault(m => m.Name == member.Name); + + if (baseMember != null) + { + var baseDoc = GetMemberElement(baseMember); + if (baseDoc != null) + { + var nodes = + baseDoc.Nodes().OfType().ToArray(); + child.ReplaceWith(nodes); + } + else + { + ProcessInheritdocInterfaceElements(member, child); + } + } + else + { + ProcessInheritdocInterfaceElements(member, child); + } + } + } + } + + private void ProcessInheritdocInterfaceElements( + MemberInfo member, + XElement child) + { + if (member.DeclaringType is { }) + { + foreach (var baseInterface in member.DeclaringType + .GetTypeInfo().ImplementedInterfaces) + { + var baseMember = baseInterface.GetTypeInfo() + .DeclaredMembers.SingleOrDefault(m => + m.Name.EqualsOrdinal(member.Name)); + if (baseMember != null) + { + var baseDoc = GetMemberElement(baseMember); + if (baseDoc != null) + { + child.ReplaceWith( + baseDoc.Nodes().OfType().ToArray()); + } + } + } + } + } + + private static string? RemoveLineBreakWhiteSpaces(string? documentation) + { + if (string.IsNullOrWhiteSpace(documentation)) + { + return null; + } + + documentation = + "\n" + documentation.Replace("\r", string.Empty).Trim('\n'); + + var whitespace = + Regex.Match(documentation, "(\\n[ \\t]*)").Value; + + documentation = documentation.Replace(whitespace, "\n"); + + return documentation.Trim('\n').Trim(); + } + + private static MemberName GetMemberElementName(MemberInfo member) + { + char prefixCode; + + var memberName = + member is Type { FullName: { Length: > 0 } } memberType + ? memberType.FullName + : member.DeclaringType is null + ? member.Name + : member.DeclaringType.FullName + "." + member.Name; + + switch (member.MemberType) + { + case MemberTypes.Constructor: + memberName = memberName.Replace(".ctor", "#ctor"); + goto case MemberTypes.Method; + + case MemberTypes.Method: + prefixCode = 'M'; + + var paramTypesList = string.Join(",", + ((MethodBase)member).GetParameters() + .Select(x => Regex + .Replace(x.ParameterType.FullName!, + "(`[0-9]+)|(, .*?PublicKeyToken=[0-9a-z]*)", + string.Empty) + .Replace("[[", "{") + .Replace("]]", "}") + .Replace("],[", ",")) + .ToArray()); + + if (!string.IsNullOrEmpty(paramTypesList)) + { + memberName += "(" + paramTypesList + ")"; + } + + break; + + case MemberTypes.Event: + prefixCode = 'E'; + break; + + case MemberTypes.Field: + prefixCode = 'F'; + break; + + case MemberTypes.NestedType: + memberName = memberName?.Replace('+', '.'); + goto case MemberTypes.TypeInfo; + + case MemberTypes.TypeInfo: + prefixCode = 'T'; + break; + + case MemberTypes.Property: + prefixCode = 'P'; + break; + + default: + throw new ArgumentException( + "Unknown member type.", + nameof(member)); + } + + return new MemberName( + $"{prefixCode}:{memberName?.Replace("+", ".")}"); + } + + private ref struct MemberName + { + private const string GetMemberDocPathFormat = "/doc/members/member[@name='{0}']"; + private const string ReturnsPathFormat = "{0}/returns"; + private const string ParamsPathFormat = "{0}/param[@name='{1}']"; + + public MemberName(string name) + { + Value = name; + Path = string.Format( + CultureInfo.InvariantCulture, + GetMemberDocPathFormat, + name); + ReturnsPath = string.Format( + CultureInfo.InvariantCulture, + ReturnsPathFormat, + Path); + } + + public string Value { get; } + + public string Path { get; } + + public string ReturnsPath { get; } + + public string GetParameterPath(string name) + { + return string.Format( + CultureInfo.InvariantCulture, + ParamsPathFormat, + Path, + name); + } + } +} + + From c138420dc34de1b6d4345d874398f9e2b39a6fe5 Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Thu, 9 Oct 2025 18:52:49 +0200 Subject: [PATCH 4/8] Back to default, huge alloc optimizations --- .../IXmlDocumentationFileResolver.cs | 4 +- .../XmlDocumentationFileResolver.cs | 33 +- .../Conventions/XmlDocumentationProvider.cs | 386 +++++++++--------- .../XmlDocumentProviderBenchmarks.cs | 2 +- 4 files changed, 200 insertions(+), 225 deletions(-) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs index bc90e521594..51aa804f78a 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs @@ -1,6 +1,6 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; -using System.Xml.XPath; +using System.Xml.Linq; namespace HotChocolate.Types.Descriptors; @@ -13,5 +13,5 @@ public interface IXmlDocumentationFileResolver /// Trues to resolve an XML documentation file from the given assembly.. /// bool TryGetXmlDocument(Assembly assembly, - [NotNullWhen(true)] out XPathNavigator? document); + [NotNullWhen(true)] out XDocument? document); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs index 3b3ce8b4df5..2d25ebf48d4 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs @@ -1,8 +1,7 @@ using System.Collections.Concurrent; using System.Diagnostics.CodeAnalysis; using System.Reflection; -using System.Xml; -using System.Xml.XPath; +using System.Xml.Linq; using IOPath = System.IO.Path; namespace HotChocolate.Types.Descriptors; @@ -13,7 +12,7 @@ public class XmlDocumentationFileResolver : IXmlDocumentationFileResolver private readonly Func? _resolveXmlDocumentationFileName; - private readonly ConcurrentDictionary _cache = + private readonly ConcurrentDictionary _cache = new(StringComparer.OrdinalIgnoreCase); public XmlDocumentationFileResolver() @@ -26,8 +25,9 @@ public XmlDocumentationFileResolver(Func? resolveXmlDocumentat _resolveXmlDocumentationFileName = resolveXmlDocumentationFileName; } - public bool TryGetXmlDocument(Assembly assembly, - [NotNullWhen(true)] out XPathNavigator? document) + public bool TryGetXmlDocument( + Assembly assembly, + [NotNullWhen(true)] out XDocument? document) { var fullName = assembly.GetName().FullName; @@ -37,10 +37,7 @@ public bool TryGetXmlDocument(Assembly assembly, if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) { - var xml = File.ReadAllText(xmlDocumentFileName); - xml = AddXmlSpacePreserve(xml); - using var reader = XmlReader.Create(new StringReader(xml), new XmlReaderSettings { IgnoreWhitespace = false }); - doc = new XPathDocument(reader).CreateNavigator(); + doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); } _cache[fullName] = doc; @@ -134,22 +131,4 @@ public bool TryGetXmlDocument(Assembly assembly, return null; } } - string AddXmlSpacePreserve(string xml) - { - // Skip metatag - var startIndex = 0; - if (xml.StartsWith("", StringComparison.Ordinal) + 2; - } - - var i = xml.IndexOf('>', startIndex); - if (i < 0) - { - return xml; - } - - var head = xml.Substring(startIndex, i - startIndex); - return head.Trim() != " _stringBuilderPool; @@ -81,9 +74,9 @@ public XmlDocumentationProvider( return null; } - var summaryNode = element.SelectSingleNode(s_summaryXPath); - var returnsNode = element.SelectSingleNode(s_returnsXPath); - var exceptionNodes = element.Select(s_exceptionXPath); + var summaryNode = element.Element(SummaryElementName); + var returnsNode = element.Element(ReturnsElementName); + var exceptionNodes = element.Descendants(ExceptionElementName); return ComposeMemberDescription( summaryNode, @@ -92,9 +85,9 @@ public XmlDocumentationProvider( } private string? ComposeMemberDescription( - XPathNavigator? summary, - XPathNavigator? returns, - XPathNodeIterator errors) + XElement? summary, + XElement? returns, + IEnumerable errors) { var description = _stringBuilderPool.Get(); @@ -118,7 +111,7 @@ public XmlDocumentationProvider( AppendErrorDescription(errors, description, needsNewLine); - return RemoveLineBreakWhiteSpaces(description); + return description.Length == 0 ? null : RemoveLineBreakWhiteSpaces(description); } finally { @@ -127,107 +120,112 @@ public XmlDocumentationProvider( } private void AppendErrorDescription( - XPathNodeIterator errors, + IEnumerable errors, StringBuilder description, bool needsNewLine) { var errorCount = 0; - while (errors.MoveNext()) + foreach (var error in errors) { - var error = errors.Current; - if(string.IsNullOrEmpty(error?.Value)) - { - continue; - } - - var code = error.GetAttribute(Code, string.Empty); - if (!string.IsNullOrEmpty(code)) + if (!error.IsEmpty) { - if (errorCount == 0) + var code = error.Attribute(Code); + if (code is not null) { - AppendNewLineIfNeeded(description, needsNewLine); - description.AppendLine("**Errors:**"); - } - else - { - description.AppendLine(); + var codeValue = code.Value; + if (!string.IsNullOrEmpty(codeValue)) + { + if (errorCount == 0) + { + AppendNewLineIfNeeded(description, needsNewLine); + description.AppendLine("**Errors:**"); + } + else + { + description.AppendLine(); + } + + description.Append($"{++errorCount}. "); + description.Append($"{codeValue}: "); + + AppendText(error, description); + } } - - description.Append($"{++errorCount}. "); - description.Append($"{code}: "); - - AppendText(error, description); } } } private static void AppendText( - XPathNavigator? element, + XElement? element, StringBuilder description) { - if (element is null || string.IsNullOrWhiteSpace(element.Value)) + if (element?.IsEmpty != false) { return; } - var children = element.SelectChildren(XPathNodeType.All); - while (children.MoveNext()) + foreach (var node in element.Nodes()) { - var child = children.Current; - switch (child?.NodeType) + if (node is not XElement currentElement) { - case XPathNodeType.Text: - case XPathNodeType.SignificantWhitespace: - case XPathNodeType.Whitespace: - description.Append(child.Value); - break; + if (node is XText text) + { + description.Append(text.Value); + } - case XPathNodeType.Element: - var localName = child.LocalName; - if (localName == Paramref) - { - var nameAttr = child.GetAttribute(Name, string.Empty); - description.Append(nameAttr); - break; - } + continue; + } - if (localName != See) - { - description.Append(child.Value); - break; - } + if (currentElement.Name == Paramref) + { + var nameAttribute = currentElement.Attribute(Name); + if (nameAttribute != null) + { + description.Append(nameAttribute.Value); + continue; + } + } - // handle - var langword = child.GetAttribute(Langword, string.Empty); - if (!string.IsNullOrEmpty(langword)) - { - description.Append(langword); - break; - } + if (currentElement.Name != See) + { + description.Append(currentElement.Value); + continue; + } - if (!string.IsNullOrEmpty(child.Value)) - { - description.Append(child.Value); - break; - } + var attribute = currentElement.Attribute(Langword); + if (attribute != null) + { + description.Append(attribute.Value); + continue; + } - var cref = child.GetAttribute(Cref, string.Empty); - if (!string.IsNullOrEmpty(cref)) + if (!currentElement.IsEmpty) + { + description.Append(currentElement.Value); + } + else + { + attribute = currentElement.Attribute(Cref); + if (attribute != null) + { + var value = attribute.Value.AsSpan().Trim(['!', ':', ' ']); + + var lastDotIndex = value.LastIndexOf('.'); + if (lastDotIndex >= 0) { - // TODO - description.Append( - cref.Trim('!', ':', ' ') - .Split('.').Last()); - break; + value = value[(lastDotIndex + 1)..]; } - var href = child.GetAttribute(Href, string.Empty); - if (!string.IsNullOrEmpty(href)) + description.Append(value); + } + else + { + attribute = currentElement.Attribute(Href); + if (attribute != null) { - description.Append(href); + description.Append(attribute.Value); } - - break; + } } } } @@ -243,7 +241,7 @@ private void AppendNewLineIfNeeded( } } - private XPathNavigator? GetMemberElement(MemberInfo member) + private XElement? GetMemberElement(MemberInfo member) { try { @@ -252,13 +250,17 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(member); - var element = document.SelectSingleNode(name.Path); - if (element == null) - { - return null; - } - - element = ReplaceInheritdocElements(member, element); + var x = document + .Element("doc")? + .Element("members")? + .Elements("member"); + var element = x?.FirstOrDefault(m => m.Attribute("name")?.Value == name); + if (element == null) + { + return null; + } + + ReplaceInheritdocElements(member, element); return element; } @@ -271,7 +273,7 @@ private void AppendNewLineIfNeeded( } } - private XPathNavigator? GetParameterElement(ParameterInfo parameter) + private XElement? GetParameterElement(ParameterInfo parameter) { try { @@ -280,27 +282,28 @@ private void AppendNewLineIfNeeded( out var document)) { var name = GetMemberElementName(parameter.Member); - var result = document.SelectSingleNode(name.Path); + var x = document + .Element("doc")? + .Element("members")? + .Elements("member"); + var element = x?.FirstOrDefault(m => m.Attribute("name")?.Value == name); - if (result is null) + if (element is null) { return null; } - result = ReplaceInheritdocElements(parameter.Member, result); + ReplaceInheritdocElements(parameter.Member, element); if (parameter.IsRetval || string.IsNullOrEmpty(parameter.Name)) { - result = result.SelectSingleNode(MemberName.RelativeReturnsPath); - } - else - { - result = result.SelectSingleNode( - name.GetRelativeParameterPath(parameter.Name)); + return element.Element("returns"); } - return result; + return element + .Elements("param") + .FirstOrDefault(m => m.Attribute("name")?.Value == parameter.Name); } return null; @@ -317,8 +320,7 @@ private void ProcessInheritdocInterfaceElements( { if (member.DeclaringType is { }) { - foreach (var baseInterface in member.DeclaringType - .GetTypeInfo().ImplementedInterfaces) + foreach (var baseInterface in member.DeclaringType.GetInterfaces()) { var baseMember = baseInterface.GetTypeInfo() .DeclaredMembers.SingleOrDefault(m => @@ -328,31 +330,29 @@ private void ProcessInheritdocInterfaceElements( var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - var nodes = XElement.Parse(baseDoc.OuterXml, LoadOptions.PreserveWhitespace).Nodes().OfType().ToArray(); - child.ReplaceWith(nodes); + // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw + child.ReplaceWith(baseDoc.Nodes()); } } } } } - private XPathNavigator ReplaceInheritdocElements( + private void ReplaceInheritdocElements( MemberInfo member, - XPathNavigator element) + XElement element) { - if (member.DeclaringType?.BaseType is null || !element.InnerXml.Contains(Inheritdoc, StringComparison.Ordinal)) + var baseType = member.DeclaringType?.BaseType; + if (baseType is null) { - return element; + return; } - XElement xElement = XElement.Parse(element.OuterXml, LoadOptions.PreserveWhitespace); - var inheritDocNodes = xElement.XPathSelectElements($"//*[local-name()='{Inheritdoc}']"); - - foreach (var child in inheritDocNodes) + var children = element.Elements(Inheritdoc); + foreach (var child in children) { var baseMember = - member.DeclaringType.BaseType - .GetTypeInfo().DeclaredMembers + baseType?.GetTypeInfo().DeclaredMembers .SingleOrDefault(m => m.Name == member.Name); if (baseMember != null) @@ -360,7 +360,8 @@ private XPathNavigator ReplaceInheritdocElements( var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - var nodes = XElement.Parse(baseDoc.OuterXml, LoadOptions.PreserveWhitespace).Nodes().OfType().ToArray(); + // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw + var nodes = baseDoc.Nodes(); child.ReplaceWith(nodes); } else @@ -373,8 +374,6 @@ private XPathNavigator ReplaceInheritdocElements( ProcessInheritdocInterfaceElements(member, child); } } - - return xElement.CreateNavigator(); } private static string? RemoveLineBreakWhiteSpaces(StringBuilder stringBuilder) @@ -406,99 +405,96 @@ private XPathNavigator ReplaceInheritdocElements( return materializedString.Trim('\n', ' '); } - private static MemberName GetMemberElementName(MemberInfo member) + private string GetMemberElementName(MemberInfo member) { char prefixCode; - - var memberName = - member is Type { FullName: { Length: > 0 } } memberType - ? memberType.FullName - : member.DeclaringType is null - ? member.Name - : member.DeclaringType.FullName + "." + member.Name; - - switch (member.MemberType) + var builder = _stringBuilderPool.Get(); + try { - case MemberTypes.Constructor: - memberName = memberName.Replace(".ctor", "#ctor"); - goto case MemberTypes.Method; - - case MemberTypes.Method: - prefixCode = 'M'; - - var paramTypesList = string.Join(",", - ((MethodBase)member).GetParameters() - .Select(x => Regex - .Replace(x.ParameterType.FullName!, - "(`[0-9]+)|(, .*?PublicKeyToken=[0-9a-z]*)", - string.Empty) - .Replace("[[", "{") - .Replace("]]", "}") - .Replace("],[", ",")) - .ToArray()); - - if (!string.IsNullOrEmpty(paramTypesList)) - { - memberName += "(" + paramTypesList + ")"; - } - - break; + if (member is Type { FullName.Length: > 0 } memberType) + { + builder.Append(memberType.FullName); + } + else if (member.DeclaringType is null) + { + builder.Append(member.Name); + } + else + { + builder.Append(member.DeclaringType.FullName).Append('.').Append(member.Name); + } - case MemberTypes.Event: - prefixCode = 'E'; - break; + switch (member.MemberType) + { + case MemberTypes.Constructor: + builder.Replace(".ctor", "#ctor"); + goto case MemberTypes.Method; - case MemberTypes.Field: - prefixCode = 'F'; - break; + case MemberTypes.Method: + prefixCode = 'M'; - case MemberTypes.NestedType: - memberName = memberName?.Replace('+', '.'); - goto case MemberTypes.TypeInfo; + var parameters = ((MethodBase)member).GetParameters(); + if (parameters.Length > 0) + { + builder.Append('('); + for (var index = 0; index < parameters.Length; index++) + { + var parameterInfo = parameters[index]; + var result = MyRegex1().Replace(parameterInfo.ParameterType.FullName!, m => m.Value switch + { + "[[" => "{", + "]]" => "}", + "],[" => ",", + _ => "" + }); + builder.Append(result); + if (index < parameters.Length - 1) + { + builder.Append(','); + } + } + + builder.Append(')'); + } - case MemberTypes.TypeInfo: - prefixCode = 'T'; - break; + break; - case MemberTypes.Property: - prefixCode = 'P'; - break; + case MemberTypes.Event: + prefixCode = 'E'; + break; - default: - throw new ArgumentException( - "Unknown member type.", - nameof(member)); - } + case MemberTypes.Field: + prefixCode = 'F'; + break; - return new MemberName( - $"{prefixCode}:{memberName?.Replace("+", ".")}"); - } + case MemberTypes.NestedType: + builder.Replace('+', '.'); + goto case MemberTypes.TypeInfo; - private ref struct MemberName - { - private const string GetMemberDocPathFormat = "/doc/members/member[@name='{0}']"; - private const string ParamsPathFormat = "./param[@name='{0}']"; - internal const string RelativeReturnsPath = "./returns"; + case MemberTypes.TypeInfo: + prefixCode = 'T'; + break; - public MemberName(string name) - { - Path = string.Format( - CultureInfo.InvariantCulture, - GetMemberDocPathFormat, - name); - } + case MemberTypes.Property: + prefixCode = 'P'; + break; - public string Path { get; } + default: + throw new ArgumentException( + "Unknown member type.", + nameof(member)); + } - public string GetRelativeParameterPath(string name) + return builder.Insert(0, prefixCode).Insert(1, ':').Replace("+", ".").ToString(); + } + finally { - return string.Format( - CultureInfo.InvariantCulture, - ParamsPathFormat, - name); + _stringBuilderPool.Return(builder); } } [GeneratedRegex("(\\n[ \\t]*)")] private static partial Regex MyRegex(); + [GeneratedRegex(@"(`\d+)|(, .*?PublicKeyToken=[0-9a-z]*)|\[\[|\]\]|\],\[")] + private static partial Regex MyRegex1(); } diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs index 38d82aa1243..f9222df694a 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -38,7 +38,7 @@ public void RunBenchmarks() { var config = ManualConfig.Create(DefaultConfig.Instance) .AddDiagnoser(MemoryDiagnoser.Default) - .AddJob(Job.Dry) + .AddJob(Job.ShortRun) .AddExporter(MarkdownExporter.GitHub); var summary = BenchmarkRunner.Run(typeof(Bench), config); From 8782e8cce9aaaa757b830e88d6678251098b4535 Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Thu, 9 Oct 2025 19:41:05 +0200 Subject: [PATCH 5/8] Further (agressive) optimizations. --- .../Conventions/DescriptorContext.cs | 2 +- ...solver.cs => IXmlDocumentationResolver.cs} | 6 +- .../Conventions/XmlDocumentationProvider.cs | 98 +++++++++++-------- ...esolver.cs => XmlDocumentationResolver.cs} | 28 +++--- .../DefaultNamingConventionsTests.cs | 12 +-- .../XmlDocumentProviderBenchmarks.cs | 14 +-- .../XmlDocumentationProviderTests.cs | 50 +++++----- .../Descriptors/DescriptorContextTests.cs | 4 +- 8 files changed, 116 insertions(+), 98 deletions(-) rename src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/{IXmlDocumentationFileResolver.cs => IXmlDocumentationResolver.cs} (59%) rename src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/{XmlDocumentationFileResolver.cs => XmlDocumentationResolver.cs} (76%) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DescriptorContext.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DescriptorContext.cs index 469fd8b58d3..35c3ec50225 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DescriptorContext.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/DescriptorContext.cs @@ -67,7 +67,7 @@ public INamingConventions Naming _naming ??= GetConventionOrDefault(() => Options.UseXmlDocumentation ? new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver( + new XmlDocumentationResolver( Options.ResolveXmlDocumentationFileName), _serviceHelper.GetStringBuilderPool())) : new DefaultNamingConventions( diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationResolver.cs similarity index 59% rename from src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs rename to src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationResolver.cs index 51aa804f78a..ccc08e3ac4f 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/IXmlDocumentationResolver.cs @@ -7,11 +7,11 @@ namespace HotChocolate.Types.Descriptors; /// /// Resolves an XML documentation file from an assembly. /// -public interface IXmlDocumentationFileResolver +public interface IXmlDocumentationResolver { /// - /// Trues to resolve an XML documentation file from the given assembly.. + /// Trues to resolve an XML documentation element from the given assembly.. /// bool TryGetXmlDocument(Assembly assembly, - [NotNullWhen(true)] out XDocument? document); + [NotNullWhen(true)] out IReadOnlyDictionary? memberLookup); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs index 37d9bc75fb7..e1aaa90a940 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs @@ -21,14 +21,14 @@ public partial class XmlDocumentationProvider : IDocumentationProvider private const string Paramref = "paramref"; private const string Name = "name"; - private readonly IXmlDocumentationFileResolver _fileResolver; + private readonly IXmlDocumentationResolver _documentationResolver; private readonly ObjectPool _stringBuilderPool; public XmlDocumentationProvider( - IXmlDocumentationFileResolver fileResolver, + IXmlDocumentationResolver documentationResolver, ObjectPool stringBuilderPool) { - _fileResolver = fileResolver ?? throw new ArgumentNullException(nameof(fileResolver)); + _documentationResolver = documentationResolver ?? throw new ArgumentNullException(nameof(documentationResolver)); _stringBuilderPool = stringBuilderPool; } @@ -230,14 +230,13 @@ private static void AppendText( } } - private void AppendNewLineIfNeeded( + private static void AppendNewLineIfNeeded( StringBuilder description, bool condition) { if (condition) { - description.AppendLine(); - description.AppendLine(); + description.Append("\n\n"); } } @@ -245,20 +244,15 @@ private void AppendNewLineIfNeeded( { try { - if (_fileResolver.TryGetXmlDocument( + if (_documentationResolver.TryGetXmlDocument( member.Module.Assembly, - out var document)) + out var elementLookup)) { var name = GetMemberElementName(member); - var x = document - .Element("doc")? - .Element("members")? - .Elements("member"); - var element = x?.FirstOrDefault(m => m.Attribute("name")?.Value == name); - if (element == null) - { - return null; - } + if (!elementLookup.TryGetValue(name, out var element)) + { + return null; + } ReplaceInheritdocElements(member, element); @@ -277,18 +271,12 @@ private void AppendNewLineIfNeeded( { try { - if (_fileResolver.TryGetXmlDocument( + if (_documentationResolver.TryGetXmlDocument( parameter.Member.Module.Assembly, - out var document)) + out var elementLookup)) { var name = GetMemberElementName(parameter.Member); - var x = document - .Element("doc")? - .Element("members")? - .Elements("member"); - var element = x?.FirstOrDefault(m => m.Attribute("name")?.Value == name); - - if (element is null) + if (!elementLookup.TryGetValue(name, out var element)) { return null; } @@ -331,6 +319,7 @@ private void ProcessInheritdocInterfaceElements( if (baseDoc != null) { // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw + // TODO: clone / lock / replace ? child.ReplaceWith(baseDoc.Nodes()); } } @@ -342,16 +331,22 @@ private void ReplaceInheritdocElements( MemberInfo member, XElement element) { + if (!element.Value.Contains(Inheritdoc, StringComparison.Ordinal)) + { + return; + } + var baseType = member.DeclaringType?.BaseType; if (baseType is null) { return; } + MemberInfo? baseMember = null; var children = element.Elements(Inheritdoc); foreach (var child in children) { - var baseMember = + baseMember ??= baseType?.GetTypeInfo().DeclaredMembers .SingleOrDefault(m => m.Name == member.Name); @@ -361,6 +356,7 @@ private void ReplaceInheritdocElements( if (baseDoc != null) { // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw + // TODO: lock / replace ? var nodes = baseDoc.Nodes(); child.ReplaceWith(nodes); } @@ -383,19 +379,34 @@ private void ReplaceInheritdocElements( return null; } - stringBuilder.Replace("\r", string.Empty); - if (stringBuilder[0] != '\n') + if (stringBuilder[^1] == '\n') { - stringBuilder.Insert(0, '\n'); + stringBuilder.Remove(stringBuilder.Length - 1, 1); } - if (stringBuilder[^1] == '\n') + var containsNewLineChar = false; + foreach (var chunk in stringBuilder.GetChunks()) { - stringBuilder.Remove(stringBuilder.Length - 1, 1); + if (chunk.Span.Contains('\n')) + { + containsNewLineChar = true; + break; + } + } + + if (!containsNewLineChar) + { + return stringBuilder.ToString().Trim(); + } + + stringBuilder.Replace("\r", string.Empty); + if (stringBuilder[0] != '\n') + { + stringBuilder.Insert(0, '\n'); } var materializedString = stringBuilder.ToString(); - var whitespace = MyRegex().Match(materializedString).Value; + var whitespace = DetectWhitespaceIndentRegex().Match(materializedString).Value; if (!string.IsNullOrEmpty(whitespace)) { stringBuilder.Replace(whitespace, "\n"); @@ -440,13 +451,15 @@ private string GetMemberElementName(MemberInfo member) for (var index = 0; index < parameters.Length; index++) { var parameterInfo = parameters[index]; - var result = MyRegex1().Replace(parameterInfo.ParameterType.FullName!, m => m.Value switch - { - "[[" => "{", - "]]" => "}", - "],[" => ",", - _ => "" - }); + var result = NormalizeParameterNameRegex() + .Replace(parameterInfo.ParameterType.FullName!, + static m => m.Value switch + { + "[[" => "{", + "]]" => "}", + "],[" => ",", + _ => "" + }); builder.Append(result); if (index < parameters.Length - 1) { @@ -494,7 +507,8 @@ private string GetMemberElementName(MemberInfo member) } [GeneratedRegex("(\\n[ \\t]*)")] - private static partial Regex MyRegex(); + private static partial Regex DetectWhitespaceIndentRegex(); + [GeneratedRegex(@"(`\d+)|(, .*?PublicKeyToken=[0-9a-z]*)|\[\[|\]\]|\],\[")] - private static partial Regex MyRegex1(); + private static partial Regex NormalizeParameterNameRegex(); } diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs similarity index 76% rename from src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs rename to src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs index 2d25ebf48d4..f904311e313 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationFileResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs @@ -6,45 +6,49 @@ namespace HotChocolate.Types.Descriptors; -public class XmlDocumentationFileResolver : IXmlDocumentationFileResolver +public class XmlDocumentationResolver : IXmlDocumentationResolver { private const string Bin = "bin"; private readonly Func? _resolveXmlDocumentationFileName; - private readonly ConcurrentDictionary _cache = - new(StringComparer.OrdinalIgnoreCase); + private readonly ConcurrentDictionary?> _cache = + new(StringComparer.Ordinal); - public XmlDocumentationFileResolver() + public XmlDocumentationResolver() { _resolveXmlDocumentationFileName = null; } - public XmlDocumentationFileResolver(Func? resolveXmlDocumentationFileName) + public XmlDocumentationResolver(Func? resolveXmlDocumentationFileName) { _resolveXmlDocumentationFileName = resolveXmlDocumentationFileName; } public bool TryGetXmlDocument( Assembly assembly, - [NotNullWhen(true)] out XDocument? document) + [NotNullWhen(true)] out IReadOnlyDictionary? memberLookup) { var fullName = assembly.GetName().FullName; - if (!_cache.TryGetValue(fullName, out var doc)) + if (!_cache.TryGetValue(fullName, out memberLookup)) { var xmlDocumentFileName = GetXmlDocumentationPath(assembly); - if (xmlDocumentFileName is not null && File.Exists(xmlDocumentFileName)) { - doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); + var doc = XDocument.Load(xmlDocumentFileName, LoadOptions.PreserveWhitespace); + memberLookup = + doc.Element("doc")? + .Element("members")? + .Elements("member") + .Where(static x => x.Attribute("name") != null) + .ToDictionary(static x => x.Attribute("name")!.Value, static x => x); } - _cache[fullName] = doc; + _cache.TryAdd(fullName, memberLookup); } - document = doc; - return document != null; + return memberLookup != null; } private string? GetXmlDocumentationPath(Assembly? assembly) diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/DefaultNamingConventionsTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/DefaultNamingConventionsTests.cs index 4b76e42a431..7f44e5c27c8 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/DefaultNamingConventionsTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/DefaultNamingConventionsTests.cs @@ -19,7 +19,7 @@ public void GetFormattedFieldName_ReturnsFormattedFieldName(string fieldName, st // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act @@ -44,7 +44,7 @@ public void GetEnumName(string runtimeName, string expectedSchemaName) // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act @@ -64,7 +64,7 @@ public void GetEnumValueDescription_NoDescription(object value) // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act @@ -80,7 +80,7 @@ public void GetEnumValueDescription_XmlDescription() // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act @@ -96,7 +96,7 @@ public void GetEnumValueDescription_AttributeDescription() // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act var result = namingConventions.GetEnumValueDescription(Foo.Baz); @@ -117,7 +117,7 @@ public void Input_Naming_Convention(Type type, string expectedName) // arrange var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); // act diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs index f9222df694a..936bf59b41a 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -32,8 +32,8 @@ public XmlDocumentProviderBenchmarks(ITestOutputHelper testOutputHelper) _testOutputHelper = testOutputHelper; } - //[Fact(Skip = "Run manually when performance regression testing is required")] - [Fact] + [Fact(Skip = "Run manually when performance regression testing is required")] + //[Fact] public void RunBenchmarks() { var config = ManualConfig.Create(DefaultConfig.Instance) @@ -51,13 +51,13 @@ public void RunBenchmarks() public class Bench { - private static XmlDocumentationProvider s_documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), - new NoOpStringBuilderPool()); + private static readonly XmlDocumentationProvider s_documentationProvider = new XmlDocumentationProvider( + new XmlDocumentationResolver(), + new DefaultObjectPoolProvider().CreateStringBuilderPool()); - private static OldXmlDocumentationProvider s_oldDocumentationProvider = new OldXmlDocumentationProvider( + private static readonly OldXmlDocumentationProvider s_oldDocumentationProvider = new OldXmlDocumentationProvider( new OldXmlDocumentationFileResolver(), - new NoOpStringBuilderPool()); + new DefaultObjectPoolProvider().CreateStringBuilderPool()); // Example parameterization [Params(1, 10, 100)] public int N { get; set; } diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs index a807d4329ce..61b469c7a2c 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs @@ -10,7 +10,7 @@ public void When_xml_doc_is_missing_then_description_is_empty() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -25,7 +25,7 @@ public void When_xml_doc_with_multiple_breaks_is_read_then_they_are_not_stripped { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -45,7 +45,7 @@ public void When_description_has_see_tag_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -65,7 +65,7 @@ public void When_description_has_paramref_tag_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -84,7 +84,7 @@ public void When_description_has_generic_tags_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -101,7 +101,7 @@ public void When_type_has_description_then_it_it_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -117,7 +117,7 @@ public void When_we_use_custom_documentation_files_they_are_correctly_loaded() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(_ => "Dummy.xml"), + new XmlDocumentationResolver(_ => "Dummy.xml"), new NoOpStringBuilderPool()); // act @@ -133,7 +133,7 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -152,7 +152,7 @@ public void When_method_has_inheritdoc_then_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -169,7 +169,7 @@ public void When_property_has_inheritdoc_then_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -186,7 +186,7 @@ public void When_type_is_an_interface_then_description_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -202,7 +202,7 @@ public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -221,7 +221,7 @@ public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -238,7 +238,7 @@ public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -255,7 +255,7 @@ public void When_class_implements_interface_and_property_has_description_then_pr { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -272,7 +272,7 @@ public void When_class_implements_interface_and_method_has_description_then_meth { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -289,7 +289,7 @@ public void When_class_implements_interface_and_method_has_description_then_meth { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -308,7 +308,7 @@ public void When_class_has_description_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -324,7 +324,7 @@ public void When_method_has_exceptions_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -340,7 +340,7 @@ public void When_method_has_exceptions_then_exceptions_with_no_code_will_be_igno { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -356,7 +356,7 @@ public void When_method_has_only_exceptions_with_no_code_then_error_section_will { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -372,7 +372,7 @@ public void When_method_has_no_exceptions_then_it_is_ignored() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -388,7 +388,7 @@ public void When_method_has_returns_then_it_is_converted() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -404,7 +404,7 @@ public void When_method_has_no_returns_then_it_is_ignored() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act @@ -420,7 +420,7 @@ public void When_method_has_dictionary_args_then_it_is_found() { // arrange var documentationProvider = new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool()); // act diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/DescriptorContextTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/DescriptorContextTests.cs index e06b242e16e..59eb9a209c6 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/DescriptorContextTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/DescriptorContextTests.cs @@ -13,7 +13,7 @@ public void Create_With_Custom_NamingConventions() var options = new SchemaOptions(); var namingConventions = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); var services = new DictionaryServiceProvider( typeof(INamingConventions), @@ -40,7 +40,7 @@ public void Create_With_Custom_NamingConventions_AsIConvention() var options = new SchemaOptions(); var naming = new DefaultNamingConventions( new XmlDocumentationProvider( - new XmlDocumentationFileResolver(), + new XmlDocumentationResolver(), new NoOpStringBuilderPool())); var namingConventionKey = new ConventionKey(typeof(INamingConventions), null); From def4a6dd4ffb2ca9703e4a2164cab9b7e6979fe9 Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Thu, 9 Oct 2025 23:38:39 +0200 Subject: [PATCH 6/8] Fix Inhertidoc again, ensure threadsafety --- .../Conventions/XmlDocumentationProvider.cs | 54 ++-- .../XmlDocumentProviderBenchmarks.cs | 306 ++++++++++-------- .../XmlDocumentationProviderTests.cs | 6 + 3 files changed, 199 insertions(+), 167 deletions(-) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs index e1aaa90a940..065fe6f6321 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs @@ -2,7 +2,6 @@ using System.Text; using System.Text.RegularExpressions; using System.Xml.Linq; -using HotChocolate.Utilities; using Microsoft.Extensions.ObjectPool; namespace HotChocolate.Types.Descriptors; @@ -20,16 +19,25 @@ public partial class XmlDocumentationProvider : IDocumentationProvider private const string Code = "code"; private const string Paramref = "paramref"; private const string Name = "name"; + private const BindingFlags BindingFlags = + System.Reflection.BindingFlags.Instance + | System.Reflection.BindingFlags.Static + | System.Reflection.BindingFlags.Public + | System.Reflection.BindingFlags.NonPublic + | System.Reflection.BindingFlags.DeclaredOnly; private readonly IXmlDocumentationResolver _documentationResolver; private readonly ObjectPool _stringBuilderPool; + private readonly bool _noCacheMutation; public XmlDocumentationProvider( IXmlDocumentationResolver documentationResolver, - ObjectPool stringBuilderPool) + ObjectPool stringBuilderPool, + bool noCacheMutation = true) { _documentationResolver = documentationResolver ?? throw new ArgumentNullException(nameof(documentationResolver)); _stringBuilderPool = stringBuilderPool; + _noCacheMutation = noCacheMutation; } public string? GetDescription(Type type) => @@ -254,7 +262,7 @@ private static void AppendNewLineIfNeeded( return null; } - ReplaceInheritdocElements(member, element); + element = ReplaceInheritdocElements(member, element); return element; } @@ -281,7 +289,7 @@ private static void AppendNewLineIfNeeded( return null; } - ReplaceInheritdocElements(parameter.Member, element); + element = ReplaceInheritdocElements(parameter.Member, element); if (parameter.IsRetval || string.IsNullOrEmpty(parameter.Name)) @@ -304,61 +312,53 @@ private static void AppendNewLineIfNeeded( private void ProcessInheritdocInterfaceElements( MemberInfo member, - XElement child) + XElement mutableElement) { if (member.DeclaringType is { }) { foreach (var baseInterface in member.DeclaringType.GetInterfaces()) { - var baseMember = baseInterface.GetTypeInfo() - .DeclaredMembers.SingleOrDefault(m => - m.Name.EqualsOrdinal(member.Name)); + var baseMember = baseInterface.GetMember(member.Name, BindingFlags).SingleOrDefault(); if (baseMember != null) { var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw - // TODO: clone / lock / replace ? - child.ReplaceWith(baseDoc.Nodes()); + // Note: This implicitly mutates the xelement + mutableElement.ReplaceWith(baseDoc.Nodes()); } } } } } - private void ReplaceInheritdocElements( + private XElement ReplaceInheritdocElements( MemberInfo member, XElement element) { - if (!element.Value.Contains(Inheritdoc, StringComparison.Ordinal)) + if (element.Element(Inheritdoc) is null) { - return; + return element; } var baseType = member.DeclaringType?.BaseType; if (baseType is null) { - return; + return element; } - MemberInfo? baseMember = null; - var children = element.Elements(Inheritdoc); - foreach (var child in children) + // Deep copy to ensure that we do not mutate the original element from the cache. + var elementCopy = _noCacheMutation ? new XElement(element) : element; + var baseMember = baseType.GetMember(member.Name, BindingFlags).SingleOrDefault(); + foreach (var child in elementCopy.Elements(Inheritdoc)) { - baseMember ??= - baseType?.GetTypeInfo().DeclaredMembers - .SingleOrDefault(m => m.Name == member.Name); - if (baseMember != null) { var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - // TODO: This implicitly mutates the document (which is cached!) and is not threadsafe -> potential flaw - // TODO: lock / replace ? - var nodes = baseDoc.Nodes(); - child.ReplaceWith(nodes); + // Note: This implicitly mutates the elementCopy + child.ReplaceWith(baseDoc.Nodes()); } else { @@ -370,6 +370,8 @@ private void ReplaceInheritdocElements( ProcessInheritdocInterfaceElements(member, child); } } + + return element; } private static string? RemoveLineBreakWhiteSpaces(StringBuilder stringBuilder) diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs index 936bf59b41a..def280cdb73 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -32,13 +32,13 @@ public XmlDocumentProviderBenchmarks(ITestOutputHelper testOutputHelper) _testOutputHelper = testOutputHelper; } - [Fact(Skip = "Run manually when performance regression testing is required")] - //[Fact] + //[Fact(Skip = "Run manually when performance regression testing is required")] + [Fact] public void RunBenchmarks() { var config = ManualConfig.Create(DefaultConfig.Instance) .AddDiagnoser(MemoryDiagnoser.Default) - .AddJob(Job.ShortRun) + .AddJob(Job.Dry) .AddExporter(MarkdownExporter.GitHub); var summary = BenchmarkRunner.Run(typeof(Bench), config); @@ -53,7 +53,13 @@ public class Bench { private static readonly XmlDocumentationProvider s_documentationProvider = new XmlDocumentationProvider( new XmlDocumentationResolver(), - new DefaultObjectPoolProvider().CreateStringBuilderPool()); + new DefaultObjectPoolProvider().CreateStringBuilderPool(), + false); + + private static readonly XmlDocumentationProvider s_safeDocumentationProvider = new XmlDocumentationProvider( + new XmlDocumentationResolver(), + new DefaultObjectPoolProvider().CreateStringBuilderPool(), + true); private static readonly OldXmlDocumentationProvider s_oldDocumentationProvider = new OldXmlDocumentationProvider( new OldXmlDocumentationFileResolver(), @@ -62,6 +68,7 @@ public class Bench // Example parameterization [Params(1, 10, 100)] public int N { get; set; } + /* [Benchmark] public void When_xml_doc_is_missing_then_description_is_empty() { @@ -215,7 +222,7 @@ public void When_we_use_custom_documentation_files_they_are_correctly_loaded_old typeof(BaseBaseClass)); } } - +*/ [Benchmark] public void When_parameter_has_inheritdoc_then_it_is_resolved() { @@ -229,13 +236,14 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved() .Single(p => p.Name == "baz")); } } + [Benchmark] - public void When_parameter_has_inheritdoc_then_it_is_resolved_old() + public void When_parameter_has_inheritdoc_then_it_is_resolved_threadSafeAndNoCacheMutation() { for (int i = 0; i < N; i++) { // act - s_oldDocumentationProvider.GetDescription( + s_safeDocumentationProvider.GetDescription( typeof(ClassWithInheritdoc) .GetMethod(nameof(ClassWithInheritdoc.Bar))! .GetParameters() @@ -244,151 +252,166 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved_old() } [Benchmark] - public void When_method_has_inheritdoc_then_it_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(ClassWithInheritdoc) - .GetMethod(nameof(ClassWithInheritdoc.Bar))!); - } - } - - [Benchmark] - public void When_method_has_inheritdoc_then_it_is_resolved_old() - { - for (int i = 0; i < N; i++) - { - // act - s_oldDocumentationProvider.GetDescription( - typeof(ClassWithInheritdoc) - .GetMethod(nameof(ClassWithInheritdoc.Bar))!); - } - } - - [Benchmark] - public void When_property_has_inheritdoc_then_it_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(ClassWithInheritdoc) - .GetProperty(nameof(ClassWithInheritdoc.Foo))!); - } - } - - [Benchmark] - public void When_property_has_inheritdoc_then_it_is_resolved_old() + public void When_parameter_has_inheritdoc_then_it_is_resolved_old() { for (int i = 0; i < N; i++) { // act s_oldDocumentationProvider.GetDescription( typeof(ClassWithInheritdoc) - .GetProperty(nameof(ClassWithInheritdoc.Foo))!); - } - } - - [Benchmark] - public void When_type_is_an_interface_then_description_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(IBaseBaseInterface)); - } - } - [Benchmark] - public void When_type_is_an_interface_then_description_is_resolved_old() - { - for (int i = 0; i < N; i++) - { - // act - s_oldDocumentationProvider.GetDescription( - typeof(IBaseBaseInterface)); - } - } - - [Benchmark] - public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! - .GetParameters() - .Single(p => p.Name == "baz")); - } - } - - [Benchmark] - public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved_old() - { - for (int i = 0; i < N; i++) - { - // act - s_oldDocumentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + .GetMethod(nameof(ClassWithInheritdoc.Bar))! .GetParameters() .Single(p => p.Name == "baz")); } } - - [Benchmark] - public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); - } - } - - [Benchmark] - public void When_property_has_inheritdoc_on_interface_then_it_is_resolved_old() - { - for (int i = 0; i < N; i++) - { - // act - s_oldDocumentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); - } - } - - [Benchmark] - public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() - { - for (int i = 0; i < N; i++) - { - // act - s_documentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); - } - } - - [Benchmark] - public void When_method_has_inheritdoc_then_on_interface_it_is_resolved_old() - { - for (int i = 0; i < N; i++) - { - // act - s_oldDocumentationProvider.GetDescription( - typeof(ClassWithInheritdocOnInterface) - .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); - } - } - + // [Benchmark] + // public void When_method_has_inheritdoc_then_it_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(ClassWithInheritdoc) + // .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + // } + // } + // + // [Benchmark] + // public void When_method_has_inheritdoc_then_it_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(ClassWithInheritdoc) + // .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + // } + // } + // + // [Benchmark] + // public void When_property_has_inheritdoc_then_it_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(ClassWithInheritdoc) + // .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + // } + // } + // + // [Benchmark] + // public void When_property_has_inheritdoc_then_it_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(ClassWithInheritdoc) + // .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + // } + // } + // + // [Benchmark] + // public void When_type_is_an_interface_then_description_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(IBaseBaseInterface)); + // } + // } + // [Benchmark] + // public void When_type_is_an_interface_then_description_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(IBaseBaseInterface)); + // } + // } + // + // [Benchmark] + // public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + // .GetParameters() + // .Single(p => p.Name == "baz")); + // } + // } + // + // [Benchmark] + // public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + // .GetParameters() + // .Single(p => p.Name == "baz")); + // } + // } + // + // + // [Benchmark] + // public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + // } + // } + // + // [Benchmark] + // public void When_property_has_inheritdoc_on_interface_then_it_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + // } + // } + // + // [Benchmark] + // public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_documentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + // } + // } + // + // [Benchmark] + // public void When_method_has_inheritdoc_then_on_interface_it_is_resolved_old() + // { + // for (int i = 0; i < N; i++) + // { + // // act + // s_oldDocumentationProvider.GetDescription( + // typeof(ClassWithInheritdocOnInterface) + // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + // } + // } + + /* [Benchmark] public void When_class_implements_interface_and_property_has_description_then_property_description_is_used() { @@ -643,6 +666,7 @@ public void When_method_has_dictionary_args_then_it_is_found_old() typeof(WithDictionaryArgs).GetMethod(nameof(WithDictionaryArgs.Method))!); } } + */ } } diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs index 61b469c7a2c..a66388ab54c 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentationProviderTests.cs @@ -143,6 +143,12 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved() .GetParameters() .Single(p => p.Name == "baz")); + parameterXml = documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + // assert Assert.Equal("Parameter details.", parameterXml); } From 3bd95dcbf5466ac577382f9e3de2ff7dda95d4dc Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Fri, 10 Oct 2025 18:57:00 +0200 Subject: [PATCH 7/8] Ensure thread-safety. --- .../Conventions/XmlDocumentationProvider.cs | 56 ++-- .../Conventions/XmlDocumentationResolver.cs | 7 +- .../XmlDocumentProviderBenchmarks.cs | 303 ++++++++---------- 3 files changed, 172 insertions(+), 194 deletions(-) diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs index 065fe6f6321..98803dc5eae 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationProvider.cs @@ -28,16 +28,13 @@ public partial class XmlDocumentationProvider : IDocumentationProvider private readonly IXmlDocumentationResolver _documentationResolver; private readonly ObjectPool _stringBuilderPool; - private readonly bool _noCacheMutation; public XmlDocumentationProvider( IXmlDocumentationResolver documentationResolver, - ObjectPool stringBuilderPool, - bool noCacheMutation = true) + ObjectPool stringBuilderPool) { _documentationResolver = documentationResolver ?? throw new ArgumentNullException(nameof(documentationResolver)); _stringBuilderPool = stringBuilderPool; - _noCacheMutation = noCacheMutation; } public string? GetDescription(Type type) => @@ -164,10 +161,10 @@ private void AppendErrorDescription( } private static void AppendText( - XElement? element, + XElement element, StringBuilder description) { - if (element?.IsEmpty != false) + if (element.IsEmpty) { return; } @@ -262,9 +259,7 @@ private static void AppendNewLineIfNeeded( return null; } - element = ReplaceInheritdocElements(member, element); - - return element; + return ReplaceInheritdocElements(member, element); } return null; @@ -310,11 +305,10 @@ private static void AppendNewLineIfNeeded( } } - private void ProcessInheritdocInterfaceElements( - MemberInfo member, - XElement mutableElement) + private IEnumerable? ProcessInheritdocInterfaceElements( + MemberInfo member) { - if (member.DeclaringType is { }) + if (member.DeclaringType is not null) { foreach (var baseInterface in member.DeclaringType.GetInterfaces()) { @@ -324,12 +318,13 @@ private void ProcessInheritdocInterfaceElements( var baseDoc = GetMemberElement(baseMember); if (baseDoc != null) { - // Note: This implicitly mutates the xelement - mutableElement.ReplaceWith(baseDoc.Nodes()); + return baseDoc.Nodes(); } } } } + + return null; } private XElement ReplaceInheritdocElements( @@ -347,31 +342,32 @@ private XElement ReplaceInheritdocElements( return element; } - // Deep copy to ensure that we do not mutate the original element from the cache. - var elementCopy = _noCacheMutation ? new XElement(element) : element; + // Shallow copy to ensure that we do not mutate the original element from the cache. + // We use a shallow copy instead of a deep copy (new XElement(element)) to avoid the allocation + // overhead since we only need to replace a few (generally 1) inheritdoc-elements. + var elementCopy = new XElement(element.Name); + var baseMember = baseType.GetMember(member.Name, BindingFlags).SingleOrDefault(); - foreach (var child in elementCopy.Elements(Inheritdoc)) + foreach (var child in element.Elements()) { + if (child.Name != Inheritdoc) + { + elementCopy.Add(child); + continue; + } + if (baseMember != null) { var baseDoc = GetMemberElement(baseMember); - if (baseDoc != null) - { - // Note: This implicitly mutates the elementCopy - child.ReplaceWith(baseDoc.Nodes()); - } - else - { - ProcessInheritdocInterfaceElements(member, child); - } + elementCopy.Add(baseDoc != null ? baseDoc.Nodes() : ProcessInheritdocInterfaceElements(member)); } else { - ProcessInheritdocInterfaceElements(member, child); + elementCopy.Add(ProcessInheritdocInterfaceElements(member)); } } - return element; + return elementCopy; } private static string? RemoveLineBreakWhiteSpaces(StringBuilder stringBuilder) @@ -420,7 +416,6 @@ private XElement ReplaceInheritdocElements( private string GetMemberElementName(MemberInfo member) { - char prefixCode; var builder = _stringBuilderPool.Get(); try { @@ -437,6 +432,7 @@ private string GetMemberElementName(MemberInfo member) builder.Append(member.DeclaringType.FullName).Append('.').Append(member.Name); } + char prefixCode; switch (member.MemberType) { case MemberTypes.Constructor: diff --git a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs index f904311e313..3b57cc91c46 100644 --- a/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs +++ b/src/HotChocolate/Core/src/Types/Types/Descriptors/Conventions/XmlDocumentationResolver.cs @@ -42,7 +42,12 @@ public bool TryGetXmlDocument( .Element("members")? .Elements("member") .Where(static x => x.Attribute("name") != null) - .ToDictionary(static x => x.Attribute("name")!.Value, static x => x); + .ToDictionary(static x => x.Attribute("name")!.Value, static delegate(XElement x) + { + // Optimize memory usage: We already stored the name as key in the dictionary. + x.RemoveAttributes(); + return x; + }); } _cache.TryAdd(fullName, memberLookup); diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs index def280cdb73..6d7c4b131f7 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -38,7 +38,7 @@ public void RunBenchmarks() { var config = ManualConfig.Create(DefaultConfig.Instance) .AddDiagnoser(MemoryDiagnoser.Default) - .AddJob(Job.Dry) + .AddJob(Job.ShortRun) .AddExporter(MarkdownExporter.GitHub); var summary = BenchmarkRunner.Run(typeof(Bench), config); @@ -53,22 +53,15 @@ public class Bench { private static readonly XmlDocumentationProvider s_documentationProvider = new XmlDocumentationProvider( new XmlDocumentationResolver(), - new DefaultObjectPoolProvider().CreateStringBuilderPool(), - false); - - private static readonly XmlDocumentationProvider s_safeDocumentationProvider = new XmlDocumentationProvider( - new XmlDocumentationResolver(), - new DefaultObjectPoolProvider().CreateStringBuilderPool(), - true); + new DefaultObjectPoolProvider().CreateStringBuilderPool()); private static readonly OldXmlDocumentationProvider s_oldDocumentationProvider = new OldXmlDocumentationProvider( new OldXmlDocumentationFileResolver(), new DefaultObjectPoolProvider().CreateStringBuilderPool()); // Example parameterization - [Params(1, 10, 100)] public int N { get; set; } + [Params(1, 10, 100, 1000, 10000, 100000)] public int N { get; set; } - /* [Benchmark] public void When_xml_doc_is_missing_then_description_is_empty() { @@ -222,7 +215,7 @@ public void When_we_use_custom_documentation_files_they_are_correctly_loaded_old typeof(BaseBaseClass)); } } -*/ + [Benchmark] public void When_parameter_has_inheritdoc_then_it_is_resolved() { @@ -238,12 +231,12 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved() } [Benchmark] - public void When_parameter_has_inheritdoc_then_it_is_resolved_threadSafeAndNoCacheMutation() + public void When_parameter_has_inheritdoc_then_it_is_resolved_old() { for (int i = 0; i < N; i++) { // act - s_safeDocumentationProvider.GetDescription( + s_oldDocumentationProvider.GetDescription( typeof(ClassWithInheritdoc) .GetMethod(nameof(ClassWithInheritdoc.Bar))! .GetParameters() @@ -252,166 +245,151 @@ public void When_parameter_has_inheritdoc_then_it_is_resolved_threadSafeAndNoCac } [Benchmark] - public void When_parameter_has_inheritdoc_then_it_is_resolved_old() + public void When_method_has_inheritdoc_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_it_is_resolved_old() { for (int i = 0; i < N; i++) { // act s_oldDocumentationProvider.GetDescription( typeof(ClassWithInheritdoc) - .GetMethod(nameof(ClassWithInheritdoc.Bar))! + .GetMethod(nameof(ClassWithInheritdoc.Bar))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdoc) + .GetProperty(nameof(ClassWithInheritdoc.Foo))!); + } + } + + [Benchmark] + public void When_type_is_an_interface_then_description_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(IBaseBaseInterface)); + } + } + [Benchmark] + public void When_type_is_an_interface_then_description_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(IBaseBaseInterface)); + } + } + + [Benchmark] + public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! + .GetParameters() + .Single(p => p.Name == "baz")); + } + } + + [Benchmark] + public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! .GetParameters() .Single(p => p.Name == "baz")); } } - // [Benchmark] - // public void When_method_has_inheritdoc_then_it_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(ClassWithInheritdoc) - // .GetMethod(nameof(ClassWithInheritdoc.Bar))!); - // } - // } - // - // [Benchmark] - // public void When_method_has_inheritdoc_then_it_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(ClassWithInheritdoc) - // .GetMethod(nameof(ClassWithInheritdoc.Bar))!); - // } - // } - // - // [Benchmark] - // public void When_property_has_inheritdoc_then_it_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(ClassWithInheritdoc) - // .GetProperty(nameof(ClassWithInheritdoc.Foo))!); - // } - // } - // - // [Benchmark] - // public void When_property_has_inheritdoc_then_it_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(ClassWithInheritdoc) - // .GetProperty(nameof(ClassWithInheritdoc.Foo))!); - // } - // } - // - // [Benchmark] - // public void When_type_is_an_interface_then_description_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(IBaseBaseInterface)); - // } - // } - // [Benchmark] - // public void When_type_is_an_interface_then_description_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(IBaseBaseInterface)); - // } - // } - // - // [Benchmark] - // public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! - // .GetParameters() - // .Single(p => p.Name == "baz")); - // } - // } - // - // [Benchmark] - // public void When_parameter_has_inheritdoc_on_interface_then_it_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))! - // .GetParameters() - // .Single(p => p.Name == "baz")); - // } - // } - // - // - // [Benchmark] - // public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); - // } - // } - // - // [Benchmark] - // public void When_property_has_inheritdoc_on_interface_then_it_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); - // } - // } - // - // [Benchmark] - // public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_documentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); - // } - // } - // - // [Benchmark] - // public void When_method_has_inheritdoc_then_on_interface_it_is_resolved_old() - // { - // for (int i = 0; i < N; i++) - // { - // // act - // s_oldDocumentationProvider.GetDescription( - // typeof(ClassWithInheritdocOnInterface) - // .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); - // } - // } - - /* + + [Benchmark] + public void When_property_has_inheritdoc_on_interface_then_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + } + } + + [Benchmark] + public void When_property_has_inheritdoc_on_interface_then_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetProperty(nameof(ClassWithInheritdocOnInterface.Foo))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_on_interface_it_is_resolved() + { + for (int i = 0; i < N; i++) + { + // act + s_documentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + } + } + + [Benchmark] + public void When_method_has_inheritdoc_then_on_interface_it_is_resolved_old() + { + for (int i = 0; i < N; i++) + { + // act + s_oldDocumentationProvider.GetDescription( + typeof(ClassWithInheritdocOnInterface) + .GetMethod(nameof(ClassWithInheritdocOnInterface.Bar))!); + } + } + [Benchmark] public void When_class_implements_interface_and_property_has_description_then_property_description_is_used() { @@ -666,7 +644,6 @@ public void When_method_has_dictionary_args_then_it_is_found_old() typeof(WithDictionaryArgs).GetMethod(nameof(WithDictionaryArgs.Method))!); } } - */ } } From d3833a9e90f41b4e6bc7d87b87fc8fd070c48663 Mon Sep 17 00:00:00 2001 From: NOlbert <45466413+N-Olbert@users.noreply.github.com> Date: Fri, 10 Oct 2025 22:53:37 +0200 Subject: [PATCH 8/8] Skip bench --- .../Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs index 6d7c4b131f7..501c4c94eee 100644 --- a/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs +++ b/src/HotChocolate/Core/test/Types.Tests/Types/Descriptors/Conventions/XmlDocumentProviderBenchmarks.cs @@ -32,8 +32,7 @@ public XmlDocumentProviderBenchmarks(ITestOutputHelper testOutputHelper) _testOutputHelper = testOutputHelper; } - //[Fact(Skip = "Run manually when performance regression testing is required")] - [Fact] + [Fact(Skip = "Run manually when required")] public void RunBenchmarks() { var config = ManualConfig.Create(DefaultConfig.Instance)