Skip to content

Commit 5a10aa6

Browse files
AustinWisejkotas
andauthored
Improve the performance of ConditionalWeakTable.TryGetValue (#80059)
* [NativeAOT] Fix Objective-C reference tracking Fixes #80032 * Move implementation-specific comment out of public doc comment * Duplicate code for TryGetHashCode implementations. * Replace comments with a passing test. * Add moke test for restricted callouts. * Remove TryGetHashCode from Mono It does not guarantee that hash codes are non-zero. * Add test coverage for untracked objective objects. * Implement RuntimeHelpers.TryGetHashCode for Mono * Remove unneeded MONO_COMPONENT_API * Remove Mono intrinsic for InternalGetHashCode This is dead code because this method no longer lives on System.Object. * Move interpreter transforms to correct class. * Rename and move icall to match convention. Co-authored-by: Jan Kotas <jkotas@microsoft.com>
1 parent b592364 commit 5a10aa6

File tree

17 files changed

+257
-23
lines changed

17 files changed

+257
-23
lines changed

src/coreclr/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.CoreCLR.cs

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,17 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH
114114
[MethodImpl(MethodImplOptions.InternalCall)]
115115
public static extern int GetHashCode(object? o);
116116

117+
/// <summary>
118+
/// If a hash code has been assigned to the object, it is returned. Otherwise zero is
119+
/// returned.
120+
/// </summary>
121+
/// <remarks>
122+
/// The advantage of this over <see cref="GetHashCode" /> is that it avoids assigning a hash
123+
/// code to the object if it does not already have one.
124+
/// </remarks>
125+
[MethodImpl(MethodImplOptions.InternalCall)]
126+
internal static extern int TryGetHashCode(object o);
127+
117128
[MethodImpl(MethodImplOptions.InternalCall)]
118129
public static extern new bool Equals(object? o1, object? o2);
119130

src/coreclr/classlibnative/bcltype/objectnative.cpp

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -128,6 +128,47 @@ FCIMPL1(INT32, ObjectNative::GetHashCode, Object* obj) {
128128
}
129129
FCIMPLEND
130130

131+
FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) {
132+
133+
CONTRACTL
134+
{
135+
FCALL_CHECK;
136+
}
137+
CONTRACTL_END;
138+
139+
VALIDATEOBJECT(obj);
140+
141+
if (obj == 0)
142+
return 0;
143+
144+
OBJECTREF objRef(obj);
145+
146+
{
147+
DWORD bits = objRef->GetHeader()->GetBits();
148+
149+
if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX)
150+
{
151+
if (bits & BIT_SBLK_IS_HASHCODE)
152+
{
153+
// Common case: the object already has a hash code
154+
return bits & MASK_HASHCODE;
155+
}
156+
else
157+
{
158+
// We have a sync block index. There may be a hash code stored within the sync block.
159+
SyncBlock *psb = objRef->PassiveGetSyncBlock();
160+
if (psb != NULL)
161+
{
162+
return psb->GetHashCode();
163+
}
164+
}
165+
}
166+
}
167+
168+
return 0;
169+
}
170+
FCIMPLEND
171+
131172
//
132173
// Compare by ref for normal classes, by value for value types.
133174
//

src/coreclr/classlibnative/bcltype/objectnative.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ class ObjectNative
3030
// If the Class object doesn't exist then you must call the GetClass() method.
3131
static FCDECL1(Object*, GetObjectValue, Object* vThisRef);
3232
static FCDECL1(INT32, GetHashCode, Object* vThisRef);
33+
static FCDECL1(INT32, TryGetHashCode, Object* vThisRef);
3334
static FCDECL2(FC_BOOL_RET, Equals, Object *pThisRef, Object *pCompareRef);
3435
static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
3536
static FCDECL1(Object*, GetClass, Object* pThis);

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.NativeAot.cs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,19 @@ public static unsafe int GetHashCode(object o)
104104
return ObjectHeader.GetHashCode(o);
105105
}
106106

107+
/// <summary>
108+
/// If a hash code has been assigned to the object, it is returned. Otherwise zero is
109+
/// returned.
110+
/// </summary>
111+
/// <remarks>
112+
/// The advantage of this over <see cref="GetHashCode" /> is that it avoids assigning a hash
113+
/// code to the object if it does not already have one.
114+
/// </remarks>
115+
internal static int TryGetHashCode(object o)
116+
{
117+
return ObjectHeader.TryGetHashCode(o);
118+
}
119+
107120
[Obsolete("OffsetToStringData has been deprecated. Use string.GetPinnableReference() instead.")]
108121
public static int OffsetToStringData
109122
{

src/coreclr/nativeaot/System.Private.CoreLib/src/System/Threading/ObjectHeader.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,38 @@ public static unsafe int GetHashCode(object o)
8585
}
8686
}
8787

88+
/// <summary>
89+
/// If a hash code has been assigned to the object, it is returned. Otherwise zero is
90+
/// returned.
91+
/// </summary>
92+
public static unsafe int TryGetHashCode(object o)
93+
{
94+
if (o == null)
95+
return 0;
96+
97+
fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef())
98+
{
99+
int* pHeader = GetHeaderPtr(ppMethodTable);
100+
int bits = *pHeader;
101+
int hashOrIndex = bits & MASK_HASHCODE_INDEX;
102+
if ((bits & BIT_SBLK_IS_HASHCODE) != 0)
103+
{
104+
// Found the hash code in the header
105+
Debug.Assert(hashOrIndex != 0);
106+
return hashOrIndex;
107+
}
108+
109+
if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0)
110+
{
111+
// Look up the hash code in the SyncTable
112+
return SyncTable.GetHashCode(hashOrIndex);
113+
}
114+
115+
// The hash code has not yet been set.
116+
return 0;
117+
}
118+
}
119+
88120
/// <summary>
89121
/// Assigns a hash code to the object in a thread-safe way.
90122
/// </summary>

src/coreclr/vm/ecalllist.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,7 @@ FCFuncStart(gRuntimeHelpers)
552552
FCFuncElement("GetSpanDataFrom", ArrayNative::GetSpanDataFrom)
553553
FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate)
554554
FCFuncElement("GetHashCode", ObjectNative::GetHashCode)
555+
FCFuncElement("TryGetHashCode", ObjectNative::TryGetHashCode)
555556
FCFuncElement("Equals", ObjectNative::Equals)
556557
FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone)
557558
FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack)

src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConditionalWeakTable.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public ConditionalWeakTable()
4848
/// </param>
4949
/// <returns>Returns "true" if key was found, "false" otherwise.</returns>
5050
/// <remarks>
51-
/// The key may get garbaged collected during the TryGetValue operation. If so, TryGetValue
51+
/// The key may get garbage collected during the TryGetValue operation. If so, TryGetValue
5252
/// may at its discretion, return "false" and set "value" to the default (as if the key was not present.)
5353
/// </remarks>
5454
public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value)
@@ -538,7 +538,17 @@ internal int FindEntry(TKey key, out object? value)
538538
{
539539
Debug.Assert(key != null); // Key already validated as non-null.
540540

541-
int hashCode = RuntimeHelpers.GetHashCode(key) & int.MaxValue;
541+
int hashCode = RuntimeHelpers.TryGetHashCode(key);
542+
543+
if (hashCode == 0)
544+
{
545+
// No hash code has been assigned to the key, so therefore it has not been added
546+
// to any ConditionalWeakTable.
547+
value = null;
548+
return -1;
549+
}
550+
551+
hashCode &= int.MaxValue;
542552
int bucket = hashCode & (_buckets.Length - 1);
543553
for (int entriesIndex = Volatile.Read(ref _buckets[bucket]); entriesIndex != -1; entriesIndex = _entries[entriesIndex].Next)
544554
{

src/mono/System.Private.CoreLib/src/System/Runtime/CompilerServices/RuntimeHelpers.Mono.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,22 @@ public static int GetHashCode(object? o)
4444
return InternalGetHashCode(o);
4545
}
4646

47+
[MethodImplAttribute(MethodImplOptions.InternalCall)]
48+
private static extern int InternalTryGetHashCode(object? o);
49+
50+
/// <summary>
51+
/// If a hash code has been assigned to the object, it is returned. Otherwise zero is
52+
/// returned.
53+
/// </summary>
54+
/// <remarks>
55+
/// The advantage of this over <see cref="GetHashCode" /> is that it avoids assigning a hash
56+
/// code to the object if it does not already have one.
57+
/// </remarks>
58+
public static int TryGetHashCode(object? o)
59+
{
60+
return InternalTryGetHashCode(o);
61+
}
62+
4763
public static new bool Equals(object? o1, object? o2)
4864
{
4965
if (o1 == o2)

src/mono/mono/metadata/icall-def.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -427,7 +427,8 @@ HANDLES(RUNH_1, "GetObjectValue", ves_icall_System_Runtime_CompilerServices_Runt
427427
HANDLES(RUNH_6, "GetSpanDataFrom", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetSpanDataFrom, gpointer, 3, (MonoClassField_ptr, MonoType_ptr, gpointer))
428428
HANDLES(RUNH_2, "GetUninitializedObjectInternal", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetUninitializedObjectInternal, MonoObject, 1, (MonoType_ptr))
429429
HANDLES(RUNH_3, "InitializeArray", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray, void, 2, (MonoArray, MonoClassField_ptr))
430-
HANDLES(RUNH_7, "InternalGetHashCode", mono_object_hash_icall, int, 1, (MonoObject))
430+
HANDLES(RUNH_7, "InternalGetHashCode", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalGetHashCode, int, 1, (MonoObject))
431+
HANDLES(RUNH_8, "InternalTryGetHashCode", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalTryGetHashCode, int, 1, (MonoObject))
431432
HANDLES(RUNH_3a, "PrepareMethod", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_PrepareMethod, void, 3, (MonoMethod_ptr, gpointer, int))
432433
HANDLES(RUNH_4, "RunClassConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunClassConstructor, void, 1, (MonoType_ptr))
433434
HANDLES(RUNH_5, "RunModuleConstructor", ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_RunModuleConstructor, void, 1, (MonoImage_ptr))

src/mono/mono/metadata/icall.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,18 @@ ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InitializeArray (MonoAr
10721072
#endif
10731073
}
10741074

1075+
int
1076+
ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalGetHashCode (MonoObjectHandle obj, MonoError* error)
1077+
{
1078+
return mono_object_hash_internal (MONO_HANDLE_RAW (obj));
1079+
}
1080+
1081+
int
1082+
ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_InternalTryGetHashCode (MonoObjectHandle obj, MonoError* error)
1083+
{
1084+
return mono_object_try_get_hash_internal (MONO_HANDLE_RAW (obj));
1085+
}
1086+
10751087
MonoObjectHandle
10761088
ves_icall_System_Runtime_CompilerServices_RuntimeHelpers_GetObjectValue (MonoObjectHandle obj, MonoError *error)
10771089
{

0 commit comments

Comments
 (0)