Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Enable Mono for using faster invoke stubs #89108

Merged
merged 4 commits into from
Jul 25, 2023
Merged

Enable Mono for using faster invoke stubs #89108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1f001d5
Unbox a true nullable without Opcodes.Unbox
steveharter Jul 18, 2023
37f55c3
Normalize UIntPtr to IntPtr for Mono
steveharter Jul 19, 2023
24a6cab
Use faster unbox
steveharter Jul 20, 2023
41aa0e7
Remove unnecessary casts since Unbox is not called
steveharter Jul 25, 2023
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
8 changes: 1 addition & 7 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/InvokeUtils.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,18 +111,12 @@ public static object ConvertOrWiden(Type srcType, object srcObject, Type dstType

private static bool TryConvertPointer(object srcObject, [NotNullWhen(true)] out object? dstPtr)
{
if (srcObject is IntPtr)
if (srcObject is IntPtr or UIntPtr)
{
dstPtr = srcObject;
return true;
}

if (srcObject is UIntPtr)
{
dstPtr = (IntPtr)(UIntPtr)srcObject;
return true;
}

// The source pointer should already have been converted to an IntPtr.
Debug.Assert(srcObject is not Pointer);

Expand Down
21 changes: 17 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/InvokerEmitUtil.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,11 @@ public static unsafe InvokeFunc_Obj4Args CreateInvokeDelegate_Obj4Args(MethodBas

if (parameterType.IsPointer)
{
il.Emit(OpCodes.Unbox_Any, typeof(IntPtr));
Unbox(il, typeof(IntPtr));
}
else if (parameterType.IsValueType)
{
il.Emit(OpCodes.Unbox_Any, parameterType);
Unbox(il, parameterType);
}
}

Expand Down Expand Up @@ -126,11 +126,11 @@ public static unsafe InvokeFunc_ObjSpanArgs CreateInvokeDelegate_ObjSpanArgs(Met

if (parameterType.IsPointer)
{
il.Emit(OpCodes.Unbox_Any, typeof(IntPtr));
Unbox(il, typeof(IntPtr));
}
else if (parameterType.IsValueType)
{
il.Emit(OpCodes.Unbox_Any, parameterType);
Unbox(il, parameterType);
}
}

Expand Down Expand Up @@ -196,6 +196,15 @@ public static unsafe InvokeFunc_RefArgs CreateInvokeDelegate_RefArgs(MethodBase
return (InvokeFunc_RefArgs)dm.CreateDelegate(typeof(InvokeFunc_RefArgs), target: null);
}

private static void Unbox(ILGenerator il, Type parameterType)
{
// Unbox without using OpCodes.Unbox\UnboxAny to avoid a type check since that was already done by reflection.
// Also required for unboxing true nullables created by reflection since that is not necessarily a valid CLI operation.
Debug.Assert(parameterType.IsValueType);
il.Emit(OpCodes.Call, Methods.Object_GetRawData());
il.Emit(OpCodes.Ldobj, parameterType);
steveharter marked this conversation as resolved.
Show resolved Hide resolved
}

private static void EmitCallAndReturnHandling(ILGenerator il, MethodBase method, bool emitNew, bool backwardsCompat)
{
// For CallStack reasons, don't inline target method.
Expand Down Expand Up @@ -316,6 +325,10 @@ public static MethodInfo Span_get_Item() =>
public static MethodInfo ThrowHelper_Throw_NullReference_InvokeNullRefReturned() =>
s_ThrowHelper_Throw_NullReference_InvokeNullRefReturned ??= typeof(ThrowHelper).GetMethod(nameof(ThrowHelper.Throw_NullReference_InvokeNullRefReturned))!;

private static MethodInfo? s_Object_GetRawData;
public static MethodInfo Object_GetRawData() =>
s_Object_GetRawData ??= typeof(RuntimeHelpers).GetMethod(nameof(RuntimeHelpers.GetRawData), BindingFlags.NonPublic | BindingFlags.Static)!;

private static MethodInfo? s_Pointer_Box;
public static MethodInfo Pointer_Box() =>
s_Pointer_Box ??= typeof(Pointer).GetMethod(nameof(Pointer.Box), new[] { typeof(void*), typeof(Type) })!;
Expand Down
27 changes: 14 additions & 13 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/MethodBaseInvoker.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ internal static void ThrowTargetParameterCountException()
bool copyBack = false;
Span<bool> shouldCopyBack = new(ref copyBack);

CheckArguments(parametersSpan, copyOfArgs, shouldCopyBack, binder, culture, invokeAttr);
steveharter marked this conversation as resolved.
Show resolved Hide resolved

object? ret;
if ((_strategy & InvokerStrategy.StrategyDetermined_ObjSpanArgs) == 0)
{
DetermineStrategy_ObjSpanArgs(ref _strategy, ref _invokeFunc_ObjSpanArgs, _method, _needsByRefStrategy, backwardsCompat: true);
}

CheckArguments(parametersSpan, copyOfArgs, shouldCopyBack, binder, culture, invokeAttr);

if (_invokeFunc_ObjSpanArgs is not null)
{
try
Expand All @@ -98,12 +98,12 @@ internal static void ThrowTargetParameterCountException()
{
throw new TargetInvocationException(e);
}

CopyBack(parameters, copyOfArgs, shouldCopyBack);
return ret;
}
else
{
ret = InvokeDirectByRefWithFewArgs(obj, copyOfArgs, invokeAttr);
}

ret = InvokeDirectByRefWithFewArgs(obj, copyOfArgs, invokeAttr);
CopyBack(parameters, copyOfArgs, shouldCopyBack);
return ret;
}
Expand All @@ -121,14 +121,14 @@ internal static void ThrowTargetParameterCountException()
Span<object?> copyOfArgs = stackArgStorage._args.AsSpan(_argCount);
Span<bool> shouldCopyBack = stackArgStorage._shouldCopyBack.AsSpan(_argCount);

CheckArguments(parameters, copyOfArgs, shouldCopyBack, binder, culture, invokeAttr);

object? ret;
if ((_strategy & InvokerStrategy.StrategyDetermined_ObjSpanArgs) == 0)
{
DetermineStrategy_ObjSpanArgs(ref _strategy, ref _invokeFunc_ObjSpanArgs, _method, _needsByRefStrategy, backwardsCompat: true);
}

CheckArguments(parameters, copyOfArgs, shouldCopyBack, binder, culture, invokeAttr);

if (_invokeFunc_ObjSpanArgs is not null)
{
try
Expand All @@ -139,12 +139,13 @@ internal static void ThrowTargetParameterCountException()
{
throw new TargetInvocationException(e);
}

CopyBack(parameters, copyOfArgs, shouldCopyBack);
return ret;
}
else
{
ret = InvokeDirectByRefWithFewArgs(obj, copyOfArgs, invokeAttr);

}

ret = InvokeDirectByRefWithFewArgs(obj, copyOfArgs, invokeAttr);
CopyBack(parameters, copyOfArgs, shouldCopyBack);
return ret;
}
Expand Down Expand Up @@ -378,7 +379,7 @@ BindingFlags invokeAttr
}
else if (!ReferenceEquals(arg.GetType(), sigType))
{
// Determine if we can use the fast path for byref types
// Determine if we can use the fast path for byref types.
if (TryByRefFastPath(sigType, ref arg))
{
// Fast path when the value's type matches the signature type of a byref parameter.
Expand Down
6 changes: 2 additions & 4 deletions src/libraries/System.Private.CoreLib/src/System/Reflection/MethodInvokerCommon.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,11 @@ ref InvokeFunc_ObjSpanArgs?
}
else
{
#if !MONO // Temporary until Mono can unbox a true Nullable<T>
if (RuntimeFeature.IsDynamicCodeSupported)
{
invokeFunc_ObjSpanArgs = CreateInvokeDelegate_ObjSpanArgs(method, backwardsCompat);
}
#endif

strategy |= InvokerStrategy.StrategyDetermined_ObjSpanArgs;
}
}
Expand All @@ -136,12 +135,11 @@ internal static void DetermineStrategy_Obj4Args(
}
else
{
#if !MONO // Temporary until Mono can unbox a true Nullable<T>
if (RuntimeFeature.IsDynamicCodeSupported)
{
invokeFunc_Obj4Args = CreateInvokeDelegate_Obj4Args(method, backwardsCompat);
}
#endif

strategy |= InvokerStrategy.StrategyDetermined_Obj4Args;
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/mono/System.Private.CoreLib/src/System/RuntimeType.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1792,9 +1792,9 @@ private CheckValueStatus TryChangeTypeSpecial(
}
else if (IsPointer)
{
Type? vtype = value.GetType();
if (vtype == typeof(IntPtr) || vtype == typeof(UIntPtr))
if (value is IntPtr or UIntPtr)
return CheckValueStatus.Success;

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed ?

Copy link
Member Author

@steveharter steveharter Jul 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for this. With the last commit, it is no longer necessary. Also removed from core CLR.

It was necessary previously since the Opcodes.UnboxAny would throw a InvalidCastException for UIntPtr arguments.

if (value is Pointer pointer)
{
Type pointerType = pointer.GetPointerType();
Expand Down