Make SetParameterProperties free of allocations

Author: SteveSandersonMS

src/Components/src/Microsoft.AspNetCore.Components/Microsoft.AspNetCore.Components.csproj

@@ -1,8 +1,9 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
     <TargetFramework>netstandard2.0</TargetFramework>
     <Description>Components feature for ASP.NET Core.</Description>
+    <AllowUnsafeBlocks>true</AllowUnsafeBlocks>
   </PropertyGroup>
 
   <ItemGroup>

src/Components/src/Microsoft.AspNetCore.Components/ParameterCollectionExtensions.cs

@@ -5,7 +5,6 @@
 using System;
 using System.Collections.Concurrent;
 using System.Collections.Generic;
-using System.Linq;
 using System.Reflection;
 
 namespace Microsoft.AspNetCore.Components
@@ -17,15 +16,16 @@ public static class ParameterCollectionExtensions
     {
         private const BindingFlags _bindablePropertyFlags = BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance | BindingFlags.IgnoreCase;
 
-        private readonly static IDictionary<Type, IDictionary<string, IPropertySetter>> _cachedParameterWriters = new ConcurrentDictionary<Type, IDictionary<string, IPropertySetter>>();
+        private readonly static ConcurrentDictionary<Type, WritersForType> _cachedWritersByType
+            = new ConcurrentDictionary<Type, WritersForType>();
 
         /// <summary>
-        /// Iterates through the <see cref="ParameterCollection"/>, assigning each parameter
-        /// to a property of the same name on <paramref name="target"/>.
+        /// For each parameter property on <paramref name="target"/>, updates its value to
+        /// match the corresponding entry in the <see cref="ParameterCollection"/>.
         /// </summary>
         /// <param name="parameterCollection">The <see cref="ParameterCollection"/>.</param>
         /// <param name="target">An object that has a public writable property matching each parameter's name and type.</param>
-        public static void SetParameterProperties(
+        public unsafe static void SetParameterProperties(
             in this ParameterCollection parameterCollection,
             object target)
         {
@@ -35,26 +35,36 @@ public static void SetParameterProperties(
             }
 
             var targetType = target.GetType();
-            if (!_cachedParameterWriters.TryGetValue(targetType, out var parameterWriters))
+            if (!_cachedWritersByType.TryGetValue(targetType, out var writers))
             {
-                parameterWriters = CreateParameterWriters(targetType);
-                _cachedParameterWriters[targetType] = parameterWriters;
+                writers = new WritersForType(targetType);
+                _cachedWritersByType[targetType] = writers;
             }
 
-            var localParameterWriter = parameterWriters.Values.ToList();
+            // We only want to iterate through the parameterCollection once, and by the end of it,
+            // need to have tracked which of the parameter properties haven't yet been written.
+            // To avoid allocating any list/dictionary to track that, here we stackalloc an array
+            // of flags and set them based on the indices of the writers we use.
+            var numWriters = writers.WritersByIndex.Count;
+            var numUsedWriters = 0;
+
+            // TODO: Once we're able to move to netstandard2.1, this can be changed to be
+            // a Span<bool> and then the enclosing method no longer needs to be 'unsafe'
+            bool* usageFlags = stackalloc bool[numWriters];
 
             foreach (var parameter in parameterCollection)
             {
                 var parameterName = parameter.Name;
-                if (!parameterWriters.TryGetValue(parameterName, out var parameterWriter))
+                if (!writers.WritersByName.TryGetValue(parameterName, out var writerWithIndex))
                 {
                     ThrowForUnknownIncomingParameterName(targetType, parameterName);
                 }
 
                 try
                 {
-                    parameterWriter.SetValue(target, parameter.Value);
-                    localParameterWriter.Remove(parameterWriter);
+                    writerWithIndex.Writer.SetValue(target, parameter.Value);
+                    usageFlags[writerWithIndex.Index] = true;
+                    numUsedWriters++;
                 }
                 catch (Exception ex)
                 {
@@ -64,44 +74,27 @@ public static void SetParameterProperties(
                 }
             }
 
-            foreach (var nonUsedParameter in localParameterWriter)
-            {
-                nonUsedParameter.SetDefaultValue(target);
-            }
-        }
-
-        internal static IEnumerable<PropertyInfo> GetCandidateBindableProperties(Type targetType)
-            => MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags);
-
-        private static IDictionary<string, IPropertySetter> CreateParameterWriters(Type targetType)
-        {
-            var result = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
-
-            foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
+            // Now we can determine whether any writers have not been used, and if there are
+            // some unused ones, find them.
+            for (var index = 0; numUsedWriters < numWriters; index++)
             {
-                var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute))
-                    || propertyInfo.IsDefined(typeof(CascadingParameterAttribute));
-                if (!shouldCreateWriter)
+                if (index >= numWriters)
                 {
-                    continue;
+                    // This should not be possible
+                    throw new InvalidOperationException("Ran out of writers before marking them all as used.");
                 }
 
-                var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
-
-                var propertyName = propertyInfo.Name;
-                if (result.ContainsKey(propertyName))
+                if (!usageFlags[index])
                 {
-                    throw new InvalidOperationException(
-                        $"The type '{targetType.FullName}' declares more than one parameter matching the " +
-                        $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique.");
+                    writers.WritersByIndex[index].SetDefaultValue(target);
+                    numUsedWriters++;
                 }
-
-                result.Add(propertyName, propertySetter);
             }
-
-            return result;
         }
 
+        internal static IEnumerable<PropertyInfo> GetCandidateBindableProperties(Type targetType)
+            => MemberAssignment.GetPropertiesIncludingInherited(targetType, _bindablePropertyFlags);
+
         private static void ThrowForUnknownIncomingParameterName(Type targetType, string parameterName)
         {
             // We know we're going to throw by this stage, so it doesn't matter that the following
@@ -129,5 +122,47 @@ private static void ThrowForUnknownIncomingParameterName(Type targetType, string
                     $"matching the name '{parameterName}'.");
             }
         }
+
+        class WritersForType
+        {
+            public Dictionary<string, (int Index, IPropertySetter Writer)> WritersByName { get; }
+            public List<IPropertySetter> WritersByIndex { get; }
+
+            public WritersForType(Type targetType)
+            {
+                var propertySettersByName = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
+                foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
+                {
+                    var shouldCreateWriter = propertyInfo.IsDefined(typeof(ParameterAttribute))
+                        || propertyInfo.IsDefined(typeof(CascadingParameterAttribute));
+                    if (!shouldCreateWriter)
+                    {
+                        continue;
+                    }
+
+                    var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo);
+
+                    var propertyName = propertyInfo.Name;
+                    if (propertySettersByName.ContainsKey(propertyName))
+                    {
+                        throw new InvalidOperationException(
+                            $"The type '{targetType.FullName}' declares more than one parameter matching the " +
+                            $"name '{propertyName.ToLowerInvariant()}'. Parameter names are case-insensitive and must be unique.");
+                    }
+
+                    propertySettersByName.Add(propertyName, propertySetter);
+                }
+
+                // Now we know all the entries, construct the resulting list/dictionary
+                // with well-defined indices
+                WritersByIndex = new List<IPropertySetter>();
+                WritersByName = new Dictionary<string, (int, IPropertySetter)>(StringComparer.OrdinalIgnoreCase);
+                foreach (var pair in propertySettersByName)
+                {
+                    WritersByName.Add(pair.Key, (WritersByIndex.Count, pair.Value));
+                    WritersByIndex.Add(pair.Value);
+                }
+            }
+        }
     }
 }