Speed up multiple attributes overwrite detection. Fixes #24467

Author: SteveSandersonMS

src/Components/Components/src/Rendering/MultipleAttributesDictionary.cs

@@ -0,0 +1,152 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using System.Diagnostics;
+
+namespace Microsoft.AspNetCore.Components.Rendering
+{
+    /// <summary>
+    /// A specialized alternative dictionary tuned entirely towards speeding up RenderTreeBuilder's
+    /// ProcessDuplicateAttributes method.
+    /// </summary>
+    /// <remarks>
+    /// It's faster than a normal Dictionary[string, int] because:
+    ///
+    ///  1. It uses a hash function optimized for the "no match" case. If we expect most lookups to
+    ///     result in no match, which we do for attribute writing, then it's sufficient to consider
+    ///     just a small fraction of the string when building a hash.
+    ///  2. It has an API designed around the add-or-return-existing semantics needed for this
+    ///     process, with a lot less abstraction and extensibility than Dictionary[string, int]
+    /// 
+    /// In total, this improves the ComplexTable benchmark by 6-7%.
+    ///
+    /// This dictionary shouldn't be used in other situations because it may perform much worse than
+    /// a Dictionary[string, int] if most of the lookups/insertions match existing entries.
+    /// </remarks>
+    internal class MultipleAttributesDictionary
+    {
+        public const int InitialCapacity = 79;
+
+        private string[] _keys = new string[InitialCapacity];
+        private int[] _values = new int[InitialCapacity];
+        private int _capacity = InitialCapacity;
+
+        public void Clear()
+        {
+            Array.Clear(_keys, 0, _keys.Length);
+            Array.Clear(_values, 0, _values.Length);
+        }
+
+        public bool TryAdd(string key, int value, out int existingValue)
+        {
+            if (TryFindIndex(key, out var index))
+            {
+                existingValue = _values[index];
+                return false;
+            }
+            else
+            {
+                if (index < 0) // Indicates that storage is full
+                {
+                    ExpandStorage();
+                    TryFindIndex(key, out index);
+                    Debug.Assert(index >= 0);
+                }
+
+                _keys[index] = key;
+                _values[index] = value;
+                existingValue = default;
+                return true;
+            }
+        }
+
+        public void Replace(string key, int value)
+        {
+            if (TryFindIndex(key, out var index))
+            {
+                _values[index] = value;
+            }
+            else
+            {
+                throw new InvalidOperationException($"Key not found: '{key}'");
+            }
+        }
+
+        private bool TryFindIndex(string key, out int existingIndexOrInsertionPosition)
+        {
+            var hashCode = GetSimpleHashCode(key);
+            var startIndex = hashCode % _capacity;
+            if (startIndex < 0)
+            {
+                startIndex += _capacity;
+            }
+            var candidateIndex = startIndex;
+
+            do
+            {
+                var candidateKey = _keys[candidateIndex];
+                if (candidateKey == null)
+                {
+                    existingIndexOrInsertionPosition = candidateIndex;
+                    return false;
+                }
+
+                if (string.Equals(candidateKey, key, StringComparison.OrdinalIgnoreCase))
+                {
+                    existingIndexOrInsertionPosition = candidateIndex;
+                    return true;
+                }
+
+                if (++candidateIndex >= _capacity)
+                {
+                    candidateIndex = 0;
+                }
+            }
+            while (candidateIndex != startIndex);
+
+            // We didn't find the key, and there's no empty slot in which we could insert it.
+            // Storage is full.
+            existingIndexOrInsertionPosition = -1;
+            return false;
+        }
+
+        private void ExpandStorage()
+        {
+            var oldKeys = _keys;
+            var oldValues = _values;
+            _capacity = _capacity * 2;
+            _keys = new string[_capacity];
+            _values = new int[_capacity];
+
+            for (var i = 0; i < oldKeys.Length; i++)
+            {
+                var key = oldKeys[i];
+                if (!(key is null))
+                {
+                    var value = oldValues[i];
+                    var didInsert = TryAdd(key, value, out _);
+                    Debug.Assert(didInsert);
+                }
+            }
+        }
+
+        private static int GetSimpleHashCode(string key)
+        {
+            var keyLength = key.Length;
+            if (keyLength > 0)
+            {
+                // Consider just the first and middle characters.
+                // This will produce a distinct result for a sufficiently large
+                // proportion of attribute names.
+                return unchecked(
+                    char.ToLowerInvariant(key[0])
+                    + 31 * char.ToLowerInvariant(key[keyLength / 2]));
+            }
+            else
+            {
+                return default;
+            }
+        }
+    }
+}

src/Components/Components/src/Rendering/RenderTreeBuilder.cs

@@ -27,7 +27,7 @@ public sealed class RenderTreeBuilder : IDisposable
         private readonly Stack<int> _openElementIndices = new Stack<int>();
         private RenderTreeFrameType? _lastNonAttributeFrameType;
         private bool _hasSeenAddMultipleAttributes;
-        private Dictionary<string, int>? _seenAttributeNames;
+        private MultipleAttributesDictionary? _seenAttributeNames;
 
         /// <summary>
         /// The reserved parameter name used for supplying child content.
@@ -707,41 +707,39 @@ internal void ProcessDuplicateAttributes(int first)
             }
 
             // Now that we've found the last attribute, we can iterate backwards and process duplicates.
-            var seenAttributeNames = (_seenAttributeNames ??= new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase));
+            var seenAttributeNames = (_seenAttributeNames ??= new MultipleAttributesDictionary());
             for (var i = last; i >= first; i--)
             {
                 ref var frame = ref buffer[i];
                 Debug.Assert(frame.FrameTypeField == RenderTreeFrameType.Attribute, $"Frame type is {frame.FrameTypeField} at {i}");
 
-                if (!seenAttributeNames.TryGetValue(frame.AttributeNameField, out var index))
+                if (!seenAttributeNames.TryAdd(frame.AttributeNameField, i, out var index))
                 {
-                    // This is the first time seeing this attribute name. Add to the dictionary and move on.
-                    seenAttributeNames.Add(frame.AttributeNameField, i);
-                }
-                else if (index < i)
-                {
-                    // This attribute is overriding a "silent frame" where we didn't create a frame for an AddAttribute call.
-                    // This is the case for a null event handler, or bool false value.
-                    //
-                    // We need to update our tracking, in case the attribute appeared 3 or more times.
-                    seenAttributeNames[frame.AttributeNameField] = i;
-                }
-                else if (index > i)
-                {
-                    // This attribute has been overridden. For now, blank out its name to *mark* it. We'll do a pass
-                    // later to wipe it out.
-                    frame = default;
-                }
-                else
-                {
-                    // OK so index == i. How is that possible? Well it's possible for a "silent frame" immediately
-                    // followed by setting the same attribute. Think of it this way, when we create a "silent frame"
-                    // we have to track that attribute name with *some* index.
-                    //
-                    // The only index value we can safely use is _entries.Count (next available). This is fine because
-                    // we never use these indexes to look stuff up, only for comparison.
-                    //
-                    // That gets you here, and there's no action to take.
+                    if (index < i)
+                    {
+                        // This attribute is overriding a "silent frame" where we didn't create a frame for an AddAttribute call.
+                        // This is the case for a null event handler, or bool false value.
+                        //
+                        // We need to update our tracking, in case the attribute appeared 3 or more times.
+                        seenAttributeNames.Replace(frame.AttributeNameField, i);
+                    }
+                    else if (index > i)
+                    {
+                        // This attribute has been overridden. For now, blank out its name to *mark* it. We'll do a pass
+                        // later to wipe it out.
+                        frame = default;
+                    }
+                    else
+                    {
+                        // OK so index == i. How is that possible? Well it's possible for a "silent frame" immediately
+                        // followed by setting the same attribute. Think of it this way, when we create a "silent frame"
+                        // we have to track that attribute name with *some* index.
+                        //
+                        // The only index value we can safely use is _entries.Count (next available). This is fine because
+                        // we never use these indexes to look stuff up, only for comparison.
+                        //
+                        // That gets you here, and there's no action to take.
+                    }
                 }
             }
 
@@ -780,8 +778,8 @@ internal void TrackAttributeName(string name)
                 return;
             }
 
-            var seenAttributeNames = (_seenAttributeNames ??= new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase));
-            seenAttributeNames[name] = _entries.Count; // See comment in ProcessAttributes for why this is OK.
+            var seenAttributeNames = (_seenAttributeNames ??= new MultipleAttributesDictionary());
+            seenAttributeNames.TryAdd(name, _entries.Count, out _); // See comment in ProcessAttributes for why this is OK.
         }
 
         void IDisposable.Dispose()

src/Components/Components/test/Rendering/MultipleAttributesDictionaryTest.cs

@@ -0,0 +1,151 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
+using System;
+using Xunit;
+
+namespace Microsoft.AspNetCore.Components.Rendering
+{
+    public class MultipleAttributesDictionaryTest
+    {
+        [Fact]
+        public void CanStoreAndRetrieveValues()
+        {
+            var instance = new MultipleAttributesDictionary();
+
+            // Add key1
+            Assert.True(instance.TryAdd("key1", 123, out var existingValue1));
+            Assert.Equal(default, existingValue1);
+
+            // Add key2
+            Assert.True(instance.TryAdd("key2", 456, out var existingValue2));
+            Assert.Equal(default, existingValue2);
+
+            // Can't add key1 again. Instead we retrieve the existing value.
+            Assert.False(instance.TryAdd("key1", 1000, out var existingValue3));
+            Assert.Equal(123, existingValue3);
+
+            // Same for KEY1, showing the keys are case-insensitive, and we didn't overwrite last time.
+            Assert.False(instance.TryAdd("KEY1", 2000, out var existingValue4));
+            Assert.Equal(123, existingValue4);
+        }
+
+        [Fact]
+        public void HandlesKeysThatVaryOnlyByOneChar()
+        {
+            // Arrange
+            // This test case is intended to produce clashing hashes
+            var instance = new MultipleAttributesDictionary();
+            instance.TryAdd("SomeLongKey", 123, out _);
+
+            // Act
+            var didAddSecondEntry = instance.TryAdd("SXmeLongKey", 456, out _);
+
+            // Assert
+            Assert.True(didAddSecondEntry);
+            Assert.False(instance.TryAdd("SomeLongKey", 0, out var existingValue1));
+            Assert.False(instance.TryAdd("SXmeLongKey", 0, out var existingValue2));
+            Assert.Equal(123, existingValue1);
+            Assert.Equal(456, existingValue2);
+        }
+
+        [Fact]
+        public void CanClear()
+        {
+            // Arrange
+            var instance = new MultipleAttributesDictionary();
+            instance.TryAdd("X", 123, out _);
+
+            // Act
+            instance.Clear();
+
+            // Assert
+            Assert.True(instance.TryAdd("X", 456, out var existingValue));
+            Assert.Equal(default, existingValue);
+        }
+
+        [Fact]
+        public void AllowsEmptyStringKey()
+        {
+            // Arrange
+            var instance = new MultipleAttributesDictionary();
+
+            // Act
+            instance.TryAdd(string.Empty, 1, out _);
+
+            // Assert
+            Assert.False(instance.TryAdd(string.Empty, 0, out var storedValue));
+            Assert.Equal(1, storedValue);
+        }
+
+        [Fact]
+        public void CanReplaceExistingValues()
+        {
+            // Arrange
+            var instance = new MultipleAttributesDictionary();
+            instance.TryAdd("somekey", 123, out _);
+
+            // Act
+            instance.Replace("SomeKey", 456);
+            instance.Replace("SomeKey", 789);
+
+            // Assert
+            Assert.False(instance.TryAdd("SOMEKEY", 0, out var storedValue));
+            Assert.Equal(789, storedValue);
+        }
+
+        [Fact]
+        public void CannotReplaceNonExistingValues()
+        {
+            // Arrange
+            var instance = new MultipleAttributesDictionary();
+            instance.TryAdd("somekey", 123, out _);
+
+            // Act
+            Assert.Throws<InvalidOperationException>(() =>
+            {
+                instance.Replace("otherkey", 456);
+            });
+        }
+
+        [Fact]
+        public void CanExpandStorage()
+        {
+            // Arrange
+            var instance = new MultipleAttributesDictionary();
+            int index;
+            for (index = 0; index < MultipleAttributesDictionary.InitialCapacity; index++)
+            {
+                Assert.True(instance.TryAdd($"key{index}", index, out _));
+            }
+
+            // Act 1: Store the same amount again
+            var doubledCapacity = 2 * MultipleAttributesDictionary.InitialCapacity;
+            for (; index < doubledCapacity; index++)
+            {
+                Assert.True(instance.TryAdd($"key{index}", index, out _));
+            }
+
+            // Assert: Verify contents
+            for (var i = 0; i < doubledCapacity; i++)
+            {
+                Assert.False(instance.TryAdd($"key{i}", 0, out var storedValue));
+                Assert.Equal(i, storedValue);
+            }
+
+            // Act 2: Store a lot more
+            var largeCapacity = 100 * MultipleAttributesDictionary.InitialCapacity;
+            for (; index < largeCapacity; index++)
+            {
+                Assert.True(instance.TryAdd($"key{index}", index, out _));
+            }
+
+            // Assert: Verify contents
+            for (var i = 0; i < largeCapacity; i++)
+            {
+                Assert.False(instance.TryAdd($"key{i}", 0, out var storedValue));
+                Assert.Equal(i, storedValue);
+            }
+        }
+    }
+}