diff --git a/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs index c256d31a55..2dfc36afa4 100644 --- a/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs +++ b/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() { diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs b/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs index 9fe777358b..13c433b387 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs @@ -97,7 +97,6 @@ namespace Microsoft.AspNetCore.Routing /// Only public instance non-index properties are considered. /// 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[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[count]; + Array.Copy(other, 0, storage, 0, count); + _arrayStorage = storage; + _count = count; + } + else + { + _arrayStorage = Array.Empty>(); } - _arrayStorage = storage; - _count = dictionary._count; return; } if (values is IEnumerable> keyValueEnumerable) { + _arrayStorage = Array.Empty>(); + foreach (var kvp in keyValueEnumerable) { Add(kvp.Key, kvp.Value); @@ -133,6 +139,8 @@ namespace Microsoft.AspNetCore.Routing if (values is IEnumerable> stringValueEnumerable) { + _arrayStorage = Array.Empty>(); + 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>(); } } @@ -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); } /// @@ -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> { private readonly RouteValueDictionary _dictionary; diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs index c4aa286fba..ab5925e219 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs +++ b/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[]>(dict._arrayStorage); var otherStorage = Assert.IsType[]>(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[]>(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[]>(dict._arrayStorage); } + [Fact] + public void Contains_PropertyStorage_KeyValuePair_True() + { + // Arrange + var dict = new RouteValueDictionary(new { key = "value" }); + + var input = new KeyValuePair("key", "value"); + + // Act + var result = ((ICollection>)dict).Contains(input); + + // Assert + Assert.True(result); + Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); + Assert.Collection( + dict, + kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp)); + } + + [Fact] + public void Contains_PropertyStory_KeyValuePair_True_CaseInsensitive() + { + // Arrange + var dict = new RouteValueDictionary(new { key = "value" }); + + var input = new KeyValuePair("KEY", "value"); + + // Act + var result = ((ICollection>)dict).Contains(input); + + // Assert + Assert.True(result); + Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); + Assert.Collection( + dict, + kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp)); + } + + [Fact] + public void Contains_PropertyStorage_KeyValuePair_False() + { + // Arrange + var dict = new RouteValueDictionary(new { key = "value" }); + + var input = new KeyValuePair("other", "value"); + + // Act + var result = ((ICollection>)dict).Contains(input); + + // Assert + Assert.False(result); + Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); + Assert.Collection( + dict, + kvp => Assert.Equal(new KeyValuePair("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("key", "valUE"); + + // Act + var result = ((ICollection>)dict).Contains(input); + + // Assert + Assert.False(result); + Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); + Assert.Collection( + dict, + kvp => Assert.Equal(new KeyValuePair("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[]>(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("key", "value"), kvp), + kvp => Assert.Equal(new KeyValuePair("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("key", "value"), kvp), - kvp => Assert.Equal(default, kvp), - kvp => Assert.Equal(default, kvp), - kvp => Assert.Equal(default, kvp)); + dict, + kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp)); } [Fact]