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.

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

Move unboxing helpers to managed code #109135

Merged
merged 19 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
c70a57c
Move unboxing helpers to managed code
davidwrighton Oct 23, 2024
f1258a4
Add boxinghelpers.cs
davidwrighton Oct 23, 2024
02636f8
Fix issues noted in CI/Review
davidwrighton Oct 24, 2024
d8e01e3
Fix more issues found in CI
davidwrighton Oct 24, 2024
b2c6991
Fix issue where UnboxNoCheck writes too many bytes when unboxing a tr…
davidwrighton Oct 25, 2024
12d17e5
Move unboxing helpers to CastHelpers class
davidwrighton Oct 28, 2024
3f281b9
Before conversion to ref byte everywhere
davidwrighton Oct 30, 2024
8dfaedc
It should all work now, and be fast
davidwrighton Oct 30, 2024
be624da
Cleanup dead code
davidwrighton Oct 30, 2024
dec085f
Update src/coreclr/System.Private.CoreLib/src/System/Runtime/Compiler…
davidwrighton Oct 30, 2024
e22038b
Merge branch 'main' of https://github.com/dotnet/runtime into Unboxin…
davidwrighton Oct 30, 2024
e44fab4
Fix build break
davidwrighton Oct 30, 2024
2964105
And finish up the details around optimizing for the fastest case in U…
davidwrighton Oct 30, 2024
6469fbb
Merge branch 'UnboxingHelpers' of https://github.com/davidwrighton/ru…
davidwrighton Oct 30, 2024
1e51dd2
- Remove unused helper functions InitValueClass and InitValueClassPtr
davidwrighton Oct 31, 2024
ee6d42d
Performance seeking behavior has finished. The perf is amazing
davidwrighton Nov 13, 2024
7761ad8
Create GetNumInstanceFieldBytesIfContainsGCPointers helper and use it…
davidwrighton Nov 13, 2024
55e0df9
Last minute changes
davidwrighton Nov 13, 2024
6a8d1ae
Update wording
davidwrighton Nov 19, 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
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,7 @@
<Compile Include="$(BclSourcesRoot)\System\Reflection\RuntimePropertyInfo.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\TypeNameResolver.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Metadata\RuntimeTypeMetadataUpdateHandler.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\BoxingHelpers.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\CastHelpers.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\GenericsHelpers.cs" />
<Compile Include="$(BclSourcesRoot)\System\Runtime\CompilerServices\InitHelpers.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,7 @@ private static unsafe void CopyImplUnBoxEachElement(Array sourceArray, int sourc

if (pDestMT->IsNullable)
{
RuntimeHelpers.Unbox_Nullable(ref dest, pDestMT, obj);
BoxingHelpers.Unbox_Nullable(ref dest, pDestMT, obj);
}
else if (obj is null || RuntimeHelpers.GetMethodTable(obj) != pDestMT)
{
Expand Down Expand Up @@ -546,7 +546,7 @@ private unsafe void InternalSetValue(object? value, nint flattenedIndex)
{
if (pElementMethodTable->IsNullable)
{
RuntimeHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value);
BoxingHelpers.Unbox_Nullable(ref offsetDataRef, pElementMethodTable, value);
}
else
{
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
// 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.InteropServices;

namespace System.Runtime.CompilerServices
{
[StackTraceHidden]
[DebuggerStepThrough]
internal static unsafe partial class BoxingHelpers
Copy link
Member

Choose a reason for hiding this comment

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

Most of these types are going to be loaded on startup path. It feels a bit too fine grained to have separate type for the few unboxing helpers.

Would it be better for the unboxing helpers to live in CastHelpers? They are coupled anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, seems reasonable to me. I had thought I might be moving the allocation based helpers over too so this class wouldn't be so small, but honestly, just copying the assembly routines from NativeAOT seems like a better path for most of those.

{
[DebuggerHidden]
private static unsafe void InitValueClass(ref byte destBytes, MethodTable *pMT)
{
uint numInstanceFieldBytes = pMT->GetNumInstanceFieldBytes();
if (((uint)Unsafe.AsPointer(ref destBytes) | numInstanceFieldBytes & ((uint)sizeof(void*) - 1)) != 0)
{
// If we have a non-pointer aligned instance field bytes count, or a non-aligned destBytes, we can zero out the data byte by byte
// And we do not need to concern ourselves with references
SpanHelpers.ClearWithoutReferences(ref destBytes, numInstanceFieldBytes);
}
else
{
// Otherwise, use the helper which is safe for that situation
SpanHelpers.ClearWithReferences(ref Unsafe.As<byte, IntPtr>(ref destBytes), (nuint)numInstanceFieldBytes / (nuint)sizeof(IntPtr));
}
}

[DebuggerHidden]
[MethodImpl(MethodImplOptions.NoInlining)]
private static bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb)
{
if (pMTa == pMTb)
{
return true;
}

if (pMTa->HasTypeEquivalence && pMTb->HasTypeEquivalence)
{
return false;
}

return RuntimeHelpers.AreTypesEquivalent(pMTa, pMTb);
}

[DebuggerHidden]
private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT)
{
if (!typeMT->IsNullable)
{
return false;
}

MethodTable *pMTNullableArg = typeMT->InstantiationArg0();
if (pMTNullableArg == boxedMT)
{
return true;
}
else
{
return AreTypesEquivalent(pMTNullableArg, boxedMT);
}
}

[DebuggerHidden]
internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj)
{
if (obj == null)
{
InitValueClass(ref destPtr, typeMT);
}
else
{
if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj)))
{
// For safety's sake, also allow true nullables to be unboxed normally.
// This should not happen normally, but we want to be robust
if (typeMT == RuntimeHelpers.GetMethodTable(obj))
{
Unsafe.CopyBlockUnaligned(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes());
return;
}
CastHelpers.ThrowInvalidCastException(obj, typeMT);
}

// Set the hasValue field on the Nullable type. It MUST always be placed at the start of the object.
*(bool*)destPtr = true;
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't look correct. The cast is casting the value of the reference into pointer. Unsafe.As would be correct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that seems wrong. I kept waffling between refs and pointers here. Thanks.

ref byte destValuePtr = ref typeMT->GetNullableValueFieldReferenceAndSize(ref destPtr, out uint size);
Unsafe.CopyBlockUnaligned(ref destValuePtr, ref RuntimeHelpers.GetRawData(obj), size);
}
}

internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj)
{
// must be a value type
Debug.Assert(pMT1->IsValueType);

MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj);
if ((pMT1->IsPrimitive && pMT2->IsPrimitive &&
pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) ||
AreTypesEquivalent(pMT1, pMT2))
{
return ref RuntimeHelpers.GetRawData(obj);
}

CastHelpers.ThrowInvalidCastException(obj, pMT1);
return ref Unsafe.AsRef<byte>(null);
}

internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2)
{
if (pMT1 == pMT2 ||
(pMT1->IsPrimitive && pMT2->IsPrimitive &&
pMT1->GetPrimitiveCorElementType() == pMT2->GetPrimitiveCorElementType()) ||
AreTypesEquivalent(pMT1, pMT2))
{
return;
}

CastHelpers.ThrowInvalidCastException(pMT1, pMT2);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,20 +8,25 @@ namespace System.Runtime.CompilerServices
{
[StackTraceHidden]
[DebuggerStepThrough]
internal static unsafe class CastHelpers
internal static unsafe partial class CastHelpers
{
// In coreclr the table is allocated and written to on the native side.
internal static int[]? s_table;

[LibraryImport(RuntimeHelpers.QCall)]
internal static partial void ThrowInvalidCastException(void* fromTypeHnd, void* toTypeHnd);

internal static void ThrowInvalidCastException(object fromTypeHnd, void* toTypeHnd)
{
ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromTypeHnd), toTypeHnd);
}

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object IsInstanceOfAny_NoCacheLookup(void* toTypeHnd, object obj);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern object ChkCastAny_NoCacheLookup(void* toTypeHnd, object obj);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern ref byte Unbox_Helper(void* toTypeHnd, object obj);

[MethodImpl(MethodImplOptions.InternalCall)]
private static extern void WriteBarrier(ref object? dst, object? obj);

Expand Down Expand Up @@ -365,13 +370,13 @@ internal static unsafe class CastHelpers
}

[DebuggerHidden]
private static ref byte Unbox(void* toTypeHnd, object obj)
private static ref byte Unbox(MethodTable* toTypeHnd, object obj)
{
// This will throw NullReferenceException if obj is null.
if (RuntimeHelpers.GetMethodTable(obj) == toTypeHnd)
return ref obj.GetRawData();

return ref Unbox_Helper(toTypeHnd, obj);
return ref BoxingHelpers.Unbox_Helper(toTypeHnd, obj);
}

[DebuggerHidden]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -443,9 +443,6 @@ internal static unsafe bool ObjectHasComponentSize(object obj)
[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern unsafe object? Box(MethodTable* methodTable, ref byte data);

[MethodImpl(MethodImplOptions.InternalCall)]
internal static extern unsafe void Unbox_Nullable(ref byte destination, MethodTable* toTypeHnd, object? obj);

// Given an object reference, returns its MethodTable*.
//
// WARNING: The caller has to ensure that MethodTable* does not get unloaded. The most robust way
Expand Down Expand Up @@ -872,6 +869,12 @@ public TypeHandle GetArrayElementTypeHandle()
/// </summary>
[MethodImpl(MethodImplOptions.InternalCall)]
public extern MethodTable* GetMethodTableMatchingParentClass(MethodTable* parent);

[MethodImpl(MethodImplOptions.InternalCall)]
public extern ref byte GetNullableValueFieldReferenceAndSize(ref byte nullableAddr, out uint size);

[MethodImpl(MethodImplOptions.InternalCall)]
public extern MethodTable* InstantiationArg0();
}

// Subset of src\vm\methodtable.h
Expand Down
4 changes: 2 additions & 2 deletions src/coreclr/inc/jithelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,8 @@
DYNAMICJITHELPER(CORINFO_HELP_BOX, JIT_Box, METHOD__NIL)
JITHELPER(CORINFO_HELP_BOX_NULLABLE, JIT_Box, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_UNBOX, NULL, METHOD__CASTHELPERS__UNBOX)
JITHELPER(CORINFO_HELP_UNBOX_TYPETEST, JIT_Unbox_TypeTest, METHOD__NIL)
JITHELPER(CORINFO_HELP_UNBOX_NULLABLE, JIT_Unbox_Nullable, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_UNBOX_TYPETEST,NULL, METHOD__BOXINGHELPERS__UNBOX_TYPETEST)
DYNAMICJITHELPER(CORINFO_HELP_UNBOX_NULLABLE,NULL, METHOD__BOXINGHELPERS__UNBOX_NULLABLE)

JITHELPER(CORINFO_HELP_GETREFANY, JIT_GetRefAny, METHOD__NIL)
DYNAMICJITHELPER(CORINFO_HELP_ARRADDR_ST, NULL, METHOD__CASTHELPERS__STELEMREF)
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/vm/JitQCallHelpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,6 @@ class MethodDesc;
extern "C" void * QCALLTYPE ResolveVirtualFunctionPointer(QCall::ObjectHandleOnStack obj, CORINFO_CLASS_HANDLE classHnd, CORINFO_METHOD_HANDLE methodHnd);
extern "C" CORINFO_GENERIC_HANDLE QCALLTYPE GenericHandleWorker(MethodDesc * pMD, MethodTable * pMT, LPVOID signature, DWORD dictionaryIndexAndSlot, Module* pModule);
extern "C" void QCALLTYPE InitClassHelper(MethodTable* pMT);
extern "C" void QCALLTYPE ThrowInvalidCastException(CORINFO_CLASS_HANDLE pTargetType, CORINFO_CLASS_HANDLE pSourceType);

#endif //_JITQCALLHELPERS_H
19 changes: 19 additions & 0 deletions src/coreclr/vm/comutilnative.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1818,6 +1818,25 @@ FCIMPL2(MethodTable*, MethodTableNative::GetMethodTableMatchingParentClass, Meth
}
FCIMPLEND

FCIMPL3(uint8_t*, MethodTableNative::GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize)
{
FCALL_CONTRACT;
_ASSERTE(Nullable::IsNullableType(mt));
_ASSERTE(strcmp(mt->GetApproxFieldDescListRaw()[1].GetDebugName(), "value") == 0);

*pSize = mt->GetInstantiation()[0].AsMethodTable()->GetNumInstanceFieldBytes();
return nullableAddr + mt->GetApproxFieldDescListRaw()[1].GetOffset();
}
FCIMPLEND

FCIMPL1(MethodTable*, MethodTableNative::InstantiationArg0, MethodTable* mt);
{
FCALL_CONTRACT;

return mt->GetInstantiation()[0].AsMethodTable();
}
FCIMPLEND

extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb)
{
QCALL_CONTRACT;
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/comutilnative.h
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,8 @@ class MethodTableNative {
static FCDECL1(UINT32, GetNumInstanceFieldBytes, MethodTable* mt);
static FCDECL1(CorElementType, GetPrimitiveCorElementType, MethodTable* mt);
static FCDECL2(MethodTable*, GetMethodTableMatchingParentClass, MethodTable* mt, MethodTable* parent);
static FCDECL3(uint8_t*, GetNullableValueFieldReferenceAndSize, MethodTable* mt, uint8_t* nullableAddr, uint32_t* pSize);
static FCDECL1(MethodTable*, InstantiationArg0, MethodTable* mt);
};

extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb);
Expand Down
6 changes: 5 additions & 1 deletion src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -1164,11 +1164,15 @@ DEFINE_METHOD(CASTHELPERS, CHKCASTANY, ChkCastAny, SM_Ptr
DEFINE_METHOD(CASTHELPERS, CHKCASTINTERFACE, ChkCastInterface, SM_PtrVoid_Obj_RetObj)
DEFINE_METHOD(CASTHELPERS, CHKCASTCLASS, ChkCastClass, SM_PtrVoid_Obj_RetObj)
DEFINE_METHOD(CASTHELPERS, CHKCASTCLASSSPECIAL, ChkCastClassSpecial, SM_PtrVoid_Obj_RetObj)
DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, SM_PtrVoid_Obj_RetRefByte)
DEFINE_METHOD(CASTHELPERS, UNBOX, Unbox, NoSig)
DEFINE_METHOD(CASTHELPERS, STELEMREF, StelemRef, SM_ArrObject_IntPtr_Obj_RetVoid)
DEFINE_METHOD(CASTHELPERS, LDELEMAREF, LdelemaRef, SM_ArrObject_IntPtr_PtrVoid_RetRefObj)
DEFINE_METHOD(CASTHELPERS, ARRAYTYPECHECK, ArrayTypeCheck, SM_Obj_Array_RetVoid)

DEFINE_CLASS(BOXINGHELPERS, CompilerServices, BoxingHelpers)
DEFINE_METHOD(BOXINGHELPERS, UNBOX_NULLABLE, Unbox_Nullable, NoSig)
DEFINE_METHOD(BOXINGHELPERS, UNBOX_TYPETEST, Unbox_TypeTest, NoSig)

DEFINE_CLASS(VIRTUALDISPATCHHELPERS, CompilerServices, VirtualDispatchHelpers)
DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER, VirtualFunctionPointer, NoSig)
DEFINE_METHOD(VIRTUALDISPATCHHELPERS, VIRTUALFUNCTIONPOINTER_DYNAMIC, VirtualFunctionPointer_Dynamic, NoSig)
Expand Down
4 changes: 1 addition & 3 deletions src/coreclr/vm/ecalllist.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,6 @@ FCFuncEnd()
FCFuncStart(gCastHelpers)
FCFuncElement("IsInstanceOfAny_NoCacheLookup", ::IsInstanceOfAny_NoCacheLookup)
FCFuncElement("ChkCastAny_NoCacheLookup", ::ChkCastAny_NoCacheLookup)
FCFuncElement("Unbox_Helper", ::Unbox_Helper)
FCFuncElement("JIT_Unbox_TypeTest", ::JIT_Unbox_TypeTest)
FCFuncElement("WriteBarrier", ::WriteBarrier_Helper)
FCFuncEnd()

Expand Down Expand Up @@ -356,13 +354,13 @@ FCFuncStart(gRuntimeHelpers)
FCFuncElement("AllocTailCallArgBufferWorker", TailCallHelp::AllocTailCallArgBufferWorker)
FCFuncElement("GetTailCallInfo", TailCallHelp::GetTailCallInfo)
FCFuncElement("Box", JIT_Box)
FCFuncElement("Unbox_Nullable", JIT_Unbox_Nullable)
FCFuncEnd()

FCFuncStart(gMethodTableFuncs)
FCFuncElement("GetNumInstanceFieldBytes", MethodTableNative::GetNumInstanceFieldBytes)
FCFuncElement("GetPrimitiveCorElementType", MethodTableNative::GetPrimitiveCorElementType)
FCFuncElement("GetMethodTableMatchingParentClass", MethodTableNative::GetMethodTableMatchingParentClass)
FCFuncElement("GetNullableValueFieldReferenceAndSize", MethodTableNative::GetNullableValueFieldReferenceAndSize)
FCFuncEnd()

FCFuncStart(gStubHelperFuncs)
Expand Down
Loading
Loading