Switch custom attribute hash from using CClosedHashEx to SHash (#77933)

* Switch custom attribute hash from CClosedHashEx to SHash

* Remove CClosedHashEx
Co-authored-by: NJan Kotas <jkotas@microsoft.com>

src/coreclr/inc/utilcode.h

@@ -2330,32 +2330,6 @@ inline COUNT_T HashPtr(COUNT_T currentHash, PTR_VOID ptr)
     return HashCOUNT_T(currentHash, COUNT_T(SIZE_T(dac_cast<TADDR>(ptr))));
 }
 
-inline DWORD HashThreeToOne(DWORD a, DWORD b, DWORD c)
-{
-    LIMITED_METHOD_DAC_CONTRACT;
-
-    /*
-    lookup3.c, by Bob Jenkins, May 2006, Public Domain.
-
-    These are functions for producing 32-bit hashes for hash table lookup.
-    hashword(), hashlittle(), hashlittle2(), hashbig(), mix(), and final()
-    are externally useful functions.  Routines to test the hash are included
-    if SELF_TEST is defined.  You can use this free for any purpose.  It's in
-    the public domain.  It has no warranty.
-    */
-
-    #define rot32(x,k) (((x)<<(k)) | ((x)>>(32-(k))))
-    c ^= b; c -= rot32(b,14);
-    a ^= c; a -= rot32(c,11);
-    b ^= a; b -= rot32(a,25);
-    c ^= b; c -= rot32(b,16);
-    a ^= c; a -= rot32(c,4);
-    b ^= a; b -= rot32(a,14);
-    c ^= b; c -= rot32(b,24);
-
-    return c;
-}
-
 inline ULONG HashBytes(BYTE const *pbData, size_t iSize)
 {
     LIMITED_METHOD_CONTRACT;
@@ -2771,139 +2745,6 @@ private:
     BYTE        *m_rgData;                  // Data element list.
 };
 
-//*****************************************************************************
-// IMPORTANT: This data structure is deprecated, please do not add any new uses.
-// The hashtable implementation that should be used instead is code:SHash.
-// If code:SHash does not work for you, talk to mailto:clrdeag.
-//*****************************************************************************
-template <class T> class CClosedHash : public CClosedHashBase
-{
-public:
-    CClosedHash(
-        int         iBuckets,               // How many buckets should we start with.
-        bool        bPerfect=false) :       // true if bucket size will hash with no collisions.
-        CClosedHashBase(iBuckets, sizeof(T), bPerfect)
-    {
-        WRAPPER_NO_CONTRACT;
-    }
-
-    T &operator[](int iIndex)
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T &) *(Data() + (iIndex * sizeof(T))));
-    }
-
-
-//*****************************************************************************
-// Add a new item to hash table given the key value.  If this new entry
-// exceeds maximum size, then the table will grow and be re-hashed, which
-// may cause a memory error.
-//*****************************************************************************
-    T *Add(                                 // New item to fill out on success.
-        void        *pData)                 // The value to hash on.
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T *) CClosedHashBase::Add(pData));
-    }
-
-//*****************************************************************************
-// Lookup a key value and return a pointer to the element if found.
-//*****************************************************************************
-    T *Find(                                // The item if found, 0 if not.
-        void        *pData)                 // The key to lookup.
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T *) CClosedHashBase::Find(pData));
-    }
-
-//*****************************************************************************
-// Look for an item in the table.  If it isn't found, then create a new one and
-// return that.
-//*****************************************************************************
-    T *FindOrAdd(                           // The item if found, 0 if not.
-        void        *pData,                 // The key to lookup.
-        bool        &bNew)                  // true if created.
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T *) CClosedHashBase::FindOrAdd(pData, bNew));
-    }
-
-
-//*****************************************************************************
-// The following functions are used to traverse each used entry.  This code
-// will skip over deleted and free entries freeing the caller up from such
-// logic.
-//*****************************************************************************
-    T *GetFirst()                           // The first entry, 0 if none.
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T *) CClosedHashBase::GetFirst());
-    }
-
-    T *GetNext(T *Prev)                     // The next entry, 0 if done.
-    {
-        WRAPPER_NO_CONTRACT;
-        return ((T *) CClosedHashBase::GetNext((BYTE *) Prev));
-    }
-};
-
-
-//*****************************************************************************
-// IMPORTANT: This data structure is deprecated, please do not add any new uses.
-// The hashtable implementation that should be used instead is code:SHash.
-// If code:SHash does not work for you, talk to mailto:clrdeag.
-//*****************************************************************************
-// Closed hash with typed parameters.  The derived class is the second
-//  parameter to the template.  The derived class must implement:
-//    unsigned long Hash(const T *pData);
-//    unsigned long Compare(const T *p1, T *p2);
-//    ELEMENTSTATUS Status(T *pEntry);
-//    void SetStatus(T *pEntry, ELEMENTSTATUS s);
-//    void* GetKey(T *pEntry);
-//*****************************************************************************
-template<class T, class H>class CClosedHashEx : public CClosedHash<T>
-{
-public:
-    CClosedHashEx(
-        int         iBuckets,               // How many buckets should we start with.
-        bool        bPerfect=false) :       // true if bucket size will hash with no collisions.
-        CClosedHash<T> (iBuckets, bPerfect)
-    {
-        WRAPPER_NO_CONTRACT;
-    }
-
-    unsigned int Hash(const void *pData)
-    {
-        WRAPPER_NO_CONTRACT;
-        return static_cast<H*>(this)->Hash((const T*)pData);
-    }
-
-    unsigned int Compare(const void *p1, BYTE *p2)
-    {
-        WRAPPER_NO_CONTRACT;
-        return static_cast<H*>(this)->Compare((const T*)p1, (T*)p2);
-    }
-
-    typename CClosedHash<T>::ELEMENTSTATUS Status(BYTE *p)
-    {
-        WRAPPER_NO_CONTRACT;
-        return static_cast<H*>(this)->Status((T*)p);
-    }
-
-    void SetStatus(BYTE *p, typename CClosedHash<T>::ELEMENTSTATUS s)
-    {
-        WRAPPER_NO_CONTRACT;
-        static_cast<H*>(this)->SetStatus((T*)p, s);
-    }
-
-    void* GetKey(BYTE *p)
-    {
-        WRAPPER_NO_CONTRACT;
-        return static_cast<H*>(this)->GetKey((T*)p);
-    }
-};
-
-
 //*****************************************************************************
 // IMPORTANT: This data structure is deprecated, please do not add any new uses.
 // The hashtable implementation that should be used instead is code:SHash.

src/coreclr/md/compiler/custattr_emit.cpp

@@ -864,7 +864,6 @@ HRESULT RegMeta::_IsKnownCustomAttribute(        // S_OK, S_FALSE, or error.
 {
     HRESULT     hr = S_OK;              // A result.
     CCustAttrHashKey sLookup;           // For looking up a custom attribute.
-    CCustAttrHashKey *pFound;           // Result of a lookup.
     LPCSTR      szNamespace = "";       // Namespace of custom attribute type.
     LPCSTR      szName = "";            // Name of custom attribute type.
     TypeDefRec  *pTypeDefRec = NULL;    // Parent record, when a TypeDef.
@@ -882,7 +881,8 @@ HRESULT RegMeta::_IsKnownCustomAttribute(        // S_OK, S_FALSE, or error.
     sLookup.tkType = tkCtor;
 
     // See if this custom attribute type has been seen before.
-    if ((pFound = m_caHash.Find(&sLookup)))
+    const CCustAttrHashKey* pFound = m_caHash.LookupPtr(tkCtor);
+    if (pFound)
     {   // Yes, already seen.
         *pca = pFound->ca;
         hr = (pFound->ca == CA_UNKNOWN) ? S_FALSE : S_OK;
@@ -991,9 +991,9 @@ HRESULT RegMeta::_IsKnownCustomAttribute(        // S_OK, S_FALSE, or error.
 
     // Add to hash.
     sLookup.ca = ixCa;
-    pFound = m_caHash.Add(&sLookup);
-    IfNullGo(pFound);
-    *pFound = sLookup;
+    if (!m_caHash.AddNoThrow(sLookup))
+        return E_OUTOFMEMORY;
+
     *pca = ixCa;
 
 ErrExit:

src/coreclr/md/compiler/custattr_import.cpp

@@ -22,37 +22,6 @@
 #include "corhdr.h"
 #include <metamodelrw.h>
 
-//*****************************************************************************
-// Implementation of hash for custom attribute types.
-//*****************************************************************************
-unsigned int CCustAttrHash::Hash(const CCustAttrHashKey *pData)
-{
-    return static_cast<unsigned int>(pData->tkType);
-} // unsigned long CCustAttrHash::Hash()
-unsigned int CCustAttrHash::Compare(const CCustAttrHashKey *p1, CCustAttrHashKey *p2)
-{
-    if (p1->tkType == p2->tkType)
-        return 0;
-    return 1;
-} // unsigned long CCustAttrHash::Compare()
-CCustAttrHash::ELEMENTSTATUS CCustAttrHash::Status(CCustAttrHashKey *p)
-{
-    if (p->tkType == FREE)
-        return (FREE);
-    if (p->tkType == DELETED)
-        return (DELETED);
-    return (USED);
-} // CCustAttrHash::ELEMENTSTATUS CCustAttrHash::Status()
-void CCustAttrHash::SetStatus(CCustAttrHashKey *p, CCustAttrHash::ELEMENTSTATUS s)
-{
-    p->tkType = s;
-} // void CCustAttrHash::SetStatus()
-void* CCustAttrHash::GetKey(CCustAttrHashKey *p)
-{
-    return &p->tkType;
-} // void* CCustAttrHash::GetKey()
-
-
 //*****************************************************************************
 // Get the value of a CustomAttribute, using only TypeName for lookup.
 //*****************************************************************************

src/coreclr/md/compiler/regmeta.h

@@ -92,23 +92,15 @@ struct CCustAttrHashKey
     int         ca;                     // flag indicating what the ca is.
 };
 
-class CCustAttrHash : public CClosedHashEx<CCustAttrHashKey, CCustAttrHash>
+class CustAttrHashTraits : public NoRemoveSHashTraits<DefaultSHashTraits<CCustAttrHashKey>>
 {
-    typedef CCustAttrHashKey T;
-
-    using CClosedHashEx<CCustAttrHashKey, CCustAttrHash>::Hash;
-    using CClosedHashEx<CCustAttrHashKey, CCustAttrHash>::Compare;
-    using CClosedHashEx<CCustAttrHashKey, CCustAttrHash>::Status;
-    using CClosedHashEx<CCustAttrHashKey, CCustAttrHash>::SetStatus;
-    using CClosedHashEx<CCustAttrHashKey, CCustAttrHash>::GetKey;
-
 public:
-    CCustAttrHash(int iBuckets=37) : CClosedHashEx<CCustAttrHashKey,CCustAttrHash>(iBuckets) {}
-    unsigned int Hash(const T *pData);
-    unsigned int Compare(const T *p1, T *p2);
-    ELEMENTSTATUS Status(T *pEntry);
-    void SetStatus(T *pEntry, ELEMENTSTATUS s);
-    void* GetKey(T *pEntry);
+    using key_t = mdToken;
+    static const key_t GetKey(_In_ const element_t& e) { return e.tkType; }
+    static count_t Hash(_In_ key_t key) { return static_cast<count_t>(key); }
+    static bool Equals(_In_ key_t lhs, _In_ key_t rhs) { return lhs == rhs; }
+    static bool IsNull(_In_ const element_t& e) { return e.tkType == mdTokenNil; }
+    static const element_t Null() { return CCustAttrHashKey{ mdTokenNil, 0 }; }
 };
 
 class MDInternalRW;
@@ -2048,7 +2040,7 @@ private:
     SetAPICallerType m_SetAPICaller;
 
     CorValidatorModuleType      m_ModuleType;
-    CCustAttrHash               m_caHash;   // Hashed list of custom attribute types seen.
+    SHash<CustAttrHashTraits>   m_caHash;   // Hashed list of custom attribute types seen.
 
     bool        m_bKeepKnownCa;             // Should all known CA's be kept?