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

Reimplement a few helpers with new RuntimeHelpers APIs #100846

Merged
merged 5 commits into from
Apr 11, 2024
Merged

Reimplement a few helpers with new RuntimeHelpers APIs #100846

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8abf3a9
Reimplement InternalBoxEnum using RuntimeHelpers.Box and SizeOf
jkoritzinsky Apr 9, 2024
04dbf13
Implement AllocateValueType with the new RuntimeHelpers APIs.
jkoritzinsky Apr 9, 2024
b9753aa
Update icall-def.h
jkoritzinsky Apr 10, 2024
d703087
Fix AllocateValueType behavior for Nullable<T>
jkoritzinsky Apr 10, 2024
956d362
Update src/libraries/System.Private.CoreLib/src/System/Enum.cs
jkoritzinsky Apr 10, 2024
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
3 changes: 0 additions & 3 deletions src/coreclr/System.Private.CoreLib/src/System/Enum.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,6 @@ public abstract partial class Enum
[LibraryImport(RuntimeHelpers.QCall, EntryPoint = "Enum_GetValuesAndNames")]
private static partial void GetEnumValuesAndNames(QCallTypeHandle enumType, ObjectHandleOnStack values, ObjectHandleOnStack names, Interop.BOOL getNames);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object InternalBoxEnum(RuntimeType enumType, long value);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern unsafe CorElementType InternalGetCorElementType(MethodTable* pMT);

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/System.Private.CoreLib/src/System/Reflection/MdConstant.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ internal static class MdConstant
#endregion
}

return RuntimeType.CreateEnum(fieldType, defaultValue);
return Enum.ToObject(fieldType, defaultValue);
}
else if (fieldType == typeof(DateTime))
{
Expand Down
11 changes: 0 additions & 11 deletions src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3664,9 +3664,6 @@ public override Type MakeArrayType(int rank)
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern bool CanValueSpecialCast(RuntimeType valueType, RuntimeType targetType);

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern object AllocateValueType(RuntimeType type, object? value);

private CheckValueStatus TryChangeTypeSpecial(ref object value)
{
Pointer? pointer = value as Pointer;
Expand Down Expand Up @@ -3973,14 +3970,6 @@ internal object GetUninitializedObject()

#region Legacy internal static

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object _CreateEnum(RuntimeType enumType, long value);

internal static object CreateEnum(RuntimeType enumType, long value)
{
return _CreateEnum(enumType, value);
}

#if FEATURE_COMINTEROP
[MethodImpl(MethodImplOptions.InternalCall)]
private extern object InvokeDispMethod(
Expand Down
52 changes: 2 additions & 50 deletions src/coreclr/nativeaot/System.Private.CoreLib/src/System/Enum.NativeAot.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,10 +62,8 @@ internal static EnumInfo<TStorage> GetEnumInfo<TStorage>(RuntimeType enumType, b
}
#pragma warning restore

private static unsafe object InternalBoxEnum(Type enumType, long value)
{
return ToObject(enumType.TypeHandle.ToMethodTable(), value);
}
internal static unsafe object ToObject(MethodTable* mt, long value)
=> InternalBoxEnum(new RuntimeTypeHandle(mt), value);

private static unsafe CorElementType InternalGetCorElementType(RuntimeType rt)
{
Expand Down Expand Up @@ -151,51 +149,5 @@ internal static Type InternalGetUnderlyingType(RuntimeType enumType)

return GetEnumInfo(enumType).UnderlyingType;
}

[Conditional("BIGENDIAN")]
private static unsafe void AdjustForEndianness(ref byte* pValue, MethodTable* enumEEType)
{
// On Debug builds, include the big-endian code to help deter bitrot (the "Conditional("BIGENDIAN")" will prevent it from executing on little-endian).
// On Release builds, exclude code to deter IL bloat and toolchain work.
#if BIGENDIAN || DEBUG
EETypeElementType elementType = enumEEType->ElementType;
switch (elementType)
{
case EETypeElementType.SByte:
case EETypeElementType.Byte:
pValue += sizeof(long) - sizeof(byte);
break;

case EETypeElementType.Int16:
case EETypeElementType.UInt16:
pValue += sizeof(long) - sizeof(short);
break;

case EETypeElementType.Int32:
case EETypeElementType.UInt32:
pValue += sizeof(long) - sizeof(int);
break;

case EETypeElementType.Int64:
case EETypeElementType.UInt64:
break;

default:
throw new NotSupportedException();
}
#endif //BIGENDIAN || DEBUG
}

#region ToObject

internal static unsafe object ToObject(MethodTable* enumEEType, long value)
{
Debug.Assert(enumEEType->IsEnum);

byte* pValue = (byte*)&value;
AdjustForEndianness(ref pValue, enumEEType);
return RuntimeImports.RhBox(enumEEType, ref *pValue);
}
#endregion
}
}
3 changes: 0 additions & 3 deletions src/coreclr/vm/ecalllist.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ FCFuncEnd()

FCFuncStart(gEnumFuncs)
FCFuncElement("InternalGetCorElementType", ReflectionEnum::InternalGetCorElementType)
FCFuncElement("InternalBoxEnum", ReflectionEnum::InternalBoxEnum)
FCFuncEnd()

FCFuncStart(gObjectFuncs)
Expand Down Expand Up @@ -114,9 +113,7 @@ FCFuncEnd()

FCFuncStart(gSystem_RuntimeType)
FCFuncElement("GetGUID", ReflectionInvocation::GetGUID)
FCFuncElement("_CreateEnum", ReflectionInvocation::CreateEnum)
FCFuncElement("CanValueSpecialCast", ReflectionInvocation::CanValueSpecialCast)
FCFuncElement("AllocateValueType", ReflectionInvocation::AllocateValueType)
#if defined(FEATURE_COMINTEROP)
FCFuncElement("InvokeDispMethod", ReflectionInvocation::InvokeDispMethod)
#endif // defined(FEATURE_COMINTEROP)
Expand Down
84 changes: 1 addition & 83 deletions src/coreclr/vm/reflectioninvocation.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,54 +113,6 @@ FCIMPL2(FC_BOOL_RET, ReflectionInvocation::CanValueSpecialCast, ReflectClassBase
}
FCIMPLEND

/// <summary>
/// Allocate the value type and copy the optional value into it.
/// </summary>
FCIMPL2(Object*, ReflectionInvocation::AllocateValueType, ReflectClassBaseObject *pTargetTypeUNSAFE, Object *valueUNSAFE) {
CONTRACTL
{
FCALL_CHECK;
PRECONDITION(CheckPointer(pTargetTypeUNSAFE));
PRECONDITION(CheckPointer(valueUNSAFE, NULL_OK));
}
CONTRACTL_END;

struct _gc
{
REFLECTCLASSBASEREF refTargetType;
OBJECTREF value;
OBJECTREF obj;
}gc;

gc.value = ObjectToOBJECTREF(valueUNSAFE);
gc.obj = gc.value;
gc.refTargetType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pTargetTypeUNSAFE);

HELPER_METHOD_FRAME_BEGIN_RET_PROTECT(gc);

TypeHandle targetType = gc.refTargetType->GetType();

// This method is only intended for value types; it is not called directly by any public APIs
// so we don't expect validation issues here.
_ASSERTE(targetType.IsValueType());

MethodTable* allocMT = targetType.AsMethodTable();
_ASSERTE(!allocMT->IsByRefLike());

gc.obj = allocMT->Allocate();
_ASSERTE(gc.obj != NULL);

if (gc.value != NULL) {
_ASSERTE(allocMT->IsEquivalentTo(gc.value->GetMethodTable()));
CopyValueClass(gc.obj->UnBox(), gc.value->UnBox(), allocMT);
}

HELPER_METHOD_FRAME_END();

return OBJECTREFToObject(gc.obj);
}
FCIMPLEND

FCIMPL6(void, RuntimeFieldHandle::SetValue, ReflectFieldObject *pFieldUNSAFE, Object *targetUNSAFE, Object *valueUNSAFE, ReflectClassBaseObject *pFieldTypeUNSAFE, ReflectClassBaseObject *pDeclaringTypeUNSAFE, CLR_BOOL *pIsClassInitialized) {
CONTRACTL
{
Expand Down Expand Up @@ -1535,24 +1487,6 @@ FCIMPL4(void, ReflectionInvocation::MakeTypedReference, TypedByRef * value, Obje
}
FCIMPLEND

FCIMPL2_IV(Object*, ReflectionInvocation::CreateEnum, ReflectClassBaseObject *pTypeUNSAFE, INT64 value) {
FCALL_CONTRACT;

REFLECTCLASSBASEREF refType = (REFLECTCLASSBASEREF)ObjectToOBJECTREF(pTypeUNSAFE);

TypeHandle typeHandle = refType->GetType();
_ASSERTE(typeHandle.IsEnum());
OBJECTREF obj = NULL;
HELPER_METHOD_FRAME_BEGIN_RET_1(refType);
MethodTable *pEnumMT = typeHandle.AsMethodTable();
obj = pEnumMT->Box(ArgSlotEndiannessFixup ((ARG_SLOT*)&value,
pEnumMT->GetNumInstanceFieldBytes()));

HELPER_METHOD_FRAME_END();
return OBJECTREFToObject(obj);
}
FCIMPLEND

#ifdef FEATURE_COMINTEROP
FCIMPL8(Object*, ReflectionInvocation::InvokeDispMethod, ReflectClassBaseObject* refThisUNSAFE,
StringObject* nameUNSAFE,
Expand Down Expand Up @@ -1965,7 +1899,7 @@ extern "C" void QCALLTYPE ReflectionSerialization_GetCreateUninitializedObjectIn
bool fHasSideEffectsUnused;
*ppfnAllocator = CEEJitInfo::getHelperFtnStatic(CEEInfo::getNewHelperStatic(pMT, &fHasSideEffectsUnused));
*pvAllocatorFirstArg = pMT;

pMT->EnsureInstanceActive();

if (pMT->HasPreciseInitCctors())
Expand Down Expand Up @@ -2105,22 +2039,6 @@ extern "C" void QCALLTYPE Enum_GetValuesAndNames(QCall::TypeHandle pEnumType, QC
END_QCALL;
}

FCIMPL2_IV(Object*, ReflectionEnum::InternalBoxEnum, ReflectClassBaseObject* target, INT64 value) {
FCALL_CONTRACT;

VALIDATEOBJECT(target);
OBJECTREF ret = NULL;

MethodTable* pMT = target->GetType().AsMethodTable();
HELPER_METHOD_FRAME_BEGIN_RET_0();

ret = pMT->Box(ArgSlotEndiannessFixup((ARG_SLOT*)&value, pMT->GetNumInstanceFieldBytes()));

HELPER_METHOD_FRAME_END();
return OBJECTREFToObject(ret);
}
FCIMPLEND

extern "C" int32_t QCALLTYPE ReflectionInvocation_SizeOf(QCall::TypeHandle pType)
{
QCALL_CONTRACT_NO_GC_TRANSITION;
Expand Down
3 changes: 0 additions & 3 deletions src/coreclr/vm/reflectioninvocation.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,9 @@ class ReflectionInvocation {
static FCDECL8(Object*, InvokeDispMethod, ReflectClassBaseObject* refThisUNSAFE, StringObject* nameUNSAFE, INT32 invokeAttr, Object* targetUNSAFE, PTRArray* argsUNSAFE, PTRArray* byrefModifiersUNSAFE, LCID lcid, PTRArray* namedParametersUNSAFE);
#endif // FEATURE_COMINTEROP
static FCDECL2(void, GetGUID, ReflectClassBaseObject* refThisUNSAFE, GUID * result);
static FCDECL2_IV(Object*, CreateEnum, ReflectClassBaseObject *pTypeUNSAFE, INT64 value);

// helper fcalls for invocation
static FCDECL2(FC_BOOL_RET, CanValueSpecialCast, ReflectClassBaseObject *valueType, ReflectClassBaseObject *targetType);
static FCDECL2(Object*, AllocateValueType, ReflectClassBaseObject *targetType, Object *valueUNSAFE);
};

extern "C" void QCALLTYPE ReflectionInvocation_CompileMethod(MethodDesc * pMD);
Expand All @@ -77,7 +75,6 @@ extern "C" void QCALLTYPE ReflectionSerialization_GetCreateUninitializedObjectIn
class ReflectionEnum {
public:
static FCDECL1(INT32, InternalGetCorElementType, MethodTable* pMT);
static FCDECL2_IV(Object*, InternalBoxEnum, ReflectClassBaseObject* pEnumType, INT64 value);
};

extern "C" void QCALLTYPE Enum_GetValuesAndNames(QCall::TypeHandle pEnumType, QCall::ObjectHandleOnStack pReturnValues, QCall::ObjectHandleOnStack pReturnNames, BOOL fGetNames);
Expand Down
35 changes: 26 additions & 9 deletions src/libraries/System.Private.CoreLib/src/System/Enum.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
using System.Numerics;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;

#pragma warning disable 8500 // Allow taking address of managed types

Expand Down Expand Up @@ -768,7 +769,7 @@ private static unsafe bool TryParse(Type enumType, ReadOnlySpan<char> value, boo
break;
}

result = parsed ? InternalBoxEnum(rt, longScratch) : null;
result = parsed ? InternalBoxEnum(rt.TypeHandle, longScratch) : null;
return parsed;

[MethodImpl(MethodImplOptions.NoInlining)]
Expand Down Expand Up @@ -2225,31 +2226,47 @@ public static object ToObject(Type enumType, object value)

[CLSCompliant(false)]
public static object ToObject(Type enumType, sbyte value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

public static object ToObject(Type enumType, short value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

public static object ToObject(Type enumType, int value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

public static object ToObject(Type enumType, byte value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

[CLSCompliant(false)]
public static object ToObject(Type enumType, ushort value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

[CLSCompliant(false)]
public static object ToObject(Type enumType, uint value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

public static object ToObject(Type enumType, long value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), value);
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, value);

[CLSCompliant(false)]
public static object ToObject(Type enumType, ulong value) =>
InternalBoxEnum(ValidateRuntimeType(enumType), unchecked((long)value));
InternalBoxEnum(ValidateRuntimeType(enumType).TypeHandle, unchecked((long)value));

private static object InternalBoxEnum(RuntimeTypeHandle type, long value)
{
ReadOnlySpan<byte> rawData = MemoryMarshal.AsBytes(new ReadOnlySpan<long>(ref value));
// On little-endian systems, we can always use the pointer to the start of the scratch space
// as memory layout since the least-significant bit is at the lowest address.
// For big-endian systems, the least-significant bit is at the highest address, so we need to adjust
// our starting ref to the correct offset from the end of the scratch space to get the value to box.
if (!BitConverter.IsLittleEndian)
{
int size = RuntimeHelpers.SizeOf(type);
rawData = rawData.Slice(sizeof(long) - size);
}

return RuntimeHelpers.Box(ref MemoryMarshal.GetReference(rawData), type)!;
Copy link
Member

Choose a reason for hiding this comment

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

This does some extra work (e.g. argument validation) compared to current state. Is it a measurable perf regression?

Copy link
Member Author

Choose a reason for hiding this comment

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

I wrote some basic benchmarks for Enum.ToObject and actually saw an improvement.

My benchmarks:

    private enum E
    {
        A,
        B,
        C
    }

    [Benchmark]
    public object ToObjectSameSize()
    {
        return Enum.ToObject(typeof(E), 1);
    }

    [Benchmark]
    public object ToObjectLarger()
    {
        return Enum.ToObject(typeof(E), 1L);
    }

    [Benchmark]
    public object ToObjectBoxed()
    {
        return Enum.ToObject(typeof(E), (object)1u);
    }

Results:


BenchmarkDotNet v0.13.12, Windows 10 (10.0.19045.4291/22H2/2022Update)
Intel Core i7-7700 CPU 3.60GHz (Kaby Lake), 1 CPU, 8 logical and 4 physical cores
.NET SDK 9.0.100-preview.2.24157.14
  [Host]     : .NET 7.0.5 (7.0.523.17405), X64 RyuJIT AVX2
  Job-BRSDCR : .NET 9.0.0 (42.42.42.42424), X64 RyuJIT AVX2
  Job-ASHIII : .NET 9.0.0 (9.0.24.12805), X64 RyuJIT AVX2
Method Runtime Mean Error StdDev Ratio RatioSD
ToObjectSameSize CoreRun 37.40 ns 0.809 ns 1.480 ns 0.90 0.03
ToObjectSameSize .NET 9.0 42.90 ns 0.170 ns 0.142 ns 1.00 0.00
ToObjectLarger CoreRun 38.20 ns 0.459 ns 0.407 ns 0.89 0.01
ToObjectLarger .NET 9.0 43.12 ns 0.385 ns 0.341 ns 1.00 0.00
ToObjectBoxed CoreRun 40.88 ns 0.127 ns 0.106 ns 0.80 0.01
ToObjectBoxed .NET 9.0 51.13 ns 0.874 ns 0.774 ns 1.00 0.00

}

internal static bool AreSequentialFromZero<TStorage>(TStorage[] values) where TStorage : struct, INumber<TStorage>
{
Expand Down
1 change: 1 addition & 0 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 @@ -402,6 +402,7 @@ private static bool TryByRefFastPath(RuntimeType type, ref object arg)
{
if (sigElementType.IsValueType)
{
Debug.Assert(!sigElementType.IsNullableOfT, "A true boxed Nullable<T> should never be here.");
// Make a copy to prevent the boxed instance from being directly modified by the method.
arg = RuntimeType.AllocateValueType(sigElementType, arg);
}
Expand Down
25 changes: 25 additions & 0 deletions src/libraries/System.Private.CoreLib/src/System/RuntimeType.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,31 @@ internal bool CheckValue(
return false;
}

[UnconditionalSuppressMessage("ReflectionAnalysis", "IL2067:UnrecognizedReflectionPattern",
Justification = "AllocateValueType is only called on a ValueType. You can always create an instance of a ValueType.")]
[return: NotNullIfNotNull(nameof(value))]
internal static object? AllocateValueType(RuntimeType type, object? value)
{
Debug.Assert(type.IsValueType);
Debug.Assert(!type.IsByRefLike);

if (value is not null)
{
// Make a copy of the provided value by re-boxing the existing value's underlying data.
Debug.Assert(type.IsEquivalentTo(value.GetType()));
return RuntimeHelpers.Box(ref RuntimeHelpers.GetRawData(value), type.TypeHandle)!;
}

if (type.IsNullableOfT)
{
// If the type is Nullable<T>, then create a true boxed Nullable<T> of the default Nullable<T> value.
return RuntimeMethodHandle.ReboxToNullable(null, type);
}

// Otherwise, just create a default instance of the type.
return RuntimeHelpers.GetUninitializedObject(type);
}

private CheckValueStatus TryChangeType(ref object? value, ref bool copyBack)
{
RuntimeType? sigElementType;
Expand Down
10 changes: 0 additions & 10 deletions src/mono/System.Private.CoreLib/src/System/Enum.Mono.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,22 +15,12 @@ public partial class Enum
[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void GetEnumValuesAndNames(QCallTypeHandle enumType, out ulong[] values, out string[] names);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void InternalBoxEnum(QCallTypeHandle enumType, ObjectHandleOnStack res, long value);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern CorElementType InternalGetCorElementType(QCallTypeHandle enumType);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void InternalGetUnderlyingType(QCallTypeHandle enumType, ObjectHandleOnStack res);

private static object InternalBoxEnum(RuntimeType enumType, long value)
{
object? res = null;
InternalBoxEnum(new QCallTypeHandle(ref enumType), ObjectHandleOnStack.Create(ref res), value);
return res!;
}

private static unsafe CorElementType InternalGetCorElementType(RuntimeType rt)
{
Debug.Assert(rt.IsActualEnum);
Expand Down
Loading
Loading