Skip to content
This repository was archived by the owner on Nov 20, 2018. It is now read-only.

Improve performance and reduce allocations in RouteValueDictionary. #1062

Merged
merged 1 commit into from
Nov 14, 2018
Merged

Improve performance and reduce allocations in RouteValueDictionary. #1062

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
147 changes: 145 additions & 2 deletions benchmarks/Microsoft.AspNetCore.Http.Performance/RouteValueDictionaryBenchmark.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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()
{
Expand All @@ -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;

Expand All @@ -68,7 +211,7 @@ public RouteValueDictionary ConditionalAdd_ContainsKeyAdd()

return dictionary;
}

[Benchmark]
public RouteValueDictionary ConditionalAdd_TryAdd()
{
Expand Down
87 changes: 69 additions & 18 deletions src/Microsoft.AspNetCore.Http.Abstractions/Routing/RouteValueDictionary.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ public RouteValueDictionary()
/// Only public instance non-index properties are considered.
/// </remarks>
public RouteValueDictionary(object values)
: this()
{
if (values is RouteValueDictionary dictionary)
{
Expand All @@ -109,20 +108,27 @@ public RouteValueDictionary(object values)
return;
}

var other = dictionary._arrayStorage;
var storage = new KeyValuePair<string, object>[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<string, object>[count];
Array.Copy(other, 0, storage, 0, count);
_arrayStorage = storage;
_count = count;
}
else
{
_arrayStorage = Array.Empty<KeyValuePair<string, object>>();
}

_arrayStorage = storage;
_count = dictionary._count;
return;
}

if (values is IEnumerable<KeyValuePair<string, object>> keyValueEnumerable)
{
_arrayStorage = Array.Empty<KeyValuePair<string, object>>();

foreach (var kvp in keyValueEnumerable)
{
Add(kvp.Key, kvp.Value);
Expand All @@ -133,6 +139,8 @@ public RouteValueDictionary(object values)

if (values is IEnumerable<KeyValuePair<string, string>> stringValueEnumerable)
{
_arrayStorage = Array.Empty<KeyValuePair<string, object>>();

foreach (var kvp in stringValueEnumerable)
{
Add(kvp.Key, kvp.Value);
Expand All @@ -146,7 +154,10 @@ public RouteValueDictionary(object values)
var storage = new PropertyStorage(values);
_propertyStorage = storage;
_count = storage.Properties.Length;
Copy link
Contributor

@poke poke Nov 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn’t this case leave the _arrayStorage uninitialized now? All other cases have an explicit initialization but this case does not.

Copy link
Contributor Author

@drieseng drieseng Nov 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does indeed leave array storage uninitialized, but that's not an issue as either:

  • property storage is initialized and used
    -or-
  • array storage is initialiazed and used

When initialized, we always prefer property storage over array storage. Array storage can remain uninitialized when property storage is initialized.

When property storage is initialized, but we want to manipulate the values, then the property storage is transformed to array storage. This is done using RouteValueDictionary.EnsureCapacitySlow(int capacity).

Copy link
Contributor

@poke poke Nov 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah cool, I missed that every use of the array storage would ensure the capacity before which would cause an initialization of the storage.

Since the checks always check whether the property storage is set (instead of checking when the array storage is not set), does it really make such a difference to not initialize the array storage to an empty array by default? All those _arrayStorage = Array.Empty… lines really seem verbose when they are always executed except in the one case where an explicit array is being set, or when the property storage is being set which does not need the array storage to be not set though.

Copy link
Contributor Author

@drieseng drieseng Nov 11, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just thought it would be clearer when either _propertyStorage or _arrayStorage would be initialized, and not both.

return;
}
else
{
_arrayStorage = Array.Empty<KeyValuePair<string, object>>();
}
}

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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);
}

/// <inheritdoc />
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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<KeyValuePair<string, object>>
{
private readonly RouteValueDictionary _dictionary;
Expand Down
Loading