From d8c545cf3eee2decde5902649331b42af1e13c0f Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Thu, 25 Jul 2024 22:01:30 -0700 Subject: [PATCH 1/3] Simplify initialization of RuntimeMetrics - Trigger the RuntimeMetrics initialization only when actually needed in the MeterListener constructor. - Delete the lock-ordering workaround and wrong comment introduced in #105259. Trigger the RuntimeMetrics initialization only when actually needed should make the lock-ordering workarond unnecessary. --- .../System/Diagnostics/Metrics/Instrument.cs | 11 ------- .../Diagnostics/Metrics/MeterListener.cs | 12 ++++---- .../Diagnostics/Metrics/RuntimeMetrics.cs | 30 +++++-------------- 3 files changed, 13 insertions(+), 40 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs index 8b87501f26a695..20c4607f927b1f 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/Instrument.cs @@ -89,17 +89,6 @@ protected void Publish() return; } - // MeterListener has a static constructor that creates runtime metrics instruments. - // We need to ensure this static constructor is called before starting to publish the instrument. - // This is necessary because creating runtime metrics instruments will cause re-entry to the Publish method, - // potentially resulting in a deadlock due to the SyncObject lock. - // Sequence of the deadlock: - // 1. An application creates an early instrument (e.g., Counter) before the MeterListener static constructor is executed. - // 2. Instrument.Publish is called and enters the SyncObject lock. - // 3. Within the lock block, MeterListener is called, triggering its static constructor. - // 4. The static constructor creates runtime metrics instruments, causing re-entry to Instrument.Publish and leading to a deadlock. - RuntimeHelpers.RunClassConstructor(typeof(MeterListener).TypeHandle); - List? allListeners = null; lock (Instrument.SyncObject) { diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MeterListener.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MeterListener.cs index 8466c1ac0fff97..5bbb4c7dd5ab58 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MeterListener.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/MeterListener.cs @@ -33,19 +33,17 @@ public sealed class MeterListener : IDisposable private MeasurementCallback _doubleMeasurementCallback = (instrument, measurement, tags, state) => { /* no-op */ }; private MeasurementCallback _decimalMeasurementCallback = (instrument, measurement, tags, state) => { /* no-op */ }; - static MeterListener() + /// + /// Creates a MeterListener object. + /// + public MeterListener() { #if NET9_0_OR_GREATER // This ensures that the static Meter gets created before any listeners exist. - _ = RuntimeMetrics.IsEnabled(); + RuntimeMetrics.EnsureInitialized(); #endif } - /// - /// Creates a MeterListener object. - /// - public MeterListener() { } - /// /// Callbacks to get notification when an instrument is published. /// diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs index 4af7755b21266a..b7449a552a6bcd 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs @@ -20,6 +20,11 @@ internal static class RuntimeMetrics private static readonly int s_maxGenerations = Math.Min(GC.GetGCMemoryInfo().GenerationInfo.Length, s_genNames.Length); + public static void EnsureInitialized() + { + // Dummy method to ensure that the static constructor have run and created the meters + } + static RuntimeMetrics() { AppDomain.CurrentDomain.FirstChanceException += (source, e) => @@ -33,6 +38,8 @@ static RuntimeMetrics() }; } +#pragma warning disable CA1823 // suppress unused fields warning, as the fields are used to keep the meters alive + private static readonly ObservableCounter s_gcCollections = s_meter.CreateObservableCounter( "dotnet.gc.collections", GetGarbageCollectionCounts, @@ -156,28 +163,7 @@ static RuntimeMetrics() unit: "s", description: "CPU time used by the process."); - public static bool IsEnabled() - { - return s_gcCollections.Enabled - || s_processWorkingSet.Enabled - || s_gcHeapTotalAllocated.Enabled - || s_gcLastCollectionMemoryCommitted.Enabled - || s_gcLastCollectionHeapSize.Enabled - || s_gcLastCollectionFragmentationSize.Enabled - || s_gcPauseTime.Enabled - || s_jitCompiledSize.Enabled - || s_jitCompiledMethodCount.Enabled - || s_jitCompilationTime.Enabled - || s_monitorLockContention.Enabled - || s_timerCount.Enabled - || s_threadPoolThreadCount.Enabled - || s_threadPoolCompletedWorkItems.Enabled - || s_threadPoolQueueLength.Enabled - || s_assembliesCount.Enabled - || s_exceptions.Enabled - || s_processCpuCount.Enabled - || s_processCpuTime?.Enabled is true; - } +#pragma warning restore CA1823 private static IEnumerable> GetGarbageCollectionCounts() { From 401b9825365beea133b764e2e95aa02011d71527 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Thu, 25 Jul 2024 23:22:58 -0700 Subject: [PATCH 2/3] Delete unnecessary static fields --- .../Diagnostics/Metrics/RuntimeMetrics.cs | 251 +++++++++--------- 1 file changed, 124 insertions(+), 127 deletions(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs index b7449a552a6bcd..c511ee7e0cd946 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs @@ -20,6 +20,8 @@ internal static class RuntimeMetrics private static readonly int s_maxGenerations = Math.Min(GC.GetGCMemoryInfo().GenerationInfo.Length, s_genNames.Length); + private static readonly Counter s_exceptions; + public static void EnsureInitialized() { // Dummy method to ensure that the static constructor have run and created the meters @@ -27,6 +29,114 @@ public static void EnsureInitialized() static RuntimeMetrics() { + s_meter.CreateObservableCounter( + "dotnet.gc.collections", + GetGarbageCollectionCounts, + unit: "{collection}", + description: "The number of garbage collections that have occurred since the process has started."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.process.memory.working_set", + () => Environment.WorkingSet, + unit: "By", + description: "The number of bytes of physical memory mapped to the process context."); + + s_meter.CreateObservableCounter( + "dotnet.gc.heap.total_allocated", + () => GC.GetTotalAllocatedBytes(), + unit: "By", + description: "The approximate number of bytes allocated on the managed GC heap since the process has started. The returned value does not include any native allocations."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.gc.last_collection.memory.committed_size", + () => + { + GCMemoryInfo gcInfo = GC.GetGCMemoryInfo(); + + return gcInfo.Index == 0 + ? Array.Empty>() + : [new(gcInfo.TotalCommittedBytes)]; + }, + unit: "By", + description: "The amount of committed virtual memory in use by the .NET GC, as observed during the latest garbage collection."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.gc.last_collection.heap.size", + GetHeapSizes, + unit: "By", + description: "The managed GC heap size (including fragmentation), as observed during the latest garbage collection."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.gc.last_collection.heap.fragmentation.size", + GetHeapFragmentation, + unit: "By", + description: "The heap fragmentation, as observed during the latest garbage collection."); + + s_meter.CreateObservableCounter( + "dotnet.gc.pause.time", + () => GC.GetTotalPauseDuration().TotalSeconds, + unit: "s", + description: "The total amount of time paused in GC since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.jit.compiled_il.size", + () => Runtime.JitInfo.GetCompiledILBytes(), + unit: "By", + description: "Count of bytes of intermediate language that have been compiled since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.jit.compiled_methods", + () => Runtime.JitInfo.GetCompiledMethodCount(), + unit: "{method}", + description: "The number of times the JIT compiler (re)compiled methods since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.jit.compilation.time", + () => Runtime.JitInfo.GetCompilationTime().TotalSeconds, + unit: "s", + description: "The number of times the JIT compiler (re)compiled methods since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.monitor.lock_contentions", + () => Monitor.LockContentionCount, + unit: "{contention}", + description: "The number of times there was contention when trying to acquire a monitor lock since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.thread_pool.thread.count", + () => (long)ThreadPool.ThreadCount, + unit: "{thread}", + description: "The number of thread pool threads that currently exist."); + + s_meter.CreateObservableCounter( + "dotnet.thread_pool.work_item.count", + () => ThreadPool.CompletedWorkItemCount, + unit: "{work_item}", + description: "The number of work items that the thread pool has completed since the process has started."); + + s_meter.CreateObservableCounter( + "dotnet.thread_pool.queue.length", + () => ThreadPool.PendingWorkItemCount, + unit: "{work_item}", + description: "The number of work items that are currently queued to be processed by the thread pool."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.timer.count", + () => Timer.ActiveCount, + unit: "{timer}", + description: "The number of timer instances that are currently active. An active timer is registered to tick at some point in the future and has not yet been canceled."); + + s_meter.CreateObservableUpDownCounter( + "dotnet.assembly.count", + () => (long)AppDomain.CurrentDomain.GetAssemblies().Length, + unit: "{assembly}", + description: "The number of .NET assemblies that are currently loaded."); + + s_exceptions = s_meter.CreateCounter( + "dotnet.exceptions", + unit: "{exception}", + description: "The number of exceptions that have been thrown in managed code."); + AppDomain.CurrentDomain.FirstChanceException += (source, e) => { // Avoid recursion if the listener itself throws an exception while recording the measurement @@ -36,134 +146,22 @@ static RuntimeMetrics() s_exceptions.Add(1, new KeyValuePair("error.type", e.Exception.GetType().Name)); t_handlingFirstChanceException = false; }; - } -#pragma warning disable CA1823 // suppress unused fields warning, as the fields are used to keep the meters alive - - private static readonly ObservableCounter s_gcCollections = s_meter.CreateObservableCounter( - "dotnet.gc.collections", - GetGarbageCollectionCounts, - unit: "{collection}", - description: "The number of garbage collections that have occurred since the process has started."); - - private static readonly ObservableUpDownCounter s_processWorkingSet = s_meter.CreateObservableUpDownCounter( - "dotnet.process.memory.working_set", - () => Environment.WorkingSet, - unit: "By", - description: "The number of bytes of physical memory mapped to the process context."); - - private static readonly ObservableCounter s_gcHeapTotalAllocated = s_meter.CreateObservableCounter( - "dotnet.gc.heap.total_allocated", - () => GC.GetTotalAllocatedBytes(), - unit: "By", - description: "The approximate number of bytes allocated on the managed GC heap since the process has started. The returned value does not include any native allocations."); - - private static readonly ObservableUpDownCounter s_gcLastCollectionMemoryCommitted = s_meter.CreateObservableUpDownCounter( - "dotnet.gc.last_collection.memory.committed_size", - () => + s_meter.CreateObservableUpDownCounter( + "dotnet.process.cpu.count", + () => (long)Environment.ProcessorCount, + unit: "{cpu}", + description: "The number of processors available to the process."); + + if (!OperatingSystem.IsBrowser() && !OperatingSystem.IsTvOS() && !(OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst())) { - GCMemoryInfo gcInfo = GC.GetGCMemoryInfo(); - - return gcInfo.Index == 0 - ? Array.Empty>() - : [new(gcInfo.TotalCommittedBytes)]; - }, - unit: "By", - description: "The amount of committed virtual memory in use by the .NET GC, as observed during the latest garbage collection."); - - private static readonly ObservableUpDownCounter s_gcLastCollectionHeapSize = s_meter.CreateObservableUpDownCounter( - "dotnet.gc.last_collection.heap.size", - GetHeapSizes, - unit: "By", - description: "The managed GC heap size (including fragmentation), as observed during the latest garbage collection."); - - private static readonly ObservableUpDownCounter s_gcLastCollectionFragmentationSize = s_meter.CreateObservableUpDownCounter( - "dotnet.gc.last_collection.heap.fragmentation.size", - GetHeapFragmentation, - unit: "By", - description: "The heap fragmentation, as observed during the latest garbage collection."); - - private static readonly ObservableCounter s_gcPauseTime = s_meter.CreateObservableCounter( - "dotnet.gc.pause.time", - () => GC.GetTotalPauseDuration().TotalSeconds, - unit: "s", - description: "The total amount of time paused in GC since the process has started."); - - private static readonly ObservableCounter s_jitCompiledSize = s_meter.CreateObservableCounter( - "dotnet.jit.compiled_il.size", - () => Runtime.JitInfo.GetCompiledILBytes(), - unit: "By", - description: "Count of bytes of intermediate language that have been compiled since the process has started."); - - private static readonly ObservableCounter s_jitCompiledMethodCount = s_meter.CreateObservableCounter( - "dotnet.jit.compiled_methods", - () => Runtime.JitInfo.GetCompiledMethodCount(), - unit: "{method}", - description: "The number of times the JIT compiler (re)compiled methods since the process has started."); - - private static readonly ObservableCounter s_jitCompilationTime = s_meter.CreateObservableCounter( - "dotnet.jit.compilation.time", - () => Runtime.JitInfo.GetCompilationTime().TotalSeconds, - unit: "s", - description: "The number of times the JIT compiler (re)compiled methods since the process has started."); - - private static readonly ObservableCounter s_monitorLockContention = s_meter.CreateObservableCounter( - "dotnet.monitor.lock_contentions", - () => Monitor.LockContentionCount, - unit: "{contention}", - description: "The number of times there was contention when trying to acquire a monitor lock since the process has started."); - - private static readonly ObservableCounter s_threadPoolThreadCount = s_meter.CreateObservableCounter( - "dotnet.thread_pool.thread.count", - () => (long)ThreadPool.ThreadCount, - unit: "{thread}", - description: "The number of thread pool threads that currently exist."); - - private static readonly ObservableCounter s_threadPoolCompletedWorkItems = s_meter.CreateObservableCounter( - "dotnet.thread_pool.work_item.count", - () => ThreadPool.CompletedWorkItemCount, - unit: "{work_item}", - description: "The number of work items that the thread pool has completed since the process has started."); - - private static readonly ObservableCounter s_threadPoolQueueLength = s_meter.CreateObservableCounter( - "dotnet.thread_pool.queue.length", - () => ThreadPool.PendingWorkItemCount, - unit: "{work_item}", - description: "The number of work items that are currently queued to be processed by the thread pool."); - - private static readonly ObservableUpDownCounter s_timerCount = s_meter.CreateObservableUpDownCounter( - "dotnet.timer.count", - () => Timer.ActiveCount, - unit: "{timer}", - description: "The number of timer instances that are currently active. An active timer is registered to tick at some point in the future and has not yet been canceled."); - - private static readonly ObservableUpDownCounter s_assembliesCount = s_meter.CreateObservableUpDownCounter( - "dotnet.assembly.count", - () => (long)AppDomain.CurrentDomain.GetAssemblies().Length, - unit: "{assembly}", - description: "The number of .NET assemblies that are currently loaded."); - - private static readonly Counter s_exceptions = s_meter.CreateCounter( - "dotnet.exceptions", - unit: "{exception}", - description: "The number of exceptions that have been thrown in managed code."); - - private static readonly ObservableUpDownCounter s_processCpuCount = s_meter.CreateObservableUpDownCounter( - "dotnet.process.cpu.count", - () => (long)Environment.ProcessorCount, - unit: "{cpu}", - description: "The number of processors available to the process."); - - private static readonly ObservableCounter? s_processCpuTime = - OperatingSystem.IsBrowser() || OperatingSystem.IsTvOS() || (OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst()) ? - null : - s_meter.CreateObservableCounter( - "dotnet.process.cpu.time", - GetCpuTime, - unit: "s", - description: "CPU time used by the process."); - -#pragma warning restore CA1823 + s_meter.CreateObservableCounter( + "dotnet.process.cpu.time", + GetCpuTime, + unit: "s", + description: "CPU time used by the process."); + } + } private static IEnumerable> GetGarbageCollectionCounts() { @@ -183,7 +181,6 @@ private static IEnumerable> GetGarbageCollectionCounts() [SupportedOSPlatform("maccatalyst")] private static IEnumerable> GetCpuTime() { - Debug.Assert(s_processCpuTime is not null); Debug.Assert(!OperatingSystem.IsBrowser() && !OperatingSystem.IsTvOS() && !(OperatingSystem.IsIOS() && !OperatingSystem.IsMacCatalyst())); Environment.ProcessCpuUsage processCpuUsage = Environment.CpuUsage; From ce1b59bf12937eaae5023d6fd144e1311f514652 Mon Sep 17 00:00:00 2001 From: Jan Kotas Date: Fri, 26 Jul 2024 08:13:08 -0700 Subject: [PATCH 3/3] Update src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs --- .../src/System/Diagnostics/Metrics/RuntimeMetrics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs index c511ee7e0cd946..fc746ff1f3545d 100644 --- a/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs +++ b/src/libraries/System.Diagnostics.DiagnosticSource/src/System/Diagnostics/Metrics/RuntimeMetrics.cs @@ -24,7 +24,7 @@ internal static class RuntimeMetrics public static void EnsureInitialized() { - // Dummy method to ensure that the static constructor have run and created the meters + // Dummy method to ensure that the static constructor run and created the meters } static RuntimeMetrics()