Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Faster ConstructorInfo.Invoke #86855

Merged
merged 4 commits into from
May 29, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ internal static object GetComparer(RuntimeTypeHandle t)
Environment.FailFast("Unable to create comparer");
}

return RuntimeAugments.NewObject(comparerType);
return RuntimeAugments.RawNewObject(comparerType);
}

// This one is an intrinsic that is used to make enum comparisons more efficient.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ internal static object GetComparer(RuntimeTypeHandle t)
Environment.FailFast("Unable to create comparer");
}

return RuntimeAugments.NewObject(comparerType);
return RuntimeAugments.RawNewObject(comparerType);
}

//-----------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ public abstract class ExecutionEnvironment
//==============================================================================================
// Access to the underlying execution engine's object allocation routines.
//==============================================================================================
public abstract object NewObject(RuntimeTypeHandle typeHandle);
public abstract Array NewArray(RuntimeTypeHandle typeHandleForArrayType, int count);
public abstract Array NewMultiDimArray(RuntimeTypeHandle typeHandleForArrayType, int[] lengths, int[] lowerBounds);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,19 @@ protected MethodInvoker() { }
System.Diagnostics.DebugAnnotations.PreviousCallContainsDebuggerStepInCode();
return result;
}

[DebuggerGuidedStepThrough]
public object CreateInstance(object?[] arguments, Binder? binder, BindingFlags invokeAttr, CultureInfo? cultureInfo)
{
BinderBundle binderBundle = binder.ToBinderBundle(invokeAttr, cultureInfo);
bool wrapInTargetInvocationException = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
object result = CreateInstance(arguments, binderBundle, wrapInTargetInvocationException);
System.Diagnostics.DebugAnnotations.PreviousCallContainsDebuggerStepInCode();
return result;
}

protected abstract object? Invoke(object? thisObject, object?[]? arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException);
protected abstract object CreateInstance(object?[]? arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException);
public abstract Delegate CreateDelegate(RuntimeTypeHandle delegateType, object target, bool isStatic, bool isVirtual, bool isOpen);

// This property is used to retrieve the target method pointer. It is used by the RuntimeMethodHandle.GetFunctionPointer API
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,34 +66,9 @@ public static void InitializeStackTraceMetadataSupport(StackTraceMetadataCallbac
// Access to the underlying execution engine's object allocation routines.
//==============================================================================================

//
// Perform the equivalent of a "newobj", but without invoking any constructors. Other than the MethodTable, the result object is zero-initialized.
//
// Special cases:
//
// Strings: The .ctor performs both the construction and initialization
// and compiler special cases these.
//
// Nullable<T>: the boxed result is the underlying type rather than Nullable so the constructor
// cannot truly initialize it.
//
// In these cases, this helper returns "null" and ConstructorInfo.Invoke() must deal with these specially.
//
public static object NewObject(RuntimeTypeHandle typeHandle)
{
EETypePtr eeType = typeHandle.ToEETypePtr();
if (eeType.IsNullable
|| eeType == EETypePtr.EETypePtrOf<string>()
)
return null;
if (eeType.IsByRefLike)
throw new System.Reflection.TargetException();
return RuntimeImports.RhNewObject(eeType);
}

//
// Helper API to perform the equivalent of a "newobj" for any MethodTable.
// Unlike the NewObject API, this is the raw version that does not special case any MethodTable, and should be used with
// This is the raw version that does not special case any MethodTable, and should be used with
// caution for very specific scenarios.
//
public static object RawNewObject(RuntimeTypeHandle typeHandle)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,14 @@ public static object CreateInstance(
if (constructor == null)
{
if (type.IsValueType)
return RuntimeAugments.NewObject(type.TypeHandle);
{
RuntimeTypeHandle typeHandle = type.TypeHandle;

if (RuntimeAugments.IsNullable(typeHandle))
return null;

return RuntimeAugments.RawNewObject(typeHandle);
}

throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, type));
}
Expand Down Expand Up @@ -77,7 +84,14 @@ public static object CreateInstance(
if (matches.Count == 0)
{
if (numArgs == 0 && type.IsValueType)
return RuntimeAugments.NewObject(type.TypeHandle);
{
RuntimeTypeHandle typeHandle = type.TypeHandle;

if (RuntimeAugments.IsNullable(typeHandle))
return null;

return RuntimeAugments.RawNewObject(typeHandle);
}

throw new MissingMethodException(SR.Format(SR.Arg_NoDefCTor, type));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ public CustomMethodInvoker(Type thisType, Type[] parameterTypes, InvokerOptions
return result;
}

protected sealed override object CreateInstance(object?[]? arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException)
{
// Custom method invokers need to also create the instance, so we just pass a null this.
Debug.Assert((_options & InvokerOptions.AllowNullThis) != 0);
return Invoke(null, arguments, binderBundle, wrapInTargetInvocationException);
}

public sealed override Delegate CreateDelegate(RuntimeTypeHandle delegateType, object target, bool isStatic, bool isVirtual, bool isOpen)
{
if (_thisType.IsConstructedGenericType && _thisType.GetGenericTypeDefinition() == typeof(Nullable<>))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,11 @@ internal sealed class OpenMethodInvoker : MethodInvoker
throw new InvalidOperationException(SR.Arg_UnboundGenParam);
}

protected sealed override object CreateInstance(object?[]? arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException)
{
throw new InvalidOperationException(SR.Arg_UnboundGenParam);
}

public sealed override Delegate CreateDelegate(RuntimeTypeHandle delegateType, object target, bool isStatic, bool isVirtual, bool isOpen)
{
throw new InvalidOperationException(SR.Arg_UnboundGenParam);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,9 @@ public sealed override Type DeclaringType
[DebuggerGuidedStepThrough]
public sealed override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[]? parameters, CultureInfo? culture)
{
// Most objects are allocated by NewObject and their constructors return "void". But in many frameworks,
// there are "weird" cases (e.g. String) where the constructor must do both the allocation and initialization.
// Reflection.Core does not hardcode these special cases. It's up to the ExecutionEnvironment to steer
// us the right way by coordinating the implementation of NewObject and MethodInvoker.
object newObject = ReflectionCoreExecution.ExecutionEnvironment.NewObject(this.DeclaringType.TypeHandle);
object ctorAllocatedObject = this.MethodInvoker.Invoke(newObject, parameters, binder, invokeAttr, culture)!;
object ctorAllocatedObject = this.MethodInvoker.CreateInstance(parameters, binder, invokeAttr, culture);
System.Diagnostics.DebugAnnotations.PreviousCallContainsDebuggerStepInCode();
return newObject ?? ctorAllocatedObject;
return ctorAllocatedObject;
}

public sealed override MethodBase MetadataDefinitionMethod
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,6 @@ namespace Internal.Reflection.Execution
//==========================================================================================================
internal sealed partial class ExecutionEnvironmentImplementation : ExecutionEnvironment
{
public sealed override object NewObject(RuntimeTypeHandle typeHandle)
{
return RuntimeAugments.NewObject(typeHandle);
}

public sealed override Array NewArray(RuntimeTypeHandle typeHandleForArrayType, int count)
{
return RuntimeAugments.NewArray(typeHandleForArrayType, count);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using global::System;
using global::System.Threading;
using global::System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using global::System.Diagnostics;
using global::System.Collections.Generic;
Expand All @@ -18,12 +19,29 @@ namespace Internal.Reflection.Execution.MethodInvokers
//
// Implements Invoke() for non-virtual instance methods.
//
internal sealed class InstanceMethodInvoker : MethodInvokerWithMethodInvokeInfo
internal sealed unsafe class InstanceMethodInvoker : MethodInvokerWithMethodInvokeInfo
{
public InstanceMethodInvoker(MethodInvokeInfo methodInvokeInfo, RuntimeTypeHandle declaringTypeHandle)
: base(methodInvokeInfo)
{
_declaringTypeHandle = declaringTypeHandle;

if (methodInvokeInfo.Method.IsConstructor && !methodInvokeInfo.Method.IsStatic)
{
if (RuntimeAugments.IsByRefLike(declaringTypeHandle))
{
_allocatorMethod = &ThrowTargetException;
}
else
{
_allocatorMethod = (delegate*<nint, object>)RuntimeAugments.GetAllocateObjectHelperForType(declaringTypeHandle);
}
}
}

private static object ThrowTargetException(IntPtr _)
{
throw new TargetException();
}

[DebuggerGuidedStepThroughAttribute]
Expand All @@ -44,6 +62,20 @@ public InstanceMethodInvoker(MethodInvokeInfo methodInvokeInfo, RuntimeTypeHandl
return result;
}

[DebuggerGuidedStepThroughAttribute]
protected sealed override object CreateInstance(object[] arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException)
{
object thisObject = RawCalliHelper.Call<object>(_allocatorMethod, _declaringTypeHandle.Value);
MethodInvokeInfo.Invoke(
thisObject,
MethodInvokeInfo.LdFtnResult,
arguments,
binderBundle,
wrapInTargetInvocationException);
System.Diagnostics.DebugAnnotations.PreviousCallContainsDebuggerStepInCode();
return thisObject;
}

public sealed override Delegate CreateDelegate(RuntimeTypeHandle delegateType, object target, bool isStatic, bool isVirtual, bool isOpen)
{
if (isOpen)
Expand Down Expand Up @@ -74,5 +106,13 @@ public sealed override Delegate CreateDelegate(RuntimeTypeHandle delegateType, o
public sealed override IntPtr LdFtnResult => MethodInvokeInfo.LdFtnResult;

private RuntimeTypeHandle _declaringTypeHandle;
private delegate*<nint, object> _allocatorMethod;

private static class RawCalliHelper
{
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static T Call<T>(delegate*<IntPtr, T> pfn, IntPtr arg)
=> pfn(arg);
}
Comment on lines +111 to +116
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @yowl; this will need to be adjusted to use the same calling convention transform we do for Activator.CreateInstance in the next integration.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI @yowl; this will need to be adjusted to use the same calling convention transform we do for Activator.CreateInstance in the next integration.

Is the issue that I used IntPtr instead of MethodTable*? Otherwise this should be just a "regular" calli. It's in a RawCalliHelper to avoid the fat pointer penalty (we know the target doesn't need a generic context) but otherwise just any other user calli.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the issue is the mismatch of calling conventions (equivalently, the assumption that they are the same between managed and native). This is a managed calli, so it will get the shadow stack argument which the native allocator methods do not have. We solve this downstream via detouring through native code. It could be solved upstream, but not sure there is appetite for that, additionally, I suspect we'll start using custom allocators that have the shadow stack argument sooner or later for performance reasons.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see, thanks and sorry for the extra trouble!

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,11 @@ public StaticMethodInvoker(MethodInvokeInfo methodInvokeInfo)
return result;
}

protected sealed override object CreateInstance(object[] arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException)
{
throw NotImplemented.ByDesign;
}

public sealed override IntPtr LdFtnResult => MethodInvokeInfo.LdFtnResult;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ public sealed override Delegate CreateDelegate(RuntimeTypeHandle delegateType, o
return result;
}

protected sealed override object CreateInstance(object[] arguments, BinderBundle binderBundle, bool wrapInTargetInvocationException)
{
throw NotImplemented.ByDesign;
}

internal IntPtr ResolveTarget(RuntimeTypeHandle type)
{
return OpenMethodResolver.ResolveMethod(MethodInvokeInfo.VirtualResolveData, type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -369,7 +369,7 @@ private static void CreateEETypeWorker(MethodTable* pTemplateEEType, uint hashCo
if (state.GcDataSize != 0)
{
// Statics are allocated on GC heap
object obj = RuntimeAugments.NewObject(((MethodTable*)state.GcStaticDesc)->ToRuntimeTypeHandle());
object obj = RuntimeAugments.RawNewObject(((MethodTable*)state.GcStaticDesc)->ToRuntimeTypeHandle());
gcStaticData = RuntimeAugments.RhHandleAlloc(obj, GCHandleType.Normal);

pEEType->DynamicGcStaticsData = gcStaticData;
Expand Down