Improve perf of attribute duplicate check

src/libraries/System.Private.Xml/src/System/Xml/Cache/XPathNodeInfoAtom.cs

@@ -4,6 +4,7 @@
 using System;
 using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
+using System.Runtime.CompilerServices;
 using System.Text;
 using System.Xml.XPath;
 
@@ -253,23 +254,25 @@ public override int GetHashCode()
         {
             if (_hashCode == 0)
             {
-                int hashCode;
-
-                // Start with local name
-                hashCode = _localNameHash;
-
-                // Add page indexes
-                unchecked
-                {
-                    if (_pageSibling != null)
-                        hashCode += (hashCode << 7) ^ _pageSibling[0].PageInfo!.PageNumber;
-
-                    if (_pageParent != null)
-                        hashCode += (hashCode << 7) ^ _pageParent[0].PageInfo!.PageNumber;
-
-                    if (_pageSimilar != null)
-                        hashCode += (hashCode << 7) ^ _pageSimilar[0].PageInfo!.PageNumber;
-                }
+                // All fields checked by Equals must be included here to avoid hash collisions.
+                // Equals uses reference equality for strings (atomized via NameTable) and
+                // page arrays, so we use RuntimeHelpers.GetHashCode for a 1:1 match.
+                HashCode hc = default;
+                hc.Add(RuntimeHelpers.GetHashCode(_localName));
+                hc.Add(RuntimeHelpers.GetHashCode(_namespaceUri));
+                hc.Add(RuntimeHelpers.GetHashCode(_prefix));
+                if (_baseUri is not null)
+                    hc.Add(RuntimeHelpers.GetHashCode(_baseUri));
+                if (_pageSibling is not null)
+                    hc.Add(RuntimeHelpers.GetHashCode(_pageSibling));
+                if (_pageParent is not null)
+                    hc.Add(RuntimeHelpers.GetHashCode(_pageParent));
+                if (_pageSimilar is not null)
+                    hc.Add(RuntimeHelpers.GetHashCode(_pageSimilar));
+                hc.Add(_lineNumBase);
+                hc.Add(_linePosBase);
+
+                int hashCode = hc.ToHashCode();
 
                 // Save hashcode.  Don't save 0, so that it won't ever be recomputed.
                 _hashCode = ((hashCode == 0) ? 1 : hashCode);

src/libraries/System.Private.Xml/src/System/Xml/Core/XmlTextReaderImpl.cs

@@ -167,7 +167,7 @@ private enum InitInputType
         private int _attrDuplWalkCount;
         private bool _attrNeedNamespaceLookup;
         private bool _fullAttrCleanup;
-        private NodeData[]? _attrDuplSortingArray;
+        private HashSet<NodeData>? _attrDuplSet;
 
         // name table
         private XmlNameTable _nameTable;
@@ -287,7 +287,7 @@ private enum InitInputType
         private const int NodesInitialSize = 8;
         private const int InitialParsingStatesDepth = 2;
         private const int MaxByteSequenceLen = 6;  // max bytes per character
-        private const int MaxAttrDuplWalkCount = 250;
+        private const int MaxAttrDuplWalkCount = 64;
         private const int MinWhitespaceLookahedCount = 4096;
 
         private const string XmlDeclarationBeginning = "<?xml";
@@ -5017,22 +5017,15 @@ private void AttributeDuplCheck()
             }
             else
             {
-                if (_attrDuplSortingArray == null || _attrDuplSortingArray.Length < _attrCount)
-                {
-                    _attrDuplSortingArray = new NodeData[_attrCount];
-                }
-                Array.Copy(_nodes, _index + 1, _attrDuplSortingArray, 0, _attrCount);
-                Array.Sort(_attrDuplSortingArray, 0, _attrCount);
+                _attrDuplSet ??= new HashSet<NodeData>(NodeData.AtomizedNameEqualityComparer.Instance);
+                _attrDuplSet.Clear();
 
-                NodeData attr1 = _attrDuplSortingArray[0];
-                for (int i = 1; i < _attrCount; i++)
+                for (int i = _index + 1; i < _index + 1 + _attrCount; i++)
                 {
-                    NodeData attr2 = _attrDuplSortingArray[i];
-                    if (Ref.Equal(attr1.localName, attr2.localName) && Ref.Equal(attr1.ns, attr2.ns))
+                    if (!_attrDuplSet.Add(_nodes[i]))
                     {
-                        Throw(SR.Xml_DupAttributeName, attr2.GetNameWPrefix(_nameTable), attr2.LineNo, attr2.LinePos);
+                        Throw(SR.Xml_DupAttributeName, _nodes[i].GetNameWPrefix(_nameTable), _nodes[i].LineNo, _nodes[i].LinePos);
                     }
-                    attr1 = attr2;
                 }
             }
         }

src/libraries/System.Private.Xml/src/System/Xml/Core/XmlTextReaderImplHelpers.cs

@@ -8,6 +8,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
+using System.Runtime.CompilerServices;
 using System.Runtime.Versioning;
 using System.Security;
 using System.Text;
@@ -705,6 +706,30 @@ int IComparable.CompareTo(object? obj)
                     return 1;
                 }
             }
+
+            internal sealed class AtomizedNameEqualityComparer : IEqualityComparer<NodeData>
+            {
+                internal static readonly AtomizedNameEqualityComparer Instance = new AtomizedNameEqualityComparer();
+
+                public bool Equals(NodeData? x, NodeData? y)
+                {
+                    if (x is null)
+                    {
+                        return y is null;
+                    }
+
+                    return y is not null
+                        && Ref.Equal(x.localName, y.localName)
+                        && Ref.Equal(x.ns, y.ns);
+                }
+
+                public int GetHashCode(NodeData node)
+                {
+                    return HashCode.Combine(
+                        RuntimeHelpers.GetHashCode(node.localName),
+                        RuntimeHelpers.GetHashCode(node.ns));
+                }
+            }
         }
 
         //

src/libraries/System.Private.Xml/tests/Misc/AttributeReadingPerformanceTests.cs

@@ -0,0 +1,183 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Diagnostics;
+using System.IO;
+using System.Text;
+using System.Threading;
+using System.Xml.XPath;
+using Xunit;
+
+namespace System.Xml.Tests
+{
+    public abstract class AttributeReadingPerformanceTests
+    {
+        // This should match the value in XmlTextReaderImpl
+        private const int MaxAttrDuplWalkCount = 64;
+
+        private const int SmallN = 2_000;
+        private const int LargeN = 20_000;
+        private const double SizeRatio = (double)LargeN / SmallN;
+        private const double MaxRatioMultiplier = 4;
+        private static readonly TimeSpan s_timeout = TimeSpan.FromSeconds(60);
+
+        protected abstract void ReadFully(string xml, CancellationToken ct);
+
+        [Fact]
+        public void AttributeDuplicatesCheck_LongUris_SameLocalName_AboveThreshold()
+        {
+            // Exercises the HashSet duplicate-check path (number of attributes >= threshold)
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount, longUris: true, distinctLocalNames: false));
+        }
+
+        [Fact]
+        public void AttributeDuplicatesCheck_ShortUris_SameLocalName_BelowThreshold()
+        {
+            // Pairwise walk path (number of attributes < threshold)
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount - 1, longUris: false, distinctLocalNames: false));
+        }
+
+        [Fact]
+        public void AttributeDuplicatesCheck_LongUris_DistinctLocalNames_AboveThreshold()
+        {
+            // Distinct localNames starting with same letter (to bypass some optimizations)
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount, longUris: true, distinctLocalNames: true));
+        }
+
+        [Fact]
+        public void AttributeDuplicatesCheck_LongUris_SameLocalName_WellAboveThreshold()
+        {
+            // Larger amount of attributes — well above the threshold
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount * 4, longUris: true, distinctLocalNames: false));
+        }
+
+        [Fact]
+        public void AttributeDuplicatesCheck_ShortUris_DistinctLocalNames_BelowThreshold()
+        {
+            // Below threshold with distinct localNames
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount - 1, longUris: false, distinctLocalNames: true));
+        }
+
+        // We're doing full string.Equals on DEBUG on top of Ref.Equal which makes it quadratic.
+#if !DEBUG
+        [Fact]
+        public void AttributeDuplicatesCheck_LongUris_SameLocalName_BelowThreshold()
+        {
+            // Pairwise walk path with long URIs — only valid in Release
+            AssertLinearScaling(
+                n => GenerateDoc(n, attrCount: MaxAttrDuplWalkCount - 1, longUris: true, distinctLocalNames: false));
+        }
+#endif
+
+        private void AssertLinearScaling(Func<int, string> generateDoc)
+        {
+            using CancellationTokenSource cts = new(s_timeout);
+            CancellationToken ct = cts.Token;
+
+            ReadFully(generateDoc(SmallN), ct);
+
+            long smallTime = MeasureRead(generateDoc(SmallN), ct);
+            long largeTime = MeasureRead(generateDoc(LargeN), ct);
+
+            double maxAllowed = SizeRatio * MaxRatioMultiplier;
+            double actualRatio = (double)largeTime / Math.Max(smallTime, 1);
+
+            Assert.True(actualRatio <= maxAllowed,
+                $"Scaling ratio {actualRatio:F1}x exceeded {maxAllowed:F1}x limit " +
+                $"(input grew {SizeRatio:F0}x). " +
+                $"Small ({SmallN}): {smallTime} ms, Large ({LargeN}): {largeTime} ms.");
+        }
+
+        private long MeasureRead(string doc, CancellationToken ct)
+        {
+            Stopwatch sw = Stopwatch.StartNew();
+            ReadFully(doc, ct);
+            return sw.ElapsedMilliseconds;
+        }
+
+        private static string GenerateDoc(int n, int attrCount, bool longUris, bool distinctLocalNames)
+        {
+            int childCount = n / 2;
+            string garbageText = longUris ? new string('x', childCount) : "ns";
+
+            StringBuilder child = new();
+            child.Append("<child ");
+            for (int i = attrCount - 1; i >= 0; i--)
+            {
+                if (distinctLocalNames)
+                    child.Append($" x{i:X4}:a{i:X4}=\"\"");
+                else
+                    child.Append($" x{i:X4}:a=\"\"");
+            }
+            child.Append("/>");
+            string childElement = child.ToString();
+
+            StringBuilder sb = new();
+            sb.Append("<parent ");
+            for (int i = 0; i < attrCount; i++)
+            {
+                sb.Append($" xmlns:x{i:X4}=\"{garbageText}{i:X4}\"");
+            }
+            sb.Append('>');
+            for (int i = 0; i < childCount; i++)
+            {
+                sb.Append(childElement);
+            }
+            sb.Append("</parent>");
+
+            return sb.ToString();
+        }
+    }
+
+    // XmlReader.Create wraps XmlTextReaderImpl in XmlAsyncCheckReader
+    public class AttributeReadingPerformanceTests_XmlReaderCreate : AttributeReadingPerformanceTests
+    {
+        protected override void ReadFully(string xml, CancellationToken ct)
+        {
+            using XmlReader xr = XmlReader.Create(new StringReader(xml));
+            while (xr.Read()) { ct.ThrowIfCancellationRequested(); }
+        }
+    }
+
+    // XmlNodeReader reads from a pre-parsed DOM tree (XmlDocument).
+    // Construction is not timed — only the read traversal.
+    public class AttributeReadingPerformanceTests_XmlNodeReader : AttributeReadingPerformanceTests
+    {
+        protected override void ReadFully(string xml, CancellationToken ct)
+        {
+            XmlDocument doc = new();
+            doc.LoadXml(xml);
+            using XmlReader xr = new XmlNodeReader(doc);
+            while (xr.Read()) { ct.ThrowIfCancellationRequested(); }
+        }
+    }
+
+    // XPathNavigatorReader reads from an XPathDocument's XPath data model
+    public class AttributeReadingPerformanceTests_XPathNavigatorReader : AttributeReadingPerformanceTests
+    {
+        protected override void ReadFully(string xml, CancellationToken ct)
+        {
+            ct.ThrowIfCancellationRequested();
+            XPathDocument doc = new(new StringReader(xml));
+            ct.ThrowIfCancellationRequested();
+            using XmlReader xr = doc.CreateNavigator().ReadSubtree();
+            while (xr.Read()) { ct.ThrowIfCancellationRequested(); }
+        }
+    }
+
+    // XmlReader.Create(XmlReader, settings) wraps a reader in XmlSubtreeReader
+    public class AttributeReadingPerformanceTests_WrappedReader : AttributeReadingPerformanceTests
+    {
+        protected override void ReadFully(string xml, CancellationToken ct)
+        {
+            using XmlReader inner = XmlReader.Create(new StringReader(xml));
+            using XmlReader xr = XmlReader.Create(inner, new XmlReaderSettings());
+            while (xr.Read()) { ct.ThrowIfCancellationRequested(); }
+        }
+    }
+}

src/libraries/System.Private.Xml/tests/System.Private.Xml.Tests.csproj

@@ -32,6 +32,7 @@
     <Compile Include="AllowDefaultResolverContext.cs" />
     <Compile Include="ExceptionVerifier.cs" />
     <Compile Include="$(CommonTestPath)\TestUtilities\System\DisableParallelizationPerAssembly.cs" Link="Common\TestUtilities\System\DisableParallelizationPerAssembly.cs" />
+    <Compile Include="Misc\AttributeReadingPerformanceTests.cs" />
   </ItemGroup>
 
   <ItemGroup>