Type key hash tweaks. (#61234)

* use HashTypeKey(), remove ComputeHash()

* nonrecursive HashTypeHandle

* remove instrumentation from pendingload.cpp

* two more uses of `ComputeHash()`

* Make GCC happy

* couple tweaks

* unnecessary full fences.

Author: VSadov

src/coreclr/vm/clsload.cpp

@@ -3634,7 +3634,7 @@ ClassLoader::LoadTypeHandleForTypeKey_Body(
     }
     EX_HOOK
     {
-        LOG((LF_CLASSLOADER, LL_INFO10, "Caught an exception loading: %x, %0x (Module)\n", pTypeKey->IsConstructed() ? pTypeKey->ComputeHash() : pTypeKey->GetTypeToken(), pTypeKey->GetModule()));
+        LOG((LF_CLASSLOADER, LL_INFO10, "Caught an exception loading: %x, %0x (Module)\n", pTypeKey->IsConstructed() ? HashTypeKey(pTypeKey) : pTypeKey->GetTypeToken(), pTypeKey->GetModule()));
 
         if (!GetThread()->HasThreadStateNC(Thread::TSNC_LoadsTypeViolation))
         {

src/coreclr/vm/dacenumerablehash.inl

@@ -135,11 +135,9 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::BaseInsertEntry(DacEnumerableHa
     // Prepare to link the new entry at the head of the bucket chain.
     pVolatileEntry->m_pNextEntry = (GetBuckets())[dwBucket];
 
-    // Make sure that all writes to the entry are visible before publishing the entry.
-    MemoryBarrier();
-
     // Publish the entry by pointing the bucket at it.
-    (GetBuckets())[dwBucket] = pVolatileEntry;
+    // Make sure that all writes to the entry are visible before publishing the entry.
+    VolatileStore(&(GetBuckets())[dwBucket], pVolatileEntry);
 
     m_cEntries++;
 
@@ -207,15 +205,13 @@ void DacEnumerableHashTable<DAC_ENUM_HASH_ARGS>::GrowTable()
     }
 
     // Make sure that all writes are visible before publishing the new array.
-    MemoryBarrier();
-    m_pBuckets = pNewBuckets;
+    VolatileStore(&m_pBuckets, pNewBuckets);
 
     // The new number of buckets has to be published last (prior to this readers may miscalculate a bucket
     // index, but the result will always be in range and they'll simply walk the wrong chain and get a miss,
     // prompting a retry under the lock). If we let the count become visible unordered wrt to the bucket array
     // itself a reader could potentially read buckets from beyond the end of the old bucket list).
-    MemoryBarrier();
-    m_cBuckets = cNewBuckets;
+    VolatileStore(&m_cBuckets, cNewBuckets);
 }
 
 // Returns the next prime larger (or equal to) than the number given.

src/coreclr/vm/gdbjit.h

@@ -387,7 +387,7 @@ class NotifyGdb
         static count_t Hash(key_t k)
         {
             LIMITED_METHOD_CONTRACT;
-            return k->ComputeHash();
+            return HashTypeKey(k);
         }
 
         static const element_t Null() { LIMITED_METHOD_CONTRACT; return element_t(key_t(),VALUE()); }

src/coreclr/vm/pendingload.cpp

@@ -114,7 +114,7 @@ BOOL PendingTypeLoadTable::InsertValue(PendingTypeLoadEntry *pData)
 
     _ASSERTE(m_dwNumBuckets != 0);
 
-    DWORD           dwHash = pData->GetTypeKey().ComputeHash();
+    DWORD           dwHash = HashTypeKey(&pData->GetTypeKey());
     DWORD           dwBucket = dwHash % m_dwNumBuckets;
     PendingTypeLoadTable::TableEntry * pNewEntry = AllocNewEntry();
     if (pNewEntry == NULL)
@@ -130,7 +130,6 @@ BOOL PendingTypeLoadTable::InsertValue(PendingTypeLoadEntry *pData)
     return TRUE;
 }
 
-
 BOOL PendingTypeLoadTable::DeleteValue(TypeKey *pKey)
 {
     CONTRACTL
@@ -145,7 +144,7 @@ BOOL PendingTypeLoadTable::DeleteValue(TypeKey *pKey)
 
     _ASSERTE(m_dwNumBuckets != 0);
 
-    DWORD           dwHash = pKey->ComputeHash();
+    DWORD           dwHash = HashTypeKey(pKey);
     DWORD           dwBucket = dwHash % m_dwNumBuckets;
     PendingTypeLoadTable::TableEntry * pSearch;
     PendingTypeLoadTable::TableEntry **ppPrev = &m_pBuckets[dwBucket];
@@ -182,7 +181,7 @@ PendingTypeLoadTable::TableEntry *PendingTypeLoadTable::FindItem(TypeKey *pKey)
     _ASSERTE(m_dwNumBuckets != 0);
 
 
-    DWORD           dwHash = pKey->ComputeHash();
+    DWORD           dwHash = HashTypeKey(pKey);
     DWORD           dwBucket = dwHash % m_dwNumBuckets;
     PendingTypeLoadTable::TableEntry * pSearch;
 

src/coreclr/vm/pendingload.h

@@ -166,7 +166,7 @@ class PendingTypeLoadEntry
     }
 #endif //DACCESS_COMPILE
 
-    TypeKey GetTypeKey()
+    TypeKey& GetTypeKey()
     {
         LIMITED_METHOD_CONTRACT;
         return m_typeKey;

src/coreclr/vm/typehash.cpp

@@ -141,10 +141,10 @@ DWORD EETypeHashTable::GetCount()
     return BaseGetElementCount();
 }
 
-static DWORD HashTypeHandle(DWORD level, TypeHandle t);
+static DWORD HashTypeHandle(TypeHandle t);
 
 // Calculate hash value for a type def or instantiated type def
-static DWORD HashPossiblyInstantiatedType(DWORD level, mdTypeDef token, Instantiation inst)
+static DWORD HashPossiblyInstantiatedType(mdTypeDef token, Instantiation inst)
 {
     CONTRACTL
     {
@@ -161,25 +161,18 @@ static DWORD HashPossiblyInstantiatedType(DWORD level, mdTypeDef token, Instanti
     dwHash = ((dwHash << 5) + dwHash) ^ token;
     if (!inst.IsEmpty())
     {
-        dwHash = ((dwHash << 5) + dwHash) ^ inst.GetNumArgs();
-
-        // Hash two levels of the hiearchy. A simple nesting of generics instantiations is
-        // pretty common in generic collections, e.g.: ICollection<KeyValuePair<TKey, TValue>>
-        if (level < 2)
+        // Hash n type parameters
+        for (DWORD i = 0; i < inst.GetNumArgs(); i++)
         {
-            // Hash n type parameters
-            for (DWORD i = 0; i < inst.GetNumArgs(); i++)
-            {
-                dwHash = ((dwHash << 5) + dwHash) ^ HashTypeHandle(level+1, inst[i]);
-            }
+            dwHash = ((dwHash << 5) + dwHash) ^ inst[i].AsTAddr();
         }
     }
 
     return dwHash;
 }
 
 // Calculate hash value for a function pointer type
-static DWORD HashFnPtrType(DWORD level, BYTE callConv, DWORD numArgs, TypeHandle *retAndArgTypes)
+static DWORD HashFnPtrType(BYTE callConv, DWORD numArgs, TypeHandle *retAndArgTypes)
 {
     WRAPPER_NO_CONTRACT;
     SUPPORTS_DAC;
@@ -188,31 +181,29 @@ static DWORD HashFnPtrType(DWORD level, BYTE callConv, DWORD numArgs, TypeHandle
     dwHash = ((dwHash << 5) + dwHash) ^ ELEMENT_TYPE_FNPTR;
     dwHash = ((dwHash << 5) + dwHash) ^ callConv;
     dwHash = ((dwHash << 5) + dwHash) ^ numArgs;
-    if (level < 1)
+
+    for (DWORD i = 0; i <= numArgs; i++)
     {
-        for (DWORD i = 0; i <= numArgs; i++)
-        {
-            dwHash = ((dwHash << 5) + dwHash) ^ HashTypeHandle(level+1, retAndArgTypes[i]);
-        }
+        dwHash = ((dwHash << 5) + dwHash) ^ retAndArgTypes[i].AsTAddr();
     }
 
     return dwHash;
 }
 
 // Calculate hash value for an array/pointer/byref type
-static DWORD HashParamType(DWORD level, CorElementType kind, TypeHandle typeParam)
+static DWORD HashParamType(CorElementType kind, TypeHandle typeParam)
 {
     WRAPPER_NO_CONTRACT;
     INT_PTR dwHash = 5381;
 
     dwHash = ((dwHash << 5) + dwHash) ^ kind;
-    dwHash = ((dwHash << 5) + dwHash) ^ HashTypeHandle(level, typeParam);
+    dwHash = ((dwHash << 5) + dwHash) ^ typeParam.AsTAddr();
 
     return dwHash;
 }
 
 // Calculate hash value from type handle
-static DWORD HashTypeHandle(DWORD level, TypeHandle t)
+static DWORD HashTypeHandle(TypeHandle t)
 {
     CONTRACTL
     {
@@ -229,29 +220,30 @@ static DWORD HashTypeHandle(DWORD level, TypeHandle t)
 
     if (t.HasTypeParam())
     {
-        retVal = HashParamType(level, t.GetInternalCorElementType(), t.GetTypeParam());
-    }
-    else if (t.IsGenericVariable())
-    {
-        retVal = (dac_cast<PTR_TypeVarTypeDesc>(t.AsTypeDesc())->GetToken());
+        retVal = HashParamType(t.GetInternalCorElementType(), t.GetTypeParam());
     }
     else if (t.HasInstantiation())
     {
-        retVal = HashPossiblyInstantiatedType(level, t.GetCl(), t.GetInstantiation());
+        retVal = HashPossiblyInstantiatedType(t.GetCl(), t.GetInstantiation());
     }
     else if (t.IsFnPtrType())
     {
         FnPtrTypeDesc* pTD = t.AsFnPtrType();
-        retVal = HashFnPtrType(level, pTD->GetCallConv(), pTD->GetNumArgs(), pTD->GetRetAndArgTypesPointer());
+        retVal = HashFnPtrType(pTD->GetCallConv(), pTD->GetNumArgs(), pTD->GetRetAndArgTypesPointer());
+    }
+    else if (t.IsGenericVariable())
+    {
+        _ASSERTE(!"Generic variables are unexpected here.");
+        retVal = t.AsTAddr();
     }
     else
-        retVal = HashPossiblyInstantiatedType(level, t.GetCl(), Instantiation());
+        retVal = HashPossiblyInstantiatedType(t.GetCl(), Instantiation());
 
     return retVal;
 }
 
 // Calculate hash value from key
-static DWORD HashTypeKey(TypeKey* pKey)
+DWORD HashTypeKey(TypeKey* pKey)
 {
     CONTRACTL
     {
@@ -265,15 +257,15 @@ static DWORD HashTypeKey(TypeKey* pKey)
 
     if (pKey->GetKind() == ELEMENT_TYPE_CLASS)
     {
-        return HashPossiblyInstantiatedType(0, pKey->GetTypeToken(), pKey->GetInstantiation());
+        return HashPossiblyInstantiatedType(pKey->GetTypeToken(), pKey->GetInstantiation());
     }
     else if (pKey->GetKind() == ELEMENT_TYPE_FNPTR)
     {
-        return HashFnPtrType(0, pKey->GetCallConv(), pKey->GetNumArgs(), pKey->GetRetAndArgTypes());
+        return HashFnPtrType(pKey->GetCallConv(), pKey->GetNumArgs(), pKey->GetRetAndArgTypes());
     }
     else
     {
-        return HashParamType(0, pKey->GetKind(), pKey->GetElementType());
+        return HashParamType(pKey->GetKind(), pKey->GetElementType());
     }
 }
 
@@ -552,7 +544,7 @@ VOID EETypeHashTable::InsertValue(TypeHandle data)
 
     pNewEntry->SetTypeHandle(data);
 
-    BaseInsertEntry(HashTypeHandle(0, data), pNewEntry);
+    BaseInsertEntry(HashTypeHandle(data), pNewEntry);
 }
 
 #endif // #ifndef DACCESS_COMPILE

src/coreclr/vm/typehash.h

@@ -26,6 +26,8 @@
 //
 //========================================================================================
 
+DWORD HashTypeKey(TypeKey* pKey);
+
 // One of these is present for each element in the table
 // It simply chains together (hash,data) pairs
 typedef DPTR(struct EETypeHashEntry) PTR_EETypeHashEntry;

src/coreclr/vm/typekey.h

@@ -213,7 +213,6 @@ class TypeKey
         return TypeKey::Equals(this, pKey);
     }
 
-    // Comparison and hashing
     static BOOL Equals(const TypeKey *pKey1, const TypeKey *pKey2)
     {
         WRAPPER_NO_CONTRACT;
@@ -257,33 +256,6 @@ class TypeKey
             return TRUE;
         }
     }
-
-    DWORD ComputeHash() const
-    {
-        LIMITED_METHOD_CONTRACT;
-        DWORD_PTR hashLarge;
-
-        if (m_kind == ELEMENT_TYPE_CLASS)
-        {
-            hashLarge = ((DWORD_PTR)u.asClass.m_pModule ^ (DWORD_PTR)u.asClass.m_numGenericArgs ^ (DWORD_PTR)u.asClass.m_typeDef);
-        }
-        else if (CorTypeInfo::IsModifier_NoThrow(m_kind) || m_kind == ELEMENT_TYPE_VALUETYPE)
-        {
-            hashLarge = (u.asParamType.m_paramType ^ (DWORD_PTR) u.asParamType.m_rank);
-        }
-        else hashLarge = 0;
-
-#if POINTER_BITS == 32
-        return hashLarge;
-#else
-        DWORD hash = *(DWORD *)&hashLarge;
-        for (unsigned i = 1; i < POINTER_BITS / 32; i++)
-        {
-            hash ^= ((DWORD *)&hashLarge)[i];
-        }
-        return hash;
-#endif
-    }
 };