From 742a9e3f3b23653374d64213015fd9d78d17f151 Mon Sep 17 00:00:00 2001
From: David Obando <daobando@microsoft.com>
Date: Mon, 2 May 2016 19:28:20 -0700
Subject: [PATCH] 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.
---
 .../ModelBinding/ModelStateEntry.cs           | 13 ++++++++++-
 .../DefaultCollectionValidationStrategy.cs    | 23 +++++++++++++++----
 ...plicitIndexCollectionValidationStrategy.cs |  2 +-
 3 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateEntry.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateEntry.cs
index 0b8bd32d4b..a4f6391bbc 100644
--- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateEntry.cs
+++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateEntry.cs
@@ -10,6 +10,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
     /// </summary>
     public abstract class ModelStateEntry
     {
+        private ModelErrorCollection _errors;
         /// <summary>
         /// Gets the raw value from the request associated with this entry.
         /// </summary>
@@ -23,7 +24,17 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// <summary>
         /// Gets the <see cref="ModelErrorCollection"/> for this entry.
         /// </summary>
-        public ModelErrorCollection Errors { get; } = new ModelErrorCollection();
+        public ModelErrorCollection Errors
+        {
+            get
+            {
+                if (_errors == null)
+                {
+                    _errors = new ModelErrorCollection();
+                }
+                return _errors;
+            }
+        }
 
         /// <summary>
         /// Gets or sets the <see cref="ModelValidationState"/> for this entry.
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultCollectionValidationStrategy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultCollectionValidationStrategy.cs
index fd9f02d49b..3823c8ac20 100644
--- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultCollectionValidationStrategy.cs
+++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultCollectionValidationStrategy.cs
@@ -1,11 +1,14 @@
 // 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.
 
+using System;
 using System.Collections;
+using System.Collections.Concurrent;
 using System.Collections.Generic;
 using System.Reflection;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Validation;
+using System.Linq.Expressions;
 
 namespace Microsoft.AspNetCore.Mvc.Internal
 {
@@ -44,7 +47,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal
         /// <summary>
         /// Gets an instance of <see cref="DefaultCollectionValidationStrategy"/>.
         /// </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()
         {
@@ -60,10 +64,21 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             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);
-            return (IEnumerator)getEnumeratorMethod.Invoke(null, new object[] { model });
+            Func<object, IEnumerator> getEnumerator = _genericGetEnumeratorCache.GetOrAdd(
+                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.
diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ExplicitIndexCollectionValidationStrategy.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ExplicitIndexCollectionValidationStrategy.cs
index 16f57ebc8f..f4f940f8ae 100644
--- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/ExplicitIndexCollectionValidationStrategy.cs
+++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/ExplicitIndexCollectionValidationStrategy.cs
@@ -56,7 +56,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             string key,
             object model)
         {
-            var enumerator = DefaultCollectionValidationStrategy.GetEnumeratorForElementType(metadata, model);
+            var enumerator = DefaultCollectionValidationStrategy.Instance.GetEnumeratorForElementType(metadata, model);
             return new Enumerator(metadata.ElementMetadata, key, ElementKeys, enumerator);
         }