Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Type key hash tweaks. #61234

Merged
merged 7 commits into from
Nov 8, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/coreclr/vm/clsload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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))
{
Expand Down
12 changes: 4 additions & 8 deletions src/coreclr/vm/dacenumerablehash.inl
Original file line number Diff line number Diff line change
Expand Up @@ -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++;

Expand Down Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/gdbjit.h
Original file line number Diff line number Diff line change
Expand Up @@ -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()); }
Expand Down
7 changes: 3 additions & 4 deletions src/coreclr/vm/pendingload.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -130,7 +130,6 @@ BOOL PendingTypeLoadTable::InsertValue(PendingTypeLoadEntry *pData)
return TRUE;
}


BOOL PendingTypeLoadTable::DeleteValue(TypeKey *pKey)
{
CONTRACTL
Expand All @@ -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];
Expand Down Expand Up @@ -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;

Expand Down
2 changes: 1 addition & 1 deletion src/coreclr/vm/pendingload.h
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class PendingTypeLoadEntry
}
#endif //DACCESS_COMPILE

TypeKey GetTypeKey()
TypeKey& GetTypeKey()
Copy link
Member Author

@VSadov VSadov Nov 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GCC wanted & here, since we pass the result by reference. Not sure how it worked with MSVC.

{
LIMITED_METHOD_CONTRACT;
return m_typeKey;
Expand Down
60 changes: 26 additions & 34 deletions src/coreclr/vm/typehash.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand All @@ -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;
Expand All @@ -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
{
Expand All @@ -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
{
Expand All @@ -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());
}
}

Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/coreclr/vm/typehash.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 0 additions & 28 deletions src/coreclr/vm/typekey.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
}
};


Expand Down