Merge pull request #6516 from benaadams/WorkerThreadStart

WorkerThreadStart volatile read+cmpxchg loop

1 files changed, 55 insertions, 26 deletions

diff --git a/src/vm/win32threadpool.cpp b/src/vm/win32threadpool.cpp
index 2c58710f11..1121417492 100644
--- a/src/vm/win32threadpool.cpp
+++ b/src/vm/win32threadpool.cpp
@@ -85,7 +85,8 @@ SVAL_IMPL(LONG,ThreadpoolMgr,MaxFreeCPThreads);                   // = MaxFreeCP
 
 Volatile<LONG> ThreadpoolMgr::NumCPInfrastructureThreads = 0;      // number of threads currently busy handling draining cycle
 
-SVAL_IMPL(ThreadpoolMgr::ThreadCounter, ThreadpoolMgr, WorkerCounter);
+// Cacheline aligned, hot variable
+DECLSPEC_ALIGN(64) SVAL_IMPL(ThreadpoolMgr::ThreadCounter, ThreadpoolMgr, WorkerCounter);
 
 SVAL_IMPL(LONG,ThreadpoolMgr,MinLimitTotalWorkerThreads);          // = MaxLimitCPThreadsPerCPU * number of CPUS
 SVAL_IMPL(LONG,ThreadpoolMgr,MaxLimitTotalWorkerThreads);        // = MaxLimitCPThreadsPerCPU * number of CPUS
@@ -95,9 +96,11 @@ LONG    ThreadpoolMgr::cpuUtilizationAverage = 0;
 
 HillClimbing ThreadpoolMgr::HillClimbingInstance;
 
-Volatile<LONG> ThreadpoolMgr::PriorCompletedWorkRequests = 0;
-Volatile<DWORD> ThreadpoolMgr::PriorCompletedWorkRequestsTime;
-Volatile<DWORD> ThreadpoolMgr::NextCompletedWorkRequestsTime;
+// Cacheline aligned, 3 hot variables updated in a group
+DECLSPEC_ALIGN(64) LONG ThreadpoolMgr::PriorCompletedWorkRequests = 0;
+DWORD ThreadpoolMgr::PriorCompletedWorkRequestsTime;
+DWORD ThreadpoolMgr::NextCompletedWorkRequestsTime;
+
 LARGE_INTEGER ThreadpoolMgr::CurrentSampleStartTime;
 
 int ThreadpoolMgr::ThreadAdjustmentInterval;
@@ -111,8 +114,12 @@ int ThreadpoolMgr::ThreadAdjustmentInterval;
 #define SUSPEND_TIME GATE_THREAD_DELAY+100      // milliseconds to suspend during SuspendProcessing
 
 LONG ThreadpoolMgr::Initialization=0;           // indicator of whether the threadpool is initialized.
-Volatile<unsigned int> ThreadpoolMgr::LastDequeueTime; // used to determine if work items are getting thread starved
-int ThreadpoolMgr::offset_counter = 0;
+
+// Cacheline aligned, hot variable
+DECLSPEC_ALIGN(64) unsigned int ThreadpoolMgr::LastDequeueTime; // used to determine if work items are getting thread starved
+
+// Move out of from preceeding variables' cache line
+DECLSPEC_ALIGN(64) int ThreadpoolMgr::offset_counter = 0;
 
 SPTR_IMPL(WorkRequest,ThreadpoolMgr,WorkRequestHead);        // Head of work request queue
 SPTR_IMPL(WorkRequest,ThreadpoolMgr,WorkRequestTail);        // Head of work request queue
@@ -135,15 +142,19 @@ CLRSemaphore* ThreadpoolMgr::RetiredWorkerSemaphore;
 CrstStatic ThreadpoolMgr::TimerQueueCriticalSection;
 HANDLE ThreadpoolMgr::TimerThread=NULL;
 Thread *ThreadpoolMgr::pTimerThread=NULL;
-DWORD ThreadpoolMgr::LastTickCount;
+
+// Cacheline aligned, hot variable
+DECLSPEC_ALIGN(64) DWORD ThreadpoolMgr::LastTickCount;
 
 #ifdef _DEBUG
 DWORD ThreadpoolMgr::TickCountAdjustment=0;
 #endif
 
-LONG  ThreadpoolMgr::GateThreadStatus=GATE_THREAD_STATUS_NOT_RUNNING;
+// Cacheline aligned, hot variable
+DECLSPEC_ALIGN(64) LONG  ThreadpoolMgr::GateThreadStatus=GATE_THREAD_STATUS_NOT_RUNNING;
 
-ThreadpoolMgr::RecycledListsWrapper ThreadpoolMgr::RecycledLists;
+// Move out of from preceeding variables' cache line
+DECLSPEC_ALIGN(64) ThreadpoolMgr::RecycledListsWrapper ThreadpoolMgr::RecycledLists;
 
 ThreadpoolMgr::TimerInfo *ThreadpoolMgr::TimerInfosToBeRecycled = NULL;
 
@@ -1189,7 +1200,7 @@ void ThreadpoolMgr::AdjustMaxWorkersActive()
 
     DWORD currentTicks = GetTickCount();
     LONG totalNumCompletions = Thread::GetTotalThreadPoolCompletionCount();
-    LONG numCompletions = totalNumCompletions - PriorCompletedWorkRequests;
+    LONG numCompletions = totalNumCompletions - VolatileLoad(&PriorCompletedWorkRequests);
 
     LARGE_INTEGER startTime = CurrentSampleStartTime;
     LARGE_INTEGER endTime;
@@ -1252,9 +1263,10 @@ void ThreadpoolMgr::AdjustMaxWorkersActive()
         }
 
         PriorCompletedWorkRequests = totalNumCompletions;
+        NextCompletedWorkRequestsTime = currentTicks + ThreadAdjustmentInterval;
+        MemoryBarrier(); // flush previous writes (especially NextCompletedWorkRequestsTime)
         PriorCompletedWorkRequestsTime = currentTicks;
-        NextCompletedWorkRequestsTime = PriorCompletedWorkRequestsTime + ThreadAdjustmentInterval;
-        CurrentSampleStartTime = endTime;
+        CurrentSampleStartTime = endTime;;
     }
 }
 
@@ -1271,7 +1283,8 @@ void ThreadpoolMgr::MaybeAddWorkingWorker()
 
     _ASSERTE(!CLRThreadpoolHosted());
 
-    ThreadCounter::Counts counts = WorkerCounter.GetCleanCounts();
+    // counts volatile read paired with CompareExchangeCounts loop set
+    ThreadCounter::Counts counts = WorkerCounter.DangerousGetDirtyCounts();
     ThreadCounter::Counts newCounts;
     while (true)
     {
@@ -1325,7 +1338,9 @@ void ThreadpoolMgr::MaybeAddWorkingWorker()
             // Of course, there's no guarantee *that* will work - but hopefully enough time will have passed
             // to allow whoever's using all the memory right now to release some.
             //
-            counts = WorkerCounter.GetCleanCounts();
+
+            // counts volatile read paired with CompareExchangeCounts loop set
+            counts = WorkerCounter.DangerousGetDirtyCounts();
             while (true)
             {
                 //
@@ -2286,7 +2301,8 @@ Work:
             #ifdef FEATURE_COMINTEROP
             if (pThread->SetApartment(Thread::AS_InMTA, TRUE) != Thread::AS_InMTA)
             {
-                counts = WorkerCounter.GetCleanCounts();
+                // counts volatile read paired with CompareExchangeCounts loop set
+                counts = WorkerCounter.DangerousGetDirtyCounts();
                 while (true)
                 {
                     newCounts = counts;
@@ -2311,7 +2327,8 @@ Work:
 
     // make sure there's really work.  If not, go back to sleep
 
-    counts = WorkerCounter.GetCleanCounts();
+    // counts volatile read paired with CompareExchangeCounts loop set
+    counts = WorkerCounter.DangerousGetDirtyCounts();
     while (true)
     {
         _ASSERTE(counts.NumActive > 0);
@@ -2403,6 +2420,7 @@ RetryRetire:
         if (WAIT_OBJECT_0 == result)
         {
             foundWork = true;
+
             counts = WorkerCounter.GetCleanCounts();
             FireEtwThreadPoolWorkerThreadRetirementStop(counts.NumActive, counts.NumRetired, GetClrInstanceId());
             goto Work;
@@ -2435,7 +2453,9 @@ RetryRetire:
             // if we don't hit zero, then there's another retired thread that will pick up this signal.  So it's ok
             // to exit.
             //
-            counts = WorkerCounter.GetCleanCounts();
+
+            // counts volatile read paired with CompareExchangeCounts loop set
+            counts = WorkerCounter.DangerousGetDirtyCounts();
             while (true)
             {
                 if (counts.NumRetired == 0)
@@ -2491,7 +2511,8 @@ RetryWaitForWork:
 
             DangerousNonHostedSpinLockHolder tal(&ThreadAdjustmentLock);
 
-            counts = WorkerCounter.GetCleanCounts();
+            // counts volatile read paired with CompareExchangeCounts loop set
+            counts = WorkerCounter.DangerousGetDirtyCounts();
             while (true)
             {
                 if (counts.NumActive == counts.NumWorking)
@@ -3830,7 +3851,9 @@ Top:
             // and the state of the rest of the IOCP threads, we need to figure out whether to de-activate (exit) this thread, retire this thread,
             // or transition to "working."
             //
-            oldCounts = CPThreadCounter.GetCleanCounts();
+
+            // counts volatile read paired with CompareExchangeCounts loop set
+            oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
             newCounts = oldCounts;
             enterRetirement = false;
 
@@ -3913,7 +3936,8 @@ Top:
                             // We can now exit; decrement the retired count.
                             while (true)
                             {
-                                oldCounts = CPThreadCounter.GetCleanCounts();
+                                // counts volatile read paired with CompareExchangeCounts loop set
+                                oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                                 newCounts = oldCounts;
                                 newCounts.NumRetired--;
                                 if (oldCounts == CPThreadCounter.CompareExchangeCounts(newCounts, oldCounts))
@@ -3927,7 +3951,8 @@ Top:
                         // put back into rotation -- we need a thread
                         while (true)
                         {
-                            oldCounts = CPThreadCounter.GetCleanCounts();
+                            // counts volatile read paired with CompareExchangeCounts loop set
+                            oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                             newCounts = oldCounts;
                             newCounts.NumRetired--;
                             newCounts.NumActive++;
@@ -3968,7 +3993,8 @@ Top:
                     //GC event, and some have not.
                     while (true)
                     {
-                        oldCounts = CPThreadCounter.GetCleanCounts();
+                        // counts volatile read paired with CompareExchangeCounts loop set
+                        oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                         newCounts = oldCounts;
                         newCounts.NumWorking--;
                         if (oldCounts == CPThreadCounter.CompareExchangeCounts(newCounts, oldCounts))
@@ -3981,7 +4007,8 @@ Top:
 
                     while (true)
                     {
-                        oldCounts = CPThreadCounter.GetCleanCounts();
+                        // counts volatile read paired with CompareExchangeCounts loop set
+                        oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                         newCounts = oldCounts;
                         newCounts.NumWorking++;
                         if (oldCounts == CPThreadCounter.CompareExchangeCounts(newCounts, oldCounts))
@@ -4256,7 +4283,8 @@ void ThreadpoolMgr::GrowCompletionPortThreadpoolIfNeeded()
                         // if thread creation failed, we have to adjust the counts back down.
                         while (true)
                         {
-                            oldCounts = CPThreadCounter.GetCleanCounts();
+                            // counts volatile read paired with CompareExchangeCounts loop set
+                            oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                             newCounts = oldCounts;
                             newCounts.NumActive--;
                             newCounts.NumWorking--;
@@ -4686,7 +4714,8 @@ DWORD __stdcall ThreadpoolMgr::GateThreadStart(LPVOID lpArgs)
                     // IOCP threads are created as "active" and "working"
                     while (true)
                     {
-                        oldCounts = CPThreadCounter.GetCleanCounts();
+                        // counts volatile read paired with CompareExchangeCounts loop set
+                        oldCounts = CPThreadCounter.DangerousGetDirtyCounts();
                         newCounts = oldCounts;
                         newCounts.NumActive++;
                         newCounts.NumWorking++;
@@ -4794,7 +4823,7 @@ BOOL ThreadpoolMgr::SufficientDelaySinceLastDequeue()
 
     #define DEQUEUE_DELAY_THRESHOLD (GATE_THREAD_DELAY * 2)
 
-    unsigned delay = GetTickCount() - LastDequeueTime;
+    unsigned delay = GetTickCount() - VolatileLoad(&LastDequeueTime);
     unsigned tooLong;
 
     if(cpuUtilization < CpuUtilizationLow)