From 3eb9546cd54389a6b7a39f72053e627c64e6d86b Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 18 Aug 2020 12:16:17 -0700 Subject: [PATCH 1/2] Make PropertySetter a concrete type * Use the pattern from PropertyHelpers to set property values * Tweaks to Blazor WebAssembly tests to allow running tests locally --- .../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 eddb39a937bf..10c9b1aa06b6 100644 --- a/src/Components/Components/src/ComponentFactory.cs +++ b/src/Components/Components/src/ComponentFactory.cs @@ -63,7 +63,7 @@ private Action CreateInitializer(Type type) ( 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 1b6e43d7ba4e..dd6aafab26e2 100644 --- a/src/Components/Components/src/Reflection/ComponentProperties.cs +++ b/src/Components/Components/src/Reflection/ComponentProperties.cs @@ -144,7 +144,7 @@ public static void SetProperties(in ParameterView parameters, object target) } } - 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 @@ private static void ThrowForInvalidCaptureUnmatchedValuesParameterType(Type targ 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 @@ public WritersForType(Type targetType) $"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 @@ public WritersForType(Type targetType) 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 d6a60e2395ae..503a4f11792e 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).GetTypeInfo().GetDeclaredMethod(nameof(CallPropertySetter))!; + + 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); - void SetValue(object target, object 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 4510d4e81c36..1d0afbe49add 100644 --- a/src/Components/Components/src/Reflection/MemberAssignment.cs +++ b/src/Components/Components/src/Reflection/MemberAssignment.cs @@ -44,46 +44,5 @@ public static IEnumerable GetPropertiesIncludingInherited( 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 000000000000..0556b5271ed6 --- /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 c3c3ae618fbf..c72ab9dccac4 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.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 @@ public static async Task Main(string[] args) { 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 @@ public static async Task Main(string[] args) includeMetadata: firstRun, isStressRun: isStressRun); + if (!isStressRun) + { + PrettyPrint(results); + } + firstRun = false; } while (isStressRun && !stressRunCancellation.IsCancellationRequested); @@ -230,6 +249,17 @@ private static void FormatAsBenchmarksOutput(BenchmarkResult benchmarkResult, bo 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 f9d7ef47efa2..974854909960 100644 --- a/src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs +++ b/src/Components/benchmarkapps/Wasm.Performance/Driver/Selenium.cs @@ -16,12 +16,7 @@ class Selenium 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 d3e261a592e0..8157ceac2247 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 From d019b2565ab8755e1acff03632293b8c299b5e37 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Mon, 24 Aug 2020 14:49:57 -0700 Subject: [PATCH 2/2] Update src/Components/Components/src/Reflection/IPropertySetter.cs --- src/Components/Components/src/Reflection/IPropertySetter.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Components/Components/src/Reflection/IPropertySetter.cs b/src/Components/Components/src/Reflection/IPropertySetter.cs index 503a4f11792e..5cd1cb0494d5 100644 --- a/src/Components/Components/src/Reflection/IPropertySetter.cs +++ b/src/Components/Components/src/Reflection/IPropertySetter.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Components.Reflection internal sealed class PropertySetter { private static readonly MethodInfo CallPropertySetterOpenGenericMethod = - typeof(PropertySetter).GetTypeInfo().GetDeclaredMethod(nameof(CallPropertySetter))!; + typeof(PropertySetter).GetMethod(nameof(CallPropertySetter), BindingFlags.NonPublic | BindingFlags.Static)!; private readonly Action _setterDelegate;