Optimize ListStorage in RouteValueDictionary

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

@@ -77,7 +77,7 @@ namespace Microsoft.AspNetCore.Routing
                         throw new ArgumentException(message, nameof(values));
                     }
 
-                    listStorage._inner.Add(kvp);
+                    listStorage.Add(kvp);
                 }
 
                 return;
@@ -96,7 +96,7 @@ namespace Microsoft.AspNetCore.Routing
                         throw new ArgumentException(message, nameof(values));
                     }
 
-                    listStorage._inner.Add(new KeyValuePair<string, object>(kvp.Key, kvp.Value));
+                    listStorage.Add(new KeyValuePair<string, object>(kvp.Key, kvp.Value));
                 }
 
                 return;
@@ -162,7 +162,7 @@ namespace Microsoft.AspNetCore.Routing
             {
                 Upgrade();
 
-                var list = ((ListStorage)_storage)._inner;
+                var list = (ListStorage)_storage;
                 var keys = new string[list.Count];
                 for (var i = 0; i < keys.Length; i++)
                 {
@@ -188,7 +188,7 @@ namespace Microsoft.AspNetCore.Routing
             {
                 Upgrade();
 
-                var list = ((ListStorage)_storage)._inner;
+                var list = (ListStorage)_storage;
                 var values = new object[list.Count];
                 for (var i = 0; i < values.Length; i++)
                 {
@@ -223,7 +223,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             for (var i = 0; i < list.Count; i++)
             {
                 if (string.Equals(list[i].Key, key, StringComparison.OrdinalIgnoreCase))
@@ -246,7 +246,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             list.Clear();
         }
 
@@ -260,7 +260,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             for (var i = 0; i < list.Count; i++)
             {
                 if (string.Equals(list[i].Key, item.Key, StringComparison.OrdinalIgnoreCase))
@@ -305,7 +305,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             list.CopyTo(array, arrayIndex);
         }
 
@@ -337,7 +337,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             for (var i = 0; i < list.Count; i++)
             {
                 if (string.Equals(list[i].Key, item.Key, StringComparison.OrdinalIgnoreCase) &&
@@ -366,7 +366,7 @@ namespace Microsoft.AspNetCore.Routing
 
             Upgrade();
 
-            var list = ((ListStorage)_storage)._inner;
+            var list = (ListStorage)_storage;
             for (var i = 0; i < list.Count; i++)
             {
                 if (string.Equals(list[i].Key, key, StringComparison.OrdinalIgnoreCase))
@@ -445,7 +445,7 @@ namespace Microsoft.AspNetCore.Routing
         {
             public abstract int Count { get; }
 
-            public abstract KeyValuePair<string, object> this[int index] { get; }
+            public abstract KeyValuePair<string, object> this[int index] { get; set; }
 
             public abstract void Upgrade(ref Storage storage);
 
@@ -458,42 +458,115 @@ namespace Microsoft.AspNetCore.Routing
 
         internal class ListStorage : Storage
         {
-            internal readonly List<KeyValuePair<string, object>> _inner;
+            private KeyValuePair<string, object>[] _items;
+            private int _count;
+
+            private static readonly KeyValuePair<string, object>[] _emptyArray = new KeyValuePair<string, object>[0];
 
             public ListStorage()
             {
-                _inner = new List<KeyValuePair<string, object>>();
+                _items = _emptyArray;
             }
 
             public ListStorage(int capacity)
             {
-                _inner = new List<KeyValuePair<string, object>>(capacity);
+                if (capacity == 0)
+                {
+                    _items = _emptyArray;
+                }
+                else
+                {
+                    _items = new KeyValuePair<string, object>[capacity];
+                }
             }
 
             public ListStorage(ListStorage other)
             {
-                // Perf: Don't call the copy constructor, that would box the enumerator.
+                if (other.Count == 0)
-                _inner = new List<KeyValuePair<string, object>>(other._inner.Capacity);
-                for (var i = 0; i < other._inner.Count; i++)
                 {
-                    _inner.Add(other._inner[i]);
+                    _items = _emptyArray;
+                }
+                else
+                {
+                    _items = new KeyValuePair<string, object>[other.Count];
+                    for (var i = 0; i < other.Count; i++)
+                    {
+                        this.Add(other[i]);
+                    }
                 }
             }
 
-            public override int Count => _inner.Count;
+            public int Capacity => _items.Length;
 
-            public override KeyValuePair<string, object> this[int index] => _inner[index];
+            public override int Count => _count;
+
+            public override KeyValuePair<string, object> this[int index]
+            {
+                get
+                {
+                    if (index < 0 || index >= _count)
+                    {
+                        throw new ArgumentOutOfRangeException(nameof(index));
+                    }
+
+                    return _items[index];
+                }
+                set
+                {
+                    if (index < 0 || index >= _count)
+                    {
+                        throw new ArgumentOutOfRangeException(nameof(index));
+                    }
+
+                    _items[index] = value;
+                }
+            }
+
+            public void Add(KeyValuePair<string, object> item)
+            {
+                if (_count == _items.Length)
+                {
+                    EnsureCapacity(_count + 1);
+                }
+
+                _items[_count++] = item;
+            }
+
+            public void RemoveAt(int index)
+            {
+                _count--;
+
+                for (var i = index; i < _count; i++)
+                {
+                    _items[i] = _items[i + 1];
+                }
+
+                _items[_count] = default(KeyValuePair<string, object>);
+            }
 
             public void Clear()
             {
-                _inner.Clear();
+                for (var i = 0; i < _count; i++)
+                {
+                    _items[i] = default(KeyValuePair<string, object>);
+                }
+
+                _count = 0;
+            }
+
+            public void CopyTo(KeyValuePair<string, object>[] array, int arrayIndex)
+            {
+                for (var i = 0; i < _count; i++)
+                {
+                    array[arrayIndex++] = _items[i];
+                }
             }
 
             public override bool ContainsKey(string key)
             {
-                for (var i = 0; i < _inner.Count; i++)
+                for (var i = 0; i < Count; i++)
                 {
-                    var kvp = _inner[i];
+                    var kvp = _items[i];
                     if (string.Equals(key, kvp.Key, StringComparison.OrdinalIgnoreCase))
                     {
                         return true;
@@ -505,25 +578,25 @@ namespace Microsoft.AspNetCore.Routing
 
             public override bool TrySetValue(string key, object value)
             {
-                for (var i = 0; i < _inner.Count; i++)
+                for (var i = 0; i < Count; i++)
                 {
-                    var kvp = _inner[i];
+                    var kvp = _items[i];
                     if (string.Equals(key, kvp.Key, StringComparison.OrdinalIgnoreCase))
                     {
-                        _inner[i] = new KeyValuePair<string, object>(key, value);
+                        _items[i] = new KeyValuePair<string, object>(key, value);
                         return true;
                     }
                 }
 
-                _inner.Add(new KeyValuePair<string, object>(key, value));
+                Add(new KeyValuePair<string, object>(key, value));
                 return true;
             }
 
             public override bool TryGetValue(string key, out object value)
             {
-                for (var i = 0; i < _inner.Count; i++)
+                for (var i = 0; i < Count; i++)
                 {
-                    var kvp = _inner[i];
+                    var kvp = _items[i];
                     if (string.Equals(key, kvp.Key, StringComparison.OrdinalIgnoreCase))
                     {
                         value = kvp.Value;
@@ -539,6 +612,18 @@ namespace Microsoft.AspNetCore.Routing
             {
                 // Do nothing.
             }
+
+            private void EnsureCapacity(int min)
+            {
+                var newLength = _items.Length == 0 ? 4 : _items.Length * 2;
+                var newItems = new KeyValuePair<string, object>[newLength];
+                for (var i = 0; i < _count; i++)
+                {
+                    newItems[i] = _items[i];
+                }
+
+                _items = newItems;
+            }
         }
 
         internal class PropertyStorage : Storage
@@ -578,6 +663,11 @@ namespace Microsoft.AspNetCore.Routing
                     var property = _properties[index];
                     return new KeyValuePair<string, object>(property.Name, property.GetValue(_value));
                 }
+                set
+                {
+                    // PropertyStorage never sets a value.
+                    throw new NotImplementedException();
+                }
             }
 
             public override bool TryGetValue(string key, out object value)
@@ -665,6 +755,10 @@ namespace Microsoft.AspNetCore.Routing
                 {
                     throw new NotImplementedException();
                 }
+                set
+                {
+                    throw new NotImplementedException();
+                }
             }
 
             public override bool ContainsKey(string key)

test/Microsoft.AspNetCore.Mvc.Routing.Abstractions.Tests/RouteValueDictionaryTests.cs

@@ -53,7 +53,6 @@ namespace Microsoft.AspNetCore.Routing.Tests
             var storage = Assert.IsType<RouteValueDictionary.ListStorage>(dict._storage);
             var otherStorage = Assert.IsType<RouteValueDictionary.ListStorage>(other._storage);
             Assert.NotSame(otherStorage, storage);
-            Assert.NotSame(otherStorage._inner, storage._inner);
         }
 
         [Fact]
@@ -328,7 +327,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
         [Fact]
         public void CreateFromObject_MixedCaseThrows()
         {
-            // Arrange		
+            // Arrange
             var obj = new { controller = "Home", Controller = "Home" };
 
             var message =
@@ -336,7 +335,7 @@ namespace Microsoft.AspNetCore.Routing.Tests
                 $"only by casing. This is not supported by {nameof(RouteValueDictionary)} which uses " +
                 $"case-insensitive comparisons.";
 
-            // Act & Assert		
+            // Act & Assert
             var exception = Assert.Throws<InvalidOperationException>(() =>
             {
                 var dictionary = new RouteValueDictionary(obj);
@@ -1438,6 +1437,55 @@ namespace Microsoft.AspNetCore.Routing.Tests
             Assert.IsType<RouteValueDictionary.ListStorage>(dict._storage);
         }
 
+        [Fact]
+        public void ListStorage_DynamicallyAdjustsCapacity()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary();
+
+            // Act 1
+            dict.Add("key", "value");
+
+            // Assert 1
+            var storage = Assert.IsType<RouteValueDictionary.ListStorage>(dict._storage);
+            Assert.Equal(4, storage.Capacity);
+
+            // Act 2
+            dict.Add("key2", "value2");
+            dict.Add("key3", "value3");
+            dict.Add("key4", "value4");
+            dict.Add("key5", "value5");
+
+            // Assert 2
+            Assert.Equal(8, storage.Capacity);
+        }
+
+        [Fact]
+        public void ListStorage_RemoveAt_RearrangesInnerArray()
+        {
+            // Arrange
+            var dict = new RouteValueDictionary();
+            dict.Add("key", "value");
+            dict.Add("key2", "value2");
+            dict.Add("key3", "value3");
+
+            // Assert 1
+            var storage = Assert.IsType<RouteValueDictionary.ListStorage>(dict._storage);
+            Assert.Equal(3, storage.Count);
+
+            // Act
+            dict.Remove("key2");
+
+            // Assert 2
+            Assert.Equal(2, storage.Count);
+            Assert.Equal("key", storage[0].Key);
+            Assert.Equal("value", storage[0].Value);
+            Assert.Equal("key3", storage[1].Key);
+            Assert.Equal("value3", storage[1].Value);
+
+            Assert.Throws<ArgumentOutOfRangeException>(() => storage[2]);
+        }
+
         private class RegularType
         {
             public bool IsAwesome { get; set; }