Fix #3217 - Optimize IList.GetEnumerator allocations
This change fixes call sites on the main request path for a simple site (model binding, validation, views) that allocate boxed list enumerators. Some cases aren't addressed by this change because the fix is too invasive or requires changing an important contract to take IList instead of IEnumerable. Will follow up on those case by case in order of importance.
This commit is contained in:
parent
28aec3f5cc
commit
d37f5aeb31
|
|
@ -162,10 +162,12 @@ namespace Microsoft.AspNet.Mvc.Controllers
|
|||
OperationBindingContext operationContext,
|
||||
ModelStateDictionary modelState,
|
||||
IDictionary<string, object> arguments,
|
||||
IEnumerable<ParameterDescriptor> parameterMetadata)
|
||||
IList<ParameterDescriptor> parameterMetadata)
|
||||
{
|
||||
foreach (var parameter in parameterMetadata)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < parameterMetadata.Count; i++)
|
||||
{
|
||||
var parameter = parameterMetadata[i];
|
||||
var modelBindingResult = await BindModelAsync(parameter, modelState, operationContext);
|
||||
if (modelBindingResult.IsModelSet)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -20,9 +20,10 @@ namespace Microsoft.AspNet.Mvc.Filters
|
|||
{
|
||||
if (context.ActionContext.ActionDescriptor.FilterDescriptors != null)
|
||||
{
|
||||
foreach (var item in context.Results)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < context.Results.Count; i++)
|
||||
{
|
||||
ProvideFilter(context, item);
|
||||
ProvideFilter(context, context.Results[i]);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -41,8 +41,11 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
|
|||
var matchingRouteConstraints = tree.Select(context.RouteData.Values);
|
||||
|
||||
var candidates = new List<ActionSelectorCandidate>();
|
||||
foreach (var action in matchingRouteConstraints)
|
||||
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < matchingRouteConstraints.Count; i++)
|
||||
{
|
||||
var action = matchingRouteConstraints[i];
|
||||
var constraints = GetConstraints(context.HttpContext, action);
|
||||
candidates.Add(new ActionSelectorCandidate(action, constraints));
|
||||
}
|
||||
|
|
@ -54,8 +57,10 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
|
|||
if (matchingActionConstraints != null)
|
||||
{
|
||||
matchingActions = new List<ActionDescriptor>(matchingActionConstraints.Count);
|
||||
foreach (var candidate in matchingActionConstraints)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < matchingActionConstraints.Count; i++)
|
||||
{
|
||||
var candidate = matchingActionConstraints[i];
|
||||
matchingActions.Add(candidate.Action);
|
||||
}
|
||||
}
|
||||
|
|
@ -107,12 +112,16 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
|
|||
// Find the next group of constraints to process. This will be the lowest value of
|
||||
// order that is higher than startingOrder.
|
||||
int? order = null;
|
||||
foreach (var candidate in candidates)
|
||||
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < candidates.Count; i++)
|
||||
{
|
||||
var candidate = candidates[i];
|
||||
if (candidate.Constraints != null)
|
||||
{
|
||||
foreach (var constraint in candidate.Constraints)
|
||||
for (var j = 0; j < candidate.Constraints.Count; j++)
|
||||
{
|
||||
var constraint = candidate.Constraints[j];
|
||||
if ((startingOrder == null || constraint.Order > startingOrder) &&
|
||||
(order == null || constraint.Order < order))
|
||||
{
|
||||
|
|
@ -137,16 +146,19 @@ namespace Microsoft.AspNet.Mvc.Infrastructure
|
|||
constraintContext.Candidates = candidates;
|
||||
constraintContext.RouteContext = context;
|
||||
|
||||
foreach (var candidate in candidates)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < candidates.Count; i++)
|
||||
{
|
||||
var candidate = candidates[i];
|
||||
var isMatch = true;
|
||||
var foundMatchingConstraint = false;
|
||||
|
||||
if (candidate.Constraints != null)
|
||||
{
|
||||
constraintContext.CurrentCandidate = candidate;
|
||||
foreach (var constraint in candidate.Constraints)
|
||||
for (var j = 0; j < candidate.Constraints.Count; j++)
|
||||
{
|
||||
var constraint = candidate.Constraints[j];
|
||||
if (constraint.Order == order)
|
||||
{
|
||||
foundMatchingConstraint = true;
|
||||
|
|
|
|||
|
|
@ -49,8 +49,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
|
|||
{
|
||||
RuntimeHelpers.EnsureSufficientExecutionStack();
|
||||
|
||||
foreach (var binder in ModelBinders)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < ModelBinders.Count; i++)
|
||||
{
|
||||
var binder = ModelBinders[i];
|
||||
var result = await binder.BindModelAsync(bindingContext);
|
||||
if (result != ModelBindingResult.NoResult)
|
||||
{
|
||||
|
|
|
|||
|
|
@ -30,9 +30,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
|
|||
/// <inheritdoc />
|
||||
public void GetValidators(ClientValidatorProviderContext context)
|
||||
{
|
||||
foreach (var validatorProvider in ValidatorProviders)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < ValidatorProviders.Count; i++)
|
||||
{
|
||||
validatorProvider.GetValidators(context);
|
||||
ValidatorProviders[i].GetValidators(context);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -27,11 +27,13 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
|
|||
/// </summary>
|
||||
public IReadOnlyList<IModelValidatorProvider> ValidatorProviders { get; }
|
||||
|
||||
/// <inheritdoc />
|
||||
public void GetValidators(ModelValidatorProviderContext context)
|
||||
{
|
||||
foreach (var validatorProvider in ValidatorProviders)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < ValidatorProviders.Count; i++)
|
||||
{
|
||||
validatorProvider.GetValidators(context);
|
||||
ValidatorProviders[i].GetValidators(context);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -15,9 +15,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
|
|||
/// <inheritdoc />
|
||||
public void GetValidators(ModelValidatorProviderContext context)
|
||||
{
|
||||
foreach (var metadata in context.ValidatorMetadata)
|
||||
//Perf: Avoid allocations here
|
||||
for (var i = 0; i < context.ValidatorMetadata.Count; i++)
|
||||
{
|
||||
var validator = metadata as IModelValidator;
|
||||
var validator = context.ValidatorMetadata[i] as IModelValidator;
|
||||
if (validator != null)
|
||||
{
|
||||
context.Validators.Add(validator);
|
||||
|
|
|
|||
|
|
@ -47,13 +47,17 @@ namespace Microsoft.AspNet.Mvc.Routing
|
|||
// The common case for MVC has no catch-alls, so avoid allocating.
|
||||
List<ActionDescriptor> filtered = null;
|
||||
|
||||
foreach (var action in results)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < results.Count; i++)
|
||||
{
|
||||
var action = results[i];
|
||||
|
||||
var actionHasCatchAll = false;
|
||||
if (action.RouteConstraints != null)
|
||||
{
|
||||
foreach (var constraint in action.RouteConstraints)
|
||||
for (var j = 0; j < action.RouteConstraints.Count; j++)
|
||||
{
|
||||
var constraint = action.RouteConstraints[j];
|
||||
if (constraint.KeyHandling == RouteKeyHandling.CatchAll)
|
||||
{
|
||||
actionHasCatchAll = true;
|
||||
|
|
|
|||
|
|
@ -22,9 +22,10 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Validation
|
|||
throw new ArgumentNullException(nameof(context));
|
||||
}
|
||||
|
||||
foreach (var metadata in context.ValidatorMetadata)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < context.ValidatorMetadata.Count; i++)
|
||||
{
|
||||
var validator = metadata as IClientModelValidator;
|
||||
var validator = context.ValidatorMetadata[i] as IClientModelValidator;
|
||||
if (validator != null)
|
||||
{
|
||||
context.Validators.Add(validator);
|
||||
|
|
|
|||
|
|
@ -198,17 +198,24 @@ namespace Microsoft.AspNet.Mvc.Razor
|
|||
}
|
||||
else
|
||||
{
|
||||
// For traditional routes, lookup the key in RouteConstraints if the key is RequireKey.
|
||||
var match = actionDescriptor.RouteConstraints.FirstOrDefault(
|
||||
constraint => string.Equals(constraint.RouteKey, key, StringComparison.OrdinalIgnoreCase));
|
||||
if (match != null && match.KeyHandling != RouteKeyHandling.CatchAll)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < actionDescriptor.RouteConstraints.Count; i++)
|
||||
{
|
||||
if (match.KeyHandling == RouteKeyHandling.DenyKey)
|
||||
var constraint = actionDescriptor.RouteConstraints[i];
|
||||
if (string.Equals(constraint.RouteKey, key, StringComparison.Ordinal))
|
||||
{
|
||||
return null;
|
||||
}
|
||||
if (constraint.KeyHandling == RouteKeyHandling.DenyKey)
|
||||
{
|
||||
return null;
|
||||
}
|
||||
else if (constraint.KeyHandling == RouteKeyHandling.RequireKey)
|
||||
{
|
||||
normalizedValue = constraint.RouteValue;
|
||||
}
|
||||
|
||||
normalizedValue = match.RouteValue;
|
||||
// Duplicate keys in RouteConstraints are not allowed.
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -266,9 +273,10 @@ namespace Microsoft.AspNet.Mvc.Razor
|
|||
expanderContext.Values = new Dictionary<string, string>(StringComparer.Ordinal);
|
||||
|
||||
// 1. Populate values from viewLocationExpanders.
|
||||
foreach (var expander in _viewLocationExpanders)
|
||||
// Perf: Avoid allocations
|
||||
for( var i = 0; i < _viewLocationExpanders.Count; i++)
|
||||
{
|
||||
expander.PopulateValues(expanderContext);
|
||||
_viewLocationExpanders[i].PopulateValues(expanderContext);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -800,8 +800,10 @@ namespace Microsoft.AspNet.Mvc.ViewFeatures
|
|||
var htmlSummary = new TagBuilder("ul");
|
||||
foreach (var modelState in modelStates)
|
||||
{
|
||||
foreach (var modelError in modelState.Errors)
|
||||
// Perf: Avoid allocations
|
||||
for (var i = 0; i < modelState.Errors.Count; i++)
|
||||
{
|
||||
var modelError = modelState.Errors[i];
|
||||
var errorText = ValidationHelpers.GetUserErrorMessageOrDefault(modelError, modelState: null);
|
||||
|
||||
if (!string.IsNullOrEmpty(errorText))
|
||||
|
|
|
|||
Loading…
Reference in New Issue