Code review feedback

src/coreclr/src/vm/eventpipe.cpp

@@ -265,35 +265,40 @@ void EventPipe::EnableViaEnvironmentVariables()
     if (gcGenAnalysisConfigured == 1 && gcGenAnalysisState == 0)
 #endif
     {
-        LPCWSTR outputPath = nullptr;
-        outputPath = W("gcgenaware.nettrace");
-        NewHolder<EventPipeProviderConfiguration> pProviders = nullptr;
-        int providerCnt = 1;
-        pProviders = new EventPipeProviderConfiguration[providerCnt];
-        const uint64_t GenAwareKeyword                  = 0x10000000000; // This keyword is necessary for the start and stop events
-        const uint64_t GCHeapAndTypeNamesKeyword        = 0x00001000000; // This keyword is necessary for the type names
-        const uint64_t GCHeapSurvivalAndMovementKeyword = 0x00000400000; // This keyword is necessary for the generation range data.
-        const uint64_t GCHeapDumpKeyword                = 0x00000100000; // This keyword is necessary for enabling walking the heap
-        const uint64_t TypeKeyword                      = 0x00000080000; // This keyword is necessary for enabling BulkType events
-        const uint64_t keyword                          = GenAwareKeyword|GCHeapAndTypeNamesKeyword|GCHeapSurvivalAndMovementKeyword|GCHeapDumpKeyword|TypeKeyword;
-        pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), keyword, 5, nullptr);
-        gcGenAnalysisEventPipeSessionId = EventPipe::Enable(
-            outputPath,
-            1024,
-            pProviders,
-            providerCnt,
-            EventPipeSessionType::File,
-            EventPipeSerializationFormat::NetTraceV4,
-            false,
-            nullptr
-        );
-        if (gcGenAnalysisEventPipeSessionId > 0)
-        {
-            gcGenAnalysisEventPipeSession= EventPipe::GetSession(gcGenAnalysisEventPipeSessionId);
-            gcGenAnalysisEventPipeSession->Pause();
-            EventPipe::StartStreaming(gcGenAnalysisEventPipeSessionId);
-            gcGenAnalysisState = GcGenAnalysisState::Enabled;
-        }
+        EnableGenerationalAwareSession();
+    }
+}
+
+void EventPipe::EnableGenerationalAwareSession()
+{
+    LPCWSTR outputPath = nullptr;
+    outputPath = W("gcgenaware.nettrace");
+    NewHolder<EventPipeProviderConfiguration> pProviders = nullptr;
+    int providerCnt = 1;
+    pProviders = new EventPipeProviderConfiguration[providerCnt];
+    const uint64_t GenAwareKeyword                  = 0x10000000000; // This keyword is necessary for the start and stop events
+    const uint64_t GCHeapAndTypeNamesKeyword        = 0x00001000000; // This keyword is necessary for the type names
+    const uint64_t GCHeapSurvivalAndMovementKeyword = 0x00000400000; // This keyword is necessary for the generation range data.
+    const uint64_t GCHeapDumpKeyword                = 0x00000100000; // This keyword is necessary for enabling walking the heap
+    const uint64_t TypeKeyword                      = 0x00000080000; // This keyword is necessary for enabling BulkType events
+    const uint64_t keyword                          = GenAwareKeyword|GCHeapAndTypeNamesKeyword|GCHeapSurvivalAndMovementKeyword|GCHeapDumpKeyword|TypeKeyword;
+    pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), keyword, 5, nullptr);
+    gcGenAnalysisEventPipeSessionId = EventPipe::Enable(
+        outputPath,
+        1024,
+        pProviders,
+        providerCnt,
+        EventPipeSessionType::File,
+        EventPipeSerializationFormat::NetTraceV4,
+        false,
+        nullptr
+    );
+    if (gcGenAnalysisEventPipeSessionId > 0)
+    {
+        gcGenAnalysisEventPipeSession= EventPipe::GetSession(gcGenAnalysisEventPipeSessionId);
+        gcGenAnalysisEventPipeSession->Pause();
+        EventPipe::StartStreaming(gcGenAnalysisEventPipeSessionId);
+        gcGenAnalysisState = GcGenAnalysisState::Enabled;
     }
 }
 

src/coreclr/src/vm/eventpipe.h

@@ -51,6 +51,9 @@ class EventPipe
     // Initialize environment variable based session
     static void EnableViaEnvironmentVariables();
 
+    // Initialize generation aware analysis sesssion
+    static void EnableGenerationalAwareSession();
+
     // Shutdown the event pipe.
     static void Shutdown();
 

src/coreclr/src/vm/eventpipesession.h

@@ -96,6 +96,14 @@ class EventPipeSession
 
     void DisableIpcStreamingThread();
 
+    // Note - access to this field is NOT synchronized
+    //
+    // This field is currently modified in EventPipe::EnableViaEnvironmentVariables() during process startup
+    // and GCToEEInterface::AnalyzeSurvivorsFinished() while the GC has already synchronized all the threads.
+    // 
+    // It is read in EventPipeSession::WriteEvent(). While it is possible for other preemptive threads to read
+    // the field while GC is happening, it should not happen because the only gcGenAwareSession only subscribe
+    // to GC events.
     bool m_paused;
 
 public:

src/coreclr/src/vm/finalizerthread.cpp

@@ -274,40 +274,7 @@ VOID FinalizerThread::FinalizerThreadWorker(void *args)
             fclose(fopen("trace.nettrace.completed","w+"));
 #ifdef GEN_ANALYSIS_STRESS
             {
-                LPCWSTR outputPath = nullptr;
-                outputPath = W("gcgenaware.nettrace");
-                NewHolder<EventPipeProviderConfiguration> pProviders = nullptr;
-                int providerCnt = 1;
-                pProviders = new EventPipeProviderConfiguration[providerCnt];
-                const uint64_t GenAwareKeyword                  = 0x10000000000; // This keyword is necessary for the start and stop events
-                const uint64_t GCHeapAndTypeNamesKeyword        = 0x00001000000; // This keyword is necessary for the type names
-                const uint64_t GCHeapSurvivalAndMovementKeyword = 0x00000400000; // This keyword is necessary for the generation range data.
-                const uint64_t GCHeapDumpKeyword                = 0x00000100000; // This keyword is necessary for enabling walking the heap
-                const uint64_t TypeKeyword                      = 0x00000080000; // This keyword is necessary for enabling BulkType events
-                const uint64_t keyword                          = GenAwareKeyword|GCHeapAndTypeNamesKeyword|GCHeapSurvivalAndMovementKeyword|GCHeapDumpKeyword|TypeKeyword;
-                pProviders[0] = EventPipeProviderConfiguration(W("Microsoft-Windows-DotNETRuntime"), keyword, 5, nullptr);
-                gcGenAnalysisEventPipeSessionId = EventPipe::Enable(
-                    outputPath,
-                    1024,
-                    pProviders,
-                    providerCnt,
-                    EventPipeSessionType::File,
-                    EventPipeSerializationFormat::NetTraceV4,
-                    false,
-                    nullptr
-                );
-                if (gcGenAnalysisEventPipeSessionId > 0)
-                {
-                    gcGenAnalysisEventPipeSession= EventPipe::GetSession(gcGenAnalysisEventPipeSessionId);
-                    // gcGenAnalysisEventPipeSession == nullptr is possible if EventPipe::Shutdown() is called
-                    // right after the Enable() call completes above and before EventPipe::GetSession() is called.
-                    if (gcGenAnalysisEventPipeSession != nullptr)
-                    {
-                        gcGenAnalysisEventPipeSession->Pause();
-                        EventPipe::StartStreaming(gcGenAnalysisEventPipeSessionId);
-                        gcGenAnalysisState = GcGenAnalysisState::Enabled;
-                    }
-                }
+                EventPipe::EnableGenerationalAwareSession();
             }
 #endif
         }