-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Changes from 15 commits
c70a57c
f1258a4
02636f8
d8e01e3
b2c6991
12d17e5
3f281b9
8dfaedc
be624da
dec085f
e22038b
e44fab4
2964105
6469fbb
1e51dd2
ee6d42d
7761ad8
55e0df9
6a8d1ae
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -2,28 +2,39 @@ | |||||
// The .NET Foundation licenses this file to you under the MIT license. | ||||||
|
||||||
using System.Diagnostics; | ||||||
using System.Diagnostics.CodeAnalysis; | ||||||
using System.Runtime.InteropServices; | ||||||
|
||||||
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); | ||||||
|
||||||
[DoesNotReturn] | ||||||
internal static void ThrowInvalidCastException(object fromType, void* toTypeHnd) | ||||||
{ | ||||||
ThrowInvalidCastException(RuntimeHelpers.GetMethodTable(fromType), toTypeHnd); | ||||||
throw null!; // Provide hint to the inliner that this method does not return | ||||||
} | ||||||
|
||||||
[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); | ||||||
private static extern void WriteBarrier(ref object? dst, object? obj); | ||||||
|
||||||
[MethodImpl(MethodImplOptions.InternalCall)] | ||||||
private static extern void WriteBarrier(ref object? dst, object? obj); | ||||||
private static extern void UnboxNullableValue(ref byte destPtr, MethodTable* typeMT, object obj); | ||||||
|
||||||
// IsInstanceOf test used for unusual cases (naked type parameters, variant generic types) | ||||||
// Unlike the IsInstanceOfInterface and IsInstanceOfClass functions, | ||||||
|
@@ -365,7 +376,7 @@ 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) | ||||||
|
@@ -492,5 +503,141 @@ private static unsafe void ArrayTypeCheck_Helper(object obj, void* elementType) | |||||
ThrowArrayMismatchException(); | ||||||
} | ||||||
} | ||||||
|
||||||
// Helpers for Unboxing | ||||||
#if FEATURE_TYPEEQUIVALENCE | ||||||
[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); | ||||||
} | ||||||
#endif // FEATURE_TYPEEQUIVALENCE | ||||||
|
||||||
[DebuggerHidden] | ||||||
[MethodImpl(MethodImplOptions.AggressiveInlining)] | ||||||
private static bool IsNullableForType(MethodTable* typeMT, MethodTable* boxedMT) | ||||||
{ | ||||||
if (!typeMT->IsNullable) | ||||||
{ | ||||||
return false; | ||||||
} | ||||||
|
||||||
// Normally getting the first generic argument involves checking the PerInstInfo to get the count of generic dictionaries | ||||||
// in the hierarchy, and then doing a bit of math to find the right dictionary, but since we know this is nullable | ||||||
// we can do a simple double deference to do the same thing. | ||||||
Debug.Assert(typeMT->InstantiationArg0() == **typeMT->PerInstInfo); | ||||||
MethodTable *pMTNullableArg = **typeMT->PerInstInfo; | ||||||
if (pMTNullableArg == boxedMT) | ||||||
{ | ||||||
return true; | ||||||
} | ||||||
else | ||||||
{ | ||||||
#if FEATURE_TYPEEQUIVALENCE | ||||||
return AreTypesEquivalent(pMTNullableArg, boxedMT); | ||||||
#else | ||||||
return false; | ||||||
#endif // FEATURE_TYPEEQUIVALENCE | ||||||
} | ||||||
} | ||||||
|
||||||
[DebuggerHidden] | ||||||
[MethodImpl(MethodImplOptions.NoInlining)] | ||||||
private static void Unbox_Nullable_NotIsNullableForType(ref byte destPtr, MethodTable* typeMT, object 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)) | ||||||
{ | ||||||
CastHelpers.ThrowInvalidCastException(obj, typeMT); | ||||||
} | ||||||
Buffer.BulkMoveWithWriteBarrier(ref destPtr, ref RuntimeHelpers.GetRawData(obj), typeMT->GetNumInstanceFieldBytes()); | ||||||
} | ||||||
|
||||||
[DebuggerHidden] | ||||||
internal static void Unbox_Nullable(ref byte destPtr, MethodTable* typeMT, object? obj) | ||||||
{ | ||||||
if (obj == null) | ||||||
{ | ||||||
if (!typeMT->ContainsGCPointers) | ||||||
{ | ||||||
SpanHelpers.ClearWithoutReferences(ref destPtr, typeMT->GetNumInstanceFieldBytes()); | ||||||
} | ||||||
else | ||||||
{ | ||||||
// If the type ContainsGCPointers, we can compute the size without resorting to loading the BaseSizePadding field from the EEClass | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
nuint numInstanceFieldBytes = typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr)); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It may be nice to move this to a property or method on MethodTable. There are more places where this micro-optimization can be used. The property impl can assert that this is only used on when |
||||||
// Otherwise, use the helper which is safe for that situation | ||||||
SpanHelpers.ClearWithReferences(ref Unsafe.As<byte, IntPtr>(ref destPtr), (typeMT->BaseSize - (nuint)(2 * sizeof(IntPtr))) / (nuint)sizeof(IntPtr)); | ||||||
} | ||||||
} | ||||||
else | ||||||
{ | ||||||
if (!IsNullableForType(typeMT, RuntimeHelpers.GetMethodTable(obj))) | ||||||
{ | ||||||
Unbox_Nullable_NotIsNullableForType(ref destPtr, typeMT, obj); | ||||||
} | ||||||
else | ||||||
{ | ||||||
UnboxNullableValue(ref destPtr, typeMT, obj); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the reason why we are not just calling BulkMoveWithWriteBarrier/Memmove here to copy the value? I would expect it to be faster. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It turns out that there is a fair amount of type system structure that would need to be ported to managed code to call BulkMoveWithWriteBarrier. I made the decision to pull this out into its own FCALL here. Especially since I expect that most paths will need to do a BulkMoveWithWriteBarrier which isn't that different in perf. I can explore pulling enough type system structure for this if you'd like me to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Of course, now that I look at this, I realized that I can actually encode the needed bits for this particular load into the space used by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to make sure that I understand - is the main missing piece There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Its ValueAddr + the computation of the size of the instance field bytes of the Value field. That requires access to the EEClass + access to a fielddesc, etc. It boils down to not that many instructions, but it IS a fair number of concepts and type system structures. That is a task probably worth doing at some point, but I'd like to avoid doing it now. The new encoding of data, which is redundant with existing data, allows for extremely cheap access, and is simple to encode into the managed code without adding more concepts than |
||||||
} | ||||||
} | ||||||
} | ||||||
|
||||||
[DebuggerHidden] | ||||||
[MethodImpl(MethodImplOptions.NoInlining)] | ||||||
internal static ref byte Unbox_Helper(MethodTable* pMT1, object obj) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
// must be a value type | ||||||
Debug.Assert(pMT1->IsValueType); | ||||||
|
||||||
MethodTable* pMT2 = RuntimeHelpers.GetMethodTable(obj); | ||||||
if ((!pMT1->IsPrimitive || !pMT2->IsPrimitive || | ||||||
pMT1->GetPrimitiveCorElementType() != pMT2->GetPrimitiveCorElementType()) | ||||||
#if FEATURE_TYPEEQUIVALENCE | ||||||
&& !AreTypesEquivalent(pMT1, pMT2) | ||||||
#endif // FEATURE_TYPEEQUIVALENCE | ||||||
) | ||||||
{ | ||||||
CastHelpers.ThrowInvalidCastException(obj, pMT1); | ||||||
} | ||||||
|
||||||
return ref RuntimeHelpers.GetRawData(obj); | ||||||
} | ||||||
|
||||||
[DebuggerHidden] | ||||||
[MethodImpl(MethodImplOptions.NoInlining)] | ||||||
internal static void Unbox_TypeTest_Helper(MethodTable *pMT1, MethodTable *pMT2) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
{ | ||||||
if ((!pMT1->IsPrimitive || !pMT2->IsPrimitive || | ||||||
pMT1->GetPrimitiveCorElementType() != pMT2->GetPrimitiveCorElementType()) | ||||||
#if FEATURE_TYPEEQUIVALENCE | ||||||
&& !AreTypesEquivalent(pMT1, pMT2) | ||||||
#endif // FEATURE_TYPEEQUIVALENCE | ||||||
) | ||||||
{ | ||||||
CastHelpers.ThrowInvalidCastException(pMT1, pMT2); | ||||||
} | ||||||
} | ||||||
|
||||||
[DebuggerHidden] | ||||||
internal static void Unbox_TypeTest(MethodTable *pMT1, MethodTable *pMT2) | ||||||
{ | ||||||
if (pMT1 == pMT2) | ||||||
return; | ||||||
else | ||||||
Unbox_TypeTest_Helper(pMT1, pMT2); | ||||||
} | ||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -448,9 +448,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 | ||
|
@@ -706,6 +703,16 @@ internal unsafe struct MethodTable | |
[FieldOffset(ElementTypeOffset)] | ||
public void* ElementType; | ||
|
||
/// <summary> | ||
/// The PerInstInfo is used to describe the generic arguments and dictionary of this type. | ||
/// It points as a PerInstInfo, which is an array of pointers to generic dictionaries, which then point | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This does not parse for me. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated the wording. Hopefully it is a bit more clear although for sure, the actual design of this is a very confusing structure. |
||
/// to the actual type arguments + the contents of the generic dictionary. The size of the PerInstInfo is | ||
/// defined in the negative space of that structure, and the size of the generic dictionary is described | ||
/// in the DictionaryLayout of the associated canonical MethodTable. | ||
/// </summary> | ||
[FieldOffset(ElementTypeOffset)] | ||
public MethodTable*** PerInstInfo; | ||
|
||
/// <summary> | ||
/// This interface map used to list out the set of interfaces. Only meaningful if InterfaceCount is non-zero. | ||
/// </summary> | ||
|
@@ -730,6 +737,7 @@ internal unsafe struct MethodTable | |
private const uint enum_flag_Category_Mask = 0x000F0000; | ||
private const uint enum_flag_Category_ValueType = 0x00040000; | ||
private const uint enum_flag_Category_Nullable = 0x00050000; | ||
private const uint enum_flag_Category_IsPrimitiveMask = 0x000E0000; | ||
private const uint enum_flag_Category_PrimitiveValueType = 0x00060000; // sub-category of ValueType, Enum or primitive value type | ||
private const uint enum_flag_Category_TruePrimitive = 0x00070000; // sub-category of ValueType, Primitive (ELEMENT_TYPE_I, etc.) | ||
private const uint enum_flag_Category_Array = 0x00080000; | ||
|
@@ -780,7 +788,9 @@ internal unsafe struct MethodTable | |
|
||
public bool NonTrivialInterfaceCast => (Flags & enum_flag_NonTrivialInterfaceCast) != 0; | ||
|
||
#if FEATURE_TYPEEQUIVALENCE | ||
public bool HasTypeEquivalence => (Flags & enum_flag_HasTypeEquivalence) != 0; | ||
#endif // FEATURE_TYPEEQUIVALENCE | ||
|
||
public bool HasFinalizer => (Flags & enum_flag_HasFinalizer) != 0; | ||
|
||
|
@@ -820,7 +830,7 @@ public int MultiDimensionalArrayRank | |
public bool IsByRefLike => (Flags & (enum_flag_HasComponentSize | enum_flag_IsByRefLike)) == enum_flag_IsByRefLike; | ||
|
||
// Warning! UNLIKE the similarly named Reflection api, this method also returns "true" for Enums. | ||
public bool IsPrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_PrimitiveValueType or enum_flag_Category_TruePrimitive; | ||
public bool IsPrimitive => (Flags & enum_flag_Category_IsPrimitiveMask) == enum_flag_Category_PrimitiveValueType; | ||
|
||
public bool IsTruePrimitive => (Flags & enum_flag_Category_Mask) is enum_flag_Category_TruePrimitive; | ||
|
||
|
@@ -877,6 +887,9 @@ public TypeHandle GetArrayElementTypeHandle() | |
/// </summary> | ||
[MethodImpl(MethodImplOptions.InternalCall)] | ||
public extern MethodTable* GetMethodTableMatchingParentClass(MethodTable* parent); | ||
|
||
[MethodImpl(MethodImplOptions.InternalCall)] | ||
public extern MethodTable* InstantiationArg0(); | ||
} | ||
|
||
[StructLayout(LayoutKind.Sequential)] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I know that this is pre-existing comment.) This is not for safety's sake. We would not give up any safety if we threw for boxed
Nullable<T>
that should not exist here. It is just to hide bugs. Have you tried deleting this to see whether anything fails?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are some cases in func eval which claim to use this pathway. In addition, it IS a pathway that has been used in reflection in the past, so I don't want to remove this path.