From 29cbf6e1069d4f400a567f29a4becae47c45e9f3 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 24 Aug 2020 18:54:28 -0700 Subject: [PATCH] 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 --- .../Components/src/ComponentFactory.cs | 2 +- .../src/Reflection/ComponentProperties.cs | 21 ++++---- .../src/Reflection/IPropertySetter.cs | 49 +++++++++++++++++-- .../src/Reflection/MemberAssignment.cs | 41 ---------------- .../Wasm.Performance/Directory.Build.props | 11 +++++ .../Wasm.Performance/Driver/Program.cs | 32 +++++++++++- .../Wasm.Performance/Driver/Selenium.cs | 7 +-- .../TestApp/Wasm.Performance.TestApp.csproj | 5 ++ 8 files changed, 107 insertions(+), 61 deletions(-) create mode 100644 src/Components/benchmarkapps/Wasm.Performance/Directory.Build.props diff --git a/src/Components/Components/src/ComponentFactory.cs b/src/Components/Components/src/ComponentFactory.cs index eddb39a937..10c9b1aa06 100644 --- a/src/Components/Components/src/ComponentFactory.cs +++ b/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; diff --git a/src/Components/Components/src/Reflection/ComponentProperties.cs b/src/Components/Components/src/Reflection/ComponentProperties.cs index 1b6e43d7ba..dd6aafab26 100644 --- a/src/Components/Components/src/Reflection/ComponentProperties.cs +++ b/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 _underlyingWriters; - private readonly ConcurrentDictionary _referenceEqualityWritersCache; + private readonly Dictionary _underlyingWriters; + private readonly ConcurrentDictionary _referenceEqualityWritersCache; public WritersForType(Type targetType) { - _underlyingWriters = new Dictionary(StringComparer.OrdinalIgnoreCase); - _referenceEqualityWritersCache = new ConcurrentDictionary(ReferenceEqualityComparer.Instance); + _underlyingWriters = new Dictionary(StringComparer.OrdinalIgnoreCase); + _referenceEqualityWritersCache = new ConcurrentDictionary(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. diff --git a/src/Components/Components/src/Reflection/IPropertySetter.cs b/src/Components/Components/src/Reflection/IPropertySetter.cs index d6a60e2395..5cd1cb0494 100644 --- a/src/Components/Components/src/Reflection/IPropertySetter.cs +++ b/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 _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) + callPropertySetterClosedGenericMethod.CreateDelegate(typeof(Action), propertySetterAsAction); + } + + public bool Cascading { get; init; } + + public void SetValue(object target, object value) => _setterDelegate(target, value); + + private static void CallPropertySetter( + Action setter, + object target, + object value) + where TTarget : notnull + { + if (value == null) + { + setter((TTarget)target, default!); + } + else + { + setter((TTarget)target, (TValue)value); + } + } } } diff --git a/src/Components/Components/src/Reflection/MemberAssignment.cs b/src/Components/Components/src/Reflection/MemberAssignment.cs index 4510d4e81c..1d0afbe49a 100644 --- a/src/Components/Components/src/Reflection/MemberAssignment.cs +++ b/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 : IPropertySetter where TTarget : notnull - { - private readonly Action _setterDelegate; - - public PropertySetter(MethodInfo setMethod, bool cascading) - { - _setterDelegate = (Action)Delegate.CreateDelegate( - typeof(Action), 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); - } - } - } } } diff --git a/src/Components/benchmarkapps/Wasm.Performance/Directory.Build.props b/src/Components/benchmarkapps/Wasm.Performance/Directory.Build.props new file mode 100644 index 0000000000..0556b5271e --- /dev/null +++ b/src/Components/benchmarkapps/Wasm.Performance/Directory.Build.props @@ -0,0 +1,11 @@ + + + + + + + + + + + diff --git a/src/Components/benchmarkapps/Wasm.Performance/Driver/Program.cs b/src/Components/benchmarkapps/Wasm.Performance/Driver/Program.cs index c3c3ae618f..c72ab9dcca 100644 --- a/src/Components/benchmarkapps/Wasm.Performance/Driver/Program.cs +++ b/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(); 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[] diff --git a/src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs b/src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs index f9d7ef47ef..9748549099 100644 --- a/src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs +++ b/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 WaitForServerAsync(int port, CancellationToken cancellationToken) { diff --git a/src/Components/benchmarkapps/Wasm.Performance/TestApp/Wasm.Performance.TestApp.csproj b/src/Components/benchmarkapps/Wasm.Performance/TestApp/Wasm.Performance.TestApp.csproj index d3e261a592..8157ceac22 100644 --- a/src/Components/benchmarkapps/Wasm.Performance/TestApp/Wasm.Performance.TestApp.csproj +++ b/src/Components/benchmarkapps/Wasm.Performance/TestApp/Wasm.Performance.TestApp.csproj @@ -3,6 +3,11 @@ $(DefaultNetCoreTargetFramework) true + + false