Improve performance and reduce allocations in RouteValueDictionary. (#1062)

benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs

@@ -10,6 +10,7 @@ namespace Microsoft.AspNetCore.Routing
     {
         private RouteValueDictionary _arrayValues;
         private RouteValueDictionary _propertyValues;
+        private RouteValueDictionary _arrayValuesEmpty;
 
         // We modify the route value dictionaries in many of these benchmarks.
         [IterationSetup]
@@ -21,9 +22,22 @@ namespace Microsoft.AspNetCore.Routing
                 { "controller", "Home" },
                 { "id", "17" },
             };
+            _arrayValuesEmpty = new RouteValueDictionary();
             _propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17" });
         }
 
+        [Benchmark]
+        public void Ctor_Values_RouteValueDictionary_EmptyArray()
+        {
+            new RouteValueDictionary(_arrayValuesEmpty);
+        }
+
+        [Benchmark]
+        public void Ctor_Values_RouteValueDictionary_Array()
+        {
+            new RouteValueDictionary(_arrayValues);
+        }
+
         [Benchmark]
         public RouteValueDictionary AddSingleItem()
         {
@@ -47,7 +61,136 @@ namespace Microsoft.AspNetCore.Routing
         }
 
         [Benchmark]
-        public RouteValueDictionary ConditionalAdd_ContainsKeyAdd()
+        public void ContainsKey_Array_Found()
+        {
+            _arrayValues.ContainsKey("id");
+        }
+
+        [Benchmark]
+        public void ContainsKey_Array_NotFound()
+        {
+            _arrayValues.ContainsKey("name");
+        }
+
+        [Benchmark]
+        public void ContainsKey_Properties_Found()
+        {
+            _propertyValues.ContainsKey("id");
+        }
+
+        [Benchmark]
+        public void ContainsKey_Properties_NotFound()
+        {
+            _propertyValues.ContainsKey("name");
+        }
+
+        [Benchmark]
+        public void TryAdd_Properties_AtCapacity_KeyExists()
+        {
+            var propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17", area = "root" });
+            propertyValues.TryAdd("id", "15");
+        }
+
+        [Benchmark]
+        public void TryAdd_Properties_AtCapacity_KeyDoesNotExist()
+        {
+            var propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17", area = "root" });
+            _propertyValues.TryAdd("name", "Service");
+        }
+
+        [Benchmark]
+        public void TryAdd_Properties_NotAtCapacity_KeyExists()
+        {
+            var propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17" });
+            propertyValues.TryAdd("id", "15");
+        }
+
+        [Benchmark]
+        public void TryAdd_Properties_NotAtCapacity_KeyDoesNotExist()
+        {
+            var propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17" });
+            _propertyValues.TryAdd("name", "Service");
+        }
+
+        [Benchmark]
+        public void TryAdd_Array_AtCapacity_KeyExists()
+        {
+            var arrayValues = new RouteValueDictionary
+                {
+                    { "action", "Index" },
+                    { "controller", "Home" },
+                    { "id", "17" },
+                    { "area", "root" }
+                };
+            arrayValues.TryAdd("id", "15");
+        }
+
+        [Benchmark]
+        public void TryAdd_Array_AtCapacity_KeyDoesNotExist()
+        {
+            var arrayValues = new RouteValueDictionary
+                {
+                    { "action", "Index" },
+                    { "controller", "Home" },
+                    { "id", "17" },
+                    { "area", "root" }
+                };
+            arrayValues.TryAdd("name", "Service");
+        }
+
+        [Benchmark]
+        public void TryAdd_Array_NotAtCapacity_KeyExists()
+        {
+            var arrayValues = new RouteValueDictionary
+                {
+                    { "action", "Index" },
+                    { "controller", "Home" },
+                    { "id", "17" }
+                };
+            arrayValues.TryAdd("id", "15");
+        }
+
+        [Benchmark]
+        public void TryAdd_Array_NotAtCapacity_KeyDoesNotExist()
+        {
+            var arrayValues = new RouteValueDictionary
+                {
+                    { "action", "Index" },
+                    { "controller", "Home" },
+                    { "id", "17" },
+                };
+            arrayValues.TryAdd("name", "Service");
+        }
+
+        [Benchmark]
+        public void ConditionalAdd_Array()
+        {
+            var arrayValues = new RouteValueDictionary()
+                {
+                    { "action", "Index" },
+                    { "controller", "Home" },
+                    { "id", "17" },
+                };
+
+            if (!arrayValues.ContainsKey("name"))
+            {
+                arrayValues.Add("name", "Service");
+            }
+        }
+
+        [Benchmark]
+        public void ConditionalAdd_Properties()
+        {
+            var propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17" });
+
+            if (!propertyValues.ContainsKey("name"))
+            {
+                propertyValues.Add("name", "Service");
+            }
+        }
+
+        [Benchmark]
+        public RouteValueDictionary ConditionalAdd_ContainsKey_Array()
         {
             var dictionary = _arrayValues;
 
@@ -68,7 +211,7 @@ namespace Microsoft.AspNetCore.Routing
 
             return dictionary;
         }
-        
+
         [Benchmark]
         public RouteValueDictionary ConditionalAdd_TryAdd()
         {

src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs

@@ -97,7 +97,6 @@ namespace Microsoft.AspNetCore.Routing
         /// Only public instance non-index properties are considered.
         /// </remarks>
         public RouteValueDictionary(object values)
-            : this()
         {
             if (values is RouteValueDictionary dictionary)
             {
@@ -109,20 +108,27 @@ namespace Microsoft.AspNetCore.Routing
                     return;
                 }
 
-                var other = dictionary._arrayStorage;
-                var storage = new KeyValuePair<string, object>[other.Length];
-                if (dictionary._count != 0)
+                var count = dictionary._count;
+                if (count > 0)
                 {
-                    Array.Copy(other, 0, storage, 0, dictionary._count);
+                    var other = dictionary._arrayStorage;
+                    var storage = new KeyValuePair<string, object>[count];
+                    Array.Copy(other, 0, storage, 0, count);
+                    _arrayStorage = storage;
+                    _count = count;
+                }
+                else
+                {
+                    _arrayStorage = Array.Empty<KeyValuePair<string, object>>();
                 }
 
-                _arrayStorage = storage;
-                _count = dictionary._count;
                 return;
             }
 
             if (values is IEnumerable<KeyValuePair<string, object>> keyValueEnumerable)
             {
+                _arrayStorage = Array.Empty<KeyValuePair<string, object>>();
+
                 foreach (var kvp in keyValueEnumerable)
                 {
                     Add(kvp.Key, kvp.Value);
@@ -133,6 +139,8 @@ namespace Microsoft.AspNetCore.Routing
 
             if (values is IEnumerable<KeyValuePair<string, string>> stringValueEnumerable)
             {
+                _arrayStorage = Array.Empty<KeyValuePair<string, object>>();
+
                 foreach (var kvp in stringValueEnumerable)
                 {
                     Add(kvp.Key, kvp.Value);
@@ -146,7 +154,10 @@ namespace Microsoft.AspNetCore.Routing
                 var storage = new PropertyStorage(values);
                 _propertyStorage = storage;
                 _count = storage.Properties.Length;
-                return;
+            }
+            else
+            {
+                _arrayStorage = Array.Empty<KeyValuePair<string, object>>();
             }
         }
 
@@ -260,8 +271,7 @@ namespace Microsoft.AspNetCore.Routing
 
             EnsureCapacity(_count + 1);
 
-            var index = FindIndex(key);
-            if (index >= 0)
+            if (ContainsKeyArray(key))
             {
                 var message = Resources.FormatRouteValueDictionary_DuplicateKey(key, nameof(RouteValueDictionary));
                 throw new ArgumentException(message, nameof(key));
@@ -305,7 +315,18 @@ namespace Microsoft.AspNetCore.Routing
                 ThrowArgumentNullExceptionForKey();
             }
 
-            return TryGetValue(key, out var _);
+            return ContainsKeyCore(key);
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool ContainsKeyCore(string key)
+        {
+            if (_propertyStorage == null)
+            {
+                return ContainsKeyArray(key);
+            }
+
+            return ContainsKeyProperties(key);
         }
 
         /// <inheritdoc />
@@ -460,13 +481,7 @@ namespace Microsoft.AspNetCore.Routing
                 ThrowArgumentNullExceptionForKey();
             }
 
-            // Since this is an attempt to write to the dictionary, just make it an array if it isn't. If the code
-            // path we're on event tries to write to the dictionary, it will likely get 'upgraded' at some point,
-            // so we do it here to keep the code size and complexity down.
-            EnsureCapacity(Count);
-
-            var index = FindIndex(key);
-            if (index >= 0)
+            if (ContainsKeyCore(key))
             {
                 return false;
             }
@@ -603,6 +618,42 @@ namespace Microsoft.AspNetCore.Routing
             return false;
         }
 
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool ContainsKeyArray(string key)
+        {
+            var array = _arrayStorage;
+            var count = _count;
+
+            // Elide bounds check for indexing.
+            if ((uint)count <= (uint)array.Length)
+            {
+                for (var i = 0; i < count; i++)
+                {
+                    if (string.Equals(array[i].Key, key, StringComparison.OrdinalIgnoreCase))
+                    {
+                        return true;
+                    }
+                }
+            }
+
+            return false;
+        }
+
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private bool ContainsKeyProperties(string key)
+        {
+            var properties = _propertyStorage.Properties;
+            for (var i = 0; i < properties.Length; i++)
+            {
+                if (string.Equals(properties[i].Name, key, StringComparison.OrdinalIgnoreCase))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
+
         public struct Enumerator : IEnumerator<KeyValuePair<string, object>>
         {
             private readonly RouteValueDictionary _dictionary;

test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs

@@ -51,6 +51,8 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.Equal(other, dict);
+            Assert.Single(dict._arrayStorage);
+            Assert.Null(dict._propertyStorage);
 
             var storage = Assert.IsType<KeyValuePair<string, object>[]>(dict._arrayStorage);
             var otherStorage = Assert.IsType<KeyValuePair<string, object>[]>(other._arrayStorage);
@@ -68,6 +70,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.Equal(other, dict);
+            Assert.Null(dict._arrayStorage);
 
             var storage = dict._propertyStorage;
             var otherStorage = other._propertyStorage;
@@ -259,6 +262,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
             Assert.Empty(dict);
         }
 
@@ -273,6 +277,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
             Assert.Empty(dict);
         }
 
@@ -287,6 +292,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
             Assert.Collection(
                 dict.OrderBy(kvp => kvp.Key),
                 kvp =>
@@ -314,6 +320,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
             Assert.Collection(
                 dict.OrderBy(kvp => kvp.Key),
                 kvp => { Assert.Equal("DerivedProperty", kvp.Key); Assert.Equal(5, kvp.Value); });
@@ -330,6 +337,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
             Assert.Empty(dict);
         }
 
@@ -918,6 +926,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.Empty(dict);
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
         }
 
         [Fact]
@@ -932,6 +941,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.Empty(dict);
             Assert.Null(dict._propertyStorage);
+            Assert.Empty(dict._arrayStorage);
         }
 
         [Fact]
@@ -949,10 +959,11 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.Empty(dict);
             Assert.IsType<KeyValuePair<string, object>[]>(dict._arrayStorage);
+            Assert.Null(dict._propertyStorage);
         }
 
         [Fact]
-        public void Contains_KeyValuePair_True()
+        public void Contains_ListStorage_KeyValuePair_True()
         {
             // Arrange
             var dict = new RouteValueDictionary()
@@ -971,7 +982,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
         }
 
         [Fact]
-        public void Contains_KeyValuePair_True_CaseInsensitive()
+        public void Contains_ListStory_KeyValuePair_True_CaseInsensitive()
         {
             // Arrange
             var dict = new RouteValueDictionary()
@@ -990,7 +1001,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
         }
 
         [Fact]
-        public void Contains_KeyValuePair_False()
+        public void Contains_ListStorage_KeyValuePair_False()
         {
             // Arrange
             var dict = new RouteValueDictionary()
@@ -1010,7 +1021,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
         // Value comparisons use the default equality comparer.
         [Fact]
-        public void Contains_KeyValuePair_False_ValueComparisonIsDefault()
+        public void Contains_ListStorage_KeyValuePair_False_ValueComparisonIsDefault()
         {
             // Arrange
             var dict = new RouteValueDictionary()
@@ -1028,6 +1039,87 @@ namespace Microsoft.AspNetCore.Routing.Tests
             Assert.IsType<KeyValuePair<string, object>[]>(dict._arrayStorage);
         }
 
+        [Fact]
+        public void Contains_PropertyStorage_KeyValuePair_True()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary(new { key = "value" });
+
+            var input = new KeyValuePair<string, object>("key", "value");
+
+            // Act
+            var result = ((ICollection<KeyValuePair<string, object>>)dict).Contains(input);
+
+            // Assert
+            Assert.True(result);
+            Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
+            Assert.Collection(
+                dict,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp));
+        }
+
+        [Fact]
+        public void Contains_PropertyStory_KeyValuePair_True_CaseInsensitive()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary(new { key = "value" });
+
+            var input = new KeyValuePair<string, object>("KEY", "value");
+
+            // Act
+            var result = ((ICollection<KeyValuePair<string, object>>)dict).Contains(input);
+
+            // Assert
+            Assert.True(result);
+            Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
+            Assert.Collection(
+                dict,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp));
+        }
+
+        [Fact]
+        public void Contains_PropertyStorage_KeyValuePair_False()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary(new { key = "value" });
+
+            var input = new KeyValuePair<string, object>("other", "value");
+
+            // Act
+            var result = ((ICollection<KeyValuePair<string, object>>)dict).Contains(input);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
+            Assert.Collection(
+                dict,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp));
+        }
+
+        // Value comparisons use the default equality comparer.
+        [Fact]
+        public void Contains_PropertyStorage_KeyValuePair_False_ValueComparisonIsDefault()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary(new { key = "value" });
+
+            var input = new KeyValuePair<string, object>("key", "valUE");
+
+            // Act
+            var result = ((ICollection<KeyValuePair<string, object>>)dict).Contains(input);
+
+            // Assert
+            Assert.False(result);
+            Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
+            Assert.Collection(
+                dict,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp));
+        }
+
         [Fact]
         public void ContainsKey_EmptyStorage()
         {
@@ -1066,6 +1158,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.False(result);
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
         }
 
         [Fact]
@@ -1080,6 +1173,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.True(result);
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
         }
 
         [Fact]
@@ -1094,6 +1188,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             // Assert
             Assert.True(result);
             Assert.NotNull(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
         }
 
         [Fact]
@@ -1393,6 +1488,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
             Assert.IsType<KeyValuePair<string, object>[]>(dict._arrayStorage);
         }
 
+
         [Fact]
         public void Remove_KeyAndOutValue_EmptyStorage()
         {
@@ -1634,9 +1730,28 @@ namespace Microsoft.AspNetCore.Routing.Tests
             Assert.True(result);
         }
 
-        // We always 'upgrade' if you are trying to write to the dictionary.
         [Fact]
-        public void TryAdd_ConvertsPropertyStorage_ToArrayStorage()
+        public void TryAdd_PropertyStorage_KeyDoesNotExist_ConvertsPropertyStorageToArrayStorage()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary(new { key = "value", });
+
+            // Act
+            var result = dict.TryAdd("otherKey", "value");
+
+            // Assert
+            Assert.True(result);
+            Assert.Null(dict._propertyStorage);
+            Assert.Collection(
+                dict._arrayStorage,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp),
+                kvp => Assert.Equal(new KeyValuePair<string, object>("otherKey", "value"), kvp),
+                kvp => Assert.Equal(default, kvp),
+                kvp => Assert.Equal(default, kvp));
+        }
+
+        [Fact]
+        public void TryAdd_PropertyStory_KeyExist_DoesNotConvertPropertyStorageToArrayStorage()
         {
             // Arrange
             var dict = new RouteValueDictionary(new { key = "value", });
@@ -1646,13 +1761,11 @@ namespace Microsoft.AspNetCore.Routing.Tests
 
             // Assert
             Assert.False(result);
-            Assert.Null(dict._propertyStorage);
+            Assert.Null(dict._arrayStorage);
+            Assert.NotNull(dict._propertyStorage);
             Assert.Collection(
-                dict._arrayStorage,
-                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp),
-                kvp => Assert.Equal(default, kvp),
-                kvp => Assert.Equal(default, kvp),
-                kvp => Assert.Equal(default, kvp));
+                dict,
+                kvp => Assert.Equal(new KeyValuePair<string, object>("key", "value"), kvp));
         }
 
         [Fact]