From e2645159c31deca1624a57670734864c27bb0a58 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Tue, 1 Oct 2019 12:30:00 +0200 Subject: [PATCH 1/7] Add benchmark for TypedClientBuilder --- .../TypedClientBuilderBenchmark.cs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) create mode 100644 src/SignalR/perf/Microbenchmarks/TypedClientBuilderBenchmark.cs diff --git a/src/SignalR/perf/Microbenchmarks/TypedClientBuilderBenchmark.cs b/src/SignalR/perf/Microbenchmarks/TypedClientBuilderBenchmark.cs new file mode 100644 index 000000000000..8a386fd22845 --- /dev/null +++ b/src/SignalR/perf/Microbenchmarks/TypedClientBuilderBenchmark.cs @@ -0,0 +1,31 @@ +// 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.Threading; +using System.Threading.Tasks; +using BenchmarkDotNet.Attributes; +using Microsoft.AspNetCore.SignalR.Internal; + +namespace Microsoft.AspNetCore.SignalR.Microbenchmarks +{ + public class TypedClientBuilderBenchmark + { + private static readonly IClientProxy Dummy = new DummyProxy(); + + [Benchmark] + public ITestClient Build() + { + return TypedClientBuilder.Build(Dummy); + } + + public interface ITestClient { } + + private class DummyProxy : IClientProxy + { + public Task SendCoreAsync(string method, object[] args, CancellationToken cancellationToken = default) + { + return Task.CompletedTask; + } + } + } +} From eb4ecca306cf4190fbc57a19f986ca29349e2296 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Tue, 1 Oct 2019 12:30:53 +0200 Subject: [PATCH 2/7] Use compiled expression as factory for typed clients --- .../server/Core/src/Internal/TypedClientBuilder.cs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index a333c600a87c..1402d62e27ac 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Linq.Expressions; using System.Reflection; using System.Reflection.Emit; using System.Threading; @@ -20,6 +21,8 @@ internal static class TypedClientBuilder private static readonly PropertyInfo CancellationTokenNoneProperty = typeof(CancellationToken).GetProperty("None", BindingFlags.Public | BindingFlags.Static); + private static readonly Type[] ParameterTypes = new Type[] { typeof(IClientProxy) }; + public static T Build(IClientProxy proxy) { return _builder.Value(proxy); @@ -40,7 +43,11 @@ private static Func GenerateClientBuilder() var moduleBuilder = assemblyBuilder.DefineDynamicModule(ClientModuleName); var clientType = GenerateInterfaceImplementation(moduleBuilder); - return proxy => (T)Activator.CreateInstance(clientType, proxy); + var ctor = clientType.GetConstructor(ParameterTypes); + var proxy = Expression.Parameter(typeof(IClientProxy), "proxy"); + var ctorCall = Expression.New(ctor, proxy); + + return Expression.Lambda>(ctorCall, proxy).Compile(); } private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) From 3cc4fdccf63ed54bf56eb2ff037eaba247b9383e Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Tue, 1 Oct 2019 12:31:20 +0200 Subject: [PATCH 3/7] Some code cleanup --- .../Core/src/Internal/TypedClientBuilder.cs | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index 1402d62e27ac..8d2599b43320 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -52,13 +52,11 @@ private static Func GenerateClientBuilder() private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) { - var type = moduleBuilder.DefineType( - ClientModuleName + "." + typeof(T).Name + "Impl", - TypeAttributes.Public, - typeof(Object), - new[] { typeof(T) }); + var name = ClientModuleName + "." + typeof(T).Name + "Impl"; - var proxyField = type.DefineField("_proxy", typeof(IClientProxy), FieldAttributes.Private); + var type = moduleBuilder.DefineType(name, TypeAttributes.Public, typeof(object), new[] { typeof(T) }); + + var proxyField = type.DefineField("_proxy", typeof(IClientProxy), FieldAttributes.Private | FieldAttributes.InitOnly); BuildConstructor(type, proxyField); @@ -88,19 +86,13 @@ private static IEnumerable GetAllInterfaceMethods(Type interfaceType private static void BuildConstructor(TypeBuilder type, FieldInfo proxyField) { - var method = type.DefineMethod(".ctor", System.Reflection.MethodAttributes.Public | System.Reflection.MethodAttributes.HideBySig); - - var ctor = typeof(object).GetConstructor(BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic, - null, new Type[] { }, null); - - method.SetReturnType(typeof(void)); - method.SetParameters(typeof(IClientProxy)); + var ctor = type.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, ParameterTypes); - var generator = method.GetILGenerator(); + var generator = ctor.GetILGenerator(); // Call object constructor generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Call, ctor); + generator.Emit(OpCodes.Call, typeof(object).GetConstructors().Single()); // Assign constructor argument to the proxyField generator.Emit(OpCodes.Ldarg_0); // type @@ -213,7 +205,7 @@ private static void VerifyInterface(Type interfaceType) foreach (var method in interfaceType.GetMethods()) { - VerifyMethod(interfaceType, method); + VerifyMethod(method); } foreach (var parent in interfaceType.GetInterfaces()) @@ -222,7 +214,7 @@ private static void VerifyInterface(Type interfaceType) } } - private static void VerifyMethod(Type interfaceType, MethodInfo interfaceMethod) + private static void VerifyMethod(MethodInfo interfaceMethod) { if (interfaceMethod.ReturnType != typeof(Task)) { From 095f741ace7d175b3d18077f28e66d6b212e934c Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Wed, 2 Oct 2019 11:23:09 +0200 Subject: [PATCH 4/7] Remove System.Linq.Expressions and use DynamicMethod --- .../Core/src/Internal/TypedClientBuilder.cs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index 8d2599b43320..4cdb54215091 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Linq; -using System.Linq.Expressions; using System.Reflection; using System.Reflection.Emit; using System.Threading; @@ -43,11 +42,21 @@ private static Func GenerateClientBuilder() var moduleBuilder = assemblyBuilder.DefineDynamicModule(ClientModuleName); var clientType = GenerateInterfaceImplementation(moduleBuilder); - var ctor = clientType.GetConstructor(ParameterTypes); - var proxy = Expression.Parameter(typeof(IClientProxy), "proxy"); - var ctorCall = Expression.New(ctor, proxy); + return GenerateFactoryFunction(clientType); + } + + private static Func GenerateFactoryFunction(Type type) + { + var ctor = type.GetConstructor(ParameterTypes); + var method = new DynamicMethod(nameof(Build), type, ParameterTypes, type); + + var generator = method.GetILGenerator(); + + generator.Emit(OpCodes.Ldarg_0); + generator.Emit(OpCodes.Newobj, ctor); + generator.Emit(OpCodes.Ret); - return Expression.Lambda>(ctorCall, proxy).Compile(); + return (Func)method.CreateDelegate(typeof(Func)); } private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) From dc7247f937f81230a9d3eb00509199cf75853e45 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Wed, 2 Oct 2019 12:00:34 +0200 Subject: [PATCH 5/7] Get rid of DynamicMethod and just emit a factory function on the type itself --- .../Core/src/Internal/TypedClientBuilder.cs | 36 ++++++++++--------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index 4cdb54215091..fe5c522bdfc9 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -42,21 +42,8 @@ private static Func GenerateClientBuilder() var moduleBuilder = assemblyBuilder.DefineDynamicModule(ClientModuleName); var clientType = GenerateInterfaceImplementation(moduleBuilder); - return GenerateFactoryFunction(clientType); - } - - private static Func GenerateFactoryFunction(Type type) - { - var ctor = type.GetConstructor(ParameterTypes); - var method = new DynamicMethod(nameof(Build), type, ParameterTypes, type); - - var generator = method.GetILGenerator(); - - generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Newobj, ctor); - generator.Emit(OpCodes.Ret); - - return (Func)method.CreateDelegate(typeof(Func)); + var factoryMethod = clientType.GetMethod(nameof(Build), BindingFlags.Public | BindingFlags.Static); + return (Func)factoryMethod.CreateDelegate(typeof(Func)); } private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) @@ -67,7 +54,9 @@ private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) var proxyField = type.DefineField("_proxy", typeof(IClientProxy), FieldAttributes.Private | FieldAttributes.InitOnly); - BuildConstructor(type, proxyField); + var ctor = BuildConstructor(type, proxyField); + + BuildFactoryMethod(type, ctor); foreach (var method in GetAllInterfaceMethods(typeof(T))) { @@ -93,7 +82,7 @@ private static IEnumerable GetAllInterfaceMethods(Type interfaceType } } - private static void BuildConstructor(TypeBuilder type, FieldInfo proxyField) + private static ConstructorInfo BuildConstructor(TypeBuilder type, FieldInfo proxyField) { var ctor = type.DefineConstructor(MethodAttributes.Public, CallingConventions.Standard, ParameterTypes); @@ -108,6 +97,8 @@ private static void BuildConstructor(TypeBuilder type, FieldInfo proxyField) generator.Emit(OpCodes.Ldarg_1); // type proxyfield generator.Emit(OpCodes.Stfld, proxyField); // type.proxyField = proxyField generator.Emit(OpCodes.Ret); + + return ctor; } private static void BuildMethod(TypeBuilder type, MethodInfo interfaceMethodInfo, FieldInfo proxyField) @@ -195,6 +186,17 @@ private static void BuildMethod(TypeBuilder type, MethodInfo interfaceMethodInfo generator.Emit(OpCodes.Ret); // Return the Task returned by 'invokeMethod' } + private static void BuildFactoryMethod(TypeBuilder type, ConstructorInfo ctor) + { + var method = type.DefineMethod(nameof(Build), MethodAttributes.Public | MethodAttributes.Static, CallingConventions.Standard, typeof(T), ParameterTypes); + + var generator = method.GetILGenerator(); + + generator.Emit(OpCodes.Ldarg_0); // Load the IClientProxy argument onto the stack + generator.Emit(OpCodes.Newobj, ctor); // Call the generated constructor with the proxy + generator.Emit(OpCodes.Ret); // Return the typed client + } + private static void VerifyInterface(Type interfaceType) { if (!interfaceType.IsInterface) From d70add092994d424aaca1bf8e908e9796799d510 Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Wed, 2 Oct 2019 12:35:43 +0200 Subject: [PATCH 6/7] Added comment explaining factory method --- src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index fe5c522bdfc9..b027be41983d 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -56,6 +56,9 @@ private static Type GenerateInterfaceImplementation(ModuleBuilder moduleBuilder) var ctor = BuildConstructor(type, proxyField); + // Because a constructor doesn't return anything, it can't be wrapped in a + // delegate directly, so we emit a factory method that just takes the IClientProxy, + // invokes the constructor (using newobj) and returns the new instance of type T. BuildFactoryMethod(type, ctor); foreach (var method in GetAllInterfaceMethods(typeof(T))) From 965665504e2b86c38a5c642df0a4aca0fc3c129b Mon Sep 17 00:00:00 2001 From: Kristian Hellang Date: Tue, 8 Oct 2019 09:41:57 +0200 Subject: [PATCH 7/7] Cache ConstructorInfo for object constructor --- src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs index b027be41983d..da171d9a47cd 100644 --- a/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs +++ b/src/SignalR/server/Core/src/Internal/TypedClientBuilder.cs @@ -20,6 +20,8 @@ internal static class TypedClientBuilder private static readonly PropertyInfo CancellationTokenNoneProperty = typeof(CancellationToken).GetProperty("None", BindingFlags.Public | BindingFlags.Static); + private static readonly ConstructorInfo ObjectConstructor = typeof(object).GetConstructors().Single(); + private static readonly Type[] ParameterTypes = new Type[] { typeof(IClientProxy) }; public static T Build(IClientProxy proxy) @@ -93,7 +95,7 @@ private static ConstructorInfo BuildConstructor(TypeBuilder type, FieldInfo prox // Call object constructor generator.Emit(OpCodes.Ldarg_0); - generator.Emit(OpCodes.Call, typeof(object).GetConstructors().Single()); + generator.Emit(OpCodes.Call, ObjectConstructor); // Assign constructor argument to the proxyField generator.Emit(OpCodes.Ldarg_0); // type