From c9be018c7e2cce41157b37a17da7189d772635bc Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Sat, 10 Nov 2018 16:07:57 +0100 Subject: [PATCH] Improve performance and reduce allocations in RouteValueDictionary. --- .../RouteValueDictionaryBenchmark.cs | 147 +++++++++++++++++- .../Routing/RouteValueDictionary.cs | 87 ++++++++--- .../RouteValueDictionaryTests.cs | 131 ++++++++++++++-- 3 files changed, 336 insertions(+), 29 deletions(-) diff --git a/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs index c256d31a..2dfc36af 100644 --- a/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs +++ b/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs @@ -10,6 +10,7 @@ public class RouteValueDictionaryBenchmark { 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 @@ public void Setup() { "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 @@ public RouteValueDictionary AddThreeItems() } [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 @@ public RouteValueDictionary ConditionalAdd_ContainsKeyAdd() 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 9fe77735..13c433b3 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs @@ -97,7 +97,6 @@ public RouteValueDictionary() /// Only public instance non-index properties are considered. /// public RouteValueDictionary(object values) - : this() { if (values is RouteValueDictionary dictionary) { @@ -109,20 +108,27 @@ public RouteValueDictionary(object values) 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 @@ public RouteValueDictionary(object values) if (values is IEnumerable> stringValueEnumerable) { + _arrayStorage = Array.Empty>(); + foreach (var kvp in stringValueEnumerable) { Add(kvp.Key, kvp.Value); @@ -146,7 +154,10 @@ public RouteValueDictionary(object values) var storage = new PropertyStorage(values); _propertyStorage = storage; _count = storage.Properties.Length; - return; + } + else + { + _arrayStorage = Array.Empty>(); } } @@ -260,8 +271,7 @@ public void Add(string key, object value) 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 @@ public bool ContainsKey(string key) 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 @@ public bool TryAdd(string key, object value) 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 @@ private bool TryFindItem(string key, out object value) 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 c4aa286f..ab5925e2 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs @@ -51,6 +51,8 @@ public void CreateFromRouteValueDictionary_WithArrayStorage_CopiesStorage() // 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 @@ public void CreateFromRouteValueDictionary_WithPropertyStorage_CopiesStorage() // Assert Assert.Equal(other, dict); + Assert.Null(dict._arrayStorage); var storage = dict._propertyStorage; var otherStorage = other._propertyStorage; @@ -259,6 +262,7 @@ public void CreateFromObject_CopiesPropertiesFromRegularType_IgnoresStatic() // Assert Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); Assert.Empty(dict); } @@ -273,6 +277,7 @@ public void CreateFromObject_CopiesPropertiesFromRegularType_IgnoresSetOnly() // Assert Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); Assert.Empty(dict); } @@ -287,6 +292,7 @@ public void CreateFromObject_CopiesPropertiesFromRegularType_IncludesInherited() // Assert Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); Assert.Collection( dict.OrderBy(kvp => kvp.Key), kvp => @@ -314,6 +320,7 @@ public void CreateFromObject_CopiesPropertiesFromRegularType_WithHiddenProperty( // 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 @@ public void CreateFromObject_CopiesPropertiesFromRegularType_WithIndexerProperty // Assert Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); Assert.Empty(dict); } @@ -918,6 +926,7 @@ public void Clear_PropertyStorage_AlreadyEmpty() // Assert Assert.Empty(dict); Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); } [Fact] @@ -932,6 +941,7 @@ public void Clear_PropertyStorage() // Assert Assert.Empty(dict); Assert.Null(dict._propertyStorage); + Assert.Empty(dict._arrayStorage); } [Fact] @@ -949,10 +959,11 @@ public void Clear_ListStorage() // 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 @@ public void Contains_KeyValuePair_True() } [Fact] - public void Contains_KeyValuePair_True_CaseInsensitive() + public void Contains_ListStory_KeyValuePair_True_CaseInsensitive() { // Arrange var dict = new RouteValueDictionary() @@ -990,7 +1001,7 @@ public void Contains_KeyValuePair_True_CaseInsensitive() } [Fact] - public void Contains_KeyValuePair_False() + public void Contains_ListStorage_KeyValuePair_False() { // Arrange var dict = new RouteValueDictionary() @@ -1010,7 +1021,7 @@ public void Contains_KeyValuePair_False() // 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 @@ public void Contains_KeyValuePair_False_ValueComparisonIsDefault() 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 @@ public void ContainsKey_PropertyStorage_False() // Assert Assert.False(result); Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); } [Fact] @@ -1080,6 +1173,7 @@ public void ContainsKey_PropertyStorage_True() // Assert Assert.True(result); Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); } [Fact] @@ -1094,6 +1188,7 @@ public void ContainsKey_PropertyStorage_True_CaseInsensitive() // Assert Assert.True(result); Assert.NotNull(dict._propertyStorage); + Assert.Null(dict._arrayStorage); } [Fact] @@ -1393,6 +1488,7 @@ public void Remove_ListStorage_True_CaseInsensitive() Assert.IsType[]>(dict._arrayStorage); } + [Fact] public void Remove_KeyAndOutValue_EmptyStorage() { @@ -1634,27 +1730,44 @@ public void TryAdd_EmptyStringIsAllowed() 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("key", "value"); + var result = dict.TryAdd("otherKey", "value"); // Assert - Assert.False(result); + Assert.True(result); Assert.Null(dict._propertyStorage); Assert.Collection( dict._arrayStorage, kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp), - kvp => Assert.Equal(default, 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", }); + + // Act + var result = dict.TryAdd("key", "value"); + + // Assert + Assert.False(result); + Assert.Null(dict._arrayStorage); + Assert.NotNull(dict._propertyStorage); + Assert.Collection( + dict, + kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp)); + } + [Fact] public void TryAdd_EmptyStorage_CanAdd() {