Code review feedback

src/coreclr/src/gc/gc.cpp

@@ -20909,26 +20909,19 @@ void gc_heap::mark_phase (int condemned_gen_number, BOOL mark_only_p)
     if (gc_t_join.joined())
 #endif //MULTIPLE_HEAPS
     {
-        uint64_t promoted_bytes = 0;
+        uint64_t total_promoted_bytes = 0;
 #ifdef HEAP_ANALYZE
         heap_analyze_enabled = FALSE;
 #ifdef MULTIPLE_HEAPS
         for (int i = 0; i < n_heaps; i++)
         {
-            promoted_bytes += g_promoted[i * 16];
+            promoted_bytes (i);
         }
 #else
-        promoted_bytes = g_promoted;
+        total_promoted_bytes = promoted_bytes (0);
 #endif //MULTIPLE_HEAPS
 
-        GCToEEInterface::AnalyzeSurvivorsFinished(condemned_gen_number, promoted_bytes, [](){
-            g_theGCHeap->DiagDescrGenerations([](void*, int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
-            {
-                uint64_t range = static_cast<uint64_t>(rangeEnd - rangeStart);
-                uint64_t rangeReserved = static_cast<uint64_t>(rangeEndReserved - rangeStart);
-                FIRE_EVENT(GCGenerationRange, generation, rangeStart, range, rangeReserved);
-            }, nullptr);
-        });
+        GCToEEInterface::AnalyzeSurvivorsFinished (condemned_gen_number, total_promoted_bytes, reportGenerationBounds);
 #endif // HEAP_ANALYZE
         GCToEEInterface::AfterGcScanRoots (condemned_gen_number, max_generation, &sc);
 

src/coreclr/src/gc/gcconfig.h

@@ -130,7 +130,7 @@ class GCConfigStringHolder
     INT_CONFIG   (GCHeapHardLimitLOHPercent, "GCHeapHardLimitLOHPercent", "System.GC.HeapHardLimitLOHPercent", 0,        "Specifies the GC heap LOH usage as a percentage of the total memory")                    \
     INT_CONFIG   (GCHeapHardLimitPOHPercent, "GCHeapHardLimitPOHPercent", "System.GC.HeapHardLimitPOHPercent", 0,        "Specifies the GC heap POH usage as a percentage of the total memory")                    \
     INT_CONFIG   (GCEnabledInstructionSets,  "GCEnabledInstructionSets",  NULL,                                -1,       "Specifies whether GC can use AVX2 or AVX512F - 0 for neither, 1 for AVX2, 3 for AVX512F")\
-    INT_CONFIG   (GCGenAnalysisGen,       "GCGenAnalysisGen",       NULL,                             999,               "Specifies which generation to analysis")                                                 \
+    INT_CONFIG   (GCGenAnalysisGen,       "GCGenAnalysisGen",       NULL,                             -1,                "Specifies which generation to analysis")                                                 \
     INT_CONFIG   (GCGenAnalysisBytes,     "GCGenAnalysisBytes",     NULL,                             0,                 "Specifies how much promoted bytes to trigger generation analysis")                       \
 
 // This class is responsible for retreiving configuration information

src/coreclr/src/gc/gcee.cpp

@@ -53,6 +53,11 @@ void GCHeap::UpdatePreGCCounters()
 #endif // BACKGROUND_GC
 
     FIRE_EVENT(GCStart_V2, count, depth, reason, static_cast<uint32_t>(type));
+    reportGenerationBounds();
+}
+
+void reportGenerationBounds()
+{
     g_theGCHeap->DiagDescrGenerations([](void*, int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
     {
         uint64_t range = static_cast<uint64_t>(rangeEnd - rangeStart);
@@ -148,12 +153,7 @@ void GCHeap::UpdatePostGCCounters()
 #endif //FEATURE_EVENT_TRACE
 
 #ifdef FEATURE_EVENT_TRACE
-    g_theGCHeap->DiagDescrGenerations([](void*, int generation, uint8_t* rangeStart, uint8_t* rangeEnd, uint8_t* rangeEndReserved)
-    {
-        uint64_t range = static_cast<uint64_t>(rangeEnd - rangeStart);
-        uint64_t rangeReserved = static_cast<uint64_t>(rangeEndReserved - rangeStart);
-        FIRE_EVENT(GCGenerationRange, generation, rangeStart, range, rangeReserved);
-    }, nullptr);
+    reportGenerationBounds();
 
     FIRE_EVENT(GCEnd_V1, static_cast<uint32_t>(pSettings->gc_index), condemned_gen);
 

src/coreclr/src/gc/gcimpl.h

@@ -317,4 +317,6 @@ class GCHeap : public IGCHeapInternal
     virtual void Shutdown();
 };
 
+void reportGenerationBounds();
+
 #endif  // GCIMPL_H_

src/coreclr/src/vm/gcenv.ee.cpp

@@ -15,7 +15,7 @@
 int event_pipe_state = 0;
 EventPipeSession* pEventPipeSession = nullptr;
 uint64_t sessionId = (uint64_t)-1;
-bool gcGenAnalysis = false;
+int gcGenAnalysis = 0;
 int64_t gcGenAnalysisGen = 999;
 int64_t gcGenAnalysisBytes = 0;
 
@@ -30,18 +30,25 @@ void GCToEEInterface::SuspendEE(SUSPEND_REASON reason)
 
     g_pDebugInterface->SuspendForGarbageCollectionStarted();
 
-    if (GetIntConfigValue("GCGenAnalysisGen", nullptr, &gcGenAnalysisGen))
+    if (gcGenAnalysis == 0)
     {
-        if (GetIntConfigValue("GCGenAnalysisBytes", nullptr, &gcGenAnalysisBytes))
+        if (GetIntConfigValue("GCGenAnalysisGen", nullptr, &gcGenAnalysisGen))
         {
-            gcGenAnalysis = true;
+            if (GetIntConfigValue("GCGenAnalysisBytes", nullptr, &gcGenAnalysisBytes))
+            {
+                gcGenAnalysis = 1;
+            }
+            else
+            {
+                gcGenAnalysis = 2;
+            }
         }
     }
-    if (gcGenAnalysis && event_pipe_state == 0)
+    if (gcGenAnalysis == 1 && event_pipe_state == 0)
     {
         event_pipe_state = 1;
         LPCWSTR outputPath = nullptr;
-        outputPath = W("trace.nettrace");        
+        outputPath = W("gcgenaware.nettrace");
         NewHolder<EventPipeProviderConfiguration> pProviders = nullptr;
         int providerCnt = 1;
         pProviders = new EventPipeProviderConfiguration[providerCnt];
@@ -1652,7 +1659,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(int condemnedGeneration, uint64_t
 
     if (event_pipe_state == 1)
     {
-        if (condemnedGeneration == gcGenAnalysisGen && promoted_bytes > (uint64_t)gcGenAnalysisBytes)
+        if (condemnedGeneration == gcGenAnalysisGen && (promoted_bytes > (uint64_t)gcGenAnalysisBytes))
         {
             event_pipe_state = 2;
             pEventPipeSession->Resume();