Enable IL for testing; improve perf for ByRef param validation

dotnet · Apr 21, 2022 · a1f7a5c · a1f7a5c
1 parent 40eb391
commit a1f7a5c
Show file tree

Hide file tree

Showing 11 changed files with 234 additions and 175 deletions.
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs
@@ -579,23 +579,9 @@ Signature LazyCreateSignature()
 
         [DebuggerHidden]
         [DebuggerStepThrough]
-        internal unsafe object? InvokeNonEmitUnsafe(object? obj, IntPtr* arguments, BindingFlags invokeAttr)
+        internal unsafe object? InvokeNonEmitUnsafe(object? obj, IntPtr* arguments)
         {
-            if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0)
-            {
-                try
-                {
-                    return RuntimeMethodHandle.InvokeMethod(obj, (void**)arguments, Signature, isConstructor: false);
-                }
-                catch (Exception e)
-                {
-                    throw new TargetInvocationException(e);
-                }
-            }
-            else
-            {
-                return RuntimeMethodHandle.InvokeMethod(obj, (void**)arguments, Signature, isConstructor: false);
-            }
+            return RuntimeMethodHandle.InvokeMethod(obj, (void**)arguments, Signature, isConstructor: false);
         }
 
         public override object[] GetCustomAttributes(Type attributeType, bool inherit)

diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethodInvoker.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethodInvoker.cs
@@ -1,9 +1,6 @@
 // Licensed to the .NET Foundation under one or more agreements.
 // The .NET Foundation licenses this file to you under the MIT license.
 
-using System.Diagnostics;
-using System.Runtime.CompilerServices;
-
 namespace System.Reflection.Emit
 {
     internal sealed partial class DynamicMethodInvoker
@@ -17,8 +14,23 @@ public DynamicMethodInvoker(DynamicMethod dynamicMethod)
 
         public unsafe object? InvokeUnsafe(object? obj, IntPtr* args, BindingFlags invokeAttr)
         {
-            // Todo: add strategy for calling IL Emit-based version
-            return _dynamicMethod.InvokeNonEmitUnsafe(obj, args, invokeAttr);
+            // Always use the slow path; the Emit-based fast path can be added but in general dynamic
+            // methods are invoked through a direct delegate, not through Invoke().
+            if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0)
+            {
+                try
+                {
+                    return _dynamicMethod.InvokeNonEmitUnsafe(obj, args);
+                }
+                catch (Exception e)
+                {
+                    throw new TargetInvocationException(e);
+                }
+            }
+            else
+            {
+                return _dynamicMethod.InvokeNonEmitUnsafe(obj, args);
+            }
         }
     }
 }
diff --git a/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.CoreCLR.cs
@@ -95,26 +95,15 @@ Signature LazyCreateSignature()
 
         internal BindingFlags BindingFlags => m_bindingFlags;
 
+#pragma warning disable CA1822 // Mark members as static
+        internal bool SupportsNewInvoke => true;
+#pragma warning restore CA1822 // Mark members as static
 
         [DebuggerStepThrough]
         [DebuggerHidden]
         internal unsafe object InvokeNonEmitUnsafe(object? obj, IntPtr* args, Span<object?> argsForTemporaryMonoSupport, BindingFlags invokeAttr)
         {
-            if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0)
-            {
-                try
-                {
-                    return RuntimeMethodHandle.InvokeMethod(obj, (void**)args, Signature, isConstructor: obj is null)!;
-                }
-                catch (Exception ex)
-                {
-                    throw new TargetInvocationException(ex);
-                }
-            }
-            else
-            {
-                return RuntimeMethodHandle.InvokeMethod(obj, (void**)args, Signature, isConstructor: obj is null)!;
-            }
+            return RuntimeMethodHandle.InvokeMethod(obj, (void**)args, Signature, isConstructor: obj is null)!;
         }
         #endregion
 

diff --git a/src/coreclr/vm/reflectioninvocation.cpp b/src/coreclr/vm/reflectioninvocation.cpp
@@ -624,14 +624,12 @@ FCIMPL4(Object*, RuntimeMethodHandle::InvokeMethod,
             else
                 pThisPtr = OBJECTREFToObject(gc.retVal);
         }
-        else
-        if (!pMeth->GetMethodTable()->IsValueType())
+        else if (!pMeth->GetMethodTable()->IsValueType())
             pThisPtr = OBJECTREFToObject(gc.target);
         else {
             if (pMeth->IsUnboxingStub())
                 pThisPtr = OBJECTREFToObject(gc.target);
             else {
-                //todo: add tests for this (calling a nullable)
                 // Create a true boxed Nullable<T> and use that as the 'this' pointer.
                 // since what is passed in is just a boxed T
                 MethodTable* pMT = pMeth->GetMethodTable();

diff --git a/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs b/src/libraries/System.Private.CoreLib/src/System/Reflection/ConstructorInvoker.cs
@@ -2,25 +2,93 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Diagnostics;
+using System.Runtime.CompilerServices;
 
 namespace System.Reflection
 {
     internal sealed partial class ConstructorInvoker
     {
-        private readonly RuntimeConstructorInfo _constructorInfo;
+        private readonly RuntimeConstructorInfo _method;
         public InvocationFlags _invocationFlags;
+        private bool _invoked;
+        private bool _strategyDetermined;
+        private InvokerEmitUtil.InvokeFunc<ConstructorInvoker>? _emitInvoke;
 
         public ConstructorInvoker(RuntimeConstructorInfo constructorInfo)
         {
-            _constructorInfo = constructorInfo;
+            _method = constructorInfo;
         }
 
         [DebuggerStepThrough]
         [DebuggerHidden]
         public unsafe object? InvokeUnsafe(object? obj, IntPtr* args, Span<object?> argsForTemporaryMonoSupport, BindingFlags invokeAttr)
         {
-            // Todo: add strategy for calling IL Emit-based version
-            return _constructorInfo.InvokeNonEmitUnsafe(obj, args, argsForTemporaryMonoSupport, invokeAttr);
+            if (!_strategyDetermined)
+            {
+                if (!_invoked)
+                {
+                    // The first time, ignoring race conditions, use the slow path.
+                    _invoked = true;
+
+#if true
+                    // TEMP HACK FOR FORCING IL ON FIRST TIME:
+                    if (RuntimeFeature.IsDynamicCodeCompiled)
+                    {
+                        _emitInvoke = InvokerEmitUtil.CreateInvokeDelegate<ConstructorInvoker>(_method);
+                    }
+                    _strategyDetermined = true;
+#endif
+                }
+                else
+                {
+                    if (RuntimeFeature.IsDynamicCodeCompiled)
+                    {
+                        _emitInvoke = InvokerEmitUtil.CreateInvokeDelegate<ConstructorInvoker>(_method);
+                    }
+
+                    _strategyDetermined = true;
+                }
+            }
+
+            if (_method.SupportsNewInvoke)
+            {
+                if ((invokeAttr & BindingFlags.DoNotWrapExceptions) == 0)
+                {
+                    try
+                    {
+                        // For the rare and broken scenario of calling the constructor directly through MethodBase.Invoke()
+                        // with a non-null 'obj', we use the slow path to avoid having two emit-based delegates.
+                        if (_emitInvoke != null && obj == null)
+                        {
+                            return _emitInvoke(this, obj, args);
+                        }
+                        else
+                        {
+                            return _method.InvokeNonEmitUnsafe(obj, args, argsForTemporaryMonoSupport, invokeAttr);
+                        }
+                    }
+                    catch (Exception e)
+                    {
+                        throw new TargetInvocationException(e);
+                    }
+                }
+                else
+                {
+                    if (_emitInvoke != null && obj == null)
+                    {
+                        return _emitInvoke(this, obj, args);
+                    }
+                    else
+                    {
+                        return _method.InvokeNonEmitUnsafe(obj, args, argsForTemporaryMonoSupport, invokeAttr);
+                    }
+                }
+            }
+            else
+            {
+                // Remove this branch once Mono has the same exception handling and managed conversion logic.
+                return _method.InvokeNonEmitUnsafe(obj, args, argsForTemporaryMonoSupport, invokeAttr);
+            }
         }
     }
 }