From 8cf0b19b0688a323eafbb65f80295deaf97efb16 Mon Sep 17 00:00:00 2001
From: Vladimir Sadov <vsadov@microsoft.com>
Date: Wed, 17 Nov 2021 10:30:29 -0800
Subject: [PATCH] A few follow up changes to LookupTypeKey change (#61718)

* re-enable optimization when a token is replaced by corresponding type handle

* compute "isNested" ony when we need it.

* no need to make more than 2 attempts at reading
---
 src/coreclr/vm/clsload.cpp | 49 +++++++++++++++++++++++++---------------------
 1 file changed, 27 insertions(+), 22 deletions(-)

diff --git a/src/coreclr/vm/clsload.cpp b/src/coreclr/vm/clsload.cpp
index 68de4d4..0578cc2 100644
--- a/src/coreclr/vm/clsload.cpp
+++ b/src/coreclr/vm/clsload.cpp
@@ -627,7 +627,6 @@ void ClassLoader::GetClassValue(NameHandleTable nhTable,
     CONTRACTL_END
 
 
-    mdToken             mdEncloser;
     EEClassHashEntry_t  *pBucket = NULL;
 
     needsToBuildHashtable = FALSE;
@@ -643,8 +642,6 @@ void ClassLoader::GetClassValue(NameHandleTable nhTable,
     }
 #endif
 
-    BOOL isNested = IsNested(pName, &mdEncloser);
-
     PTR_Assembly assembly = GetAssembly();
     PREFIX_ASSUME(assembly != NULL);
     ModuleIterator i = assembly->IterateModules();
@@ -719,6 +716,9 @@ void ClassLoader::GetClassValue(NameHandleTable nhTable,
             }
             _ASSERTE(pTable);
 
+            mdToken             mdEncloser;
+            BOOL isNested = IsNested(pName, &mdEncloser);
+
             if (isNested)
             {
                 Module *pNameModule = pName->GetTypeModule();
@@ -834,6 +834,9 @@ void ClassLoader::LazyPopulateCaseSensitiveHashTables()
     }
     CONTRACTL_END;
 
+
+    _ASSERT(m_cUnhashedModules > 0);
+
     AllocMemTracker amTracker;
     ModuleIterator i = GetAssembly()->IterateModules();
 
@@ -909,6 +912,8 @@ void ClassLoader::LazyPopulateCaseInsensitiveHashTables()
                 amTracker.SuppressRelease();
                 pModule->SetAvailableClassCaseInsHash(pNewClassCaseInsHash);
                 FastInterlockDecrement((LONG*)&m_cUnhashedModules);
+
+                _ASSERT(m_cUnhashedModules >= 0);
             }
         }
     }
@@ -1238,18 +1243,18 @@ BOOL ClassLoader::FindClassModuleThrowing(
 
     if (pBucket == NULL)
     {
+        // Take the lock. To make sure the table is not being built by another thread.
         AvailableClasses_LockHolder lh(this);
 
-        // Try again with the lock.  This will protect against another thread reallocating
-        // the hash table underneath us
-        GetClassValue(nhTable, pName, &Data, &pTable, pLookInThisModuleOnly, &foundEntry, loadFlag, needsToBuildHashtable);
-        pBucket = foundEntry.GetClassHashBasedEntryValue();
-
+        if (!needsToBuildHashtable || (m_cUnhashedModules == 0))
+        {
+            // the table should be finished now, try again
+            GetClassValue(nhTable, pName, &Data, &pTable, pLookInThisModuleOnly, &foundEntry, loadFlag, needsToBuildHashtable);
+            pBucket = foundEntry.GetClassHashBasedEntryValue();
+        }
 #ifndef DACCESS_COMPILE
-        if (needsToBuildHashtable && (pBucket == NULL) && (m_cUnhashedModules > 0))
+        else
         {
-            _ASSERT(needsToBuildHashtable);
-
             if (nhTable == nhCaseInsensitive)
             {
                 LazyPopulateCaseInsensitiveHashTables();
@@ -1268,7 +1273,7 @@ BOOL ClassLoader::FindClassModuleThrowing(
 #endif
     }
 
-    // Same check as above, but this time we've checked with the lock so the table will be populated
+    // Same check as above, but this time we've ensured that the tables are populated
     if (pBucket == NULL)
     {
 #if defined(_DEBUG_IMPL) && !defined(DACCESS_COMPILE)
@@ -1536,19 +1541,19 @@ ClassLoader::LoadTypeHandleThrowing(
             pClsLdr = pFoundModule->GetClassLoader();
             pLookInThisModuleOnly = NULL;
         }
+    }
 
 #ifndef DACCESS_COMPILE
-        // Replace AvailableClasses Module entry with found TypeHandle
-        if (!typeHnd.IsNull() &&
-            typeHnd.IsRestored() &&
-            foundEntry.GetEntryType() == HashedTypeEntry::EntryType::IsHashedClassEntry &&
-            (foundEntry.GetClassHashBasedEntryValue() != NULL) &&
-            (foundEntry.GetClassHashBasedEntryValue()->GetData() != typeHnd.AsPtr()))
-        {
-            foundEntry.GetClassHashBasedEntryValue()->SetData(typeHnd.AsPtr());
-        }
-#endif // !DACCESS_COMPILE
+    // Replace AvailableClasses Module entry with found TypeHandle
+    if (!typeHnd.IsNull() &&
+        typeHnd.IsRestored() &&
+        foundEntry.GetEntryType() == HashedTypeEntry::EntryType::IsHashedClassEntry &&
+        (foundEntry.GetClassHashBasedEntryValue() != NULL) &&
+        (foundEntry.GetClassHashBasedEntryValue()->GetData() != typeHnd.AsPtr()))
+    {
+        foundEntry.GetClassHashBasedEntryValue()->SetData(typeHnd.AsPtr());
     }
+#endif // !DACCESS_COMPILE
 
     RETURN typeHnd;
 } // ClassLoader::LoadTypeHandleThrowing
-- 
2.7.4