Profiler RequestReJITWithInliners did not work properly if all methods in the module...

- Add unused functions into rejit.cs test to expose the problematic behavior
- Fix by using tokens throughout the handling of R2R inlining information instead of converting to MethodDescs at times

diff --git a/src/coreclr/vm/rejit.cpp b/src/coreclr/vm/rejit.cpp
index 8807b09..713992b 100644 (file)
--- a/src/coreclr/vm/rejit.cpp
+++ b/src/coreclr/vm/rejit.cpp
@@ -420,7 +420,6 @@ COR_IL_MAP* ProfilerFunctionControl::GetInstrumentedMapEntries()
 #ifndef DACCESS_COMPILE
 NativeImageInliningIterator::NativeImageInliningIterator() :
         m_pModule(NULL),
-        m_pInlinee(NULL),
         m_dynamicBuffer(NULL),
         m_dynamicBufferSize(0),
         m_dynamicAvailable(0),
@@ -429,21 +428,21 @@ NativeImageInliningIterator::NativeImageInliningIterator() :
 
 }
 
-HRESULT NativeImageInliningIterator::Reset(Module *pModule, MethodDesc *pInlinee)
+HRESULT NativeImageInliningIterator::Reset(Module *pInlinerModule, MethodInModule inlinee)
 {
-    _ASSERTE(pModule != NULL);
-    _ASSERTE(pInlinee != NULL);
+    _ASSERTE(pInlinerModule != NULL);
+    _ASSERTE(inlinee.m_module != NULL);
 
-    m_pModule = pModule;
-    m_pInlinee = pInlinee;
+    m_pModule = pInlinerModule;
+    m_inlinee = inlinee;
 
     HRESULT hr = S_OK;
     EX_TRY
     {
         // Trying to use the existing buffer
         BOOL incompleteData;
-        Module *inlineeModule = m_pInlinee->GetModule();
-        mdMethodDef mdInlinee = m_pInlinee->GetMemberDef();
+        Module *inlineeModule = m_inlinee.m_module;
+        mdMethodDef mdInlinee = m_inlinee.m_methodDef;
         COUNT_T methodsAvailable = m_pModule->GetReadyToRunInliners(inlineeModule, mdInlinee, m_dynamicBufferSize, m_dynamicBuffer, &incompleteData);
 
         // If the existing buffer is not large enough, reallocate.
@@ -484,19 +483,16 @@ BOOL NativeImageInliningIterator::Next()
     return m_currentPos < m_dynamicAvailable;
 }
 
-MethodDesc *NativeImageInliningIterator::GetMethodDesc()
+MethodInModule NativeImageInliningIterator::GetMethod()
 {
     // this evaluates true when m_currentPos == s_failurePos or m_currentPos == (COUNT_T)-1
     // m_currentPos is an unsigned type
     if (m_currentPos >= m_dynamicAvailable)
     {
-        return NULL;
+        return MethodInModule();
     }
 
-    MethodInModule mm = m_dynamicBuffer[m_currentPos];
-    Module *pModule = mm.m_module;
-    mdMethodDef mdInliner = mm.m_methodDef;
-    return pModule->LookupMethodDef(mdInliner);
+    return m_dynamicBuffer[m_currentPos];
 }
 
 //---------------------------------------------------------------------------------------
@@ -623,16 +619,20 @@ HRESULT ReJitManager::UpdateActiveILVersions(
 
         if ((flags & COR_PRF_REJIT_BLOCK_INLINING) == COR_PRF_REJIT_BLOCK_INLINING)
         {
-            hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pMD, fIsRevert, flags);
+            hr = UpdateNativeInlinerActiveILVersions(&mgrToCodeActivationBatch, pModule, rgMethodDefs[i], fIsRevert, flags);
             if (FAILED(hr))
             {
                 return hr;
             }
 
-            hr = UpdateJitInlinerActiveILVersions(&mgrToCodeActivationBatch, pMD, fIsRevert, flags);
-            if (FAILED(hr))
+            if (pMD != NULL)
             {
-                return hr;
+                // If pMD is not null, then the method may have already been inlined somewhere. Go check.
+                hr = UpdateJitInlinerActiveILVersions(&mgrToCodeActivationBatch, pMD, fIsRevert, flags);
+                if (FAILED(hr))
+                {
+                    return hr;
+                }
             }
         }
     }   // for (ULONG i = 0; i < cFunctions; i++)
@@ -780,7 +780,8 @@ HRESULT ReJitManager::UpdateActiveILVersion(
 // static
 HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
     SHash<CodeActivationBatchTraits>   *pMgrToCodeActivationBatch,
-    MethodDesc                         *pInlinee,
+    Module                             *pInlineeModule,
+    mdMethodDef                         inlineeMethodDef,
     BOOL                                fIsRevert,
     COR_PRF_REJIT_FLAGS                 flags)
 {
@@ -794,7 +795,8 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
     CONTRACTL_END;
 
     _ASSERTE(pMgrToCodeActivationBatch != NULL);
-    _ASSERTE(pInlinee != NULL);
+    _ASSERTE(pInlineeModule != NULL);
+    _ASSERTE(RidFromToken(inlineeMethodDef) != 0);
 
     HRESULT hr = S_OK;
 
@@ -812,16 +814,15 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
         Module * pModule = pDomainAssembly->GetModule();
         if (pModule->HasReadyToRunInlineTrackingMap())
         {
-            inlinerIter.Reset(pModule, pInlinee);
+            inlinerIter.Reset(pModule, MethodInModule(pInlineeModule, inlineeMethodDef));
 
-            MethodDesc *pInliner = NULL;
             while (inlinerIter.Next())
             {
-                pInliner = inlinerIter.GetMethodDesc();
+                MethodInModule inliner = inlinerIter.GetMethod();
                 {
                     CodeVersionManager *pCodeVersionManager = pModule->GetCodeVersionManager();
                     CodeVersionManager::LockHolder codeVersioningLockHolder;
-                    ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(pInliner);
+                    ILCodeVersion ilVersion = pCodeVersionManager->GetActiveILCodeVersion(inliner.m_module, inliner.m_methodDef);
                     if (!ilVersion.HasDefaultIL())
                     {
                         // This method has already been ReJITted, no need to request another ReJIT at this point.
@@ -830,10 +831,10 @@ HRESULT ReJitManager::UpdateNativeInlinerActiveILVersions(
                     }
                 }
 
-                hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, pInliner->GetModule(), pInliner->GetMemberDef(), fIsRevert, flags);
+                hr = UpdateActiveILVersion(pMgrToCodeActivationBatch, inliner.m_module, inliner.m_methodDef, fIsRevert, flags);
                 if (FAILED(hr))
                 {
-                    ReportReJITError(pInliner->GetModule(), pInliner->GetMemberDef(), NULL, hr);
+                    ReportReJITError(inliner.m_module, inliner.m_methodDef, NULL, hr);
                 }
             }
         }

diff --git a/src/coreclr/vm/rejit.h b/src/coreclr/vm/rejit.h
index 77e9dd6..17d32aa 100644 (file)
--- a/src/coreclr/vm/rejit.h
+++ b/src/coreclr/vm/rejit.h
@@ -75,13 +75,13 @@ class NativeImageInliningIterator
 public:
     NativeImageInliningIterator();
 
-    HRESULT Reset(Module *pInlineeModule, MethodDesc *pInlinee);
+    HRESULT Reset(Module* pInlinerModule, MethodInModule inlinee);
     BOOL Next();
-    MethodDesc *GetMethodDesc();
+    MethodInModule GetMethod();
 
 private:
     Module *m_pModule;
-    MethodDesc *m_pInlinee;
+    MethodInModule m_inlinee;
     NewArrayHolder<MethodInModule> m_dynamicBuffer;
     COUNT_T m_dynamicBufferSize;
     COUNT_T m_dynamicAvailable;
@@ -189,7 +189,8 @@ private:
 
     static HRESULT UpdateNativeInlinerActiveILVersions(
         SHash<CodeActivationBatchTraits> *pMgrToCodeActivationBatch,
-        MethodDesc         *pInlinee,
+        Module             *pInlineeModule,
+        mdMethodDef         inlineeMethodDef,
         BOOL                fIsRevert,
         COR_PRF_REJIT_FLAGS flags);
 

diff --git a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp
index 3077bf1..068d9dd 100644 (file)
--- a/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp
+++ b/src/tests/profiler/native/rejitprofiler/rejitprofiler.cpp
@@ -287,13 +287,20 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::JITCachedFunctionSearchFinished(Functio
         COR_PRF_METHOD method;
         while (pEnum->Next(1, &method, NULL) == S_OK)
         {
-            FunctionID inlinerFuncId = GetFunctionIDFromToken(method.moduleId, method.methodId);
+            FunctionID inlinerFuncId = GetFunctionIDFromToken(method.moduleId, method.methodId, true);
 
-            // GetFunctionIDFromToken doesn't handle generics, will return NULL
+            // GetFunctionIDFromToken doesn't handle generics or not yet loaded methods, will return NULL
             if (inlinerFuncId != mdTokenNil)
             {
                 AddInlining(inlinerFuncId, functionId);
             }
+            else
+            {
+                String calleeName = GetFunctionIDName(functionId);
+                String moduleName = GetModuleIDName(GetModuleIDForFunction(functionId));
+                String inlinerModuleId = GetModuleIDName(method.moduleId);
+                INFO(L"Inlining occurred, but name could not be resolved! Inliner=ModuleId=" << inlinerModuleId << L" Token=" << std::hex << method.methodId << L",  Inlinee=" << calleeName << L" Inlinee module name=" << moduleName);
+            }
         }
     }
 
@@ -313,7 +320,7 @@ HRESULT STDMETHODCALLTYPE ReJITProfiler::GetReJITParameters(ModuleID moduleId, m
 {
     SHUTDOWNGUARD();
 
-    INFO(L"Starting to build IL for method " << GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId)));
+    INFO(L"Starting to build IL for method " << GetFunctionIDName(GetFunctionIDFromToken(moduleId, methodId, false)));
     COMPtrHolder<IUnknown> pUnk;
     HRESULT hr = _profInfo10->GetModuleMetaData(moduleId, ofWrite, IID_IMetaDataEmit2, &pUnk);
     if (FAILED(hr))
@@ -444,13 +451,21 @@ void ReJITProfiler::AddInlining(FunctionID inliner, FunctionID inlinee)
     INFO(L"Inlining occurred! Inliner=" << GetFunctionIDName(inliner) << L" Inlinee=" << calleeName << L" Inlinee module name=" << moduleName);
 }
 
-FunctionID ReJITProfiler::GetFunctionIDFromToken(ModuleID module, mdMethodDef token)
+FunctionID ReJITProfiler::GetFunctionIDFromToken(ModuleID module, mdMethodDef token, bool invalidArgNotFailure)
 {
     HRESULT hr = S_OK;
     FunctionID functionId;
-    if (FAILED(hr = pCorProfilerInfo->GetFunctionFromToken(module,
-                                                           token,
-                                                           &functionId)))
+    hr = pCorProfilerInfo->GetFunctionFromToken(module,
+                                                token,
+                                                &functionId);
+
+    if (invalidArgNotFailure && hr == E_INVALIDARG)
+    {
+        printf("Call to GetFunctionFromToken failed with E_INVALIDARG, this may be caused by the method not yet being loaded\n");
+        return mdTokenNil;
+    }
+
+    if (FAILED(hr))
     {
         printf("Call to GetFunctionFromToken failed with hr=0x%x\n", hr);
         _failures++;

diff --git a/src/tests/profiler/native/rejitprofiler/rejitprofiler.h b/src/tests/profiler/native/rejitprofiler/rejitprofiler.h
index c6d0751..59b7ff7 100644 (file)
--- a/src/tests/profiler/native/rejitprofiler/rejitprofiler.h
+++ b/src/tests/profiler/native/rejitprofiler/rejitprofiler.h
@@ -48,7 +48,7 @@ private:
 
     bool FunctionSeen(FunctionID func);
 
-    FunctionID GetFunctionIDFromToken(ModuleID module, mdMethodDef token);
+    FunctionID GetFunctionIDFromToken(ModuleID module, mdMethodDef token, bool invalidArgNotFailure);
     mdMethodDef GetMethodDefForFunction(FunctionID functionId);
     ModuleID GetModuleIDForFunction(FunctionID functionId);
 

diff --git a/src/tests/profiler/rejit/rejit.cs b/src/tests/profiler/rejit/rejit.cs
index 7a45cce..b9ea65f 100644 (file)
--- a/src/tests/profiler/rejit/rejit.cs
+++ b/src/tests/profiler/rejit/rejit.cs
@@ -78,7 +78,7 @@ namespace Profiler.Tests
         }
 
         [MethodImplAttribute(MethodImplOptions.AggressiveInlining)]
-        private static void InlineeTarget()
+        public static void InlineeTarget()
         {
             Console.WriteLine("Inline.InlineeTarget");
         }
@@ -102,4 +102,29 @@ namespace Profiler.Tests
                                           profileeOptions: ProfileeOptions.OptimizationSensitive);
         }
     }
+
+    public class SeparateClassNeverLoaded
+    {
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        private static void TriggerInliningChain()
+        {
+            Console.WriteLine("TriggerInlining");
+            // Test Inlining through another method
+            InlineeChain1();
+        }
+
+        [MethodImplAttribute(MethodImplOptions.NoInlining)]
+        private static void TriggerDirectInlining()
+        {
+            Console.WriteLine("TriggerDirectInlining");
+            RejitWithInlining.InlineeTarget();
+        }
+
+        [MethodImplAttribute(MethodImplOptions.AggressiveInlining)]
+        private static void InlineeChain1()
+        {
+            Console.WriteLine("Inline.InlineeChain1");
+            RejitWithInlining.InlineeTarget();
+        }
+    }
 }