Revert "Update added types and methoddefs on ApplyMetadata (#22617)" (#22923)
authorJan Kotas <jkotas@microsoft.com>
Thu, 28 Feb 2019 23:45:26 +0000 (15:45 -0800)
committerGitHub <noreply@github.com>
Thu, 28 Feb 2019 23:45:26 +0000 (15:45 -0800)
This reverts commit ee755e322dabc2fc280e2561b0fbaf6e90aedf54.

Documentation/Profiling/Profiler Breaking Changes.md [deleted file]
src/vm/ceeload.cpp
src/vm/ceeload.h
src/vm/clsload.cpp
src/vm/proftoeeinterfaceimpl.cpp

diff --git a/Documentation/Profiling/Profiler Breaking Changes.md b/Documentation/Profiling/Profiler Breaking Changes.md
deleted file mode 100644 (file)
index f63b227..0000000
+++ /dev/null
@@ -1,6 +0,0 @@
-# Profiler Breaking Changes #
-
-Over time we will need to modify the Profiler APIs, this document will serve as a record of any breaking changes.
-
-1. Code Versioning introduced changes documented [here](../design-docs/code-versioning-profiler-breaking-changes.md)
-2. The work to allow adding new types and methods after module load means ICorProfilerInfo7::ApplyMetadata will now potentially trigger a GC, and will not be callable in situations where a GC can not happen (for example  ICorProfilerCallback::RootReferences).
\ No newline at end of file
index 07109ae..6d1643b 100644 (file)
@@ -191,35 +191,6 @@ BOOL Module::SetTransientFlagInterlocked(DWORD dwFlag)
 }
 
 #if PROFILING_SUPPORTED 
-void Module::UpdateNewlyAddedTypes()
-{
-    CONTRACTL
-    {
-        THROWS;
-        GC_TRIGGERS;
-        INJECT_FAULT(COMPlusThrowOM(););
-    }
-    CONTRACTL_END
-
-    DWORD countTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtTypeDef);
-    DWORD countExportedTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtExportedType);
-
-    // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2
-    for (DWORD typeDefRid = m_dwTypeCount + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++)
-    {
-        GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef));
-    }
-
-    // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1
-    for (DWORD exportedTypeDef = m_dwExportedTypeCount + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++)
-    {
-        GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType));
-    }
-
-    m_dwTypeCount = countTypesAfterProfilerUpdate;
-    m_dwExportedTypeCount = countExportedTypesAfterProfilerUpdate;
-}
-
 void Module::NotifyProfilerLoadFinished(HRESULT hr)
 {
     CONTRACTL
@@ -237,10 +208,12 @@ void Module::NotifyProfilerLoadFinished(HRESULT hr)
     if (SetTransientFlagInterlocked(IS_PROFILER_NOTIFIED))
     {
         // Record how many types are already present
+        DWORD countTypesOrig = 0;
+        DWORD countExportedTypesOrig = 0;
         if (!IsResource())
         {
-            m_dwTypeCount = GetMDImport()->GetCountWithTokenKind(mdtTypeDef);
-            m_dwExportedTypeCount = GetMDImport()->GetCountWithTokenKind(mdtExportedType);
+            countTypesOrig = GetMDImport()->GetCountWithTokenKind(mdtTypeDef);
+            countExportedTypesOrig = GetMDImport()->GetCountWithTokenKind(mdtExportedType);
         }
 
         // Notify the profiler, this may cause metadata to be updated
@@ -263,7 +236,18 @@ void Module::NotifyProfilerLoadFinished(HRESULT hr)
         // assembly
         if (!IsResource())
         {
-            UpdateNewlyAddedTypes();
+            DWORD countTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtTypeDef);
+            DWORD countExportedTypesAfterProfilerUpdate = GetMDImport()->GetCountWithTokenKind(mdtExportedType);
+            // typeDefs rids 0 and 1 aren't included in the count, thus X typeDefs before means rid X+1 was valid and our incremental addition should start at X+2
+            for (DWORD typeDefRid = countTypesOrig + 2; typeDefRid < countTypesAfterProfilerUpdate + 2; typeDefRid++)
+            {
+                GetAssembly()->AddType(this, TokenFromRid(typeDefRid, mdtTypeDef));
+            }
+            // exportedType rid 0 isn't included in the count, thus X exportedTypes before means rid X was valid and our incremental addition should start at X+1
+            for (DWORD exportedTypeDef = countExportedTypesOrig + 1; exportedTypeDef < countExportedTypesAfterProfilerUpdate + 1; exportedTypeDef++)
+            {
+                GetAssembly()->AddExportedType(TokenFromRid(exportedTypeDef, mdtExportedType));
+            }
         }
 
         {
@@ -601,11 +585,6 @@ void Module::Initialize(AllocMemTracker *pamTracker, LPCWSTR szName)
     m_ModuleID = NULL;
     m_ModuleIndex.m_dwIndex = (SIZE_T)-1;
 
-    // These will be initialized in NotifyProfilerLoadFinished, set them to 
-    // a safe initial value now.
-    m_dwTypeCount = 0;
-    m_dwExportedTypeCount = 0;
-
     // Prepare statics that are known at module load time
     AllocateStatics(pamTracker);
 
@@ -1208,7 +1187,7 @@ void Module::ApplyMetaData()
     CONTRACTL
     {
         THROWS;
-        GC_TRIGGERS;
+        GC_NOTRIGGER;
         MODE_ANY;
     }
     CONTRACTL_END;
@@ -1218,13 +1197,6 @@ void Module::ApplyMetaData()
     HRESULT hr = S_OK;
     ULONG ulCount;
 
-#if PROFILING_SUPPORTED 
-    if (!IsResource())
-    {
-        UpdateNewlyAddedTypes();
-    }
-#endif // PROFILING_SUPPORTED
-
     // Ensure for TypeRef
     ulCount = GetMDImport()->GetCountWithTokenKind(mdtTypeRef) + 1;
     EnsureTypeRefCanBeStored(TokenFromRid(ulCount, mdtTypeRef));
@@ -1232,10 +1204,6 @@ void Module::ApplyMetaData()
     // Ensure for AssemblyRef
     ulCount = GetMDImport()->GetCountWithTokenKind(mdtAssemblyRef) + 1;
     EnsureAssemblyRefCanBeStored(TokenFromRid(ulCount, mdtAssemblyRef));
-
-    // Ensure for MethodDef
-    ulCount = GetMDImport()->GetCountWithTokenKind(mdtMethodDef) + 1;
-    EnsureMethodDefCanBeStored(TokenFromRid(ulCount, mdtMethodDef));
 }
 
 //
index 280c65a..6bf5fbd 100644 (file)
@@ -1627,11 +1627,6 @@ private:
     BOOL                            m_nativeImageProfiling;
     CORCOMPILE_METHOD_PROFILE_LIST *m_methodProfileList;
 
-#if PROFILING_SUPPORTED_DATA 
-    DWORD                   m_dwTypeCount;
-    DWORD                   m_dwExportedTypeCount;
-#endif // PROFILING_SUPPORTED_DATA
-
 #if defined(FEATURE_COMINTEROP)
         public:
 
@@ -2531,8 +2526,6 @@ public:
                                               DEBUGGER_INFO_SHIFT_PRIV);
     }
 
-    void UpdateNewlyAddedTypes();
-
 #ifdef PROFILING_SUPPORTED
     BOOL IsProfilerNotified() {LIMITED_METHOD_CONTRACT;  return (m_dwTransientFlags & IS_PROFILER_NOTIFIED) != 0; }
     void NotifyProfilerLoadFinished(HRESULT hr);
index 027dcd3..4ffbb3f 100644 (file)
@@ -840,8 +840,7 @@ void ClassLoader::GetClassValue(NameHandleTable nhTable,
             continue;
 
 #ifdef FEATURE_READYTORUN
-        if (nhTable == nhCaseSensitive && pCurrentClsModule->IsReadyToRun() && pCurrentClsModule->GetReadyToRunInfo()->HasHashtableOfTypes() &&
-            pCurrentClsModule->GetAvailableClassHash() == NULL)
+        if (nhTable == nhCaseSensitive && pCurrentClsModule->IsReadyToRun() && pCurrentClsModule->GetReadyToRunInfo()->HasHashtableOfTypes())
         {
             // For R2R modules, we only search the hashtable of token types stored in the module's image, and don't fallback
             // to searching m_pAvailableClasses or m_pAvailableClassesCaseIns (in fact, we don't even allocate them for R2R modules).
@@ -4185,15 +4184,6 @@ VOID ClassLoader::AddAvailableClassDontHaveLock(Module *pModule,
 #endif
 
     CrstHolder ch(&m_AvailableClassLock);
-
-    // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can
-    // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the
-    // in memory hashtable as usual.
-    if (!pModule->IsResource() && pModule->GetAvailableClassHash() == NULL)
-    {
-        LazyPopulateCaseSensitiveHashTables();
-    }
-
     AddAvailableClassHaveLock(
         pModule, 
         classdef, 
@@ -4390,15 +4380,6 @@ VOID ClassLoader::AddExportedTypeDontHaveLock(Module *pManifestModule,
     CONTRACTL_END
 
     CrstHolder ch(&m_AvailableClassLock);
-        
-    // R2R pre-computes an export table and tries to avoid populating a class hash at runtime. However the profiler can
-    // still add new types on the fly by calling here. If that occurs we fallback to the slower path of creating the
-    // in memory hashtable as usual.
-    if (!pManifestModule->IsResource() && pManifestModule->GetAvailableClassHash() == NULL)
-    {
-        LazyPopulateCaseSensitiveHashTables();
-    }
-
     AddExportedTypeHaveLock(
         pManifestModule,
         cl,
index 663b82f..8542888 100644 (file)
@@ -9661,12 +9661,12 @@ HRESULT ProfToEEInterfaceImpl::ApplyMetaData(
     CONTRACTL
     {
         NOTHROW;
-        GC_TRIGGERS;
+        GC_NOTRIGGER;
         MODE_ANY;
     }
     CONTRACTL_END;
 
-    PROFILER_TO_CLR_ENTRYPOINT_SYNC_EX(kP2EEAllowableAfterAttach | kP2EETriggers, (LF_CORPROF, LL_INFO1000, "**PROF: ApplyMetaData.\n"));
+    PROFILER_TO_CLR_ENTRYPOINT_SYNC_EX(kP2EEAllowableAfterAttach, (LF_CORPROF, LL_INFO1000, "**PROF: ApplyMetaData.\n"));
 
     if (moduleId == NULL)
     {