From a58888c154f9e0bd5ad98a0e27d1d8fd3916e127 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Mon, 7 Dec 2020 04:20:21 -0800 Subject: [PATCH 1/2] Revert "Make PortableThreadPool to check events correctly before firing them (#45666)" This reverts commit 83d19de66a596ff5e0ad0519e48dff62945cc636. --- .../System/Threading/PortableThreadPool.HillClimbing.cs | 7 +++---- .../src/System/Threading/PortableThreadPool.WaitThread.cs | 5 ++--- .../System/Threading/PortableThreadPool.WorkerThread.cs | 8 +++----- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.cs index 0275e617f15428..c7ed0ee800c934 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.HillClimbing.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Diagnostics.Tracing; namespace System.Threading { @@ -170,7 +169,7 @@ public HillClimbing() // Add the current thread count and throughput sample to our history // double throughput = numCompletions / sampleDurationSeconds; - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Informational, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadAdjustmentSample(throughput); } @@ -344,7 +343,7 @@ public HillClimbing() // Record these numbers for posterity // - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Verbose, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadAdjustmentStats(sampleDurationSeconds, throughput, threadWaveComponent.Real, throughputWaveComponent.Real, throughputErrorEstimate, _averageThroughputNoise, ratio.Real, confidence, _currentControlSetting, (ushort)newThreadWaveMagnitude); @@ -405,7 +404,7 @@ private void LogTransition(int newThreadCount, double throughput, StateOrTransit _logSize++; - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Informational, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadAdjustmentAdjustment( throughput, diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs index 2eb692706419d6..ed091b9d8fa921 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WaitThread.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Diagnostics.Tracing; using Microsoft.Win32.SafeHandles; namespace System.Threading @@ -37,7 +36,7 @@ internal partial class PortableThreadPool /// A description of the requested registration. internal void RegisterWaitHandle(RegisteredWaitHandle handle) { - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Verbose, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolIOEnqueue(handle); } @@ -76,7 +75,7 @@ internal void RegisterWaitHandle(RegisteredWaitHandle handle) internal static void CompleteWait(RegisteredWaitHandle handle, bool timedOut) { - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Verbose, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolIODequeue(handle); } diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs index ef1b905cf3bd98..4b7e3694c43dcb 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs @@ -1,8 +1,6 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Diagnostics.Tracing; - namespace System.Threading { internal partial class PortableThreadPool @@ -22,7 +20,7 @@ private static class WorkerThread AppContextConfigHelper.GetInt32Config("System.Threading.ThreadPool.UnfairSemaphoreSpinLimit", 70, false), onWait: () => { - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Informational, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadWait( (uint)ThreadPoolInstance._separated.counts.VolatileRead().NumExistingThreads); @@ -35,7 +33,7 @@ private static void WorkerThreadStart() PortableThreadPool threadPoolInstance = ThreadPoolInstance; - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Informational, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadStart( (uint)threadPoolInstance._separated.counts.VolatileRead().NumExistingThreads); @@ -107,7 +105,7 @@ private static void WorkerThreadStart() { HillClimbing.ThreadPoolHillClimber.ForceChange(newNumThreadsGoal, HillClimbing.StateOrTransition.ThreadTimedOut); - if (PortableThreadPoolEventSource.Log.IsEnabled(EventLevel.Informational, PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkerThreadStop((uint)newNumExistingThreads); } From 5785405c9d1f29be9e439ceab21e03990565ffe5 Mon Sep 17 00:00:00 2001 From: Koundinya Veluri Date: Mon, 7 Dec 2020 04:42:24 -0800 Subject: [PATCH 2/2] Add keyword and verbosity checks for events Reverted the part of https://github.com/dotnet/runtime/pull/38225/commits/e8043ffa05450b1983a1e751d0a1af80e43e1f31 regarding `IsEnabled` checks --- .../PortableThreadPool.GateThread.cs | 6 +-- .../PortableThreadPoolEventSource.cs | 54 ++++++++++++++++--- 2 files changed, 48 insertions(+), 12 deletions(-) diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs index c509fdfa008c9c..6b6897f1b287ff 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.GateThread.cs @@ -2,7 +2,6 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Diagnostics; -using System.Diagnostics.Tracing; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; @@ -44,10 +43,7 @@ private static void GateThreadStart() { Thread.Sleep(GateThreadDelayMs); - if (ThreadPool.EnableWorkerTracking && - PortableThreadPoolEventSource.Log.IsEnabled( - EventLevel.Verbose, - PortableThreadPoolEventSource.Keywords.ThreadingKeyword)) + if (ThreadPool.EnableWorkerTracking && PortableThreadPoolEventSource.Log.IsEnabled()) { PortableThreadPoolEventSource.Log.ThreadPoolWorkingThreadCount( (uint)threadPoolInstance.GetAndResetHighWatermarkCountOfThreadsProcessingUserCallbacks()); diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPoolEventSource.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPoolEventSource.cs index 711893fd8b4804..65061781083b02 100644 --- a/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPoolEventSource.cs +++ b/src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPoolEventSource.cs @@ -108,7 +108,10 @@ public unsafe void ThreadPoolWorkerThreadStart( uint RetiredWorkerThreadCount = 0, ushort ClrInstanceID = DefaultClrInstanceId) { - WriteThreadEvent(50, ActiveWorkerThreadCount); + if (IsEnabled(EventLevel.Informational, Keywords.ThreadingKeyword)) + { + WriteThreadEvent(50, ActiveWorkerThreadCount); + } } [Event(51, Level = EventLevel.Informational, Message = Messages.WorkerThread, Task = Tasks.ThreadPoolWorkerThread, Opcode = EventOpcode.Stop, Version = 0, Keywords = Keywords.ThreadingKeyword)] @@ -117,16 +120,23 @@ public void ThreadPoolWorkerThreadStop( uint RetiredWorkerThreadCount = 0, ushort ClrInstanceID = DefaultClrInstanceId) { - WriteThreadEvent(51, ActiveWorkerThreadCount); + if (IsEnabled(EventLevel.Informational, Keywords.ThreadingKeyword)) + { + WriteThreadEvent(51, ActiveWorkerThreadCount); + } } [Event(57, Level = EventLevel.Informational, Message = Messages.WorkerThread, Task = Tasks.ThreadPoolWorkerThread, Opcode = Opcodes.Wait, Version = 0, Keywords = Keywords.ThreadingKeyword)] + [MethodImpl(MethodImplOptions.NoInlining)] public void ThreadPoolWorkerThreadWait( uint ActiveWorkerThreadCount, uint RetiredWorkerThreadCount = 0, ushort ClrInstanceID = DefaultClrInstanceId) { - WriteThreadEvent(57, ActiveWorkerThreadCount); + if (IsEnabled(EventLevel.Informational, Keywords.ThreadingKeyword)) + { + WriteThreadEvent(57, ActiveWorkerThreadCount); + } } [Event(54, Level = EventLevel.Informational, Message = Messages.WorkerThreadAdjustmentSample, Task = Tasks.ThreadPoolWorkerThreadAdjustment, Opcode = Opcodes.Sample, Version = 0, Keywords = Keywords.ThreadingKeyword)] @@ -134,6 +144,11 @@ public unsafe void ThreadPoolWorkerThreadAdjustmentSample( double Throughput, ushort ClrInstanceID = DefaultClrInstanceId) { + if (!IsEnabled(EventLevel.Informational, Keywords.ThreadingKeyword)) + { + return; + } + EventData* data = stackalloc EventData[2]; data[0].DataPointer = (IntPtr)(&Throughput); data[0].Size = sizeof(double); @@ -151,6 +166,11 @@ public unsafe void ThreadPoolWorkerThreadAdjustmentAdjustment( ThreadAdjustmentReasonMap Reason, ushort ClrInstanceID = DefaultClrInstanceId) { + if (!IsEnabled(EventLevel.Informational, Keywords.ThreadingKeyword)) + { + return; + } + EventData* data = stackalloc EventData[4]; data[0].DataPointer = (IntPtr)(&AverageThroughput); data[0].Size = sizeof(double); @@ -181,6 +201,11 @@ public unsafe void ThreadPoolWorkerThreadAdjustmentStats( ushort NewThreadWaveMagnitude, ushort ClrInstanceID = DefaultClrInstanceId) { + if (!IsEnabled(EventLevel.Verbose, Keywords.ThreadingKeyword)) + { + return; + } + EventData* data = stackalloc EventData[11]; data[0].DataPointer = (IntPtr)(&Duration); data[0].Size = sizeof(double); @@ -247,8 +272,13 @@ private unsafe void ThreadPoolIOEnqueue( // FrameworkEventSource's thread transfer send/receive events instead at callers. [NonEvent] [MethodImpl(MethodImplOptions.NoInlining)] - public void ThreadPoolIOEnqueue(RegisteredWaitHandle registeredWaitHandle) => - ThreadPoolIOEnqueue((IntPtr)registeredWaitHandle.GetHashCode(), IntPtr.Zero, registeredWaitHandle.Repeating); + public void ThreadPoolIOEnqueue(RegisteredWaitHandle registeredWaitHandle) + { + if (IsEnabled(EventLevel.Verbose, Keywords.ThreadingKeyword | Keywords.ThreadTransferKeyword)) + { + ThreadPoolIOEnqueue((IntPtr)registeredWaitHandle.GetHashCode(), IntPtr.Zero, registeredWaitHandle.Repeating); + } + } [Event(64, Level = EventLevel.Verbose, Message = Messages.IO, Task = Tasks.ThreadPool, Opcode = Opcodes.IODequeue, Version = 0, Keywords = Keywords.ThreadingKeyword | Keywords.ThreadTransferKeyword)] private unsafe void ThreadPoolIODequeue( @@ -273,12 +303,22 @@ private unsafe void ThreadPoolIODequeue( // FrameworkEventSource's thread transfer send/receive events instead at callers. [NonEvent] [MethodImpl(MethodImplOptions.NoInlining)] - public void ThreadPoolIODequeue(RegisteredWaitHandle registeredWaitHandle) => - ThreadPoolIODequeue((IntPtr)registeredWaitHandle.GetHashCode(), IntPtr.Zero); + public void ThreadPoolIODequeue(RegisteredWaitHandle registeredWaitHandle) + { + if (IsEnabled(EventLevel.Verbose, Keywords.ThreadingKeyword | Keywords.ThreadTransferKeyword)) + { + ThreadPoolIODequeue((IntPtr)registeredWaitHandle.GetHashCode(), IntPtr.Zero); + } + } [Event(60, Level = EventLevel.Verbose, Message = Messages.WorkingThreadCount, Task = Tasks.ThreadPoolWorkingThreadCount, Opcode = EventOpcode.Start, Version = 0, Keywords = Keywords.ThreadingKeyword)] public unsafe void ThreadPoolWorkingThreadCount(uint Count, ushort ClrInstanceID = DefaultClrInstanceId) { + if (!IsEnabled(EventLevel.Verbose, Keywords.ThreadingKeyword)) + { + return; + } + EventData* data = stackalloc EventData[2]; data[0].DataPointer = (IntPtr)(&Count); data[0].Size = sizeof(uint);