From 0a32ed8dad70f55363603d81173ef1b44b5d75fd Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 15 Aug 2024 12:58:50 -0700 Subject: [PATCH 1/4] Remove HelperMethodFrames from Object methods Split GetHashCode into fast/slow managed functions. Split GetType into fast/slow managed functions. --- .../src/System/Object.CoreCLR.cs | 22 ++- .../RuntimeHelpers.CoreCLR.cs | 46 ++++-- .../classlibnative/bcltype/objectnative.cpp | 156 ++++-------------- .../classlibnative/bcltype/objectnative.h | 5 +- src/coreclr/vm/ecalllist.h | 6 - src/coreclr/vm/qcallentrypoints.cpp | 2 + 6 files changed, 95 insertions(+), 142 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs index 940d1622bad18..c12a44994d773 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs @@ -3,15 +3,33 @@ using System.Diagnostics; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace System { public partial class Object { + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ObjectNative_GetTypeSlow")] + private static unsafe partial void GetTypeSlow(MethodTable* methodTable, ObjectHandleOnStack ret); + // Returns a Type object which represent this object instance. [Intrinsic] - [MethodImpl(MethodImplOptions.InternalCall)] - public extern Type GetType(); + public unsafe Type GetType() + { + MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); + Type? type = pMT->AuxiliaryData->ExposedClassObject; + type ??= GetTypeWorker(pMT); + GC.KeepAlive(this); + return type; + + [MethodImpl(MethodImplOptions.NoInlining)] + static Type GetTypeWorker(MethodTable* pMT) + { + Type? type = null; + GetTypeSlow(pMT, ObjectHandleOnStack.Create(ref type)); + return type!; + } + } // Returns a new object instance that is a memberwise copy of this // object. This is always a shallow copy of the instance. The method is protected diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index e155b109886d0..26a8b12312cb6 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -220,19 +220,36 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH [MethodImpl(MethodImplOptions.InternalCall)] public static extern void PrepareDelegate(Delegate d); - [MethodImpl(MethodImplOptions.InternalCall)] - public static extern int GetHashCode(object? o); - /// /// If a hash code has been assigned to the object, it is returned. Otherwise zero is /// returned. /// - /// - /// The advantage of this over is that it avoids assigning a hash - /// code to the object if it does not already have one. - /// [MethodImpl(MethodImplOptions.InternalCall)] - internal static extern int TryGetHashCode(object o); + internal static extern int TryGetHashCode(object? o); + + [LibraryImport(QCall, EntryPoint = "ObjectNative_GetHashCodeSlow")] + private static partial int GetHashCodeSlow(ObjectHandleOnStack o); + + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public static int GetHashCode(object? o) + { + int hashCode = TryGetHashCode(o); + if (hashCode == 0) + { + return GetHashCodeWorker(o); + } + return hashCode; + + [MethodImpl(MethodImplOptions.NoInlining)] + static int GetHashCodeWorker(object? o) + { + if (o is null) + { + return 0; + } + return GetHashCodeSlow(ObjectHandleOnStack.Create(ref o)); + } + } public static new unsafe bool Equals(object? o1, object? o2) { @@ -791,11 +808,12 @@ public TypeHandle GetArrayElementTypeHandle() } // Subset of src\vm\methodtable.h - [StructLayout(LayoutKind.Explicit)] + [StructLayout(LayoutKind.Sequential)] internal unsafe struct MethodTableAuxiliaryData { - [FieldOffset(0)] private uint Flags; + private nint m_pLoaderModule; + private nint m_hExposedClassObject; private const uint enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode = 0x0002; // Whether we have checked the overridden Equals or GetHashCode private const uint enum_flag_CanCompareBitsOrUseFastGetHashCode = 0x0004; // Is any field type or sub field type overridden Equals or GetHashCode @@ -810,6 +828,14 @@ public bool CanCompareBitsOrUseFastGetHashCode return (Flags & enum_flag_CanCompareBitsOrUseFastGetHashCode) != 0; } } + + public RuntimeType? ExposedClassObject + { + get + { + return *(RuntimeType*)Unsafe.AsPointer(ref m_hExposedClassObject); + } + } } /// diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index afbda5fad9916..b1b7ceba207fe 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -19,106 +19,49 @@ #include "comsynchronizable.h" #include "eeconfig.h" - -NOINLINE static INT32 GetHashCodeHelper(OBJECTREF objRef) +extern "C" INT32 QCALLTYPE ObjectNative_GetHashCodeSlow(QCall::ObjectHandleOnStack objHandle) { - DWORD idx = 0; + QCALL_CONTRACT; - FC_INNER_PROLOG(ObjectNative::GetHashCode); + _ASSERTE(objHandle.Get() != NULL); + INT32 idx = 0; - HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB_1(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); + BEGIN_QCALL; - idx = objRef->GetHashCodeEx(); + GCX_COOP(); - HELPER_METHOD_FRAME_END(); - FC_INNER_EPILOG(); - return idx; -} + idx = objHandle.Get()->GetHashCodeEx(); -// Note that we obtain a sync block index without actually building a sync block. -// That's because a lot of objects are hashed, without requiring support for -FCIMPL1(INT32, ObjectNative::GetHashCode, Object* obj) { + END_QCALL; - CONTRACTL - { - FCALL_CHECK; - INJECT_FAULT(FCThrow(kOutOfMemoryException);); - } - CONTRACTL_END; + return idx; +} - VALIDATEOBJECT(obj); +FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) +{ + FCALL_CONTRACT; - if (obj == 0) + if (obj == NULL) return 0; - OBJECTREF objRef(obj); - + OBJECTREF objRef = ObjectToOBJECTREF(obj); + DWORD bits = objRef->GetHeader()->GetBits(); + if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) { - DWORD bits = objRef->GetHeader()->GetBits(); - - if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) + if (bits & BIT_SBLK_IS_HASHCODE) { - if (bits & BIT_SBLK_IS_HASHCODE) - { - // Common case: the object already has a hash code - return bits & MASK_HASHCODE; - } - else - { - // We have a sync block index. This means if we already have a hash code, - // it is in the sync block, otherwise we generate a new one and store it there - SyncBlock *psb = objRef->PassiveGetSyncBlock(); - if (psb != NULL) - { - DWORD hashCode = psb->GetHashCode(); - if (hashCode != 0) - return hashCode; - } - } + // Common case: the object already has a hash code + return bits & MASK_HASHCODE; } - } - - FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef)); -} -FCIMPLEND - -FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) { - - CONTRACTL - { - FCALL_CHECK; - } - CONTRACTL_END; - - VALIDATEOBJECT(obj); - - if (obj == 0) - return 0; - - OBJECTREF objRef(obj); - - { - DWORD bits = objRef->GetHeader()->GetBits(); - - if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) + else { - if (bits & BIT_SBLK_IS_HASHCODE) - { - // Common case: the object already has a hash code - return bits & MASK_HASHCODE; - } - else - { - // We have a sync block index. There may be a hash code stored within the sync block. - SyncBlock *psb = objRef->PassiveGetSyncBlock(); - if (psb != NULL) - { - return psb->GetHashCode(); - } - } + // We have a sync block index. This means if we already have a hash code, + // it is in the sync block, otherwise we will return 0, which means "not set". + SyncBlock *psb = objRef->PassiveGetSyncBlock(); + if (psb != NULL) + return psb->GetHashCode(); } } - return 0; } FCIMPLEND @@ -146,48 +89,19 @@ FCIMPL2(FC_BOOL_RET, ObjectNative::ContentEquals, Object *pThisRef, Object *pCom } FCIMPLEND -NOINLINE static Object* GetClassHelper(OBJECTREF objRef) +extern "C" void QCALLTYPE ObjectNative_GetTypeSlow(MethodTable* pMT, QCall::ObjectHandleOnStack ret) { - FC_INNER_PROLOG(ObjectNative::GetClass); - _ASSERTE(objRef != NULL); - TypeHandle typeHandle = objRef->GetTypeHandle(); - OBJECTREF refType = NULL; - - HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB_1(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, refType); - - refType = typeHandle.GetManagedClassObject(); + QCALL_CONTRACT; + _ASSERTE(pMT != NULL); - HELPER_METHOD_FRAME_END(); - FC_INNER_EPILOG(); - return OBJECTREFToObject(refType); -} + BEGIN_QCALL; -// This routine is called by the Object.GetType() routine. It is a major way to get the System.Type -FCIMPL1(Object*, ObjectNative::GetClass, Object* pThis) -{ - CONTRACTL - { - FCALL_CHECK; - INJECT_FAULT(FCThrow(kOutOfMemoryException);); - } - CONTRACTL_END; + GCX_COOP(); - OBJECTREF objRef = ObjectToOBJECTREF(pThis); - if (objRef != NULL) - { - MethodTable* pMT = objRef->GetMethodTable(); - OBJECTREF typePtr = pMT->GetManagedClassObjectIfExists(); - if (typePtr != NULL) - { - return OBJECTREFToObject(typePtr); - } - } - else - FCThrow(kNullReferenceException); + ret.Set(pMT->GetManagedClassObject()); - FC_INNER_RETURN(Object*, GetClassHelper(objRef)); + END_QCALL; } -FCIMPLEND extern "C" void QCALLTYPE ObjectNative_AllocateUninitializedClone(QCall::ObjectHandleOnStack objHandle) { @@ -200,10 +114,10 @@ extern "C" void QCALLTYPE ObjectNative_AllocateUninitializedClone(QCall::ObjectH OBJECTREF refClone = objHandle.Get(); _ASSERTE(refClone != NULL); // Should be handled at managed side MethodTable* pMT = refClone->GetMethodTable(); - + // assert that String has overloaded the Clone() method _ASSERTE(pMT != g_pStringClass); - + if (pMT->IsArray()) { objHandle.Set(DupArrayForCloning((BASEARRAYREF)refClone)); diff --git a/src/coreclr/classlibnative/bcltype/objectnative.h b/src/coreclr/classlibnative/bcltype/objectnative.h index 418fd2561d7cc..374991cef986a 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.h +++ b/src/coreclr/classlibnative/bcltype/objectnative.h @@ -24,14 +24,13 @@ class ObjectNative { public: - - static FCDECL1(INT32, GetHashCode, Object* vThisRef); static FCDECL1(INT32, TryGetHashCode, Object* vThisRef); static FCDECL2(FC_BOOL_RET, ContentEquals, Object *pThisRef, Object *pCompareRef); - static FCDECL1(Object*, GetClass, Object* pThis); static FCDECL1(FC_BOOL_RET, IsLockHeld, Object* pThisUNSAFE); }; +extern "C" INT32 QCALLTYPE ObjectNative_GetHashCodeSlow(QCall::ObjectHandleOnStack objHandle); +extern "C" void QCALLTYPE ObjectNative_GetTypeSlow(MethodTable* pMT, QCall::ObjectHandleOnStack ret); extern "C" void QCALLTYPE ObjectNative_AllocateUninitializedClone(QCall::ObjectHandleOnStack objHandle); extern "C" BOOL QCALLTYPE Monitor_Wait(QCall::ObjectHandleOnStack pThis, INT32 Timeout); extern "C" void QCALLTYPE Monitor_Pulse(QCall::ObjectHandleOnStack pThis); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 00429788f0b1c..ad7d7f85aecab 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -54,10 +54,6 @@ FCFuncStart(gDependentHandleFuncs) FCFuncElement("InternalFree", DependentHandle::InternalFree) FCFuncEnd() -FCFuncStart(gObjectFuncs) - FCFuncElement("GetType", ObjectNative::GetClass) -FCFuncEnd() - FCFuncStart(gStringFuncs) FCDynamic("FastAllocateString", ECall::FastAllocateString) FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_ArrChar_RetVoid, ECall::CtorCharArrayManaged) @@ -432,7 +428,6 @@ FCFuncEnd() FCFuncStart(gRuntimeHelpers) FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate) - FCFuncElement("GetHashCode", ObjectNative::GetHashCode) FCFuncElement("TryGetHashCode", ObjectNative::TryGetHashCode) FCFuncElement("ContentEquals", ObjectNative::ContentEquals) FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack) @@ -551,7 +546,6 @@ FCClassElement("MetadataImport", "System.Reflection", gMetaDataImport) FCClassElement("MethodTable", "System.Runtime.CompilerServices", gMethodTableFuncs) FCClassElement("ModuleHandle", "System", gCOMModuleHandleFuncs) FCClassElement("Monitor", "System.Threading", gMonitorFuncs) -FCClassElement("Object", "System", gObjectFuncs) FCClassElement("RuntimeAssembly", "System.Reflection", gRuntimeAssemblyFuncs) FCClassElement("RuntimeFieldHandle", "System", gCOMFieldHandleNewFuncs) diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index bf2a8f6ce3a32..fa9f45157437a 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -337,6 +337,8 @@ static const Entry s_QCall[] = DllImportEntry(GetFileLoadExceptionMessage) DllImportEntry(FileLoadException_GetMessageForHR) DllImportEntry(Interlocked_MemoryBarrierProcessWide) + DllImportEntry(ObjectNative_GetHashCodeSlow) + DllImportEntry(ObjectNative_GetTypeSlow) DllImportEntry(ObjectNative_AllocateUninitializedClone) DllImportEntry(Monitor_Wait) DllImportEntry(Monitor_Pulse) From b735d836407283e5c04f1743ee6b9c8f48628f11 Mon Sep 17 00:00:00 2001 From: Aaron Robinson Date: Thu, 15 Aug 2024 13:33:15 -0700 Subject: [PATCH 2/4] Apply suggestions from code review Co-authored-by: Jan Kotas --- .../System.Private.CoreLib/src/System/Object.CoreCLR.cs | 3 +-- .../Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 6 +++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs index c12a44994d773..ce1c810d8bc8e 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Object.CoreCLR.cs @@ -17,8 +17,7 @@ public partial class Object public unsafe Type GetType() { MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); - Type? type = pMT->AuxiliaryData->ExposedClassObject; - type ??= GetTypeWorker(pMT); + Type type = pMT->AuxiliaryData->ExposedClassObject ?? GetTypeWorker(pMT); GC.KeepAlive(this); return type; diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index 26a8b12312cb6..ca6a47e44b37c 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -812,8 +812,8 @@ public TypeHandle GetArrayElementTypeHandle() internal unsafe struct MethodTableAuxiliaryData { private uint Flags; - private nint m_pLoaderModule; - private nint m_hExposedClassObject; + private void* LoaderModule; + private nint ExposedClassObjectRaw; private const uint enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode = 0x0002; // Whether we have checked the overridden Equals or GetHashCode private const uint enum_flag_CanCompareBitsOrUseFastGetHashCode = 0x0004; // Is any field type or sub field type overridden Equals or GetHashCode @@ -833,7 +833,7 @@ public RuntimeType? ExposedClassObject { get { - return *(RuntimeType*)Unsafe.AsPointer(ref m_hExposedClassObject); + return *(RuntimeType*)Unsafe.AsPointer(ref ExposedClassObjectRaw); } } } From 87d203fa6d058217e356f46d69b82fd1d12512fe Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 15 Aug 2024 14:24:49 -0700 Subject: [PATCH 3/4] Remove aggressiveinline --- .../System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs index ca6a47e44b37c..88e07e8d388ce 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs @@ -230,7 +230,6 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH [LibraryImport(QCall, EntryPoint = "ObjectNative_GetHashCodeSlow")] private static partial int GetHashCodeSlow(ObjectHandleOnStack o); - [MethodImpl(MethodImplOptions.AggressiveInlining)] public static int GetHashCode(object? o) { int hashCode = TryGetHashCode(o); From 681a0b90a30d9afc121579a457ce804c0037f763 Mon Sep 17 00:00:00 2001 From: Aaron R Robinson Date: Thu, 15 Aug 2024 16:19:42 -0700 Subject: [PATCH 4/4] Poorly placed ASSERT. --- src/coreclr/classlibnative/bcltype/objectnative.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/classlibnative/bcltype/objectnative.cpp b/src/coreclr/classlibnative/bcltype/objectnative.cpp index b1b7ceba207fe..0ac42cc084d8d 100644 --- a/src/coreclr/classlibnative/bcltype/objectnative.cpp +++ b/src/coreclr/classlibnative/bcltype/objectnative.cpp @@ -23,13 +23,13 @@ extern "C" INT32 QCALLTYPE ObjectNative_GetHashCodeSlow(QCall::ObjectHandleOnSta { QCALL_CONTRACT; - _ASSERTE(objHandle.Get() != NULL); INT32 idx = 0; BEGIN_QCALL; GCX_COOP(); + _ASSERTE(objHandle.Get() != NULL); idx = objHandle.Get()->GetHashCodeEx(); END_QCALL;