From 0c8f9d0d237ae2bc25c325fce2da562c467ca313 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Tue, 2 Feb 2021 00:59:53 +0100 Subject: [PATCH 1/6] Avoid unneeded custom invocation types on interface proxy w/o target --- .../InterfaceProxyWithoutTargetContributor.cs | 16 ++++- .../InterfaceMethodWithoutTargetInvocation.cs | 61 +++++++++++++++++++ 2 files changed, 74 insertions(+), 3 deletions(-) create mode 100644 src/Castle.Core/DynamicProxy/Internal/InterfaceMethodWithoutTargetInvocation.cs diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs index cb1613cd13..d23164bb74 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs @@ -20,6 +20,7 @@ namespace Castle.DynamicProxy.Contributors using Castle.DynamicProxy.Generators; using Castle.DynamicProxy.Generators.Emitters; + using Castle.DynamicProxy.Internal; internal class InterfaceProxyWithoutTargetContributor : CompositeTypeContributor { @@ -60,6 +61,15 @@ protected override IEnumerable GetCollectors() private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) { + var methodInfo = method.Method; + + if (canChangeTarget == false && methodInfo.IsAbstract && methodInfo.IsGenericMethod == false && methodInfo.IsGenericMethodDefinition == false) + { + // We do not need to generate a custom invocation type because no custom implementation + // for `InvokeMethodOnTarget` will be needed (proceeding to target isn't possible here): + return typeof(InterfaceMethodWithoutTargetInvocation); + } + var scope = emitter.ModuleScope; Type[] invocationInterfaces; if (canChangeTarget) @@ -70,14 +80,14 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) { invocationInterfaces = new[] { typeof(IInvocation) }; } - var key = new CacheKey(method.Method, CompositionInvocationTypeGenerator.BaseType, invocationInterfaces, null); + var key = new CacheKey(methodInfo, CompositionInvocationTypeGenerator.BaseType, invocationInterfaces, null); // no locking required as we're already within a lock return scope.TypeCache.GetOrAddWithoutTakingLock(key, _ => - new CompositionInvocationTypeGenerator(method.Method.DeclaringType, + new CompositionInvocationTypeGenerator(methodInfo.DeclaringType, method, - method.Method, + methodInfo, canChangeTarget, null) .Generate(emitter, namingScope) diff --git a/src/Castle.Core/DynamicProxy/Internal/InterfaceMethodWithoutTargetInvocation.cs b/src/Castle.Core/DynamicProxy/Internal/InterfaceMethodWithoutTargetInvocation.cs new file mode 100644 index 0000000000..a1c92a2dc9 --- /dev/null +++ b/src/Castle.Core/DynamicProxy/Internal/InterfaceMethodWithoutTargetInvocation.cs @@ -0,0 +1,61 @@ +// Copyright 2004-2021 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Castle.DynamicProxy.Internal +{ + using System; + using System.ComponentModel; + using System.Diagnostics; + using System.Reflection; + +#if FEATURE_SERIALIZATION + [Serializable] +#endif + [EditorBrowsable(EditorBrowsableState.Never)] + public sealed class InterfaceMethodWithoutTargetInvocation : AbstractInvocation + { + public InterfaceMethodWithoutTargetInvocation(object target, object proxy, IInterceptor[] interceptors, MethodInfo proxiedMethod, object[] arguments) + : base(proxy, interceptors, proxiedMethod, arguments) + { + // This invocation type is suitable for interface method invocations that cannot proceed + // to a target, i.e. where `InvokeMethodOnTarget` will always throw: + + Debug.Assert(target == null, $"{nameof(InterfaceMethodWithoutTargetInvocation)} does not support targets."); + Debug.Assert(proxiedMethod.IsAbstract, $"{nameof(InterfaceMethodWithoutTargetInvocation)} does not support non-abstract methods."); + + // Why this restriction? Because it greatly benefits proxy type generation performance. + // + // For invocations that can proceed to a target, `InvokeMethodOnTarget`'s implementation + // depends on the target method's signature. Because of this, DynamicProxy needs to + // dynamically generate a separate invocation type per such method. Type generation is + // always expensive... that is, slow. + // + // However, if it is known that `InvokeMethodOnTarget` won't forward, but throw, + // no custom (dynamically generated) invocation type is needed at all, and we can use + // this unspecific invocation type instead. + } + + // The next three properties mimick the behavior seen with an interface proxy without target. + // (This is why this type's name starts with `Interface`.) A similar type could be written + // for class proxies without target, but the values returned here would be different. + + public override object InvocationTarget => null; + + public override MethodInfo MethodInvocationTarget => null; + + public override Type TargetType => null; + + protected override void InvokeMethodOnTarget() => ThrowOnNoTarget(); + } +} From db247cdb223adcda3a853e21a705887a54cc1446 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 3 Feb 2021 20:30:56 +0100 Subject: [PATCH 2/6] Test proper invocation type selection --- .../InvocationTypeReuseTestCase.cs | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs new file mode 100644 index 0000000000..ccdf4e5c22 --- /dev/null +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs @@ -0,0 +1,59 @@ +// Copyright 2004-2021 Castle Project - http://www.castleproject.org/ +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +namespace Castle.DynamicProxy.Tests +{ + using System; + + using Castle.DynamicProxy.Internal; + + using NUnit.Framework; + + /// + /// This fixture checks which types get used for proxied methods. + /// Usually, DynamicProxy generates a separate implementation type per proxied method, but + /// in some cases, it can reuse predefined implementation types. Because this is beneficial + /// for runtime performance (as it reduces the amount of dynamic type generation performed), + /// we want to ensure that those predefined types do in fact get picked when they should be. + /// + [TestFixture] + public class InvocationTypeReuseTestCase : BasePEVerifyTestCase + { + [Test] + public void Non_generic_method_of_interface_proxy_without_target__uses__InterfaceMethodWithoutTargetInvocation() + { + var recorder = new InvocationTypeRecorder(); + + var proxy = generator.CreateInterfaceProxyWithoutTarget(recorder); + proxy.Method(); + + Assert.AreEqual(typeof(InterfaceMethodWithoutTargetInvocation), recorder.InvocationType); + } + + public interface IWithNonGenericMethod + { + void Method(); + } + + private sealed class InvocationTypeRecorder : IInterceptor + { + public Type InvocationType { get; private set; } + + public void Intercept(IInvocation invocation) + { + InvocationType = invocation.GetType(); + } + } + } +} From d6c66f5f7258e312b93264028bc5c71ed762090c Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 3 Feb 2021 23:09:06 +0100 Subject: [PATCH 3/6] New invocation type should work for generic methods, too --- .../InvocationTypeReuseTestCase.cs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs b/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs index ccdf4e5c22..21e79a9a6c 100644 --- a/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs +++ b/src/Castle.Core.Tests/DynamicProxy.Tests/InvocationTypeReuseTestCase.cs @@ -41,11 +41,27 @@ public void Non_generic_method_of_interface_proxy_without_target__uses__Interfac Assert.AreEqual(typeof(InterfaceMethodWithoutTargetInvocation), recorder.InvocationType); } + [Test] + public void Generic_method_of_interface_proxy_without_target__uses__InterfaceMethodWithoutTargetInvocation() + { + var recorder = new InvocationTypeRecorder(); + + var proxy = generator.CreateInterfaceProxyWithoutTarget(recorder); + proxy.Method(42); + + Assert.AreEqual(typeof(InterfaceMethodWithoutTargetInvocation), recorder.InvocationType); + } + public interface IWithNonGenericMethod { void Method(); } + public interface IWithGenericMethod + { + void Method(T arg); + } + private sealed class InvocationTypeRecorder : IInterceptor { public Type InvocationType { get; private set; } From b87787f949741c2c1ae5446c072831c206ea3c65 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 3 Feb 2021 23:10:35 +0100 Subject: [PATCH 4/6] Some generic methods now use non-generic invocation type --- .../InterfaceProxyWithoutTargetContributor.cs | 2 +- .../Generators/MethodWithInvocationGenerator.cs | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs index d23164bb74..3910e426e3 100644 --- a/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs +++ b/src/Castle.Core/DynamicProxy/Contributors/InterfaceProxyWithoutTargetContributor.cs @@ -63,7 +63,7 @@ private Type GetInvocationType(MetaMethod method, ClassEmitter emitter) { var methodInfo = method.Method; - if (canChangeTarget == false && methodInfo.IsAbstract && methodInfo.IsGenericMethod == false && methodInfo.IsGenericMethodDefinition == false) + if (canChangeTarget == false && methodInfo.IsAbstract) { // We do not need to generate a custom invocation type because no custom implementation // for `InvokeMethodOnTarget` will be needed (proceeding to target isn't possible here): diff --git a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs b/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs index 9815661106..6d2b5d6108 100644 --- a/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs +++ b/src/Castle.Core/DynamicProxy/Generators/MethodWithInvocationGenerator.cs @@ -73,7 +73,6 @@ protected override MethodEmitter BuildProxiedMethodBody(MethodEmitter emitter, C { var invocationType = invocation; - Trace.Assert(MethodToOverride.IsGenericMethod == invocationType.IsGenericTypeDefinition); var genericArguments = Type.EmptyTypes; var constructor = invocation.GetConstructors()[0]; @@ -81,13 +80,16 @@ protected override MethodEmitter BuildProxiedMethodBody(MethodEmitter emitter, C IExpression proxiedMethodTokenExpression; if (MethodToOverride.IsGenericMethod) { - // bind generic method arguments to invocation's type arguments - genericArguments = emitter.MethodBuilder.GetGenericArguments(); - invocationType = invocationType.MakeGenericType(genericArguments); - constructor = TypeBuilder.GetConstructor(invocationType, constructor); - // Not in the cache: generic method + genericArguments = emitter.MethodBuilder.GetGenericArguments(); proxiedMethodTokenExpression = new MethodTokenExpression(MethodToOverride.MakeGenericMethod(genericArguments)); + + if (invocationType.IsGenericTypeDefinition) + { + // bind generic method arguments to invocation's type arguments + invocationType = invocationType.MakeGenericType(genericArguments); + constructor = TypeBuilder.GetConstructor(invocationType, constructor); + } } else { From 8c86054a5fb16c12737f101e001f5045648212b5 Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Tue, 2 Feb 2021 01:12:05 +0100 Subject: [PATCH 5/6] Update `ref/` contracts --- ref/Castle.Core-net45.cs | 9 +++++++++ ref/Castle.Core-netstandard2.0.cs | 8 ++++++++ ref/Castle.Core-netstandard2.1.cs | 8 ++++++++ 3 files changed, 25 insertions(+) diff --git a/ref/Castle.Core-net45.cs b/ref/Castle.Core-net45.cs index 98c4ea6ce2..f4914b2c7b 100644 --- a/ref/Castle.Core-net45.cs +++ b/ref/Castle.Core-net45.cs @@ -2787,6 +2787,15 @@ public abstract class InheritanceInvocation : Castle.DynamicProxy.AbstractInvoca public override System.Type TargetType { get; } protected abstract override void InvokeMethodOnTarget() { } } + [System.Serializable] + public sealed class InterfaceMethodWithoutTargetInvocation : Castle.DynamicProxy.AbstractInvocation + { + public InterfaceMethodWithoutTargetInvocation(object target, object proxy, Castle.DynamicProxy.IInterceptor[] interceptors, System.Reflection.MethodInfo proxiedMethod, object[] arguments) { } + public override object InvocationTarget { get; } + public override System.Reflection.MethodInfo MethodInvocationTarget { get; } + public override System.Type TargetType { get; } + protected override void InvokeMethodOnTarget() { } + } public static class TypeUtil { public static System.Type[] GetAllInterfaces(this System.Type type) { } diff --git a/ref/Castle.Core-netstandard2.0.cs b/ref/Castle.Core-netstandard2.0.cs index 829a37e2ae..e2d8604e8f 100644 --- a/ref/Castle.Core-netstandard2.0.cs +++ b/ref/Castle.Core-netstandard2.0.cs @@ -2740,6 +2740,14 @@ public abstract class InheritanceInvocation : Castle.DynamicProxy.AbstractInvoca public override System.Type TargetType { get; } protected abstract override void InvokeMethodOnTarget() { } } + public sealed class InterfaceMethodWithoutTargetInvocation : Castle.DynamicProxy.AbstractInvocation + { + public InterfaceMethodWithoutTargetInvocation(object target, object proxy, Castle.DynamicProxy.IInterceptor[] interceptors, System.Reflection.MethodInfo proxiedMethod, object[] arguments) { } + public override object InvocationTarget { get; } + public override System.Reflection.MethodInfo MethodInvocationTarget { get; } + public override System.Type TargetType { get; } + protected override void InvokeMethodOnTarget() { } + } public static class TypeUtil { public static System.Type[] GetAllInterfaces(this System.Type type) { } diff --git a/ref/Castle.Core-netstandard2.1.cs b/ref/Castle.Core-netstandard2.1.cs index f29e4c6734..00e2896aa3 100644 --- a/ref/Castle.Core-netstandard2.1.cs +++ b/ref/Castle.Core-netstandard2.1.cs @@ -2740,6 +2740,14 @@ public abstract class InheritanceInvocation : Castle.DynamicProxy.AbstractInvoca public override System.Type TargetType { get; } protected abstract override void InvokeMethodOnTarget() { } } + public sealed class InterfaceMethodWithoutTargetInvocation : Castle.DynamicProxy.AbstractInvocation + { + public InterfaceMethodWithoutTargetInvocation(object target, object proxy, Castle.DynamicProxy.IInterceptor[] interceptors, System.Reflection.MethodInfo proxiedMethod, object[] arguments) { } + public override object InvocationTarget { get; } + public override System.Reflection.MethodInfo MethodInvocationTarget { get; } + public override System.Type TargetType { get; } + protected override void InvokeMethodOnTarget() { } + } public static class TypeUtil { public static System.Type[] GetAllInterfaces(this System.Type type) { } From 7b6b193390f81bccfa26eeeae3e7c6863505106b Mon Sep 17 00:00:00 2001 From: Dominique Schuppli Date: Wed, 3 Feb 2021 20:04:14 +0100 Subject: [PATCH 6/6] Update the changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 94d0a7ab15..37f52d171d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ Enhancements: - .NET Standard 2.0 and 2.1 support (@lg2de, #485) - Non-intercepted methods on a class proxy with target are now forwarded to the target (@stakx, #571) +- Significant performance improvements with proxy type generation for interface proxies without target. (Up until now, DynamicProxy generated a separate `IInvocation` implementation type for every single proxied method – it is now able to reuse a single predefined type in many cases, thereby reducing the total amount of dynamic type generation.) (@stakx, #573) Bugfixes: - Proxying certain `[Serializable]` classes produces proxy types that fail PEVerify test (@stakx, #367)