Reduce the number of allocations during model validation
When the service receives a model (say, via a POST message) MVC validates it to ensure the model is in a correct state. Validation currently incurs in many allocations that can be avoided. This tackles two of them: 1. We're now caching the generic `GetEnumerator<T>` method infos generated on the fly during collection validation, and 2. We're now only initializing `ModelErrorCollection` on demand. The first one incurs in the additional allocation of 1 long-lived dictionary object, which will grow only to the amount of `Collection<T>` types used by the model being validated. This is expected to be a small to medium number. The second change assumes that class `ModelStateEntry` isn't thread safe, as model validation isn't multithreaded. This resolves #4434 and #4435.
This commit is contained in:
parent
c6a6aae15f
commit
742a9e3f3b
|
|
@ -10,6 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public abstract class ModelStateEntry
|
public abstract class ModelStateEntry
|
||||||
{
|
{
|
||||||
|
private ModelErrorCollection _errors;
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets the raw value from the request associated with this entry.
|
/// Gets the raw value from the request associated with this entry.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
|
|
@ -23,7 +24,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets the <see cref="ModelErrorCollection"/> for this entry.
|
/// Gets the <see cref="ModelErrorCollection"/> for this entry.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public ModelErrorCollection Errors { get; } = new ModelErrorCollection();
|
public ModelErrorCollection Errors
|
||||||
|
{
|
||||||
|
get
|
||||||
|
{
|
||||||
|
if (_errors == null)
|
||||||
|
{
|
||||||
|
_errors = new ModelErrorCollection();
|
||||||
|
}
|
||||||
|
return _errors;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets or sets the <see cref="ModelValidationState"/> for this entry.
|
/// Gets or sets the <see cref="ModelValidationState"/> for this entry.
|
||||||
|
|
|
||||||
|
|
@ -1,11 +1,14 @@
|
||||||
// Copyright (c) .NET Foundation. All rights reserved.
|
// 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.
|
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
|
||||||
|
|
||||||
|
using System;
|
||||||
using System.Collections;
|
using System.Collections;
|
||||||
|
using System.Collections.Concurrent;
|
||||||
using System.Collections.Generic;
|
using System.Collections.Generic;
|
||||||
using System.Reflection;
|
using System.Reflection;
|
||||||
using Microsoft.AspNetCore.Mvc.ModelBinding;
|
using Microsoft.AspNetCore.Mvc.ModelBinding;
|
||||||
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
|
using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
|
||||||
|
using System.Linq.Expressions;
|
||||||
|
|
||||||
namespace Microsoft.AspNetCore.Mvc.Internal
|
namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
{
|
{
|
||||||
|
|
@ -44,7 +47,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets an instance of <see cref="DefaultCollectionValidationStrategy"/>.
|
/// Gets an instance of <see cref="DefaultCollectionValidationStrategy"/>.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public static readonly IValidationStrategy Instance = new DefaultCollectionValidationStrategy();
|
public static readonly DefaultCollectionValidationStrategy Instance = new DefaultCollectionValidationStrategy();
|
||||||
|
private readonly ConcurrentDictionary<Type, Func<object, IEnumerator>> _genericGetEnumeratorCache = new ConcurrentDictionary<Type, Func<object, IEnumerator>>();
|
||||||
|
|
||||||
private DefaultCollectionValidationStrategy()
|
private DefaultCollectionValidationStrategy()
|
||||||
{
|
{
|
||||||
|
|
@ -60,10 +64,21 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
return new Enumerator(metadata.ElementMetadata, key, enumerator);
|
return new Enumerator(metadata.ElementMetadata, key, enumerator);
|
||||||
}
|
}
|
||||||
|
|
||||||
public static IEnumerator GetEnumeratorForElementType(ModelMetadata metadata, object model)
|
public IEnumerator GetEnumeratorForElementType(ModelMetadata metadata, object model)
|
||||||
{
|
{
|
||||||
var getEnumeratorMethod = _getEnumerator.MakeGenericMethod(metadata.ElementType);
|
Func<object, IEnumerator> getEnumerator = _genericGetEnumeratorCache.GetOrAdd(
|
||||||
return (IEnumerator)getEnumeratorMethod.Invoke(null, new object[] { model });
|
key: metadata.ElementType,
|
||||||
|
valueFactory: (type) => {
|
||||||
|
var getEnumeratorMethod = _getEnumerator.MakeGenericMethod(type);
|
||||||
|
var parameter = Expression.Parameter(typeof(object), "model");
|
||||||
|
var expression =
|
||||||
|
Expression.Lambda<Func<object, IEnumerator>>(
|
||||||
|
Expression.Call(null, getEnumeratorMethod, parameter),
|
||||||
|
parameter);
|
||||||
|
return expression.Compile();
|
||||||
|
});
|
||||||
|
|
||||||
|
return getEnumerator(model);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Called via reflection.
|
// Called via reflection.
|
||||||
|
|
|
||||||
|
|
@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
|
||||||
string key,
|
string key,
|
||||||
object model)
|
object model)
|
||||||
{
|
{
|
||||||
var enumerator = DefaultCollectionValidationStrategy.GetEnumeratorForElementType(metadata, model);
|
var enumerator = DefaultCollectionValidationStrategy.Instance.GetEnumeratorForElementType(metadata, model);
|
||||||
return new Enumerator(metadata.ElementMetadata, key, ElementKeys, enumerator);
|
return new Enumerator(metadata.ElementMetadata, key, ElementKeys, enumerator);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue