Partial fix for issue 22585. Fixes symbol lookup in profilers.
authorVance Morrison <vancemorrison@comcast.net>
Thu, 28 Feb 2019 00:18:42 +0000 (16:18 -0800)
committerVance Morrison <vancemorrison@comcast.net>
Thu, 28 Feb 2019 00:18:42 +0000 (16:18 -0800)
src/vm/eventtrace.cpp

index 5ca0787..e624f55 100644 (file)
@@ -6812,18 +6812,6 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(BaseDomain *pDomainFilter,
         ReJITID rejitID =
             fGetReJitIDs ? ReJitManager::GetReJitIdNoLock(pMD, codeStart) : 0;
 
-        // There are small windows of time where the heap iterator may come across a
-        // codeStart that is not yet published to the MethodDesc. This may happen if
-        // we're JITting the method right now on another thread, and have not completed
-        // yet. Detect the race, and skip the method if appropriate. (If rejitID is
-        // nonzero, there is no race, as GetReJitIdNoLock will not return a nonzero
-        // rejitID if the codeStart has not yet been published for that rejitted version
-        // of the method.) This check also catches recompilations due to EnC, which we do
-        // not want to issue events for, in order to ensure xperf's assumption that
-        // MethodDesc* + ReJITID + extent (hot vs. cold) form a unique key for code
-        // ranges of methods
-        if ((rejitID == 0) && (codeStart != PCODEToPINSTR(pMD->GetNativeCode())))
-            continue;
 
         // When we're called to announce loads, then the methodload event itself must
         // precede any supplemental events, so that the method load or method jitting
@@ -6845,9 +6833,37 @@ VOID ETW::MethodLog::SendEventsForJitMethodsHelper(BaseDomain *pDomainFilter,
             }
         }
 
-        // Send any supplemental events requested for this MethodID
-        if (fSendILToNativeMapEvent)
-            ETW::MethodLog::SendMethodILToNativeMapEvent(pMD, dwEventOptions, codeStart, rejitID);
+        // The filtering line below excludes all methods that don't go directly to the 
+        // native body (And that includes all Tiered JIT methods).   In V3.0 Tiered JIT
+        // is only by default which means important methods are filtered out.
+        //
+        // This filter used to exclude all events for a particular method, but has been
+        // moved here so it only applies to the IL->Native map.   
+        // This filter needs to be removed in its entirety, because right now all Tiered
+        // methods don't have a IL->Native map (and thus can't support source line profiling)
+        // moving it here at least minimizes the damage.  
+        //
+        // We did not remove the filter right now because currently 
+        // SendMethodILToNativeMapEvent relies on a fragile invariant where all lazy data is 
+        // known to be populated (see  g_pDebugInterface->InitializeLazyDataIfNecessary(); above)
+        // and this may not be true for tiered methods.    
+        // 
+        // There was also concern that there are races where the heap-iterator might see
+        // data between the time that the code was created and the debug information was set
+        // (there is a lock that protects the code manager data, however the JIT does not
+        // let all the data in one go (in particular the debug data is set latter).  
+        //
+        // Note that currently the  rejitID variable is likely always 0 because it seems that
+        // fGetReJitIDs=false except in the case that an appdomain is being unloaded (which I 
+        // think never happens in .NET core) 
+        // 
+        // Issue #XXXX tracks the work to fix the IL->Native mapping.  
+        if ((rejitID != 0) || (codeStart == PCODEToPINSTR(pMD->GetNativeCode())))
+        {
+            // Send any supplemental events requested for this MethodID
+            if (fSendILToNativeMapEvent)
+                ETW::MethodLog::SendMethodILToNativeMapEvent(pMD, dwEventOptions, codeStart, rejitID);
+        }
 
         // When we're called to announce unloads, then the methodunload event itself must
         // come after any supplemental events, so that the method unload event is the
@@ -7259,7 +7275,7 @@ VOID ETW::EnumerationLog::EnumerationHelper(Module *moduleFilter, BaseDomain *do
 
     if(moduleFilter)
     {
-        // Iteratate modules first because their number is ussualy smaller then the number of methods. 
+        // Iterate modules first because their number is usually smaller then the number of methods. 
         // Thus hitting a timeout due to a large number of methods will not affect modules rundown.tf g
         ETW::EnumerationLog::IterateModule(moduleFilter, enumerationOptions);