Improve the performance of ConditionalWeakTable.TryGetValue

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

@@ -114,6 +114,17 @@ public static unsafe void PrepareMethod(RuntimeMethodHandle method, RuntimeTypeH
         [MethodImpl(MethodImplOptions.InternalCall)]
         public static extern int GetHashCode(object? o);
 
+        /// <summary>
+        /// If a hash code has been assigned to the object, it is returned. Otherwise zero is
+        /// returned.
+        /// </summary>
+        /// <remarks>
+        /// The advantage of this over <see cref="GetHashCode" /> is that it avoids assigning a hash
+        /// code to the object if it does not already have one.
+        /// </remarks>
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        internal static extern int TryGetHashCode(object o);
+
         [MethodImpl(MethodImplOptions.InternalCall)]
         public static extern new bool Equals(object? o1, object? o2);
 

src/coreclr/classlibnative/bcltype/objectnative.cpp

@@ -65,6 +65,34 @@ FCIMPL1(Object*, ObjectNative::GetObjectValue, Object* obj)
 }
 FCIMPLEND
 
+static INT32 TryGetHashCodeHelper(OBJECTREF objRef)
+{
+    DWORD bits = objRef->GetHeader()->GetBits();
+
+    if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX)
+    {
+        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;
+            }
+        }
+    }
+
+    // No hash code currently assigned.
+    return 0;
+}
 
 NOINLINE static INT32 GetHashCodeHelper(OBJECTREF objRef)
 {
@@ -99,32 +127,30 @@ FCIMPL1(INT32, ObjectNative::GetHashCode, Object* obj) {
 
     OBJECTREF objRef(obj);
 
-    {
-        DWORD bits = objRef->GetHeader()->GetBits();
+    INT32 ret = TryGetHashCodeHelper(objRef);
+    if (ret != 0)
+        return ret;
 
-        if (bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX)
-        {
-            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;
-                }
-            }
-        }
+    FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef));
+}
+FCIMPLEND
+
+FCIMPL1(INT32, ObjectNative::TryGetHashCode, Object* obj) {
+
+    CONTRACTL
+    {
+        FCALL_CHECK;
     }
+    CONTRACTL_END;
 
-    FC_INNER_RETURN(INT32, GetHashCodeHelper(objRef));
+    VALIDATEOBJECT(obj);
+
+    if (obj == 0)
+        return 0;
+
+    OBJECTREF objRef(obj);
+
+    return TryGetHashCodeHelper(objRef);
 }
 FCIMPLEND
 

src/coreclr/classlibnative/bcltype/objectnative.h

@@ -30,6 +30,7 @@ class ObjectNative
     //  If the Class object doesn't exist then you must call the GetClass() method.
     static FCDECL1(Object*, GetObjectValue, Object* vThisRef);
     static FCDECL1(INT32, GetHashCode, Object* vThisRef);
+    static FCDECL1(INT32, TryGetHashCode, Object* vThisRef);
     static FCDECL2(FC_BOOL_RET, Equals, Object *pThisRef, Object *pCompareRef);
     static FCDECL1(Object*, AllocateUninitializedClone, Object* pObjUNSAFE);
     static FCDECL1(Object*, GetClass, Object* pThis);

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

@@ -104,6 +104,19 @@ public static unsafe int GetHashCode(object o)
             return ObjectHeader.GetHashCode(o);
         }
 
+        /// <summary>
+        /// If a hash code has been assigned to the object, it is returned. Otherwise zero is
+        /// returned.
+        /// </summary>
+        /// <remarks>
+        /// The advantage of this over <see cref="GetHashCode" /> is that it avoids assigning a hash
+        /// code to the object if it does not already have one.
+        /// </remarks>
+        internal static int TryGetHashCode(object o)
+        {
+            return ObjectHeader.TryGetHashCode(o);
+        }
+
         [Obsolete("OffsetToStringData has been deprecated. Use string.GetPinnableReference() instead.")]
         public static int OffsetToStringData
         {

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

@@ -49,6 +49,28 @@ internal static class ObjectHeader
             return (int*)ppMethodTable - 1;
         }
 
+        // returns zero if no hash code is currently assigned
+        private static unsafe int TryGetHashCode(int* pHeader)
+        {
+            int bits = *pHeader;
+            int hashOrIndex = bits & MASK_HASHCODE_INDEX;
+            if ((bits & BIT_SBLK_IS_HASHCODE) != 0)
+            {
+                // Found the hash code in the header
+                Debug.Assert(hashOrIndex != 0);
+                return hashOrIndex;
+            }
+
+            if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0)
+            {
+                // Look up the hash code in the SyncTable
+                return SyncTable.GetHashCode(hashOrIndex);
+            }
+
+            // The hash code has not yet been set.
+            return 0;
+        }
+
         /// <summary>
         /// Returns the hash code assigned to the object.  If no hash code has yet been assigned,
         /// it assigns one in a thread-safe way.
@@ -61,30 +83,37 @@ public static unsafe int GetHashCode(object o)
             fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef())
             {
                 int* pHeader = GetHeaderPtr(ppMethodTable);
-                int bits = *pHeader;
-                int hashOrIndex = bits & MASK_HASHCODE_INDEX;
-                if ((bits & BIT_SBLK_IS_HASHCODE) != 0)
-                {
-                    // Found the hash code in the header
-                    Debug.Assert(hashOrIndex != 0);
-                    return hashOrIndex;
-                }
-
-                if ((bits & BIT_SBLK_IS_HASH_OR_SYNCBLKINDEX) != 0)
+                int hashCode = TryGetHashCode(pHeader);
+                if (hashCode != 0)
                 {
-                    // Look up the hash code in the SyncTable
-                    int hashCode = SyncTable.GetHashCode(hashOrIndex);
-                    if (hashCode != 0)
-                    {
-                        return hashCode;
-                    }
+                    return hashCode;
                 }
 
                 // The hash code has not yet been set.  Assign some value.
                 return AssignHashCode(o, pHeader);
             }
         }
 
+        /// <summary>
+        /// If a hash code has been assigned to the object, it is returned. Otherwise zero is
+        /// returned.
+        /// </summary>
+        /// <remarks>
+        /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called
+        /// while a garbage collection in in progress.
+        /// </remarks>
+        public static unsafe int TryGetHashCode(object o)
+        {
+            if (o == null)
+                return 0;
+
+            fixed (MethodTable** ppMethodTable = &o.GetMethodTableRef())
+            {
+                int* pHeader = GetHeaderPtr(ppMethodTable);
+                return TryGetHashCode(pHeader);
+            }
+        }
+
         /// <summary>
         /// Assigns a hash code to the object in a thread-safe way.
         /// </summary>

src/coreclr/vm/ecalllist.h

@@ -552,6 +552,7 @@ FCFuncStart(gRuntimeHelpers)
     FCFuncElement("GetSpanDataFrom", ArrayNative::GetSpanDataFrom)
     FCFuncElement("PrepareDelegate", ReflectionInvocation::PrepareDelegate)
     FCFuncElement("GetHashCode", ObjectNative::GetHashCode)
+    FCFuncElement("TryGetHashCode", ObjectNative::TryGetHashCode)
     FCFuncElement("Equals", ObjectNative::Equals)
     FCFuncElement("AllocateUninitializedClone", ObjectNative::AllocateUninitializedClone)
     FCFuncElement("EnsureSufficientExecutionStack", ReflectionInvocation::EnsureSufficientExecutionStack)

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

@@ -48,8 +48,10 @@ public ConditionalWeakTable()
         /// </param>
         /// <returns>Returns "true" if key was found, "false" otherwise.</returns>
         /// <remarks>
-        /// The key may get garbaged collected during the TryGetValue operation. If so, TryGetValue
+        /// The key may get garbage collected during the TryGetValue operation. If so, TryGetValue
         /// may at its discretion, return "false" and set "value" to the default (as if the key was not present.)
+        /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called
+        /// while a garbage collection in in progress.
         /// </remarks>
         public bool TryGetValue(TKey key, [MaybeNullWhen(false)] out TValue value)
         {
@@ -520,6 +522,10 @@ internal void CreateEntryNoResize(TKey key, TValue value)
             }
 
             /// <summary>Worker for finding a key/value pair. Must hold _lock.</summary>
+            /// <remarks>
+            /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called
+            /// while a garbage collection in in progress.
+            /// </remarks>
             internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue value)
             {
                 Debug.Assert(key != null); // Key already validated as non-null
@@ -533,12 +539,26 @@ internal bool TryGetValueWorker(TKey key, [MaybeNullWhen(false)] out TValue valu
             /// Returns -1 if not found (if key expires during FindEntry, this can be treated as "not found.").
             /// Must hold _lock, or be prepared to retry the search while holding _lock.
             /// </summary>
-            /// <remarks>This method requires <paramref name="value"/> to be on the stack to be properly tracked.</remarks>
+            /// <remarks>
+            /// This method requires <paramref name="value"/> to be on the stack to be properly tracked.
+            /// This method needs to follow the rules in RestrictedCallouts.h, as it may be called
+            /// while a garbage collection in in progress.
+            /// </remarks>
             internal int FindEntry(TKey key, out object? value)
             {
                 Debug.Assert(key != null); // Key already validated as non-null.
 
-                int hashCode = RuntimeHelpers.GetHashCode(key) & int.MaxValue;
+                int hashCode = RuntimeHelpers.TryGetHashCode(key);
+
+                if (hashCode == 0)
+                {
+                    // No hash code has been assigned to the key, so therefore it has not been added
+                    // to any ConditionalWeakTable.
+                    value = null;
+                    return -1;
+                }
+
+                hashCode &= int.MaxValue;
                 int bucket = hashCode & (_buckets.Length - 1);
                 for (int entriesIndex = Volatile.Read(ref _buckets[bucket]); entriesIndex != -1; entriesIndex = _entries[entriesIndex].Next)
                 {

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

@@ -44,6 +44,12 @@ public static int GetHashCode(object? o)
             return InternalGetHashCode(o);
         }
 
+        internal static int TryGetHashCode(object o)
+        {
+            // On Mono the fast path is not yet implemented, so we just defer to GetHashCode.
+            return InternalGetHashCode(o);
+        }
+
         public static new bool Equals(object? o1, object? o2)
         {
             if (o1 == o2)