Move thread pool initialization to native side (#36789)

Move thread pool initialization to native side

- Following up from https://github.com/dotnet/runtime/pull/36697. Currently the initialization occurs when a static variable is accessed before calling into the VM, and after thinking about it some more the intent and need to initialize the thread pool can be a bit unclear
- The initialization is specific to this implementation and the managed side doesn't necessarily need to know about it
- Moved the initialization to the native side similarly to other thread pool APIs
- Added some more assertions to relevant managed-to-native entry point paths to ensure that they are not called before the thread pool is initialized
- This adds a non-volatile check when requesting a worker thread, which is already a very slow path. I ran into some issues with testing on the arm64 machine with updated locally built libcoreclr.so, so wasn't able to test that. I looked at the baseline profile and looking at time spent exclusively in ThreadPoolNative::RequestWorkerThread() and what it already does, I don't think the change would result in any significant perf difference.

diff --git a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
index 719576c..8f8b1b7 100644 (file)
--- a/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
+++ b/src/coreclr/src/System.Private.CoreLib/src/System/Threading/ThreadPool.CoreCLR.cs
@@ -192,12 +192,7 @@ namespace System.Threading
         // Time in ms for which ThreadPoolWorkQueue.Dispatch keeps executing work items before returning to the OS
         private const uint DispatchQuantum = 30;
 
-        private static bool GetEnableWorkerTracking()
-        {
-            bool enableWorkerTracking = false;
-            InitializeVMTp(ref enableWorkerTracking);
-            return enableWorkerTracking;
-        }
+        internal static readonly bool EnableWorkerTracking = GetEnableWorkerTracking();
 
         internal static bool KeepDispatching(int startTickCount)
         {
@@ -340,8 +335,8 @@ namespace System.Threading
         [MethodImpl(MethodImplOptions.InternalCall)]
         internal static extern void NotifyWorkItemProgressNative();
 
-        [DllImport(RuntimeHelpers.QCall, CharSet = CharSet.Unicode)]
-        private static extern void InitializeVMTp(ref bool enableWorkerTracking);
+        [MethodImpl(MethodImplOptions.InternalCall)]
+        private static extern bool GetEnableWorkerTracking();
 
         [MethodImpl(MethodImplOptions.InternalCall)]
         private static extern IntPtr RegisterWaitForSingleObjectNative(

diff --git a/src/coreclr/src/vm/comthreadpool.cpp b/src/coreclr/src/vm/comthreadpool.cpp
index a87d954..9a0ab92 100644 (file)
--- a/src/coreclr/src/vm/comthreadpool.cpp
+++ b/src/coreclr/src/vm/comthreadpool.cpp
@@ -217,6 +217,7 @@ FCIMPLEND
 FCIMPL0(VOID, ThreadPoolNative::NotifyRequestProgress)
 {
     FCALL_CONTRACT;
+    _ASSERTE(ThreadpoolMgr::IsInitialized()); // can't be here without requesting a thread first
 
     ThreadpoolMgr::NotifyWorkItemCompleted();
 
@@ -247,6 +248,7 @@ FCIMPLEND
 FCIMPL0(FC_BOOL_RET, ThreadPoolNative::NotifyRequestComplete)
 {
     FCALL_CONTRACT;
+    _ASSERTE(ThreadpoolMgr::IsInitialized()); // can't be here without requesting a thread first
 
     ThreadpoolMgr::NotifyWorkItemCompleted();
 
@@ -305,15 +307,14 @@ FCIMPLEND
 
 /*****************************************************************************************************/
 
-void QCALLTYPE ThreadPoolNative::InitializeVMTp(CLR_BOOL* pEnableWorkerTracking)
+FCIMPL0(FC_BOOL_RET, ThreadPoolNative::GetEnableWorkerTracking)
 {
-    QCALL_CONTRACT;
+    FCALL_CONTRACT;
 
-    BEGIN_QCALL;
-    ThreadpoolMgr::EnsureInitialized();
-    *pEnableWorkerTracking = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_ThreadPool_EnableWorkerTracking) ? TRUE : FALSE;
-    END_QCALL;
+    BOOL result = CLRConfig::GetConfigValue(CLRConfig::INTERNAL_ThreadPool_EnableWorkerTracking) ? TRUE : FALSE;
+    FC_RETURN_BOOL(result);
 }
+FCIMPLEND
 
 /*****************************************************************************************************/
 
@@ -473,6 +474,7 @@ BOOL QCALLTYPE ThreadPoolNative::RequestWorkerThread()
 
     BEGIN_QCALL;
 
+    ThreadpoolMgr::EnsureInitialized();
     ThreadpoolMgr::SetAppDomainRequestsActive();
 
     res = ThreadpoolMgr::QueueUserWorkItem(NULL,

diff --git a/src/coreclr/src/vm/comthreadpool.h b/src/coreclr/src/vm/comthreadpool.h
index 23426fa..ceef964 100644 (file)
--- a/src/coreclr/src/vm/comthreadpool.h
+++ b/src/coreclr/src/vm/comthreadpool.h
@@ -35,7 +35,7 @@ public:
     static FCDECL0(VOID, NotifyRequestProgress);
     static FCDECL0(FC_BOOL_RET, NotifyRequestComplete);
 
-    static void QCALLTYPE InitializeVMTp(CLR_BOOL* pEnableWorkerTracking);
+    static FCDECL0(FC_BOOL_RET, GetEnableWorkerTracking);
 
     static FCDECL1(void, ReportThreadStatus, CLR_BOOL isWorking);
 

diff --git a/src/coreclr/src/vm/ecalllist.h b/src/coreclr/src/vm/ecalllist.h
index 243a735..955bf98 100644 (file)
--- a/src/coreclr/src/vm/ecalllist.h
+++ b/src/coreclr/src/vm/ecalllist.h
@@ -657,7 +657,7 @@ FCFuncStart(gThreadPoolFuncs)
     FCFuncElement("GetMaxThreadsNative", ThreadPoolNative::CorGetMaxThreads)
     FCFuncElement("NotifyWorkItemComplete", ThreadPoolNative::NotifyRequestComplete)
     FCFuncElement("NotifyWorkItemProgressNative", ThreadPoolNative::NotifyRequestProgress)
-    QCFuncElement("InitializeVMTp", ThreadPoolNative::InitializeVMTp)
+    FCFuncElement("GetEnableWorkerTracking", ThreadPoolNative::GetEnableWorkerTracking)
     FCFuncElement("ReportThreadStatus", ThreadPoolNative::ReportThreadStatus)
     QCFuncElement("RequestWorkerThread", ThreadPoolNative::RequestWorkerThread)
 FCFuncEnd()

diff --git a/src/coreclr/src/vm/win32threadpool.cpp b/src/coreclr/src/vm/win32threadpool.cpp
index b7106cc..6afe8ba 100644 (file)
--- a/src/coreclr/src/vm/win32threadpool.cpp
+++ b/src/coreclr/src/vm/win32threadpool.cpp
@@ -235,7 +235,7 @@ void ThreadpoolMgr::EnsureInitialized()
 {
     CONTRACTL
     {
-        THROWS;         // Initialize can throw
+        THROWS;         // EnsureInitializedSlow can throw
         MODE_ANY;
         GC_NOTRIGGER;
     }
@@ -244,6 +244,19 @@ void ThreadpoolMgr::EnsureInitialized()
     if (IsInitialized())
         return;
 
+    EnsureInitializedSlow();
+}
+
+NOINLINE void ThreadpoolMgr::EnsureInitializedSlow()
+{
+    CONTRACTL
+    {
+        THROWS;         // Initialize can throw
+        MODE_ANY;
+        GC_NOTRIGGER;
+    }
+    CONTRACTL_END;
+
     DWORD dwSwitchCount = 0;
 
 retry:
@@ -758,7 +771,10 @@ void ThreadpoolMgr::ReportThreadStatus(bool isWorking)
         MODE_ANY;
     }
     CONTRACTL_END;
+
+    _ASSERTE(IsInitialized()); // can't be here without requesting a thread first
     _ASSERTE(CLRConfig::GetConfigValue(CLRConfig::INTERNAL_ThreadPool_EnableWorkerTracking));
+
     while (true)
     {
         WorkingThreadCounts currentCounts, newCounts;
@@ -3069,6 +3085,7 @@ void ThreadpoolMgr::DeregisterWait(WaitInfo* pArgs)
 void ThreadpoolMgr::WaitHandleCleanup(HANDLE hWaitObject)
 {
     LIMITED_METHOD_CONTRACT;
+    _ASSERTE(IsInitialized()); // cannot call cleanup before first registering
 
     WaitInfo* waitInfo = (WaitInfo*) hWaitObject;
     _ASSERTE(waitInfo->refCount > 0);

diff --git a/src/coreclr/src/vm/win32threadpool.h b/src/coreclr/src/vm/win32threadpool.h
index fb0804c..36ae2d3 100644 (file)
--- a/src/coreclr/src/vm/win32threadpool.h
+++ b/src/coreclr/src/vm/win32threadpool.h
@@ -824,7 +824,12 @@ public:
         return entry;
     }
 
+public:
     static void EnsureInitialized();
+private:
+    static void EnsureInitializedSlow();
+
+public:
     static void InitPlatformVariables();
 
     inline static BOOL IsInitialized()

diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs
index ed0bf45..2a68a0b 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.Portable.cs
@@ -317,6 +317,8 @@ namespace System.Threading
 
     public static partial class ThreadPool
     {
+        internal const bool EnableWorkerTracking = false;
+
         internal static void InitializeForThreadPoolThread() { }
 
         public static bool SetMaxThreads(int workerThreads, int completionPortThreads)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
index de78cc1..6d06533 100644 (file)
--- a/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Threading/ThreadPool.cs
@@ -607,7 +607,8 @@ namespace System.Threading
                     //
                     // Execute the workitem outside of any finally blocks, so that it can be aborted if needed.
                     //
-                    if (ThreadPool.s_enableWorkerTracking)
+#pragma warning disable CS0162 // Unreachable code detected. EnableWorkerTracking may be constant false in some runtimes.
+                    if (ThreadPool.EnableWorkerTracking)
                     {
                         bool reportedStatus = false;
                         try
@@ -630,6 +631,7 @@ namespace System.Threading
                                 ThreadPool.ReportThreadStatus(isWorking: false);
                         }
                     }
+#pragma warning restore CS0162
                     else if (workItem is Task task)
                     {
                         // Check for Task first as it's currently faster to type check
@@ -935,7 +937,6 @@ namespace System.Threading
     public static partial class ThreadPool
     {
         internal static readonly ThreadPoolWorkQueue s_workQueue = new ThreadPoolWorkQueue();
-        internal static readonly bool s_enableWorkerTracking = GetEnableWorkerTracking();
 
         /// <summary>Shim used to invoke <see cref="IAsyncStateMachineBox.MoveNext"/> of the supplied <see cref="IAsyncStateMachineBox"/>.</summary>
         internal static readonly Action<object?> s_invokeAsyncStateMachineBox = state =>

diff --git a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/ThreadPool.Mono.cs b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/ThreadPool.Mono.cs
index ae757df..4708d29 100644 (file)
--- a/src/mono/netcore/System.Private.CoreLib/src/System/Threading/ThreadPool.Mono.cs
+++ b/src/mono/netcore/System.Private.CoreLib/src/System/Threading/ThreadPool.Mono.cs
@@ -8,8 +8,6 @@ namespace System.Threading
 {
     public static partial class ThreadPool
     {
-        private static bool GetEnableWorkerTracking() => false;
-
         internal static void ReportThreadStatus(bool isWorking)
         {
         }