Minor improvments to RVD perf

This commit is contained in:
Ryan Nowak 2018-09-13 14:33:06 -07:00
parent cee960f3c5
commit 9a68f48a5c
3 changed files with 391 additions and 59 deletions

View File

@ -8,11 +8,11 @@ namespace Microsoft.AspNetCore.Routing
{
public class RouteValueDictionaryBenchmark
{
// These dictionaries are used by a few tests over and over, so don't modify them destructively.
private RouteValueDictionary _arrayValues;
private RouteValueDictionary _propertyValues;
[GlobalSetup]
// We modify the route value dictionaries in many of these benchmarks.
[IterationSetup]
public void Setup()
{
_arrayValues = new RouteValueDictionary()
@ -27,18 +27,57 @@ namespace Microsoft.AspNetCore.Routing
[Benchmark]
public RouteValueDictionary AddSingleItem()
{
var dictionary = new RouteValueDictionary();
dictionary.Add("action", "Index");
var dictionary = new RouteValueDictionary
{
{ "action", "Index" }
};
return dictionary;
}
[Benchmark]
public RouteValueDictionary AddThreeItems()
{
var dictionary = new RouteValueDictionary();
dictionary.Add("action", "Index");
dictionary.Add("controller", "Home");
dictionary.Add("id", "15");
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;
}
@ -56,7 +95,7 @@ namespace Microsoft.AspNetCore.Routing
[Benchmark]
public RouteValueDictionary ForEachThreeItems_Properties()
{
var dictionary = _arrayValues;
var dictionary = _propertyValues;
foreach (var kvp in dictionary)
{
GC.KeepAlive(kvp.Value);
@ -87,8 +126,10 @@ namespace Microsoft.AspNetCore.Routing
[Benchmark]
public RouteValueDictionary SetSingleItem()
{
var dictionary = new RouteValueDictionary();
dictionary["action"] = "Index";
var dictionary = new RouteValueDictionary
{
["action"] = "Index"
};
return dictionary;
}
@ -103,10 +144,12 @@ namespace Microsoft.AspNetCore.Routing
[Benchmark]
public RouteValueDictionary SetThreeItems()
{
var dictionary = new RouteValueDictionary();
dictionary["action"] = "Index";
dictionary["controller"] = "Home";
dictionary["id"] = "15";
var dictionary = new RouteValueDictionary
{
["action"] = "Index",
["controller"] = "Home",
["id"] = "15"
};
return dictionary;
}
@ -136,4 +179,4 @@ namespace Microsoft.AspNetCore.Routing
return dictionary;
}
}
}
}

View File

@ -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<object>.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;
}
/// <summary>
/// Attempts to the add the provided <paramref name="key"/> and <paramref name="value"/> to the dictionary.
/// </summary>
/// <param name="key">The key.</param>
/// <param name="value">The value.</param>
/// <returns>Returns <c>true</c> if the value was added. Returns <c>false</c> if the key was already present.</returns>
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<string, object>(key, value);
_count++;
return true;
}
/// <inheritdoc />
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<string, object>[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<KeyValuePair<string, object>>
{
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<string, object> Current { get; private set; }
@ -524,22 +586,36 @@ namespace Microsoft.AspNetCore.Routing
{
}
// Similar to the design of List<T>.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<string, object>(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<string, object>(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
}
}
}
}
}

View File

@ -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<KeyValuePair<string, object>[]>(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<KeyValuePair<string, object>[]>(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<KeyValuePair<string, object>[]>(dict._arrayStorage);
// The upgrade from property -> array should make space for at least 4 entries
Assert.Collection(
dict._arrayStorage,
kvp => Assert.Equal(new KeyValuePair<string, object>("age", 30), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("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<KeyValuePair<string, object>[]>(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<string, object>("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<string, object>("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<string, object>("key0", "value0"), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("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<string, object>("key0", "value0"), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("key1", "value1"), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("key2", "value2"), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("key3", "value3"), kvp),
kvp => Assert.Equal(new KeyValuePair<string, object>("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<string, object>("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; }
}
}
}
}