From 41c4a47680a2242bb389a58a18aba3b01491dab0 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 12 Sep 2018 01:40:38 -0700 Subject: [PATCH] Minor perf updates for RVD Porting changes from perf work in https://github.com/aspnet/Routing/pull/788 Includes porting/adding the RVD benchmarks, as well as a new TryAdd method. --- HttpAbstractions.sln | 19 +- ...crosoft.AspNetCore.Http.Performance.csproj | 21 ++ .../Properties/AssemblyInfo.cs | 1 + .../RouteValueDictionaryBenchmark.cs | 182 +++++++++++++++ build/dependencies.props | 2 + build/repo.props | 4 + .../Routing/RouteValueDictionary.cs | 160 +++++++++---- .../RouteValueDictionaryTests.cs | 215 +++++++++++++++++- 8 files changed, 560 insertions(+), 44 deletions(-) create mode 100644 benchmarks/Microsoft.AspNetCore.Http.Performance/Microsoft.AspNetCore.Http.Performance.csproj create mode 100644 benchmarks/Microsoft.AspNetCore.Http.Performance/Properties/AssemblyInfo.cs create mode 100644 benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs diff --git a/HttpAbstractions.sln b/HttpAbstractions.sln index fc578eb8a3..bf9b67107e 100644 --- a/HttpAbstractions.sln +++ b/HttpAbstractions.sln @@ -1,4 +1,4 @@ -Microsoft Visual Studio Solution File, Format Version 12.00 +Microsoft Visual Studio Solution File, Format Version 12.00 # Visual Studio 15 VisualStudioVersion = 15.0.26730.10 MinimumVisualStudioVersion = 15.0.26730.03 @@ -70,6 +70,10 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authen EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Microsoft.AspNetCore.Authentication.Core.Test", "test\Microsoft.AspNetCore.Authentication.Core.Test\Microsoft.AspNetCore.Authentication.Core.Test.csproj", "{A85950C5-2794-47E2-8EAA-05A1DC7C6DA7}" EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "benchmarks", "benchmarks", "{5C05BDE0-6339-40BF-8215-97AFA72DCFE1}" +EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Http.Performance", "benchmarks\Microsoft.AspNetCore.Http.Performance\Microsoft.AspNetCore.Http.Performance.csproj", "{4633ADE6-7A61-44EB-B7CE-0141C009DBAA}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -272,6 +276,18 @@ Global {A85950C5-2794-47E2-8EAA-05A1DC7C6DA7}.Release|Mixed Platforms.Build.0 = Release|Any CPU {A85950C5-2794-47E2-8EAA-05A1DC7C6DA7}.Release|x86.ActiveCfg = Release|Any CPU {A85950C5-2794-47E2-8EAA-05A1DC7C6DA7}.Release|x86.Build.0 = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|Any CPU.Build.0 = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|Mixed Platforms.ActiveCfg = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|Mixed Platforms.Build.0 = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|x86.ActiveCfg = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Debug|x86.Build.0 = Debug|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|Any CPU.ActiveCfg = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|Any CPU.Build.0 = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|Mixed Platforms.ActiveCfg = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|Mixed Platforms.Build.0 = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|x86.ActiveCfg = Release|Any CPU + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA}.Release|x86.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -296,6 +312,7 @@ Global {3D8C9A87-5DFB-4EC0-9CB6-174AD3B33852} = {A5A15F1C-885A-452A-A731-B0173DDBD913} {73CA3145-91BD-4DA5-BC74-40008DE7EA98} = {A5A15F1C-885A-452A-A731-B0173DDBD913} {A85950C5-2794-47E2-8EAA-05A1DC7C6DA7} = {F31FF137-390C-49BF-A3BD-7C6ED3597C21} + {4633ADE6-7A61-44EB-B7CE-0141C009DBAA} = {5C05BDE0-6339-40BF-8215-97AFA72DCFE1} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {D9A9994D-F09F-4209-861B-4A9036485D1F} diff --git a/benchmarks/Microsoft.AspNetCore.Http.Performance/Microsoft.AspNetCore.Http.Performance.csproj b/benchmarks/Microsoft.AspNetCore.Http.Performance/Microsoft.AspNetCore.Http.Performance.csproj new file mode 100644 index 0000000000..2cb0d4f30d --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Http.Performance/Microsoft.AspNetCore.Http.Performance.csproj @@ -0,0 +1,21 @@ + + + + netcoreapp2.2 + Exe + true + true + false + Microsoft.AspNetCore.Http + + + + + + + + + + + + diff --git a/benchmarks/Microsoft.AspNetCore.Http.Performance/Properties/AssemblyInfo.cs b/benchmarks/Microsoft.AspNetCore.Http.Performance/Properties/AssemblyInfo.cs new file mode 100644 index 0000000000..2efc4cb5fb --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Http.Performance/Properties/AssemblyInfo.cs @@ -0,0 +1 @@ +[assembly: BenchmarkDotNet.Attributes.AspNetCoreBenchmark] \ No newline at end of file diff --git a/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs b/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs new file mode 100644 index 0000000000..c256d31a55 --- /dev/null +++ b/benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs @@ -0,0 +1,182 @@ +// 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 BenchmarkDotNet.Attributes; + +namespace Microsoft.AspNetCore.Routing +{ + public class RouteValueDictionaryBenchmark + { + private RouteValueDictionary _arrayValues; + private RouteValueDictionary _propertyValues; + + // We modify the route value dictionaries in many of these benchmarks. + [IterationSetup] + public void Setup() + { + _arrayValues = new RouteValueDictionary() + { + { "action", "Index" }, + { "controller", "Home" }, + { "id", "17" }, + }; + _propertyValues = new RouteValueDictionary(new { action = "Index", controller = "Home", id = "17" }); + } + + [Benchmark] + public RouteValueDictionary AddSingleItem() + { + var dictionary = new RouteValueDictionary + { + { "action", "Index" } + }; + return dictionary; + } + + [Benchmark] + public RouteValueDictionary AddThreeItems() + { + var dictionary = new RouteValueDictionary + { + { "action", "Index" }, + { "controller", "Home" }, + { "id", "15" } + }; + return dictionary; + } + + [Benchmark] + public RouteValueDictionary ConditionalAdd_ContainsKeyAdd() + { + var dictionary = _arrayValues; + + if (!dictionary.ContainsKey("action")) + { + dictionary.Add("action", "Index"); + } + + if (!dictionary.ContainsKey("controller")) + { + dictionary.Add("controller", "Home"); + } + + if (!dictionary.ContainsKey("area")) + { + dictionary.Add("area", "Admin"); + } + + return dictionary; + } + + [Benchmark] + public RouteValueDictionary ConditionalAdd_TryAdd() + { + var dictionary = _arrayValues; + + dictionary.TryAdd("action", "Index"); + dictionary.TryAdd("controller", "Home"); + dictionary.TryAdd("area", "Admin"); + + return dictionary; + } + + [Benchmark] + public RouteValueDictionary ForEachThreeItems_Array() + { + var dictionary = _arrayValues; + foreach (var kvp in dictionary) + { + GC.KeepAlive(kvp.Value); + } + return dictionary; + } + + [Benchmark] + public RouteValueDictionary ForEachThreeItems_Properties() + { + var dictionary = _propertyValues; + foreach (var kvp in dictionary) + { + GC.KeepAlive(kvp.Value); + } + return dictionary; + } + + [Benchmark] + public RouteValueDictionary GetThreeItems_Array() + { + var dictionary = _arrayValues; + GC.KeepAlive(dictionary["action"]); + GC.KeepAlive(dictionary["controller"]); + GC.KeepAlive(dictionary["id"]); + return dictionary; + } + + [Benchmark] + public RouteValueDictionary GetThreeItems_Properties() + { + var dictionary = _propertyValues; + GC.KeepAlive(dictionary["action"]); + GC.KeepAlive(dictionary["controller"]); + GC.KeepAlive(dictionary["id"]); + return dictionary; + } + + [Benchmark] + public RouteValueDictionary SetSingleItem() + { + var dictionary = new RouteValueDictionary + { + ["action"] = "Index" + }; + return dictionary; + } + + [Benchmark] + public RouteValueDictionary SetExistingItem() + { + var dictionary = _arrayValues; + dictionary["action"] = "About"; + return dictionary; + } + + [Benchmark] + public RouteValueDictionary SetThreeItems() + { + var dictionary = new RouteValueDictionary + { + ["action"] = "Index", + ["controller"] = "Home", + ["id"] = "15" + }; + return dictionary; + } + + [Benchmark] + public RouteValueDictionary TryGetValueThreeItems_Array() + { + var dictionary = _arrayValues; + dictionary.TryGetValue("action", out var action); + dictionary.TryGetValue("controller", out var controller); + dictionary.TryGetValue("id", out var id); + GC.KeepAlive(action); + GC.KeepAlive(controller); + GC.KeepAlive(id); + return dictionary; + } + + [Benchmark] + public RouteValueDictionary TryGetValueThreeItems_Properties() + { + var dictionary = _propertyValues; + dictionary.TryGetValue("action", out var action); + dictionary.TryGetValue("controller", out var controller); + dictionary.TryGetValue("id", out var id); + GC.KeepAlive(action); + GC.KeepAlive(controller); + GC.KeepAlive(id); + return dictionary; + } + } +} diff --git a/build/dependencies.props b/build/dependencies.props index a3e49fa1f1..500c0656c9 100644 --- a/build/dependencies.props +++ b/build/dependencies.props @@ -3,7 +3,9 @@ $(MSBuildAllProjects);$(MSBuildThisFileFullPath) + 0.10.13 3.0.0-alpha1-20180907.9 + 3.0.0-alpha1-10419 3.0.0-alpha1-10419 3.0.0-alpha1-10419 3.0.0-alpha1-10419 diff --git a/build/repo.props b/build/repo.props index 17a98ac7e7..71840e75d1 100644 --- a/build/repo.props +++ b/build/repo.props @@ -12,4 +12,8 @@ + + + true + diff --git a/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs b/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs index f4153881b3..396f79b507 100644 --- a/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs +++ b/src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs @@ -155,9 +155,9 @@ namespace Microsoft.AspNetCore.Routing { get { - if (string.IsNullOrEmpty(key)) + if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } object value; @@ -167,9 +167,9 @@ namespace Microsoft.AspNetCore.Routing set { - if (string.IsNullOrEmpty(key)) + if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } // We're calling this here for the side-effect of converting from properties @@ -177,7 +177,7 @@ namespace Microsoft.AspNetCore.Routing // property storage is immutable. EnsureCapacity(_count); - var index = FindInArray(key); + var index = FindIndex(key); if (index < 0) { EnsureCapacity(_count + 1); @@ -255,12 +255,12 @@ namespace Microsoft.AspNetCore.Routing { if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } EnsureCapacity(_count + 1); - var index = FindInArray(key); + var index = FindIndex(key); if (index >= 0) { var message = Resources.FormatRouteValueDictionary_DuplicateKey(key, nameof(RouteValueDictionary)); @@ -302,7 +302,7 @@ namespace Microsoft.AspNetCore.Routing { if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } return TryGetValue(key, out var _); @@ -362,7 +362,7 @@ namespace Microsoft.AspNetCore.Routing EnsureCapacity(Count); - var index = FindInArray(item.Key); + var index = FindIndex(item.Key); var array = _arrayStorage; if (index >= 0 && EqualityComparer.Default.Equals(array[index].Value, item.Value)) { @@ -380,7 +380,7 @@ namespace Microsoft.AspNetCore.Routing { if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } if (Count == 0) @@ -390,7 +390,7 @@ namespace Microsoft.AspNetCore.Routing EnsureCapacity(Count); - var index = FindInArray(key); + var index = FindIndex(key); if (index >= 0) { _count--; @@ -404,14 +404,54 @@ namespace Microsoft.AspNetCore.Routing return false; } + /// + /// Attempts to the add the provided and to the dictionary. + /// + /// The key. + /// The value. + /// Returns true if the value was added. Returns false if the key was already present. + public bool TryAdd(string key, object value) + { + if (key == null) + { + 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) + { + return false; + } + + EnsureCapacity(Count + 1); + _arrayStorage[Count] = new KeyValuePair(key, value); + _count++; + return true; + } + /// public bool TryGetValue(string key, out object value) { if (key == null) { - throw new ArgumentNullException(nameof(key)); + ThrowArgumentNullExceptionForKey(); } + if (_propertyStorage == null) + { + return TryFindItem(key, out value); + } + + return TryGetValueSlow(key, out value); + } + + private bool TryGetValueSlow(string key, out object value) + { if (_propertyStorage != null) { var storage = _propertyStorage; @@ -423,25 +463,17 @@ namespace Microsoft.AspNetCore.Routing return true; } } - - value = default; - return false; - } - - var array = _arrayStorage; - for (var i = 0; i < _count; i++) - { - if (string.Equals(array[i].Key, key, StringComparison.OrdinalIgnoreCase)) - { - value = array[i].Value; - return true; - } } value = default; return false; } + private static void ThrowArgumentNullExceptionForKey() + { + throw new ArgumentNullException("key"); + } + [MethodImpl(MethodImplOptions.AggressiveInlining)] private void EnsureCapacity(int capacity) { @@ -456,7 +488,10 @@ namespace Microsoft.AspNetCore.Routing if (_propertyStorage != null) { var storage = _propertyStorage; - capacity = Math.Max(storage.Properties.Length, capacity); + + // If we're converting from properties, it's likely due to an 'add' to make sure we have at least + // the default amount of space. + capacity = Math.Max(DefaultCapacity, Math.Max(storage.Properties.Length, capacity)); var array = new KeyValuePair[capacity]; for (var i = 0; i < storage.Properties.Length; i++) @@ -484,10 +519,14 @@ namespace Microsoft.AspNetCore.Routing } [MethodImpl(MethodImplOptions.AggressiveInlining)] - private int FindInArray(string key) + private int FindIndex(string key) { + // Generally the bounds checking here will be elided by the JIT because this will be called + // on the same code path as EnsureCapacity. var array = _arrayStorage; - for (var i = 0; i < _count; i++) + var count = _count; + + for (var i = 0; i < count; i++) { if (string.Equals(array[i].Key, key, StringComparison.OrdinalIgnoreCase)) { @@ -498,9 +537,32 @@ namespace Microsoft.AspNetCore.Routing return -1; } + [MethodImpl(MethodImplOptions.AggressiveInlining)] + private bool TryFindItem(string key, out object value) + { + 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)) + { + value = array[i].Value; + return true; + } + } + } + + value = null; + return false; + } + public struct Enumerator : IEnumerator> { - private RouteValueDictionary _dictionary; + private readonly RouteValueDictionary _dictionary; private int _index; public Enumerator(RouteValueDictionary dictionary) @@ -513,7 +575,7 @@ namespace Microsoft.AspNetCore.Routing _dictionary = dictionary; Current = default; - _index = -1; + _index = 0; } public KeyValuePair Current { get; private set; } @@ -524,22 +586,36 @@ namespace Microsoft.AspNetCore.Routing { } + // Similar to the design of List.Enumerator - Split into fast path and slow path for inlining friendliness + [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool MoveNext() { - if (++_index < _dictionary.Count) - { - if (_dictionary._propertyStorage != null) - { - var storage = _dictionary._propertyStorage; - var property = storage.Properties[_index]; - Current = new KeyValuePair(property.Name, property.GetValue(storage.Value)); - return true; - } + var dictionary = _dictionary; - Current = _dictionary._arrayStorage[_index]; + // The uncommon case is that the propertyStorage is in use + if (dictionary._propertyStorage == null && ((uint)_index < (uint)dictionary._count)) + { + Current = dictionary._arrayStorage[_index]; + _index++; return true; } + return MoveNextRare(); + } + + private bool MoveNextRare() + { + var dictionary = _dictionary; + if (dictionary._propertyStorage != null && ((uint)_index < (uint)dictionary._count)) + { + var storage = dictionary._propertyStorage; + var property = storage.Properties[_index]; + Current = new KeyValuePair(property.Name, property.GetValue(storage.Value)); + _index++; + return true; + } + + _index = dictionary._count; Current = default; return false; } @@ -547,7 +623,7 @@ namespace Microsoft.AspNetCore.Routing public void Reset() { Current = default; - _index = -1; + _index = 0; } } @@ -595,4 +671,4 @@ namespace Microsoft.AspNetCore.Routing } } } -} +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs index 524966dc05..65a97c70ba 100644 --- a/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs +++ b/test/Microsoft.AspNetCore.Http.Abstractions.Tests/RouteValueDictionaryTests.cs @@ -380,6 +380,19 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.False(result); } + [Fact] + public void IndexGet_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var value = dict[""]; + + // Assert + Assert.Null(value); + } + [Fact] public void IndexGet_EmptyStorage_ReturnsNull() { @@ -486,6 +499,19 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.IsType[]>(dict._arrayStorage); } + [Fact] + public void IndexSet_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + dict[""] = "foo"; + + // Assert + Assert.Equal("foo", dict[""]); + } + [Fact] public void IndexSet_EmptyStorage_UpgradesToList() { @@ -747,6 +773,19 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.IsType[]>(dict._arrayStorage); } + [Fact] + public void Add_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + dict.Add("", "foo"); + + // Assert + Assert.Equal("foo", dict[""]); + } + [Fact] public void Add_PropertyStorage() { @@ -762,6 +801,14 @@ namespace Microsoft.AspNetCore.Routing.Tests kvp => { Assert.Equal("age", kvp.Key); Assert.Equal(30, kvp.Value); }, kvp => { Assert.Equal("key", kvp.Key); Assert.Equal("value", kvp.Value); }); Assert.IsType[]>(dict._arrayStorage); + + // The upgrade from property -> array should make space for at least 4 entries + Assert.Collection( + dict._arrayStorage, + kvp => Assert.Equal(new KeyValuePair("age", 30), kvp), + kvp => Assert.Equal(new KeyValuePair("key", "value"), kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp)); } [Fact] @@ -994,6 +1041,19 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.False(result); } + [Fact] + public void ContainsKey_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.ContainsKey(""); + + // Assert + Assert.False(result); + } + [Fact] public void ContainsKey_PropertyStorage_False() { @@ -1206,6 +1266,19 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.False(result); } + [Fact] + public void Remove_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.Remove(""); + + // Assert + Assert.False(result); + } + [Fact] public void Remove_PropertyStorage_Empty() { @@ -1320,6 +1393,132 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.IsType[]>(dict._arrayStorage); } + [Fact] + public void TryAdd_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.TryAdd("", "foo"); + + // Assert + Assert.True(result); + } + + // We always 'upgrade' if you are trying to write to the dictionary. + [Fact] + public void TryAdd_ConvertsPropertyStorage_ToArrayStorage() + { + // Arrange + var dict = new RouteValueDictionary(new { key = "value", }); + + // Act + var result = dict.TryAdd("key", "value"); + + // Assert + Assert.False(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(default, kvp), + kvp => Assert.Equal(default, kvp)); + } + + [Fact] + public void TryAdd_EmptyStorage_CanAdd() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.TryAdd("key", "value"); + + // Assert + Assert.True(result); + 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)); + } + + [Fact] + public void TryAdd_ArrayStorage_CanAdd() + { + // Arrange + var dict = new RouteValueDictionary() + { + { "key0", "value0" }, + }; + + // Act + var result = dict.TryAdd("key1", "value1"); + + // Assert + Assert.True(result); + Assert.Collection( + dict._arrayStorage, + kvp => Assert.Equal(new KeyValuePair("key0", "value0"), kvp), + kvp => Assert.Equal(new KeyValuePair("key1", "value1"), kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp)); + } + + [Fact] + public void TryAdd_ArrayStorage_CanAddWithResize() + { + // Arrange + var dict = new RouteValueDictionary() + { + { "key0", "value0" }, + { "key1", "value1" }, + { "key2", "value2" }, + { "key3", "value3" }, + }; + + // Act + var result = dict.TryAdd("key4", "value4"); + + // Assert + Assert.True(result); + Assert.Collection( + dict._arrayStorage, + kvp => Assert.Equal(new KeyValuePair("key0", "value0"), kvp), + kvp => Assert.Equal(new KeyValuePair("key1", "value1"), kvp), + kvp => Assert.Equal(new KeyValuePair("key2", "value2"), kvp), + kvp => Assert.Equal(new KeyValuePair("key3", "value3"), kvp), + kvp => Assert.Equal(new KeyValuePair("key4", "value4"), kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp)); + } + + [Fact] + public void TryAdd_ArrayStorage_DoesNotAddWhenKeyIsPresent() + { + // Arrange + var dict = new RouteValueDictionary() + { + { "key0", "value0" }, + }; + + // Act + var result = dict.TryAdd("key0", "value1"); + + // Assert + Assert.False(result); + Assert.Collection( + dict._arrayStorage, + kvp => Assert.Equal(new KeyValuePair("key0", "value0"), kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp), + kvp => Assert.Equal(default, kvp)); + } + [Fact] public void TryGetValue_EmptyStorage() { @@ -1335,6 +1534,20 @@ namespace Microsoft.AspNetCore.Routing.Tests Assert.Null(value); } + [Fact] + public void TryGetValue_EmptyStringIsAllowed() + { + // Arrange + var dict = new RouteValueDictionary(); + + // Act + var result = dict.TryGetValue("", out var value); + + // Assert + Assert.False(result); + Assert.Null(value); + } + [Fact] public void TryGetValue_PropertyStorage_False() { @@ -1618,4 +1831,4 @@ namespace Microsoft.AspNetCore.Routing.Tests public string State { get; set; } } } -} +} \ No newline at end of file