Make PropertySetter a concrete type (#25054)

* Make PropertySetter a concrete type

* Use the pattern from PropertyHelpers to set property values
* Tweaks to Blazor WebAssembly tests to allow running tests locally

* Update src/Components/Components/src/Reflection/IPropertySetter.cs

src/Components/Components/src/ComponentFactory.cs

@@ -63,7 +63,7 @@ namespace Microsoft.AspNetCore.Components
             (
                 propertyName: property.Name,
                 propertyType: property.PropertyType,
-                setter: MemberAssignment.CreatePropertySetter(type, property, cascading: false)
+                setter: new PropertySetter(type, property)
             )).ToArray();
 
             return Initialize;

src/Components/Components/src/Reflection/ComponentProperties.cs

@@ -144,7 +144,7 @@ namespace Microsoft.AspNetCore.Components.Reflection
                 }
             }
 
-            static void SetProperty(object target, IPropertySetter writer, string parameterName, object value)
+            static void SetProperty(object target, PropertySetter writer, string parameterName, object value)
             {
                 try
                 {
@@ -246,13 +246,13 @@ namespace Microsoft.AspNetCore.Components.Reflection
         private class WritersForType
         {
             private const int MaxCachedWriterLookups = 100;
-            private readonly Dictionary<string, IPropertySetter> _underlyingWriters;
-            private readonly ConcurrentDictionary<string, IPropertySetter?> _referenceEqualityWritersCache;
+            private readonly Dictionary<string, PropertySetter> _underlyingWriters;
+            private readonly ConcurrentDictionary<string, PropertySetter?> _referenceEqualityWritersCache;
 
             public WritersForType(Type targetType)
             {
-                _underlyingWriters = new Dictionary<string, IPropertySetter>(StringComparer.OrdinalIgnoreCase);
-                _referenceEqualityWritersCache = new ConcurrentDictionary<string, IPropertySetter?>(ReferenceEqualityComparer.Instance);
+                _underlyingWriters = new Dictionary<string, PropertySetter>(StringComparer.OrdinalIgnoreCase);
+                _referenceEqualityWritersCache = new ConcurrentDictionary<string, PropertySetter?>(ReferenceEqualityComparer.Instance);
 
                 foreach (var propertyInfo in GetCandidateBindableProperties(targetType))
                 {
@@ -271,7 +271,10 @@ namespace Microsoft.AspNetCore.Components.Reflection
                             $"The type '{targetType.FullName}' declares a parameter matching the name '{propertyName}' that is not public. Parameters must be public.");
                     }
 
-                    var propertySetter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo, cascading: cascadingParameterAttribute != null);
+                    var propertySetter = new PropertySetter(targetType, propertyInfo)
+                    {
+                        Cascading = cascadingParameterAttribute != null,
+                    };
 
                     if (_underlyingWriters.ContainsKey(propertyName))
                     {
@@ -298,17 +301,17 @@ namespace Microsoft.AspNetCore.Components.Reflection
                             ThrowForInvalidCaptureUnmatchedValuesParameterType(targetType, propertyInfo);
                         }
 
-                        CaptureUnmatchedValuesWriter = MemberAssignment.CreatePropertySetter(targetType, propertyInfo, cascading: false);
+                        CaptureUnmatchedValuesWriter = new PropertySetter(targetType, propertyInfo);
                         CaptureUnmatchedValuesPropertyName = propertyInfo.Name;
                     }
                 }
             }
 
-            public IPropertySetter? CaptureUnmatchedValuesWriter { get; }
+            public PropertySetter? CaptureUnmatchedValuesWriter { get; }
 
             public string? CaptureUnmatchedValuesPropertyName { get; }
 
-            public bool TryGetValue(string parameterName, [MaybeNullWhen(false)] out IPropertySetter writer)
+            public bool TryGetValue(string parameterName, [MaybeNullWhen(false)] out PropertySetter writer)
             {
                 // In intensive parameter-passing scenarios, one of the most expensive things we do is the
                 // lookup from parameterName to writer. Pre-5.0 that was because of the string hashing.

src/Components/Components/src/Reflection/IPropertySetter.cs

@@ -1,12 +1,55 @@
 // 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.Reflection;
+
 namespace Microsoft.AspNetCore.Components.Reflection
 {
-    internal interface IPropertySetter
+    internal sealed class PropertySetter
     {
-        bool Cascading { get; }
+        private static readonly MethodInfo CallPropertySetterOpenGenericMethod =
+            typeof(PropertySetter).GetMethod(nameof(CallPropertySetter), BindingFlags.NonPublic | BindingFlags.Static)!;
 
-        void SetValue(object target, object value);
+        private readonly Action<object, object> _setterDelegate;
+
+        public PropertySetter(Type targetType, PropertyInfo property)
+        {
+            if (property.SetMethod == null)
+            {
+                throw new InvalidOperationException($"Cannot provide a value for property " +
+                    $"'{property.Name}' on type '{targetType.FullName}' because the property " +
+                    $"has no setter.");
+            }
+
+            var setMethod = property.SetMethod;
+
+            var propertySetterAsAction =
+                setMethod.CreateDelegate(typeof(Action<,>).MakeGenericType(targetType, property.PropertyType));
+            var callPropertySetterClosedGenericMethod =
+                CallPropertySetterOpenGenericMethod.MakeGenericMethod(targetType, property.PropertyType);
+            _setterDelegate = (Action<object, object>)
+                callPropertySetterClosedGenericMethod.CreateDelegate(typeof(Action<object, object>), propertySetterAsAction);
+        }
+
+        public bool Cascading { get; init;  }
+
+        public void SetValue(object target, object value) => _setterDelegate(target, value);
+
+        private static void CallPropertySetter<TTarget, TValue>(
+            Action<TTarget, TValue> setter,
+            object target,
+            object value)
+            where TTarget : notnull
+        {
+            if (value == null)
+            {
+                setter((TTarget)target, default!);
+            }
+            else
+            {
+                setter((TTarget)target, (TValue)value);
+            }
+        }
     }
 }

src/Components/Components/src/Reflection/MemberAssignment.cs

@@ -44,46 +44,5 @@ namespace Microsoft.AspNetCore.Components.Reflection
 
             return dictionary.Values.SelectMany(p => p);
         }
-
-        public static IPropertySetter CreatePropertySetter(Type targetType, PropertyInfo property, bool cascading)
-        {
-            if (property.SetMethod == null)
-            {
-                throw new InvalidOperationException($"Cannot provide a value for property " +
-                    $"'{property.Name}' on type '{targetType.FullName}' because the property " +
-                    $"has no setter.");
-            }
-
-            return (IPropertySetter)Activator.CreateInstance(
-                typeof(PropertySetter<,>).MakeGenericType(targetType, property.PropertyType),
-                property.SetMethod,
-                cascading)!;
-        }
-
-        class PropertySetter<TTarget, TValue> : IPropertySetter where TTarget : notnull
-        {
-            private readonly Action<TTarget, TValue> _setterDelegate;
-
-            public PropertySetter(MethodInfo setMethod, bool cascading)
-            {
-                _setterDelegate = (Action<TTarget, TValue>)Delegate.CreateDelegate(
-                    typeof(Action<TTarget, TValue>), setMethod);
-                Cascading = cascading;
-            }
-
-            public bool Cascading { get; }
-
-            public void SetValue(object target, object value)
-            {
-                if (value == null)
-                {
-                    _setterDelegate((TTarget)target, default!);
-                }
-                else
-                {
-                    _setterDelegate((TTarget)target, (TValue)value);
-                }
-            }
-        }
     }
 }

src/Components/benchmarkapps/Wasm.Performance/Directory.Build.props

@@ -0,0 +1,11 @@
+<Project>
+  <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory)..\, Directory.Build.props))\Directory.Build.props" />
+
+  <PropertyGroup>
+    <!-- Makes our docker composition simpler by not redirecting build and publish output to the artifacts dir -->
+    <BaseIntermediateOutputPath />
+    <IntermediateOutputPath />
+    <BaseOutputPath />
+    <OutputPath />
+  </PropertyGroup>
+</Project>

src/Components/benchmarkapps/Wasm.Performance/Driver/Program.cs

@@ -16,6 +16,7 @@ using Microsoft.AspNetCore.Hosting.Server;
 using Microsoft.AspNetCore.Hosting.Server.Features;
 using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Hosting;
+using OpenQA.Selenium;
 using DevHostServerProgram = Microsoft.AspNetCore.Components.WebAssembly.DevServer.Server.Program;
 
 namespace Wasm.Performance.Driver
@@ -81,7 +82,20 @@ namespace Wasm.Performance.Driver
             {
                 BenchmarkResultTask = new TaskCompletionSource<BenchmarkResult>();
                 using var runCancellationToken = new CancellationTokenSource(timeForEachRun);
-                using var registration = runCancellationToken.Token.Register(() => BenchmarkResultTask.TrySetException(new TimeoutException($"Timed out after {timeForEachRun}")));
+                using var registration = runCancellationToken.Token.Register(() =>
+                {
+                    string exceptionMessage = $"Timed out after {timeForEachRun}.";
+                    try
+                    {
+                        var innerHtml = browser.FindElement(By.CssSelector(":first-child")).GetAttribute("innerHTML");
+                        exceptionMessage += Environment.NewLine + "Browser state: " + Environment.NewLine + innerHtml;
+                    }
+                    catch
+                    {
+                        // Do nothing;
+                    }
+                    BenchmarkResultTask.TrySetException(new TimeoutException(exceptionMessage));
+                });
 
                 var results = await BenchmarkResultTask.Task;
 
@@ -89,6 +103,11 @@ namespace Wasm.Performance.Driver
                     includeMetadata: firstRun,
                     isStressRun: isStressRun);
 
+                if (!isStressRun)
+                {
+                    PrettyPrint(results);
+                }
+
                 firstRun = false;
             } while (isStressRun && !stressRunCancellation.IsCancellationRequested);
 
@@ -230,6 +249,17 @@ namespace Wasm.Performance.Driver
             Console.WriteLine(builder);
         }
 
+        static void PrettyPrint(BenchmarkResult benchmarkResult)
+        {
+            Console.WriteLine();
+            Console.WriteLine("| Name | Description | Duration | NumExecutions | ");
+            Console.WriteLine("--------------------------");
+            foreach (var result in benchmarkResult.ScenarioResults)
+            {
+                Console.WriteLine($"| {result.Descriptor.Name} | {result.Name} | {result.Duration} | {result.NumExecutions} |");
+            }
+        }
+
         static IHost StartTestApp()
         {
             var args = new[]

src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs

@@ -16,12 +16,7 @@ namespace Wasm.Performance.Driver
         const int SeleniumPort = 4444;
         static bool RunHeadlessBrowser = true;
 
-        static bool PoolForBrowserLogs =
-#if DEBUG
-            true;
-#else
-            false;
-#endif
+        static bool PoolForBrowserLogs = true;
 
         private static async ValueTask<Uri> WaitForServerAsync(int port, CancellationToken cancellationToken)
         {

src/Components/benchmarkapps/Wasm.Performance/TestApp/Wasm.Performance.TestApp.csproj

@@ -3,6 +3,11 @@
   <PropertyGroup>
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <IsTestAssetProject>true</IsTestAssetProject>
+    <!--
+      Chrome in docker appears to run in to cache corruption issues when the cache is read multiple times over.
+      Clien caching isn't part of our performance measurement, so we'll skip it.
+    -->
+    <BlazorCacheBootResources>false</BlazorCacheBootResources>
   </PropertyGroup>
 
   <ItemGroup>