From ade6628af1ecbba8c35d9745f15892f95ded5e68 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 16:58:06 +0800 Subject: [PATCH 01/18] Access CanCompareBits from MethodTableAuxiliaryData --- .../RuntimeHelpers.CoreCLR.cs | 36 ++++++++++++++++++- .../src/System/ValueType.cs | 21 ++++++++++- 2 files changed, 55 insertions(+), 2 deletions(-) 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 534a251630cda..7f009df62309b 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 @@ -467,7 +467,13 @@ internal unsafe struct MethodTable // Additional conditional fields (see methodtable.h). // m_pModule - // m_pAuxiliaryData + + /// + /// A pointer to auxiliary data that is cold for method table. + /// + [FieldOffset(AuxiliaryDataOffset)] + public MethodTableAuxiliaryData* AuxiliaryData; + // union { // m_pEEClass (pointer to the EE class) // m_pCanonMT (pointer to the canonical method table) @@ -528,6 +534,12 @@ internal unsafe struct MethodTable private const int ParentMethodTableOffset = 0x10 + DebugClassNamePtr; +#if TARGET_64BIT + private const int AuxiliaryDataOffset = 0x20 + DebugClassNamePtr; +#else + private const int AuxiliaryDataOffset = 0x18 + DebugClassNamePtr; +#endif + #if TARGET_64BIT private const int ElementTypeOffset = 0x30 + DebugClassNamePtr; #else @@ -615,6 +627,28 @@ public TypeHandle GetArrayElementTypeHandle() public extern uint GetNumInstanceFieldBytes(); } + // Subset of src\vm\methodtable.h + [StructLayout(LayoutKind.Explicit)] + internal unsafe struct MethodTableAuxiliaryData + { + [FieldOffset(0)] + private uint Flags; + + private const uint enum_flag_CanCompareBitsOrUseFastGetHashCode = 0x0001; // Is any field type or sub field type overrode Equals or GetHashCode + private const uint enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode = 0x0002; // Whether we have checked the overridden Equals or GetHashCode + + public bool HasCheckedCanCompareBitsOrUseFastGetHashCode => (Flags & enum_flag_HasCheckedCanCompareBitsOrUseFastGetHashCode) != 0; + + public bool CanCompareBitsOrUseFastGetHashCode + { + get + { + Debug.Assert(HasCheckedCanCompareBitsOrUseFastGetHashCode); + return (Flags & enum_flag_CanCompareBitsOrUseFastGetHashCode) != 0; + } + } + } + /// /// A type handle, which can wrap either a pointer to a TypeDesc or to a . /// diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index cc13e37e083f0..200ae018a9a04 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -66,8 +66,27 @@ ref RuntimeHelpers.GetRawData(obj), return true; } + private static unsafe bool CanCompareBits(object obj) + { + MethodTable* pMT = RuntimeHelpers.GetMethodTable(obj); + MethodTableAuxiliaryData* pAuxData = pMT->AuxiliaryData; + bool result; + + if (pAuxData->HasCheckedCanCompareBitsOrUseFastGetHashCode) + { + result = pAuxData->CanCompareBitsOrUseFastGetHashCode; + } + else + { + result = CanCompareBitsOrUseFastGetHashCode(pMT); + } + + GC.KeepAlive(obj); + return result; + } + [MethodImpl(MethodImplOptions.InternalCall)] - private static extern bool CanCompareBits(object obj); + private static extern unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT); /*=================================GetHashCode================================== **Action: Our algorithm for returning the hashcode is a little bit complex. We look From 246168c4513a89e83cfbef0b3048d867bf4cfb34 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:05:14 +0800 Subject: [PATCH 02/18] Switch to QCall --- .../System.Private.CoreLib/src/System/ValueType.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 200ae018a9a04..9e3f7cf407ad0 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -13,12 +13,13 @@ using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace System { [Serializable] [TypeForwardedFrom("mscorlib, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089")] - public abstract class ValueType + public abstract partial class ValueType { [UnconditionalSuppressMessage("ReflectionAnalysis", "IL2075:UnrecognizedReflectionPattern", Justification = "Trimmed fields don't make a difference for equality")] @@ -85,8 +86,9 @@ private static unsafe bool CanCompareBits(object obj) return result; } - [MethodImpl(MethodImplOptions.InternalCall)] - private static extern unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT); + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_CanCompareBitsOrUseFastGetHashCode")] + [return: MarshalAs(UnmanagedType.Bool)] + private static unsafe partial bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT); /*=================================GetHashCode================================== **Action: Our algorithm for returning the hashcode is a little bit complex. We look From 580f969cef319d6adec94352fb013f26e6b023b8 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:11:09 +0800 Subject: [PATCH 03/18] Implement the QCall --- .../src/System/ValueType.cs | 4 ++- src/coreclr/vm/comutilnative.cpp | 34 +++---------------- src/coreclr/vm/comutilnative.h | 2 +- src/coreclr/vm/ecalllist.h | 1 - src/coreclr/vm/qcallentrypoints.cpp | 1 + 5 files changed, 10 insertions(+), 32 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 9e3f7cf407ad0..686911f6a034f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -67,6 +67,8 @@ ref RuntimeHelpers.GetRawData(obj), return true; } + // Return true if the valuetype does not contain pointer, is tightly packed, + // does not have floating point number field and does not override Equals method. private static unsafe bool CanCompareBits(object obj) { MethodTable* pMT = RuntimeHelpers.GetMethodTable(obj); @@ -86,7 +88,7 @@ private static unsafe bool CanCompareBits(object obj) return result; } - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_CanCompareBitsOrUseFastGetHashCode")] + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "MethodTable_CanCompareBitsOrUseFastGetHashCode")] [return: MarshalAs(UnmanagedType.Bool)] private static unsafe partial bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT); diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 027c4ae8903ae..149cdef69ab02 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1681,44 +1681,20 @@ BOOL CanCompareBitsOrUseFastGetHashCode(MethodTable* mt) return canCompareBitsOrUseFastGetHashCode; } -NOINLINE static FC_BOOL_RET CanCompareBitsHelper(MethodTable* mt, OBJECTREF objRef) +extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable * mt) { - FC_INNER_PROLOG(ValueTypeHelper::CanCompareBits); - - _ASSERTE(mt != NULL); - _ASSERTE(objRef != NULL); + QCALL_CONTRACT; BOOL ret = FALSE; - HELPER_METHOD_FRAME_BEGIN_RET_ATTRIB_1(Frame::FRAME_ATTR_EXACT_DEPTH|Frame::FRAME_ATTR_CAPTURE_DEPTH_2, objRef); + BEGIN_QCALL; ret = CanCompareBitsOrUseFastGetHashCode(mt); - HELPER_METHOD_FRAME_END(); - FC_INNER_EPILOG(); - - FC_RETURN_BOOL(ret); -} - -// Return true if the valuetype does not contain pointer, is tightly packed, -// does not have floating point number field and does not override Equals method. -FCIMPL1(FC_BOOL_RET, ValueTypeHelper::CanCompareBits, Object* obj) -{ - FCALL_CONTRACT; - - _ASSERTE(obj != NULL); - MethodTable* mt = obj->GetMethodTable(); - - if (mt->HasCheckedCanCompareBitsOrUseFastGetHashCode()) - { - FC_RETURN_BOOL(mt->CanCompareBitsOrUseFastGetHashCode()); - } - - OBJECTREF objRef(obj); + END_QCALL; - FC_INNER_RETURN(FC_BOOL_RET, CanCompareBitsHelper(mt, objRef)); + return ret; } -FCIMPLEND static INT32 FastGetValueTypeHashCodeHelper(MethodTable *mt, void *pObjRef) { diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 80d35da7b7214..88a9d548ab4d4 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -247,7 +247,6 @@ extern "C" void QCALLTYPE Interlocked_MemoryBarrierProcessWide(); class ValueTypeHelper { public: - static FCDECL1(FC_BOOL_RET, CanCompareBits, Object* obj); static FCDECL1(INT32, GetHashCode, Object* objRef); }; @@ -257,6 +256,7 @@ class MethodTableNative { }; extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); +extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); class StreamNative { public: diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 64142b1fb69d1..811630d8bcf20 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -77,7 +77,6 @@ FCFuncStart(gStringFuncs) FCFuncEnd() FCFuncStart(gValueTypeFuncs) - FCFuncElement("CanCompareBits", ValueTypeHelper::CanCompareBits) FCFuncElement("GetHashCode", ValueTypeHelper::GetHashCode) FCFuncEnd() diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 3e149e1a763a2..5fd7625c717f8 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -100,6 +100,7 @@ static const Entry s_QCall[] = DllImportEntry(QCall_GetGCHandleForTypeHandle) DllImportEntry(QCall_FreeGCHandleForTypeHandle) DllImportEntry(MethodTable_AreTypesEquivalent) + DllImportEntry(MethodTable_CanCompareBitsOrUseFastGetHashCode) DllImportEntry(RuntimeTypeHandle_MakePointer) DllImportEntry(RuntimeTypeHandle_MakeByRef) DllImportEntry(RuntimeTypeHandle_MakeSZArray) From 99b4ad9829a24525e5f9cf4df34815b583d974a3 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:26:28 +0800 Subject: [PATCH 04/18] Fast path of GetHashCode --- .../src/System/ValueType.cs | 42 ++++++++++++++++++- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 686911f6a034f..c70e0abcd2236 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -102,8 +102,46 @@ private static unsafe bool CanCompareBits(object obj) **Arguments: None. **Exceptions: None. ==============================================================================*/ - [MethodImpl(MethodImplOptions.InternalCall)] - public extern override int GetHashCode(); + public override unsafe int GetHashCode() + { + MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); + + // We don't want to expose the method table pointer in the hash code + // Let's use the typeID instead. + uint typeID = 12345; + + // To get less colliding and more evenly distributed hash codes, + // we munge the class index with two big prime numbers + int hashCode = (int)(typeID * 711650207 + 2506965631U); + + if (CanCompareBits(this)) + { + hashCode ^= FastGetValueTypeHashCodeHelper(pMT, ref this.GetRawData()); + } + else + { + object obj = this; + hashCode ^= RegularGetValueTypeHashCode(pMT, ObjectHandleOnStack.Create(ref obj)); + } + + GC.KeepAlive(this); + return hashCode; + } + + private static unsafe int FastGetValueTypeHashCodeHelper(MethodTable* pMT, ref byte data) + { + int hashCode = 0; + + // this is a struct with no refs and no "strange" offsets, just go through the obj and xor the bits + nuint size = pMT->GetNumInstanceFieldBytes(); + for (nuint i = 0; i < size; i++) + hashCode ^= Unsafe.AddByteOffset(ref data, i); + + return hashCode; + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "")] + private static unsafe partial int RegularGetValueTypeHashCode(MethodTable* pMT, ObjectHandleOnStack obj); public override string? ToString() { From 6fa466601c39a5758942085ec8774aa5a039a44b Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:29:52 +0800 Subject: [PATCH 05/18] Reduce MethodTable access --- .../src/System/ValueType.cs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index c70e0abcd2236..c3817e76bb1c5 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -37,7 +37,7 @@ public override unsafe bool Equals([NotNullWhen(true)] object? obj) // if there are no GC references in this object we can avoid reflection // and do a fast memcmp - if (CanCompareBits(this)) + if (CanCompareBitsOrUseFastGetHashCode(RuntimeHelpers.GetMethodTable(obj))) // MethodTable kept alive by access to object below { return SpanHelpers.SequenceEqual( ref RuntimeHelpers.GetRawData(this), @@ -69,28 +69,21 @@ ref RuntimeHelpers.GetRawData(obj), // Return true if the valuetype does not contain pointer, is tightly packed, // does not have floating point number field and does not override Equals method. - private static unsafe bool CanCompareBits(object obj) + private static unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT) { - MethodTable* pMT = RuntimeHelpers.GetMethodTable(obj); MethodTableAuxiliaryData* pAuxData = pMT->AuxiliaryData; - bool result; if (pAuxData->HasCheckedCanCompareBitsOrUseFastGetHashCode) { - result = pAuxData->CanCompareBitsOrUseFastGetHashCode; - } - else - { - result = CanCompareBitsOrUseFastGetHashCode(pMT); + return pAuxData->CanCompareBitsOrUseFastGetHashCode; } - GC.KeepAlive(obj); - return result; + return CanCompareBitsOrUseFastGetHashCodeHelper(pMT); } [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "MethodTable_CanCompareBitsOrUseFastGetHashCode")] [return: MarshalAs(UnmanagedType.Bool)] - private static unsafe partial bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT); + private static unsafe partial bool CanCompareBitsOrUseFastGetHashCodeHelper(MethodTable* pMT); /*=================================GetHashCode================================== **Action: Our algorithm for returning the hashcode is a little bit complex. We look @@ -114,7 +107,7 @@ public override unsafe int GetHashCode() // we munge the class index with two big prime numbers int hashCode = (int)(typeID * 711650207 + 2506965631U); - if (CanCompareBits(this)) + if (CanCompareBitsOrUseFastGetHashCode(pMT)) { hashCode ^= FastGetValueTypeHashCodeHelper(pMT, ref this.GetRawData()); } From 6d54f40b37c2a202abd6fc75cfc96ea737aab1ea Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:41:53 +0800 Subject: [PATCH 06/18] Expose GetTypeID --- .../RuntimeHelpers.CoreCLR.cs | 3 +++ .../src/System/ValueType.cs | 2 +- src/coreclr/vm/comutilnative.cpp | 21 +++++++++++++++++++ src/coreclr/vm/comutilnative.h | 1 + src/coreclr/vm/qcallentrypoints.cpp | 1 + 5 files changed, 27 insertions(+), 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 7f009df62309b..c62e7792067de 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 @@ -330,6 +330,9 @@ internal static unsafe bool ObjectHasComponentSize(object obj) [return: MarshalAs(UnmanagedType.Bool)] internal static unsafe partial bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb); + [LibraryImport(QCall, EntryPoint = "MethodTable_GetTypeID")] + internal static unsafe partial uint GetTypeID(MethodTable* pMT); + /// /// Allocate memory that is associated with the and /// will be freed if and when the is unloaded. diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index c3817e76bb1c5..8e0a1d752b15f 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -101,7 +101,7 @@ public override unsafe int GetHashCode() // We don't want to expose the method table pointer in the hash code // Let's use the typeID instead. - uint typeID = 12345; + uint typeID = RuntimeHelpers.GetTypeID(pMT); // To get less colliding and more evenly distributed hash codes, // we munge the class index with two big prime numbers diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 149cdef69ab02..cb2da0de56d03 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1900,6 +1900,27 @@ extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, Metho return bResult; } +extern "C" UINT32 QCALLTYPE MethodTable_GetTypeID(MethodTable * mt) +{ + QCALL_CONTRACT; + + UINT32 typeID = 0; + + BEGIN_QCALL; + + typeID = mt->LookupTypeID(); + if (typeID == TypeIDProvider::INVALID_TYPE_ID) + { + // If the typeID has yet to be generated, fall back to GetTypeID + // This only needs to be done once per MethodTable + typeID = mt->GetTypeID(); + } + + END_QCALL; + + return typeID; +} + static MethodTable * g_pStreamMT; static WORD g_slotBeginRead, g_slotEndRead; static WORD g_slotBeginWrite, g_slotEndWrite; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 88a9d548ab4d4..26cf87538bf74 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -257,6 +257,7 @@ class MethodTableNative { extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); +extern "C" UINT32 QCALLTYPE MethodTable_GetTypeID(MethodTable* mt); class StreamNative { public: diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 5fd7625c717f8..acb442ea934c0 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -101,6 +101,7 @@ static const Entry s_QCall[] = DllImportEntry(QCall_FreeGCHandleForTypeHandle) DllImportEntry(MethodTable_AreTypesEquivalent) DllImportEntry(MethodTable_CanCompareBitsOrUseFastGetHashCode) + DllImportEntry(MethodTable_GetTypeID) DllImportEntry(RuntimeTypeHandle_MakePointer) DllImportEntry(RuntimeTypeHandle_MakeByRef) DllImportEntry(RuntimeTypeHandle_MakeSZArray) From 5a5da0637998cb8fd15687780f44cdf25090f0d9 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:51:02 +0800 Subject: [PATCH 07/18] Call for RegularGetValueTypeHashCode --- .../src/System/ValueType.cs | 3 ++- src/coreclr/vm/comutilnative.cpp | 20 +++++++++++++++++++ src/coreclr/vm/comutilnative.h | 1 + src/coreclr/vm/qcallentrypoints.cpp | 1 + 4 files changed, 24 insertions(+), 1 deletion(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 8e0a1d752b15f..b22350986620b 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -133,7 +133,8 @@ private static unsafe int FastGetValueTypeHashCodeHelper(MethodTable* pMT, ref b return hashCode; } - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "")] + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_RegularGetValueTypeHashCode")] + [SuppressGCTransition] private static unsafe partial int RegularGetValueTypeHashCode(MethodTable* pMT, ObjectHandleOnStack obj); public override string? ToString() diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index cb2da0de56d03..fcb18e6581612 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1818,6 +1818,26 @@ static INT32 RegularGetValueTypeHashCode(MethodTable *mt, void *pObjRef) return hashCode; } +extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * pMT, QCall::ObjectHandleOnStack objHandle) +{ + CONTRACTL + { + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } CONTRACTL_END; + + INT32 ret = 0; + + BEGIN_QCALL; + + ret = RegularGetValueTypeHashCode(pMT, objHandle.Get()->UnBox()); + + END_QCALL; + + return ret; +} + // The default implementation of GetHashCode() for all value types. // Note that this implementation reveals the value of the fields. // So if the value type contains any sensitive information it should diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 26cf87538bf74..80dff467f618c 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -258,6 +258,7 @@ class MethodTableNative { extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); extern "C" UINT32 QCALLTYPE MethodTable_GetTypeID(MethodTable* mt); +extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * pMT, QCall::ObjectHandleOnStack objHandle); class StreamNative { public: diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index acb442ea934c0..4efffeacbbfc3 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -102,6 +102,7 @@ static const Entry s_QCall[] = DllImportEntry(MethodTable_AreTypesEquivalent) DllImportEntry(MethodTable_CanCompareBitsOrUseFastGetHashCode) DllImportEntry(MethodTable_GetTypeID) + DllImportEntry(ValueType_RegularGetValueTypeHashCode) DllImportEntry(RuntimeTypeHandle_MakePointer) DllImportEntry(RuntimeTypeHandle_MakeByRef) DllImportEntry(RuntimeTypeHandle_MakeSZArray) From ce7f636f17bbafeb569c15737f1e2c2a24907956 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 17:53:30 +0800 Subject: [PATCH 08/18] Cleanup unused FCall --- .../src/System/ValueType.cs | 5 ++ src/coreclr/vm/comutilnative.cpp | 60 ------------------- src/coreclr/vm/comutilnative.h | 5 -- src/coreclr/vm/ecalllist.h | 5 -- 4 files changed, 5 insertions(+), 70 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index b22350986620b..46e5060899387 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -97,6 +97,11 @@ private static unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT) ==============================================================================*/ public override unsafe int GetHashCode() { + // The default implementation of GetHashCode() for all value types. + // Note that this implementation reveals the value of the fields. + // So if the value type contains any sensitive information it should + // implement its own GetHashCode(). + MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); // We don't want to expose the method table pointer in the hash code diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index fcb18e6581612..3df1c6fa2d3e6 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1838,66 +1838,6 @@ extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * p return ret; } -// The default implementation of GetHashCode() for all value types. -// Note that this implementation reveals the value of the fields. -// So if the value type contains any sensitive information it should -// implement its own GetHashCode(). -FCIMPL1(INT32, ValueTypeHelper::GetHashCode, Object* objUNSAFE) -{ - FCALL_CONTRACT; - - if (objUNSAFE == NULL) - FCThrow(kNullReferenceException); - - OBJECTREF obj = ObjectToOBJECTREF(objUNSAFE); - VALIDATEOBJECTREF(obj); - - INT32 hashCode = 0; - MethodTable *pMT = objUNSAFE->GetMethodTable(); - - // We don't want to expose the method table pointer in the hash code - // Let's use the typeID instead. - UINT32 typeID = pMT->LookupTypeID(); - if (typeID == TypeIDProvider::INVALID_TYPE_ID) - { - // If the typeID has yet to be generated, fall back to GetTypeID - // This only needs to be done once per MethodTable - HELPER_METHOD_FRAME_BEGIN_RET_1(obj); - typeID = pMT->GetTypeID(); - HELPER_METHOD_FRAME_END(); - } - - // To get less colliding and more evenly distributed hash codes, - // we munge the class index with two big prime numbers - hashCode = typeID * 711650207 + 2506965631U; - - BOOL canUseFastGetHashCodeHelper = FALSE; - if (pMT->HasCheckedCanCompareBitsOrUseFastGetHashCode()) - { - canUseFastGetHashCodeHelper = pMT->CanCompareBitsOrUseFastGetHashCode(); - } - else - { - HELPER_METHOD_FRAME_BEGIN_RET_1(obj); - canUseFastGetHashCodeHelper = CanCompareBitsOrUseFastGetHashCode(pMT); - HELPER_METHOD_FRAME_END(); - } - - if (canUseFastGetHashCodeHelper) - { - hashCode ^= FastGetValueTypeHashCodeHelper(pMT, obj->UnBox()); - } - else - { - HELPER_METHOD_FRAME_BEGIN_RET_1(obj); - hashCode ^= RegularGetValueTypeHashCode(pMT, obj->UnBox()); - HELPER_METHOD_FRAME_END(); - } - - return hashCode; -} -FCIMPLEND - FCIMPL1(UINT32, MethodTableNative::GetNumInstanceFieldBytes, MethodTable* mt) { FCALL_CONTRACT; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 80dff467f618c..34d87337d0264 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -245,11 +245,6 @@ class COMInterlocked extern "C" void QCALLTYPE Interlocked_MemoryBarrierProcessWide(); -class ValueTypeHelper { -public: - static FCDECL1(INT32, GetHashCode, Object* objRef); -}; - class MethodTableNative { public: static FCDECL1(UINT32, GetNumInstanceFieldBytes, MethodTable* mt); diff --git a/src/coreclr/vm/ecalllist.h b/src/coreclr/vm/ecalllist.h index 811630d8bcf20..dc266b0b0d9ee 100644 --- a/src/coreclr/vm/ecalllist.h +++ b/src/coreclr/vm/ecalllist.h @@ -76,10 +76,6 @@ FCFuncStart(gStringFuncs) FCDynamicSig(COR_CTOR_METHOD_NAME, &gsig_IM_PtrSByt_Int_Int_Encoding_RetVoid, ECall::CtorSBytePtrStartLengthEncodingManaged) FCFuncEnd() -FCFuncStart(gValueTypeFuncs) - FCFuncElement("GetHashCode", ValueTypeHelper::GetHashCode) -FCFuncEnd() - FCFuncStart(gDiagnosticsDebugger) FCFuncElement("BreakInternal", DebugDebugger::Break) FCFuncElement("get_IsAttached", DebugDebugger::IsDebuggerAttached) @@ -659,7 +655,6 @@ FCClassElement("Thread", "System.Threading", gThreadFuncs) FCClassElement("ThreadPool", "System.Threading", gThreadPoolFuncs) FCClassElement("Type", "System", gSystem_Type) FCClassElement("TypedReference", "System", gTypedReferenceFuncs) -FCClassElement("ValueType", "System", gValueTypeFuncs) #ifdef FEATURE_COMINTEROP FCClassElement("Variant", "System", gVariantFuncs) #endif From 7091fb0a9766bad72e57cc6364bd0a62f92f9a62 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sat, 27 Jan 2024 18:41:23 +0800 Subject: [PATCH 09/18] Use HashCode type --- .../src/System/ValueType.cs | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 46e5060899387..1447eb9ebda35 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -114,7 +114,11 @@ public override unsafe int GetHashCode() if (CanCompareBitsOrUseFastGetHashCode(pMT)) { - hashCode ^= FastGetValueTypeHashCodeHelper(pMT, ref this.GetRawData()); + // this is a struct with no refs and no "strange" offsets + HashCode hash = default; + uint size = pMT->GetNumInstanceFieldBytes(); + hash.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref this.GetRawData(), (int)size)); + hashCode ^= hash.ToHashCode(); } else { @@ -126,18 +130,6 @@ public override unsafe int GetHashCode() return hashCode; } - private static unsafe int FastGetValueTypeHashCodeHelper(MethodTable* pMT, ref byte data) - { - int hashCode = 0; - - // this is a struct with no refs and no "strange" offsets, just go through the obj and xor the bits - nuint size = pMT->GetNumInstanceFieldBytes(); - for (nuint i = 0; i < size; i++) - hashCode ^= Unsafe.AddByteOffset(ref data, i); - - return hashCode; - } - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_RegularGetValueTypeHashCode")] [SuppressGCTransition] private static unsafe partial int RegularGetValueTypeHashCode(MethodTable* pMT, ObjectHandleOnStack obj); From a0a1e3296f19a1ec0f08c13f5f041d3c2264a39e Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 28 Jan 2024 00:36:06 +0800 Subject: [PATCH 10/18] Remove TypeID usage --- .../RuntimeHelpers.CoreCLR.cs | 3 --- .../src/System/ValueType.cs | 18 ++++++---------- src/coreclr/vm/comutilnative.cpp | 21 ------------------- src/coreclr/vm/comutilnative.h | 1 - src/coreclr/vm/qcallentrypoints.cpp | 1 - 5 files changed, 6 insertions(+), 38 deletions(-) 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 c62e7792067de..7f009df62309b 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 @@ -330,9 +330,6 @@ internal static unsafe bool ObjectHasComponentSize(object obj) [return: MarshalAs(UnmanagedType.Bool)] internal static unsafe partial bool AreTypesEquivalent(MethodTable* pMTa, MethodTable* pMTb); - [LibraryImport(QCall, EntryPoint = "MethodTable_GetTypeID")] - internal static unsafe partial uint GetTypeID(MethodTable* pMT); - /// /// Allocate memory that is associated with the and /// will be freed if and when the is unloaded. diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 1447eb9ebda35..a079bed0490bb 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -103,31 +103,25 @@ public override unsafe int GetHashCode() // implement its own GetHashCode(). MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); - - // We don't want to expose the method table pointer in the hash code - // Let's use the typeID instead. - uint typeID = RuntimeHelpers.GetTypeID(pMT); + HashCode hashCode = default; // To get less colliding and more evenly distributed hash codes, - // we munge the class index with two big prime numbers - int hashCode = (int)(typeID * 711650207 + 2506965631U); + // we munge the class index into the hashcode + hashCode.Add((IntPtr)pMT); if (CanCompareBitsOrUseFastGetHashCode(pMT)) { // this is a struct with no refs and no "strange" offsets - HashCode hash = default; uint size = pMT->GetNumInstanceFieldBytes(); - hash.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref this.GetRawData(), (int)size)); - hashCode ^= hash.ToHashCode(); + hashCode.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref this.GetRawData(), (int)size)); } else { object obj = this; - hashCode ^= RegularGetValueTypeHashCode(pMT, ObjectHandleOnStack.Create(ref obj)); + hashCode.Add(RegularGetValueTypeHashCode(pMT, ObjectHandleOnStack.Create(ref obj))); } - GC.KeepAlive(this); - return hashCode; + return hashCode.ToHashCode(); } [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_RegularGetValueTypeHashCode")] diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 3df1c6fa2d3e6..a2bc0a538da69 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1860,27 +1860,6 @@ extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, Metho return bResult; } -extern "C" UINT32 QCALLTYPE MethodTable_GetTypeID(MethodTable * mt) -{ - QCALL_CONTRACT; - - UINT32 typeID = 0; - - BEGIN_QCALL; - - typeID = mt->LookupTypeID(); - if (typeID == TypeIDProvider::INVALID_TYPE_ID) - { - // If the typeID has yet to be generated, fall back to GetTypeID - // This only needs to be done once per MethodTable - typeID = mt->GetTypeID(); - } - - END_QCALL; - - return typeID; -} - static MethodTable * g_pStreamMT; static WORD g_slotBeginRead, g_slotEndRead; static WORD g_slotBeginWrite, g_slotEndWrite; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 34d87337d0264..eb594537278e2 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -252,7 +252,6 @@ class MethodTableNative { extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); -extern "C" UINT32 QCALLTYPE MethodTable_GetTypeID(MethodTable* mt); extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * pMT, QCall::ObjectHandleOnStack objHandle); class StreamNative { diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index 4efffeacbbfc3..f9c452db1c293 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -101,7 +101,6 @@ static const Entry s_QCall[] = DllImportEntry(QCall_FreeGCHandleForTypeHandle) DllImportEntry(MethodTable_AreTypesEquivalent) DllImportEntry(MethodTable_CanCompareBitsOrUseFastGetHashCode) - DllImportEntry(MethodTable_GetTypeID) DllImportEntry(ValueType_RegularGetValueTypeHashCode) DllImportEntry(RuntimeTypeHandle_MakePointer) DllImportEntry(RuntimeTypeHandle_MakeByRef) From 808ceb43fddb4a211ce841df4a3ce553a8c65693 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 28 Jan 2024 01:29:48 +0800 Subject: [PATCH 11/18] Move each branch to managed --- .../src/System/ValueType.cs | 51 ++++- src/coreclr/vm/comutilnative.cpp | 185 ++++++------------ src/coreclr/vm/comutilnative.h | 2 +- src/coreclr/vm/qcallentrypoints.cpp | 2 +- 4 files changed, 110 insertions(+), 130 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index a079bed0490bb..9ba9ff2ccad1b 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -10,6 +10,7 @@ ** ===========================================================*/ +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Reflection; using System.Runtime.CompilerServices; @@ -95,14 +96,15 @@ private static unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT) **Arguments: None. **Exceptions: None. ==============================================================================*/ - public override unsafe int GetHashCode() + public override unsafe int GetHashCode() => GetHashCodeHelper(RuntimeHelpers.GetMethodTable(this), ref RuntimeHelpers.GetRawData(this)); + + private static unsafe int GetHashCodeHelper(MethodTable* pMT, ref byte rawData) { // The default implementation of GetHashCode() for all value types. // Note that this implementation reveals the value of the fields. // So if the value type contains any sensitive information it should // implement its own GetHashCode(). - MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); HashCode hashCode = default; // To get less colliding and more evenly distributed hash codes, @@ -113,20 +115,53 @@ public override unsafe int GetHashCode() { // this is a struct with no refs and no "strange" offsets uint size = pMT->GetNumInstanceFieldBytes(); - hashCode.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref this.GetRawData(), (int)size)); + hashCode.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref rawData, (int)size)); } else { - object obj = this; - hashCode.Add(RegularGetValueTypeHashCode(pMT, ObjectHandleOnStack.Create(ref obj))); + switch (GetHashCodeStrategy(pMT, ref rawData, out uint fieldOffset, out uint fieldSize, out MethodTable* fieldMethodTable)) + { + case ValueTypeHashCodeStrategy.ReferenceField: + hashCode.Add(Unsafe.As(ref Unsafe.AddByteOffset(ref rawData, fieldOffset)).GetHashCode()); + break; + + case ValueTypeHashCodeStrategy.DoubleField: + hashCode.Add(Unsafe.As(ref Unsafe.AddByteOffset(ref rawData, fieldOffset)).GetHashCode()); + break; + + case ValueTypeHashCodeStrategy.SingleField: + hashCode.Add(Unsafe.As(ref Unsafe.AddByteOffset(ref rawData, fieldOffset)).GetHashCode()); + break; + + case ValueTypeHashCodeStrategy.PrimitiveField: + Debug.Assert(fieldSize != 0); + hashCode.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref Unsafe.AddByteOffset(ref rawData, fieldOffset), (int)fieldSize)); + break; + + case ValueTypeHashCodeStrategy.ValueTypeField: + Debug.Assert(fieldMethodTable != null); + hashCode.Add(GetHashCodeHelper(fieldMethodTable, ref Unsafe.AddByteOffset(ref rawData, fieldOffset))); + break; + } } return hashCode.ToHashCode(); } - [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_RegularGetValueTypeHashCode")] - [SuppressGCTransition] - private static unsafe partial int RegularGetValueTypeHashCode(MethodTable* pMT, ObjectHandleOnStack obj); + // Must match the definition in src\vm\comutilnative.cpp + private enum ValueTypeHashCodeStrategy + { + None, + ReferenceField, + DoubleField, + SingleField, + PrimitiveField, + ValueTypeField, + } + + [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_GetHashCodeStrategy")] + private static unsafe partial ValueTypeHashCodeStrategy GetHashCodeStrategy( + MethodTable* pMT, ref byte rawData, out uint fieldOffset, out uint fieldSize, out MethodTable* fieldMethodTable); public override string? ToString() { diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index a2bc0a538da69..9afa5abdf3959 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1696,143 +1696,88 @@ extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodT return ret; } -static INT32 FastGetValueTypeHashCodeHelper(MethodTable *mt, void *pObjRef) +enum ValueTypeHashCodeStrategy { - CONTRACTL - { - NOTHROW; - GC_NOTRIGGER; - MODE_COOPERATIVE; - } CONTRACTL_END; - - INT32 hashCode = 0; - INT32 *pObj = (INT32*)pObjRef; - - // this is a struct with no refs and no "strange" offsets, just go through the obj and xor the bits - INT32 size = mt->GetNumInstanceFieldBytes(); - for (INT32 i = 0; i < (INT32)(size / sizeof(INT32)); i++) - hashCode ^= *pObj++; - - return hashCode; -} + None, + ReferenceField, + DoubleField, + SingleField, + PrimitiveField, + ValueTypeField, +}; -static INT32 RegularGetValueTypeHashCode(MethodTable *mt, void *pObjRef) +extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* pObjRef, UINT32* fieldOffset, UINT32* fieldSize, MethodTable** fieldMethodTable) { - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } CONTRACTL_END; + QCALL_CONTRACT; - INT32 hashCode = 0; + ValueTypeHashCodeStrategy ret = ValueTypeHashCodeStrategy::None; + *fieldOffset = 0; + *fieldSize = 0; + *fieldMethodTable = NULL; - GCPROTECT_BEGININTERIOR(pObjRef); + BEGIN_QCALL; - BOOL canUseFastGetHashCodeHelper = FALSE; - if (mt->HasCheckedCanCompareBitsOrUseFastGetHashCode()) - { - canUseFastGetHashCodeHelper = mt->CanCompareBitsOrUseFastGetHashCode(); - } - else - { - canUseFastGetHashCodeHelper = CanCompareBitsOrUseFastGetHashCode(mt); - } + // Should be handled by fast path + _ASSERTE(!mt->CanCompareBitsOrUseFastGetHashCode()); - // While we should not get here directly from ValueTypeHelper::GetHashCode, if we recurse we need to - // be able to handle getting the hashcode for an embedded structure whose hashcode is computed by the fast path. - if (canUseFastGetHashCodeHelper) - { - hashCode = FastGetValueTypeHashCodeHelper(mt, pObjRef); - } - else + GCX_COOP(); + + // it's looking ugly so we'll use the old behavior in managed code. Grab the first non-null + // field and return its hash code or 'it' as hash code + // Note that the old behavior has already been broken for value types + // that is qualified for CanUseFastGetHashCodeHelper. So maybe we should + // change the implementation here to use all fields instead of just the 1st one. + // + // + // check this approximation - we may be losing exact type information + ApproxFieldDescIterator fdIterator(mt, ApproxFieldDescIterator::INSTANCE_FIELDS); + + FieldDesc *field; + while ((field = fdIterator.Next()) != NULL) { - // it's looking ugly so we'll use the old behavior in managed code. Grab the first non-null - // field and return its hash code or 'it' as hash code - // Note that the old behavior has already been broken for value types - // that is qualified for CanUseFastGetHashCodeHelper. So maybe we should - // change the implementation here to use all fields instead of just the 1st one. - // - // - // check this approximation - we may be losing exact type information - ApproxFieldDescIterator fdIterator(mt, ApproxFieldDescIterator::INSTANCE_FIELDS); - - FieldDesc *field; - while ((field = fdIterator.Next()) != NULL) + _ASSERTE(!field->IsRVA()); + *fieldOffset = field->GetOffsetUnsafe(); + if (field->IsObjRef()) { - _ASSERTE(!field->IsRVA()); - if (field->IsObjRef()) + // if we get an object reference we get the hash code out of that + if (*(Object**)((BYTE *)pObjRef + field->GetOffsetUnsafe()) != NULL) { - // if we get an object reference we get the hash code out of that - if (*(Object**)((BYTE *)pObjRef + field->GetOffsetUnsafe()) != NULL) - { - PREPARE_SIMPLE_VIRTUAL_CALLSITE(METHOD__OBJECT__GET_HASH_CODE, (*(Object**)((BYTE *)pObjRef + field->GetOffsetUnsafe()))); - DECLARE_ARGHOLDER_ARRAY(args, 1); - args[ARGNUM_0] = PTR_TO_ARGHOLDER(*(Object**)((BYTE *)pObjRef + field->GetOffsetUnsafe())); - CALL_MANAGED_METHOD(hashCode, INT32, args); - } - else - { - // null object reference, try next - continue; - } + ret = ValueTypeHashCodeStrategy::ReferenceField; } else { - CorElementType fieldType = field->GetFieldType(); - if (fieldType == ELEMENT_TYPE_R8) - { - PREPARE_NONVIRTUAL_CALLSITE(METHOD__DOUBLE__GET_HASH_CODE); - DECLARE_ARGHOLDER_ARRAY(args, 1); - args[ARGNUM_0] = PTR_TO_ARGHOLDER(((BYTE *)pObjRef + field->GetOffsetUnsafe())); - CALL_MANAGED_METHOD(hashCode, INT32, args); - } - else if (fieldType == ELEMENT_TYPE_R4) - { - PREPARE_NONVIRTUAL_CALLSITE(METHOD__SINGLE__GET_HASH_CODE); - DECLARE_ARGHOLDER_ARRAY(args, 1); - args[ARGNUM_0] = PTR_TO_ARGHOLDER(((BYTE *)pObjRef + field->GetOffsetUnsafe())); - CALL_MANAGED_METHOD(hashCode, INT32, args); - } - else if (fieldType != ELEMENT_TYPE_VALUETYPE) - { - UINT fieldSize = field->LoadSize(); - INT32 *pValue = (INT32*)((BYTE *)pObjRef + field->GetOffsetUnsafe()); - for (INT32 j = 0; j < (INT32)(fieldSize / sizeof(INT32)); j++) - hashCode ^= *pValue++; - } - else - { - // got another value type. Get the type - TypeHandle fieldTH = field->GetFieldTypeHandleThrowing(); - _ASSERTE(!fieldTH.IsNull()); - hashCode = RegularGetValueTypeHashCode(fieldTH.GetMethodTable(), (BYTE *)pObjRef + field->GetOffsetUnsafe()); - } + // null object reference, try next + continue; + } + } + else + { + CorElementType fieldType = field->GetFieldType(); + if (fieldType == ELEMENT_TYPE_R8) + { + ret = ValueTypeHashCodeStrategy::DoubleField; + } + else if (fieldType == ELEMENT_TYPE_R4) + { + ret = ValueTypeHashCodeStrategy::SingleField; + } + else if (fieldType != ELEMENT_TYPE_VALUETYPE) + { + *fieldSize = field->LoadSize(); + ret = ValueTypeHashCodeStrategy::PrimitiveField; + } + else + { + // got another value type. Get the type + TypeHandle fieldTH = field->GetFieldTypeHandleThrowing(); + _ASSERTE(!fieldTH.IsNull()); + *fieldMethodTable = fieldTH.GetMethodTable(); + ret = ValueTypeHashCodeStrategy::ValueTypeField; } - break; } + break; } - GCPROTECT_END(); - - return hashCode; -} - -extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * pMT, QCall::ObjectHandleOnStack objHandle) -{ - CONTRACTL - { - THROWS; - GC_TRIGGERS; - MODE_COOPERATIVE; - } CONTRACTL_END; - - INT32 ret = 0; - - BEGIN_QCALL; - - ret = RegularGetValueTypeHashCode(pMT, objHandle.Get()->UnBox()); - END_QCALL; return ret; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index eb594537278e2..0a2eaec901b93 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -252,7 +252,7 @@ class MethodTableNative { extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); -extern "C" INT32 QCALLTYPE ValueType_RegularGetValueTypeHashCode(MethodTable * pMT, QCall::ObjectHandleOnStack objHandle); +extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* objHandle, UINT32* fieldOffset, UINT32* fieldSize, MethodTable** fieldMethodTable); class StreamNative { public: diff --git a/src/coreclr/vm/qcallentrypoints.cpp b/src/coreclr/vm/qcallentrypoints.cpp index f9c452db1c293..af35e0cc906e2 100644 --- a/src/coreclr/vm/qcallentrypoints.cpp +++ b/src/coreclr/vm/qcallentrypoints.cpp @@ -101,7 +101,7 @@ static const Entry s_QCall[] = DllImportEntry(QCall_FreeGCHandleForTypeHandle) DllImportEntry(MethodTable_AreTypesEquivalent) DllImportEntry(MethodTable_CanCompareBitsOrUseFastGetHashCode) - DllImportEntry(ValueType_RegularGetValueTypeHashCode) + DllImportEntry(ValueType_GetHashCodeStrategy) DllImportEntry(RuntimeTypeHandle_MakePointer) DllImportEntry(RuntimeTypeHandle_MakeByRef) DllImportEntry(RuntimeTypeHandle_MakeSZArray) From 9a8de4c1a1e74ad4f47016260741d102f13ef9d7 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 28 Jan 2024 19:16:27 +0800 Subject: [PATCH 12/18] Handle recursive case inside native code --- .../src/System/ValueType.cs | 21 +++---- src/coreclr/vm/comutilnative.cpp | 62 ++++++++++++++----- src/coreclr/vm/comutilnative.h | 2 +- 3 files changed, 54 insertions(+), 31 deletions(-) diff --git a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs index 9ba9ff2ccad1b..78301866c36dc 100644 --- a/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs +++ b/src/coreclr/System.Private.CoreLib/src/System/ValueType.cs @@ -96,15 +96,15 @@ private static unsafe bool CanCompareBitsOrUseFastGetHashCode(MethodTable* pMT) **Arguments: None. **Exceptions: None. ==============================================================================*/ - public override unsafe int GetHashCode() => GetHashCodeHelper(RuntimeHelpers.GetMethodTable(this), ref RuntimeHelpers.GetRawData(this)); - - private static unsafe int GetHashCodeHelper(MethodTable* pMT, ref byte rawData) + public override unsafe int GetHashCode() { // The default implementation of GetHashCode() for all value types. // Note that this implementation reveals the value of the fields. // So if the value type contains any sensitive information it should // implement its own GetHashCode(). + MethodTable* pMT = RuntimeHelpers.GetMethodTable(this); + ref byte rawData = ref RuntimeHelpers.GetRawData(this); HashCode hashCode = default; // To get less colliding and more evenly distributed hash codes, @@ -119,7 +119,8 @@ private static unsafe int GetHashCodeHelper(MethodTable* pMT, ref byte rawData) } else { - switch (GetHashCodeStrategy(pMT, ref rawData, out uint fieldOffset, out uint fieldSize, out MethodTable* fieldMethodTable)) + object thisRef = this; + switch (GetHashCodeStrategy(pMT, ObjectHandleOnStack.Create(ref thisRef), out uint fieldOffset, out uint fieldSize)) { case ValueTypeHashCodeStrategy.ReferenceField: hashCode.Add(Unsafe.As(ref Unsafe.AddByteOffset(ref rawData, fieldOffset)).GetHashCode()); @@ -133,15 +134,10 @@ private static unsafe int GetHashCodeHelper(MethodTable* pMT, ref byte rawData) hashCode.Add(Unsafe.As(ref Unsafe.AddByteOffset(ref rawData, fieldOffset)).GetHashCode()); break; - case ValueTypeHashCodeStrategy.PrimitiveField: + case ValueTypeHashCodeStrategy.FastGetHashCode: Debug.Assert(fieldSize != 0); hashCode.AddBytes(MemoryMarshal.CreateReadOnlySpan(ref Unsafe.AddByteOffset(ref rawData, fieldOffset), (int)fieldSize)); break; - - case ValueTypeHashCodeStrategy.ValueTypeField: - Debug.Assert(fieldMethodTable != null); - hashCode.Add(GetHashCodeHelper(fieldMethodTable, ref Unsafe.AddByteOffset(ref rawData, fieldOffset))); - break; } } @@ -155,13 +151,12 @@ private enum ValueTypeHashCodeStrategy ReferenceField, DoubleField, SingleField, - PrimitiveField, - ValueTypeField, + FastGetHashCode, } [LibraryImport(RuntimeHelpers.QCall, EntryPoint = "ValueType_GetHashCodeStrategy")] private static unsafe partial ValueTypeHashCodeStrategy GetHashCodeStrategy( - MethodTable* pMT, ref byte rawData, out uint fieldOffset, out uint fieldSize, out MethodTable* fieldMethodTable); + MethodTable* pMT, ObjectHandleOnStack objHandle, out uint fieldOffset, out uint fieldSize); public override string? ToString() { diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 9afa5abdf3959..f1b5c84656080 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1702,26 +1702,24 @@ enum ValueTypeHashCodeStrategy ReferenceField, DoubleField, SingleField, - PrimitiveField, - ValueTypeField, + FastGetHashCode, }; -extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* pObjRef, UINT32* fieldOffset, UINT32* fieldSize, MethodTable** fieldMethodTable) +static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObjRef, UINT32* fieldOffset, UINT32* fieldSize) { - QCALL_CONTRACT; - - ValueTypeHashCodeStrategy ret = ValueTypeHashCodeStrategy::None; - *fieldOffset = 0; - *fieldSize = 0; - *fieldMethodTable = NULL; - - BEGIN_QCALL; + CONTRACTL{ + THROWS; + GC_TRIGGERS; + MODE_COOPERATIVE; + } CONTRACTL_END; - // Should be handled by fast path + // Should be handled by caller _ASSERTE(!mt->CanCompareBitsOrUseFastGetHashCode()); - GCX_COOP(); + ValueTypeHashCodeStrategy ret = ValueTypeHashCodeStrategy::None; + GCPROTECT_BEGININTERIOR(pObjRef); + // it's looking ugly so we'll use the old behavior in managed code. Grab the first non-null // field and return its hash code or 'it' as hash code // Note that the old behavior has already been broken for value types @@ -1736,7 +1734,6 @@ extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* while ((field = fdIterator.Next()) != NULL) { _ASSERTE(!field->IsRVA()); - *fieldOffset = field->GetOffsetUnsafe(); if (field->IsObjRef()) { // if we get an object reference we get the hash code out of that @@ -1755,29 +1752,60 @@ extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* CorElementType fieldType = field->GetFieldType(); if (fieldType == ELEMENT_TYPE_R8) { + *fieldOffset = field->GetOffsetUnsafe(); ret = ValueTypeHashCodeStrategy::DoubleField; } else if (fieldType == ELEMENT_TYPE_R4) { + *fieldOffset = field->GetOffsetUnsafe(); ret = ValueTypeHashCodeStrategy::SingleField; } else if (fieldType != ELEMENT_TYPE_VALUETYPE) { + *fieldOffset = field->GetOffsetUnsafe(); *fieldSize = field->LoadSize(); - ret = ValueTypeHashCodeStrategy::PrimitiveField; + ret = ValueTypeHashCodeStrategy::FastGetHashCode; } else { // got another value type. Get the type TypeHandle fieldTH = field->GetFieldTypeHandleThrowing(); _ASSERTE(!fieldTH.IsNull()); - *fieldMethodTable = fieldTH.GetMethodTable(); - ret = ValueTypeHashCodeStrategy::ValueTypeField; + MethodTable* fieldMT = fieldTH.GetMethodTable(); + if (CanCompareBitsOrUseFastGetHashCode(fieldMT)) + { + *fieldOffset = field->GetOffsetUnsafe(); + *fieldSize = field->LoadSize(); + ret = ValueTypeHashCodeStrategy::FastGetHashCode; + } + else + { + UINT32 offset = 0; + ret = GetHashCodeStrategy(fieldMT, (BYTE *)pObjRef + field->GetOffsetUnsafe(), &offset, fieldSize); + *fieldOffset = field->GetOffsetUnsafe() + offset; + } } } break; } + GCPROTECT_END(); +} + +extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, QCall::ObjectHandleOnStack objHandle, UINT32* fieldOffset, UINT32* fieldSize) +{ + QCALL_CONTRACT; + + ValueTypeHashCodeStrategy ret = ValueTypeHashCodeStrategy::None; + *fieldOffset = 0; + *fieldSize = 0; + + BEGIN_QCALL; + + GCX_COOP(); + + ret = GetHashCodeStrategy(mt, objHandle.Get()->UnBox(), fieldOffset, fieldSize); + END_QCALL; return ret; diff --git a/src/coreclr/vm/comutilnative.h b/src/coreclr/vm/comutilnative.h index 0a2eaec901b93..a3c5ea65c3ca7 100644 --- a/src/coreclr/vm/comutilnative.h +++ b/src/coreclr/vm/comutilnative.h @@ -252,7 +252,7 @@ class MethodTableNative { extern "C" BOOL QCALLTYPE MethodTable_AreTypesEquivalent(MethodTable* mta, MethodTable* mtb); extern "C" BOOL QCALLTYPE MethodTable_CanCompareBitsOrUseFastGetHashCode(MethodTable* mt); -extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, void* objHandle, UINT32* fieldOffset, UINT32* fieldSize, MethodTable** fieldMethodTable); +extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, QCall::ObjectHandleOnStack objHandle, UINT32* fieldOffset, UINT32* fieldSize); class StreamNative { public: From 83e49b7f54d6965cc6628d798ee6932569ca5d46 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 28 Jan 2024 19:46:31 +0800 Subject: [PATCH 13/18] Remove Double and Single from CoreLibBinder --- src/coreclr/vm/corelib.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/coreclr/vm/corelib.h b/src/coreclr/vm/corelib.h index 9168ef4ed0884..f7966e9971cbd 100644 --- a/src/coreclr/vm/corelib.h +++ b/src/coreclr/vm/corelib.h @@ -569,12 +569,6 @@ DEFINE_METHOD(OBJECT, GET_TYPE, GetType, DEFINE_METHOD(OBJECT, GET_HASH_CODE, GetHashCode, IM_RetInt) DEFINE_METHOD(OBJECT, EQUALS, Equals, IM_Obj_RetBool) -// DEFINE_CLASS(DOUBLE, System, Double) -DEFINE_METHOD(DOUBLE, GET_HASH_CODE, GetHashCode, IM_RetInt) - -// DEFINE_CLASS(SINGLE, System, Single) -DEFINE_METHOD(SINGLE, GET_HASH_CODE, GetHashCode, IM_RetInt) - DEFINE_CLASS(__CANON, System, __Canon) BEGIN_ILLINK_FEATURE_SWITCH(System.Runtime.InteropServices.BuiltInComInterop.IsSupported, true, true) From 4303b3c713112a4628a744686085ad26b7e1d059 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Sun, 28 Jan 2024 20:26:47 +0800 Subject: [PATCH 14/18] Fix return --- src/coreclr/vm/comutilnative.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index f1b5c84656080..eb1c11025a209 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1790,6 +1790,8 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj } GCPROTECT_END(); + + return ret; } extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, QCall::ObjectHandleOnStack objHandle, UINT32* fieldOffset, UINT32* fieldSize) From 8dd42ff5fa31d3fbba7b93d14e20cfabaeb06b03 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 29 Jan 2024 18:35:41 +0800 Subject: [PATCH 15/18] Apply suggestions from code review Co-authored-by: Jan Kotas --- src/coreclr/vm/comutilnative.cpp | 22 ++++++---------------- 1 file changed, 6 insertions(+), 16 deletions(-) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index eb1c11025a209..349876803f9cc 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1718,16 +1718,7 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj ValueTypeHashCodeStrategy ret = ValueTypeHashCodeStrategy::None; - GCPROTECT_BEGININTERIOR(pObjRef); - - // it's looking ugly so we'll use the old behavior in managed code. Grab the first non-null - // field and return its hash code or 'it' as hash code - // Note that the old behavior has already been broken for value types - // that is qualified for CanUseFastGetHashCodeHelper. So maybe we should - // change the implementation here to use all fields instead of just the 1st one. - // - // - // check this approximation - we may be losing exact type information + // Grab the first non-null field and return its hash code or 'it' as hash code ApproxFieldDescIterator fdIterator(mt, ApproxFieldDescIterator::INSTANCE_FIELDS); FieldDesc *field; @@ -1736,8 +1727,9 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj _ASSERTE(!field->IsRVA()); if (field->IsObjRef()) { + GCX_COOP(); // if we get an object reference we get the hash code out of that - if (*(Object**)((BYTE *)pObjRef + field->GetOffsetUnsafe()) != NULL) + if (*(Object**)((BYTE *)objHandle->Unbox() + *fieldOffset + field->GetOffsetUnsafe()) != NULL) { ret = ValueTypeHashCodeStrategy::ReferenceField; } @@ -1780,9 +1772,8 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj } else { - UINT32 offset = 0; - ret = GetHashCodeStrategy(fieldMT, (BYTE *)pObjRef + field->GetOffsetUnsafe(), &offset, fieldSize); - *fieldOffset = field->GetOffsetUnsafe() + offset; + *fieldOffset += field->GetOffsetUnsafe(); + ret = GetHashCodeStrategy(fieldMT, objHandle, fieldOffset, fieldSize); } } } @@ -1804,9 +1795,8 @@ extern "C" INT32 QCALLTYPE ValueType_GetHashCodeStrategy(MethodTable* mt, QCall: BEGIN_QCALL; - GCX_COOP(); - ret = GetHashCodeStrategy(mt, objHandle.Get()->UnBox(), fieldOffset, fieldSize); + ret = GetHashCodeStrategy(mt, objHandle, fieldOffset, fieldSize); END_QCALL; From e519b2bbd1889167abbc30f24fea24ad951f0ee6 Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Mon, 29 Jan 2024 18:45:16 +0800 Subject: [PATCH 16/18] Complete --- src/coreclr/vm/comutilnative.cpp | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index 349876803f9cc..a1f4c46cfcc1f 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1705,12 +1705,13 @@ enum ValueTypeHashCodeStrategy FastGetHashCode, }; -static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObjRef, UINT32* fieldOffset, UINT32* fieldSize) +static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, QCall::ObjectHandleOnStack objHandle, UINT32* fieldOffset, UINT32* fieldSize) { - CONTRACTL{ + CONTRACTL + { THROWS; GC_TRIGGERS; - MODE_COOPERATIVE; + MODE_PREEMPTIVE; } CONTRACTL_END; // Should be handled by caller @@ -1729,7 +1730,7 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj { GCX_COOP(); // if we get an object reference we get the hash code out of that - if (*(Object**)((BYTE *)objHandle->Unbox() + *fieldOffset + field->GetOffsetUnsafe()) != NULL) + if (*(Object**)((BYTE *)objHandle.Get()->UnBox() + *fieldOffset + field->GetOffsetUnsafe()) != NULL) { ret = ValueTypeHashCodeStrategy::ReferenceField; } @@ -1780,8 +1781,6 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, void* pObj break; } - GCPROTECT_END(); - return ret; } From 9c80cd433bceb90b74b2a59fdafd3558cc0dd90f Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Wed, 31 Jan 2024 14:03:12 +0800 Subject: [PATCH 17/18] Handle offset of recursive case --- src/coreclr/vm/comutilnative.cpp | 9 ++++---- .../System/ValueTypeTests.cs | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/coreclr/vm/comutilnative.cpp b/src/coreclr/vm/comutilnative.cpp index a1f4c46cfcc1f..612cb9d72dc0d 100644 --- a/src/coreclr/vm/comutilnative.cpp +++ b/src/coreclr/vm/comutilnative.cpp @@ -1732,6 +1732,7 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, QCall::Obj // if we get an object reference we get the hash code out of that if (*(Object**)((BYTE *)objHandle.Get()->UnBox() + *fieldOffset + field->GetOffsetUnsafe()) != NULL) { + *fieldOffset += field->GetOffsetUnsafe(); ret = ValueTypeHashCodeStrategy::ReferenceField; } else @@ -1745,17 +1746,17 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, QCall::Obj CorElementType fieldType = field->GetFieldType(); if (fieldType == ELEMENT_TYPE_R8) { - *fieldOffset = field->GetOffsetUnsafe(); + *fieldOffset += field->GetOffsetUnsafe(); ret = ValueTypeHashCodeStrategy::DoubleField; } else if (fieldType == ELEMENT_TYPE_R4) { - *fieldOffset = field->GetOffsetUnsafe(); + *fieldOffset += field->GetOffsetUnsafe(); ret = ValueTypeHashCodeStrategy::SingleField; } else if (fieldType != ELEMENT_TYPE_VALUETYPE) { - *fieldOffset = field->GetOffsetUnsafe(); + *fieldOffset += field->GetOffsetUnsafe(); *fieldSize = field->LoadSize(); ret = ValueTypeHashCodeStrategy::FastGetHashCode; } @@ -1767,7 +1768,7 @@ static ValueTypeHashCodeStrategy GetHashCodeStrategy(MethodTable* mt, QCall::Obj MethodTable* fieldMT = fieldTH.GetMethodTable(); if (CanCompareBitsOrUseFastGetHashCode(fieldMT)) { - *fieldOffset = field->GetOffsetUnsafe(); + *fieldOffset += field->GetOffsetUnsafe(); *fieldSize = field->LoadSize(); ret = ValueTypeHashCodeStrategy::FastGetHashCode; } diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs index 422f71e11c04f..a8f7c163f7b18 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs @@ -299,6 +299,21 @@ public static void StructContainsPointerCompareTest() Assert.True(obj1.Equals(obj2)); Assert.Equal(obj1.GetHashCode(), obj2.GetHashCode()); } + + [Fact] + public static void StructContainsPointerNestedCompareTest() + { + StructContainsPointerNested obj1 = new StructContainsPointerNested(); + obj1.o = null; + obj1.value.value = 1; + + StructContainsPointerNested obj2 = new StructContainsPointerNested(); + obj1.o = null; + obj1.value.value = 2; + + Assert.False(obj1.Equals(obj2)); + Assert.NotEqual(obj1.GetHashCode(), obj2.GetHashCode()); + } public struct S { @@ -392,5 +407,11 @@ public struct StructContainsPointer public double value1; public double value2; } + + public struct StructContainsPointerNested + { + public object o; + public StructNonOverriddenEqualsOrGetHasCode value; + } } } From 946967ec3e61de672945754e1fb2df7c5365649d Mon Sep 17 00:00:00 2001 From: Huo Yaoyuan Date: Tue, 20 Feb 2024 20:55:28 +0800 Subject: [PATCH 18/18] Test equal case only --- .../tests/System.Runtime.Tests/System/ValueTypeTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs index a8f7c163f7b18..92a2c006ce204 100644 --- a/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs +++ b/src/libraries/System.Runtime/tests/System.Runtime.Tests/System/ValueTypeTests.cs @@ -308,11 +308,11 @@ public static void StructContainsPointerNestedCompareTest() obj1.value.value = 1; StructContainsPointerNested obj2 = new StructContainsPointerNested(); - obj1.o = null; - obj1.value.value = 2; + obj2.o = null; + obj2.value.value = 1; - Assert.False(obj1.Equals(obj2)); - Assert.NotEqual(obj1.GetHashCode(), obj2.GetHashCode()); + Assert.True(obj1.Equals(obj2)); + Assert.Equal(obj1.GetHashCode(), obj2.GetHashCode()); } public struct S