Reduce allocations in MSD enumeration

This commit is contained in:
Ryan Nowak 2015-10-14 20:55:57 -07:00
parent 6985015e2d
commit 054b39013c
3 changed files with 198 additions and 64 deletions

View File

@ -4,9 +4,7 @@
using System; using System;
using System.Collections; using System.Collections;
using System.Collections.Generic; using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNet.Mvc.Abstractions; using Microsoft.AspNet.Mvc.Abstractions;
using Microsoft.Extensions.Internal;
namespace Microsoft.AspNet.Mvc.ModelBinding namespace Microsoft.AspNet.Mvc.ModelBinding
{ {
@ -22,7 +20,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// </summary> /// </summary>
public static readonly int DefaultMaxAllowedErrors = 200; public static readonly int DefaultMaxAllowedErrors = 200;
private readonly IDictionary<string, ModelState> _innerDictionary; private readonly Dictionary<string, ModelState> _innerDictionary;
private int _maxAllowedErrors; private int _maxAllowedErrors;
/// <summary> /// <summary>
@ -55,7 +53,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
throw new ArgumentNullException(nameof(dictionary)); throw new ArgumentNullException(nameof(dictionary));
} }
_innerDictionary = new CopyOnWriteDictionary<string, ModelState>( _innerDictionary = new Dictionary<string, ModelState>(
dictionary, dictionary,
StringComparer.OrdinalIgnoreCase); StringComparer.OrdinalIgnoreCase);
@ -124,7 +122,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// <inheritdoc /> /// <inheritdoc />
public bool IsReadOnly public bool IsReadOnly
{ {
get { return _innerDictionary.IsReadOnly; } get { return ((ICollection<KeyValuePair<string, ModelState>>)_innerDictionary).IsReadOnly; }
} }
/// <inheritdoc /> /// <inheritdoc />
@ -153,7 +151,11 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// <inheritdoc /> /// <inheritdoc />
public ModelValidationState ValidationState public ModelValidationState ValidationState
{ {
get { return GetValidity(_innerDictionary); } get
{
var entries = FindKeysWithPrefix(string.Empty);
return GetValidity(entries, defaultState: ModelValidationState.Valid);
}
} }
/// <inheritdoc /> /// <inheritdoc />
@ -344,12 +346,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
} }
var entries = FindKeysWithPrefix(key); var entries = FindKeysWithPrefix(key);
if (!entries.Any()) return GetValidity(entries, defaultState: ModelValidationState.Unvalidated);
{
return ModelValidationState.Unvalidated;
}
return GetValidity(entries);
} }
/// <summary> /// <summary>
@ -496,9 +493,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
{ {
// If key is null or empty, clear all entries in the dictionary // If key is null or empty, clear all entries in the dictionary
// else just clear the ones that have key as prefix // else just clear the ones that have key as prefix
var entries = (string.IsNullOrEmpty(key)) ? var entries = FindKeysWithPrefix(key ?? string.Empty);
_innerDictionary : FindKeysWithPrefix(key);
foreach (var entry in entries) foreach (var entry in entries)
{ {
entry.Value.Errors.Clear(); entry.Value.Errors.Clear();
@ -523,11 +518,16 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return modelState; return modelState;
} }
private static ModelValidationState GetValidity(IEnumerable<KeyValuePair<string, ModelState>> entries) private static ModelValidationState GetValidity(PrefixEnumerable entries, ModelValidationState defaultState)
{ {
var hasEntries = false;
var validationState = ModelValidationState.Valid; var validationState = ModelValidationState.Valid;
foreach (var entry in entries) foreach (var entry in entries)
{ {
hasEntries = true;
var entryState = entry.Value.ValidationState; var entryState = entry.Value.ValidationState;
if (entryState == ModelValidationState.Unvalidated) if (entryState == ModelValidationState.Unvalidated)
{ {
@ -539,7 +539,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
validationState = entryState; validationState = entryState;
} }
} }
return validationState;
return hasEntries ? validationState : defaultState;
} }
private void EnsureMaxErrorsReachedRecorded() private void EnsureMaxErrorsReachedRecorded()
@ -591,7 +592,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
/// <inheritdoc /> /// <inheritdoc />
public bool Contains(KeyValuePair<string, ModelState> item) public bool Contains(KeyValuePair<string, ModelState> item)
{ {
return _innerDictionary.Contains(item); return ((ICollection<KeyValuePair<string, ModelState>>)_innerDictionary).Contains(item);
} }
/// <inheritdoc /> /// <inheritdoc />
@ -613,13 +614,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
throw new ArgumentNullException(nameof(array)); throw new ArgumentNullException(nameof(array));
} }
_innerDictionary.CopyTo(array, arrayIndex); ((ICollection<KeyValuePair<string, ModelState>>)_innerDictionary).CopyTo(array, arrayIndex);
} }
/// <inheritdoc /> /// <inheritdoc />
public bool Remove(KeyValuePair<string, ModelState> item) public bool Remove(KeyValuePair<string, ModelState> item)
{ {
return _innerDictionary.Remove(item); return ((ICollection<KeyValuePair<string, ModelState>>)_innerDictionary).Remove(item);
} }
/// <inheritdoc /> /// <inheritdoc />
@ -656,70 +657,204 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
return GetEnumerator(); return GetEnumerator();
} }
public IEnumerable<KeyValuePair<string, ModelState>> FindKeysWithPrefix(string prefix) public static bool StartsWithPrefix(string prefix, string key)
{ {
if (prefix == null) if (prefix == null)
{ {
throw new ArgumentNullException(nameof(prefix)); throw new ArgumentNullException(nameof(prefix));
} }
ModelState exactMatchValue; if (key == null)
if (_innerDictionary.TryGetValue(prefix, out exactMatchValue))
{ {
yield return new KeyValuePair<string, ModelState>(prefix, exactMatchValue); throw new ArgumentNullException(nameof(key));
} }
foreach (var entry in _innerDictionary) if (StringComparer.OrdinalIgnoreCase.Equals(key, prefix))
{ {
var key = entry.Key; return true;
}
if (key.Length <= prefix.Length) if (key.Length <= prefix.Length)
{
return false;
}
if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
{
if (key.StartsWith("[", StringComparison.OrdinalIgnoreCase))
{ {
continue; var subKey = key.Substring(key.IndexOf('.') + 1);
}
if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase)) if (!subKey.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
{
if (key.StartsWith("[", StringComparison.OrdinalIgnoreCase))
{ {
var subKey = key.Substring(key.IndexOf('.') + 1); return false;
if (!subKey.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
{
continue;
}
if (string.Equals(prefix, subKey, StringComparison.Ordinal))
{
yield return entry;
continue;
}
key = subKey;
} }
else
if (string.Equals(prefix, subKey, StringComparison.OrdinalIgnoreCase))
{ {
continue; return true;
} }
}
// Everything is prefixed by the empty string key = subKey;
if (prefix.Length == 0)
{
yield return entry;
} }
else else
{ {
var charAfterPrefix = key[prefix.Length]; return false;
switch (charAfterPrefix) }
}
// Everything is prefixed by the empty string
if (prefix.Length == 0)
{
return true;
}
else
{
var charAfterPrefix = key[prefix.Length];
switch (charAfterPrefix)
{
case '[':
case '.':
return true;
}
}
return false;
}
public PrefixEnumerable FindKeysWithPrefix(string prefix)
{
if (prefix == null)
{
throw new ArgumentNullException(nameof(prefix));
}
return new PrefixEnumerable(this, prefix);
}
public struct PrefixEnumerable : IEnumerable<KeyValuePair<string, ModelState>>
{
private readonly ModelStateDictionary _dictionary;
private readonly string _prefix;
public PrefixEnumerable(ModelStateDictionary dictionary, string prefix)
{
if (dictionary == null)
{
throw new ArgumentNullException(nameof(dictionary));
}
if (prefix == null)
{
throw new ArgumentNullException(nameof(prefix));
}
_dictionary = dictionary;
_prefix = prefix;
}
public PrefixEnumerator GetEnumerator()
{
return _dictionary == null ? new PrefixEnumerator() : new PrefixEnumerator(_dictionary, _prefix);
}
IEnumerator<KeyValuePair<string, ModelState>> IEnumerable<KeyValuePair<string, ModelState>>.GetEnumerator()
{
return GetEnumerator();
}
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}
public struct PrefixEnumerator : IEnumerator<KeyValuePair<string, ModelState>>
{
private readonly ModelStateDictionary _dictionary;
private readonly string _prefix;
private bool _exactMatchUsed;
private Dictionary<string, ModelState>.Enumerator _enumerator;
public PrefixEnumerator(ModelStateDictionary dictionary, string prefix)
{
if (dictionary == null)
{
throw new ArgumentNullException(nameof(dictionary));
}
if (prefix == null)
{
throw new ArgumentNullException(nameof(prefix));
}
_dictionary = dictionary;
_prefix = prefix;
_exactMatchUsed = false;
_enumerator = default(Dictionary<string, ModelState>.Enumerator);
Current = default(KeyValuePair<string, ModelState>);
}
public KeyValuePair<string, ModelState> Current { get; private set; }
object IEnumerator.Current
{
get
{
return Current;
}
}
public void Dispose()
{
}
public bool MoveNext()
{
if (_dictionary == null)
{
return false;
}
// ModelStateDictionary has a behavior where the first 'match' returned from iterating
// prefixes is the exact match for the prefix (if present). Only after looking for an
// exact match do we fall back to iteration to find 'starts-with' matches.
if (!_exactMatchUsed)
{
_exactMatchUsed = true;
_enumerator = _dictionary._innerDictionary.GetEnumerator();
ModelState entry;
if (_dictionary.TryGetValue(_prefix, out entry))
{ {
case '[': Current = new KeyValuePair<string, ModelState>(_prefix, entry);
case '.': return true;
yield return entry;
break;
} }
} }
while (_enumerator.MoveNext())
{
if (string.Equals(_prefix, _enumerator.Current.Key, StringComparison.OrdinalIgnoreCase))
{
// Skip this one. We've already handle the 'exact match' case.
}
else if (StartsWithPrefix(_prefix, _enumerator.Current.Key))
{
Current = _enumerator.Current;
return true;
}
}
return false;
}
public void Reset()
{
_exactMatchUsed = false;
_enumerator = default(Dictionary<string, ModelState>.Enumerator);
Current = default(KeyValuePair<string, ModelState>);
} }
} }
} }

View File

@ -227,7 +227,7 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
/// </remarks> /// </remarks>
protected ViewDataDictionary(ViewDataDictionary source, object model, Type declaredModelType) protected ViewDataDictionary(ViewDataDictionary source, object model, Type declaredModelType)
: this(source._metadataProvider, : this(source._metadataProvider,
new ModelStateDictionary(source.ModelState), source.ModelState,
declaredModelType, declaredModelType,
data: new CopyOnWriteDictionary<string, object>(source, StringComparer.OrdinalIgnoreCase), data: new CopyOnWriteDictionary<string, object>(source, StringComparer.OrdinalIgnoreCase),
templateInfo: new TemplateInfo(source.TemplateInfo)) templateInfo: new TemplateInfo(source.TemplateInfo))

View File

@ -2,8 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System; using System;
using System.Globalization; using System.Collections.Generic;
using Microsoft.Extensions.Internal;
using Xunit; using Xunit;
namespace Microsoft.AspNet.Mvc.ModelBinding namespace Microsoft.AspNet.Mvc.ModelBinding
@ -156,7 +155,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
Assert.Equal(0, target.ErrorCount); Assert.Equal(0, target.ErrorCount);
Assert.Equal(1, target.Count); Assert.Equal(1, target.Count);
Assert.Same(modelState, target["key"]); Assert.Same(modelState, target["key"]);
Assert.IsType<CopyOnWriteDictionary<string, ModelState>>(target.InnerDictionary); Assert.IsType<Dictionary<string, ModelState>>(target.InnerDictionary);
} }
[Fact] [Fact]