Code review feedback

cshung · cshung · commit b516f1058795 · 2025-02-06T12:14:04.000-08:00
diff --git a/src/coreclr/clr.featuredefines.props b/src/coreclr/clr.featuredefines.props
@@ -1,7 +1,6 @@
 <Project>
     <PropertyGroup>
         <FeatureCoreCLR>true</FeatureCoreCLR>
-        <FeatureEventTrace>true</FeatureEventTrace>
         <FeaturePerfTracing>true</FeaturePerfTracing>
         <ProfilingSupportedBuild>true</ProfilingSupportedBuild>
     </PropertyGroup>
@@ -33,7 +32,6 @@
         <DefineConstants Condition="'$(FeatureCominteropApartmentSupport)' == 'true'">$(DefineConstants);FEATURE_COMINTEROP_APARTMENT_SUPPORT</DefineConstants>
         <DefineConstants Condition="'$(FeatureIjw)' == 'true'">$(DefineConstants);FEATURE_IJW</DefineConstants>
         <DefineConstants Condition="'$(FeatureObjCMarshal)' == 'true'">$(DefineConstants);FEATURE_OBJCMARSHAL</DefineConstants>
-        <DefineConstants Condition="'$(FeatureEventTrace)' == 'true'">$(DefineConstants);FEATURE_EVENT_TRACE</DefineConstants>
         <DefineConstants Condition="'$(FeaturePerfTracing)' == 'true'">$(DefineConstants);FEATURE_PERFTRACING</DefineConstants>
         <DefineConstants Condition="'$(FeatureXplatEventSource)' == 'true'">$(DefineConstants);FEATURE_EVENTSOURCE_XPLAT</DefineConstants>
         <DefineConstants Condition="'$(FeatureTypeEquivalence)' == 'true'">$(DefineConstants);FEATURE_TYPEEQUIVALENCE</DefineConstants>
diff --git a/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj b/src/coreclr/nativeaot/System.Private.CoreLib/src/System.Private.CoreLib.csproj
@@ -583,11 +583,6 @@
     <Compile Include="@(EventingSourceFile)" />
   </ItemGroup>
 
-  <PropertyGroup>
-    <FeatureEventTrace>true</FeatureEventTrace>
-    <DefineConstants Condition="'$(FeatureEventTrace)' == 'true'">$(DefineConstants);FEATURE_EVENT_TRACE</DefineConstants>
-  </PropertyGroup>
-
   <Target Name="GenerateEventingFiles"
           Inputs="@(EventingGenerationScript);@(EventManifestFile);@(EventingInclusionList)"
           Outputs="@(EventingSourceFile)"
diff --git a/src/coreclr/vm/gcenv.ee.cpp b/src/coreclr/vm/gcenv.ee.cpp
@@ -1724,7 +1724,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene
         {
             if (gcGenAnalysisTrace)
             {
-#ifdef FEATURE_EVENT_TRACE
+#ifdef FEATURE_PERFTRACING
                 EventPipeAdapter::ResumeSession(gcGenAnalysisEventPipeSession);
                 FireEtwGenAwareBegin((int)gcIndex, GetClrInstanceId());
                 s_forcedGCInProgress = true;
@@ -1733,7 +1733,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene
                 reportGenerationBounds();
                 FireEtwGenAwareEnd((int)gcIndex, GetClrInstanceId());
                 EventPipeAdapter::PauseSession(gcGenAnalysisEventPipeSession);
-#endif //FEATURE_EVENT_TRACE
+#endif //FEATURE_PERFTRACING
             }
             if (gcGenAnalysisDump)
             {
diff --git a/src/coreclr/vm/genanalysis.h b/src/coreclr/vm/genanalysis.h
@@ -6,12 +6,10 @@
 #ifndef __GENANALYSIS_H__
 #define __GENANALYSIS_H__
 
-#ifndef FEATURE_EVENT_TRACE
-typedef struct _EventPipeSession EventPipeSession;
-#endif //FEATURE_EVENT_TRACE
-
 #ifdef FEATURE_PERFTRACING
 #include "eventpipeadaptertypes.h"
+#else
+typedef struct _EventPipeSession EventPipeSession;
 #endif // FEATURE_PERFTRACING
 
 enum GcGenAnalysisState
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.Internal.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.Internal.cs
@@ -9,7 +9,7 @@ namespace System.Diagnostics.Tracing
 {
     internal static partial class EventPipeInternal
     {
-#if FEATURE_EVENT_TRACE
+#if FEATURE_PERFTRACING
         //
         // These PInvokes are used by the configuration APIs to interact with EventPipe.
         //
@@ -131,6 +131,6 @@ internal static unsafe bool WaitForSessionSignal(ulong sessionID, int timeoutMs)
             return false;
         }
 #pragma warning restore IDE0060
-#endif //FEATURE_EVENT_TRACE
+#endif //FEATURE_PERFTRACING
     }
 }
diff --git a/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.cs b/src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventPipe.cs
@@ -83,7 +83,7 @@ internal sealed class EventPipeWaitHandle : WaitHandle
 
     internal static partial class EventPipeInternal
     {
-#if FEATURE_EVENT_TRACE
+#if FEATURE_PERFTRACING
         private unsafe struct EventPipeProviderConfigurationNative
         {
             private char* m_pProviderName;
@@ -162,6 +162,6 @@ internal static unsafe ulong Enable(
             return 0;
         }
 #pragma warning restore IDE0060
-#endif //FEATURE_EVENT_TRACE
+#endif //FEATURE_PERFTRACING
     }
 }
diff --git a/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj b/src/mono/System.Private.CoreLib/System.Private.CoreLib.csproj
@@ -320,11 +320,6 @@
     <Compile Include="@(EventingSourceFile)" />
   </ItemGroup>
 
-  <PropertyGroup>
-    <FeatureEventTrace>true</FeatureEventTrace>
-    <DefineConstants Condition="'$(FeatureEventTrace)' == 'true'">$(DefineConstants);FEATURE_EVENT_TRACE</DefineConstants>
-  </PropertyGroup>
-
   <Target Name="GenerateEventingFiles"
           Inputs="@(EventingGenerationScript);@(EventManifestFile)"
           Outputs="@(EventingSourceFile)"

Original file line number	Diff line number	Diff line change
`@@ -1724,7 +1724,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene`
`1724`	`1724`	`{`
`1725`	`1725`	`if (gcGenAnalysisTrace)`
`1726`	`1726`	`{`
`1727`		`-#ifdef FEATURE_EVENT_TRACE`
	`1727`	`+#ifdef FEATURE_PERFTRACING`
`1728`	`1728`	`EventPipeAdapter::ResumeSession(gcGenAnalysisEventPipeSession);`
`1729`	`1729`	`FireEtwGenAwareBegin((int)gcIndex, GetClrInstanceId());`
`1730`	`1730`	`s_forcedGCInProgress = true;`
`@@ -1733,7 +1733,7 @@ void GCToEEInterface::AnalyzeSurvivorsFinished(size_t gcIndex, int condemnedGene`
`1733`	`1733`	`reportGenerationBounds();`
`1734`	`1734`	`FireEtwGenAwareEnd((int)gcIndex, GetClrInstanceId());`
`1735`	`1735`	`EventPipeAdapter::PauseSession(gcGenAnalysisEventPipeSession);`
`1736`		`-#endif //FEATURE_EVENT_TRACE`
	`1736`	`+#endif //FEATURE_PERFTRACING`
`1737`	`1737`	`}`
`1738`	`1738`	`if (gcGenAnalysisDump)`
`1739`	`1739`	`{`
Original file line number	Diff line number	Diff line change
`@@ -9,7 +9,7 @@ namespace System.Diagnostics.Tracing`
`9`	`9`	`{`
`10`	`10`	`internal static partial class EventPipeInternal`
`11`	`11`	`{`
`12`		`-#if FEATURE_EVENT_TRACE`
	`12`	`+#if FEATURE_PERFTRACING`
`13`	`13`	`//`
`14`	`14`	`// These PInvokes are used by the configuration APIs to interact with EventPipe.`
`15`	`15`	`//`
`@@ -131,6 +131,6 @@ internal static unsafe bool WaitForSessionSignal(ulong sessionID, int timeoutMs)`
`131`	`131`	`return false;`
`132`	`132`	`}`
`133`	`133`	`#pragma warning restore IDE0060`
`134`		`-#endif //FEATURE_EVENT_TRACE`
	`134`	`+#endif //FEATURE_PERFTRACING`
`135`	`135`	`}`
`136`	`136`	`}`
Original file line number	Diff line number	Diff line change
`@@ -83,7 +83,7 @@ internal sealed class EventPipeWaitHandle : WaitHandle`
`83`	`83`
`84`	`84`	`internal static partial class EventPipeInternal`
`85`	`85`	`{`
`86`		`-#if FEATURE_EVENT_TRACE`
	`86`	`+#if FEATURE_PERFTRACING`
`87`	`87`	`private unsafe struct EventPipeProviderConfigurationNative`
`88`	`88`	`{`
`89`	`89`	`private char* m_pProviderName;`
`@@ -162,6 +162,6 @@ internal static unsafe ulong Enable(`
`162`	`162`	`return 0;`
`163`	`163`	`}`
`164`	`164`	`#pragma warning restore IDE0060`
`165`		`-#endif //FEATURE_EVENT_TRACE`
	`165`	`+#endif //FEATURE_PERFTRACING`
`166`	`166`	`}`
`167`	`167`	`}`