Skip to content

Commit

Permalink
EventSource startup initialization (#106014)
Browse files Browse the repository at this point in the history
* EventSource startup initialization

Fixes #105845

Previously MetricsEventSource wasn't being created for apps that didn't ever create a Meter. This caused a chicken-and-egg problem for RuntimeMetrics which weren't created until MetricsEventSource started a tracing session. This change ensures that MetricsEventSource will be created on demand if ETW, EventPipe, or EventListener based tooling starts a tracing session. I took some extra effort to create the EventSource in a deferred fashion to avoid eager loading System.Diagnostics.DiagnosticSource.dll when it might never be needed.

Aside from the fix there were some small improvements:
- Moved NativeRuntimeEventSource to initialize in the same place as other startup EventSources
- Removed a useless lock(EventListener.EventListenersLock) around EventPipe eventProvider registration
  • Loading branch information
noahfalk authored Aug 8, 2024
1 parent 243715e commit 679a284
Show file tree
Hide file tree
Showing 13 changed files with 247 additions and 72 deletions.
4 changes: 2 additions & 2 deletions src/coreclr/vm/corelib.h
Original file line number Diff line number Diff line change
Expand Up @@ -775,8 +775,8 @@ DEFINE_FIELD_U(rgiColumnNumber, StackFrameHelper, rgiColumnNumber)
DEFINE_FIELD_U(rgiLastFrameFromForeignExceptionStackTrace, StackFrameHelper, rgiLastFrameFromForeignExceptionStackTrace)
DEFINE_FIELD_U(iFrameCount, StackFrameHelper, iFrameCount)

DEFINE_CLASS(RUNTIME_EVENT_SOURCE, Tracing, RuntimeEventSource)
DEFINE_METHOD(RUNTIME_EVENT_SOURCE, INITIALIZE, Initialize, SM_RetVoid)
DEFINE_CLASS(EVENT_SOURCE, Tracing, EventSource)
DEFINE_METHOD(EVENT_SOURCE, INITIALIZE_DEFAULT_EVENT_SOURCES, InitializeDefaultEventSources, SM_RetVoid)

DEFINE_CLASS(STARTUP_HOOK_PROVIDER, System, StartupHookProvider)
DEFINE_METHOD(STARTUP_HOOK_PROVIDER, MANAGED_STARTUP, ManagedStartup, SM_PtrChar_RetVoid)
Expand Down
6 changes: 3 additions & 3 deletions src/coreclr/vm/corhost.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -665,11 +665,11 @@ HRESULT CorHost2::CreateAppDomainWithManager(
m_fAppDomainCreated = TRUE;

#ifdef FEATURE_PERFTRACING
// Initialize RuntimeEventSource
// Initialize default event sources
{
GCX_COOP();
MethodDescCallSite initRuntimeEventSource(METHOD__RUNTIME_EVENT_SOURCE__INITIALIZE);
initRuntimeEventSource.Call(NULL);
MethodDescCallSite initEventSources(METHOD__EVENT_SOURCE__INITIALIZE_DEFAULT_EVENT_SOURCES);
initEventSources.Call(NULL);
}
#endif // FEATURE_PERFTRACING

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
<linker>
<assembly fullname="System.Diagnostics.DiagnosticSource">
<type fullname="System.Diagnostics.Metrics.MetricsEventSource">
<!-- Used by System.Private.CoreLib via reflection to init the EventSource -->
<method name="GetInstance" />
</type>
</assembly>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ System.Diagnostics.DiagnosticSource</PackageDescription>

<ItemGroup>
<ILLinkSubstitutionsXmls Include="ILLink/ILLink.Substitutions.Shared.xml" />
<Content Include="ILLink\ILLink.Descriptors.LibraryBuild.xml" />
</ItemGroup>

<ItemGroup>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ internal sealed class MetricsEventSource : EventSource
{
public static readonly MetricsEventSource Log = new();

// Although this API isn't public, it is invoked via reflection from System.Private.CoreLib and needs the same back-compat
// consideration as a public API. See EventSource.InitializeDefaultEventSources() in System.Private.CoreLib source for more
// details. We have a unit test GetInstanceMethodIsReflectable that verifies this method isn't accidentally removed or renamed.
public static MetricsEventSource GetInstance() { return Log; }

private const string SharedSessionId = "SHARED";
private const string ClientIdKey = "ClientId";
private const string MaxHistogramsKey = "MaxHistograms";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Diagnostics.Tracing;
using System.Globalization;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text;
using System.Threading;
Expand All @@ -26,6 +27,23 @@ public MetricEventSourceTests(ITestOutputHelper output)
_output = output;
}

[Fact]
public void GetInstanceMethodIsReflectable()
{
// The startup code in System.Private.CoreLib needs to be able to get the MetricsEventSource instance via reflection. See EventSource.InitializeDefaultEventSources() in
// the System.Private.CoreLib source.
// Even though the the type isn't public this test ensures the GetInstance() API isn't removed or renamed.
Type? metricsEventSourceType = Type.GetType("System.Diagnostics.Metrics.MetricsEventSource, System.Diagnostics.DiagnosticSource", throwOnError: false);
Assert.True(metricsEventSourceType != null, "Unable to get MetricsEventSource type via reflection");

MethodInfo? getInstanceMethod = metricsEventSourceType.GetMethod("GetInstance", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Static, null, Type.EmptyTypes, null);
Assert.True(getInstanceMethod != null, "Unable to get MetricsEventSource.GetInstance method via reflection");

object? o = getInstanceMethod.Invoke(null, null);
Assert.True(o != null, "Expected non-null result invoking MetricsEventSource.GetInstance() via reflection");
Assert.True(o is EventSource, "Expected object returned from MetricsEventSource.GetInstance() to be assignable to EventSource");
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotBrowser))]
[OuterLoop("Slow and has lots of console spew")]
public void MultipleListeners_DifferentCounters()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ public static void CheckNoEventSourcesRunning(string message = "")
eventSource.Name != "System.Reflection.Runtime.Tracing" &&
eventSource.Name != "Microsoft-Windows-DotNETRuntime" &&
eventSource.Name != "System.Runtime" &&
eventSource.Name != "System.Diagnostics.Metrics" &&

// event source from xunit runner
eventSource.Name != "xUnit.TestEventSource" &&
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
<linker>
<assembly fullname="System.Private.CoreLib" feature="System.Diagnostics.Tracing.EventSource.IsSupported" featurevalue="true" featuredefault="true">
<type fullname="System.Diagnostics.Tracing.RuntimeEventSource">
<method signature="System.Void Initialize()" />
<type fullname="System.Diagnostics.Tracing.EventSource">
<method signature="System.Void InitializeDefaultEventSources()" />
</type>
</assembly>
</linker>
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ private static unsafe void Callback(byte* sourceId, int isEnabled, byte level,
}

// Register an event provider.
internal override unsafe void Register(EventSource eventSource)
internal override unsafe void Register(Guid id, string name)
{
Debug.Assert(!_gcHandle.IsAllocated);
_gcHandle = GCHandle.Alloc(this);

_provHandle = EventPipeInternal.CreateProvider(eventSource.Name, &Callback, (void*)GCHandle.ToIntPtr(_gcHandle));
_provHandle = EventPipeInternal.CreateProvider(name, &Callback, (void*)GCHandle.ToIntPtr(_gcHandle));
if (_provHandle == 0)
{
// Unable to create the provider.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,14 +103,14 @@ internal EventProvider(EventProviderType providerType)
}

/// <summary>
/// This method registers the controlGuid of this class with ETW.
/// This method registers the provider with the backing tracing mechanism, either ETW or EventPipe.
/// </summary>
internal unsafe void Register(EventSource eventSource)
internal unsafe void Register(Guid id, string name)
{
_providerName = eventSource.Name;
_providerId = eventSource.Guid;
_providerName = name;
_providerId = id;

_eventProvider.Register(eventSource);
_eventProvider.Register(id, name);
}

//
Expand Down Expand Up @@ -827,13 +827,13 @@ private static unsafe void Callback(Guid* sourceId, int isEnabled, byte level,


// Register an event provider.
internal override unsafe void Register(EventSource eventSource)
internal override unsafe void Register(Guid id, string name)
{
Debug.Assert(!_gcHandle.IsAllocated);
_gcHandle = GCHandle.Alloc(this);

long registrationHandle = 0;
_providerId = eventSource.Guid;
_providerId = id;
Guid providerId = _providerId;
uint status = Interop.Advapi32.EventRegister(
&providerId,
Expand Down Expand Up @@ -1235,7 +1235,7 @@ internal virtual void Disable()
_allKeywordMask = 0;
}

internal virtual void Register(EventSource eventSource)
internal virtual void Register(Guid id, string name)
{
}

Expand Down
Loading

0 comments on commit 679a284

Please sign in to comment.