Reduce Timer lock contention

Timer currently uses a global lock to protect a single "queue" of Timers, and any operation to create/change/fire/delete a Timer takes that same lock.  This leads to a scalability problem for code that operates on lots of timers.

This change partitions this single queue into N queues, which all operate independently.  In the .NET Framework implementation, there's already logically such a split with one queue/lock per AppDomain, and this change utilizes the same underlying support in the runtime.  As such, contention is distributed across the N locks, helping scalability.

diff --git a/src/mscorlib/src/System/Threading/Timer.cs b/src/mscorlib/src/System/Threading/Timer.cs
index 918cbd8..d46bf6b 100644 (file)
--- a/src/mscorlib/src/System/Threading/Timer.cs
+++ b/src/mscorlib/src/System/Threading/Timer.cs
@@ -40,23 +40,27 @@ namespace System.Threading
     // The data structure we've chosen is an unordered doubly-linked list of active timers.  This gives O(1) insertion
     // and removal, and O(N) traversal when finding expired timers.
     //
-    // Note that all instance methods of this class require that the caller hold a lock on TimerQueue.Instance.
+    // Note that all instance methods of this class require that the caller hold a lock on the TimerQueue instance.
     //
     internal class TimerQueue
     {
-        #region singleton pattern implementation
+        #region Shared TimerQueue instances
 
-        // The one-and-only TimerQueue for the AppDomain.
-        private static TimerQueue s_queue = new TimerQueue();
+        public static TimerQueue[] Instances { get; } = CreateTimerQueues();
 
-        public static TimerQueue Instance
+        private TimerQueue(int id)
         {
-            get { return s_queue; }
+            m_id = id;
         }
 
-        private TimerQueue()
+        private static TimerQueue[] CreateTimerQueues()
         {
-            // empty private constructor to ensure we remain a singleton.
+            var queues = new TimerQueue[Environment.ProcessorCount];
+            for (int i = 0; i < queues.Length; i++)
+            {
+                queues[i] = new TimerQueue(i);
+            }
+            return queues;
         }
 
         #endregion
@@ -112,6 +116,7 @@ namespace System.Threading
             }
         }
 
+        private readonly int m_id; // TimerQueues[m_id] == this
         private AppDomainTimerSafeHandle m_appDomainTimer;
 
         private bool m_isAppDomainTimerScheduled;
@@ -154,8 +159,9 @@ namespace System.Threading
             if (m_appDomainTimer == null || m_appDomainTimer.IsInvalid)
             {
                 Debug.Assert(!m_isAppDomainTimerScheduled);
+                Debug.Assert(m_id >= 0 && m_id < Instances.Length && this == Instances[m_id]);
 
-                m_appDomainTimer = CreateAppDomainTimer(actualDuration);
+                m_appDomainTimer = CreateAppDomainTimer(actualDuration, m_id);
                 if (!m_appDomainTimer.IsInvalid)
                 {
                     m_isAppDomainTimerScheduled = true;
@@ -185,16 +191,17 @@ namespace System.Threading
         }
 
         //
-        // The VM calls this when the native timer fires.
+        // The VM calls this when a native timer fires.
         //
-        internal static void AppDomainTimerCallback()
+        internal static void AppDomainTimerCallback(int id)
         {
-            Instance.FireNextTimers();
+            Debug.Assert(id >= 0 && id < Instances.Length && Instances[id].m_id == id);
+            Instances[id].FireNextTimers();
         }
 
         [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
         [SuppressUnmanagedCodeSecurity]
-        private static extern AppDomainTimerSafeHandle CreateAppDomainTimer(uint dueTime);
+        private static extern AppDomainTimerSafeHandle CreateAppDomainTimer(uint dueTime, int id);
 
         [DllImport(JitHelpers.QCall, CharSet = CharSet.Unicode)]
         [SuppressUnmanagedCodeSecurity]
@@ -219,6 +226,9 @@ namespace System.Threading
 
         //
         // Fire any timers that have expired, and update the native timer to schedule the rest of them.
+        // We're in a thread pool work item here, and if there are multiple timers to be fired, we want
+        // to queue all but the first one.  The first may can then be invoked synchronously or queued,
+        // a task left up to our caller, which might be firing timers from multiple queues.
         //
         private void FireNextTimers()
         {
@@ -400,6 +410,11 @@ namespace System.Threading
     internal sealed class TimerQueueTimer
     {
         //
+        // The associated timer queue.
+        //
+        private readonly TimerQueue m_associatedTimerQueue;
+
+        //
         // All fields of this class are protected by a lock on TimerQueue.Instance.
         //
         // The first four fields are maintained by TimerQueue itself.
@@ -449,6 +464,7 @@ namespace System.Threading
             m_dueTime = Timeout.UnsignedInfinite;
             m_period = Timeout.UnsignedInfinite;
             m_executionContext = ExecutionContext.Capture();
+            m_associatedTimerQueue = TimerQueue.Instances[Environment.CurrentExecutionId % TimerQueue.Instances.Length];
 
             //
             // After the following statement, the timer may fire.  No more manipulation of timer state outside of
@@ -458,12 +474,11 @@ namespace System.Threading
                 Change(dueTime, period);
         }
 
-
         internal bool Change(uint dueTime, uint period)
         {
             bool success;
 
-            lock (TimerQueue.Instance)
+            lock (m_associatedTimerQueue)
             {
                 if (m_canceled)
                     throw new ObjectDisposedException(null, SR.ObjectDisposed_Generic);
@@ -476,7 +491,7 @@ namespace System.Threading
 
                     if (dueTime == Timeout.UnsignedInfinite)
                     {
-                        TimerQueue.Instance.DeleteTimer(this);
+                        m_associatedTimerQueue.DeleteTimer(this);
                         success = true;
                     }
                     else
@@ -484,7 +499,7 @@ namespace System.Threading
                         if (FrameworkEventSource.IsInitialized && FrameworkEventSource.Log.IsEnabled(EventLevel.Informational, FrameworkEventSource.Keywords.ThreadTransfer))
                             FrameworkEventSource.Log.ThreadTransferSendObj(this, 1, string.Empty, true);
 
-                        success = TimerQueue.Instance.UpdateTimer(this, dueTime, period);
+                        success = m_associatedTimerQueue.UpdateTimer(this, dueTime, period);
                     }
                 }
             }
@@ -495,7 +510,7 @@ namespace System.Threading
 
         public void Close()
         {
-            lock (TimerQueue.Instance)
+            lock (m_associatedTimerQueue)
             {
                 // prevent ThreadAbort while updating state
                 try { }
@@ -504,7 +519,7 @@ namespace System.Threading
                     if (!m_canceled)
                     {
                         m_canceled = true;
-                        TimerQueue.Instance.DeleteTimer(this);
+                        m_associatedTimerQueue.DeleteTimer(this);
                     }
                 }
             }
@@ -516,7 +531,7 @@ namespace System.Threading
             bool success;
             bool shouldSignal = false;
 
-            lock (TimerQueue.Instance)
+            lock (m_associatedTimerQueue)
             {
                 // prevent ThreadAbort while updating state
                 try { }
@@ -530,7 +545,7 @@ namespace System.Threading
                     {
                         m_canceled = true;
                         m_notifyWhenNoCallbacksRunning = toSignal;
-                        TimerQueue.Instance.DeleteTimer(this);
+                        m_associatedTimerQueue.DeleteTimer(this);
 
                         if (m_callbacksRunning == 0)
                             shouldSignal = true;
@@ -551,7 +566,7 @@ namespace System.Threading
         {
             bool canceled = false;
 
-            lock (TimerQueue.Instance)
+            lock (m_associatedTimerQueue)
             {
                 // prevent ThreadAbort while updating state
                 try { }
@@ -569,7 +584,7 @@ namespace System.Threading
             CallCallback();
 
             bool shouldSignal = false;
-            lock (TimerQueue.Instance)
+            lock (m_associatedTimerQueue)
             {
                 // prevent ThreadAbort while updating state
                 try { }

diff --git a/src/vm/comthreadpool.cpp b/src/vm/comthreadpool.cpp
index c49f834..08c056b 100644 (file)
--- a/src/vm/comthreadpool.cpp
+++ b/src/vm/comthreadpool.cpp
@@ -100,9 +100,6 @@ DelegateInfo *DelegateInfo::MakeDelegateInfo(AppDomain *pAppDomain,
         INJECT_FAULT(COMPlusThrowOM());
     }
     CONTRACTL_END;
-
-    // If there were any DelegateInfos waiting to be released, they'll get flushed now
-    ThreadpoolMgr::FlushQueueOfTimerInfos();
     
     DelegateInfoHolder delegateInfo = (DelegateInfo*) ThreadpoolMgr::GetRecycledMemory(ThreadpoolMgr::MEMTYPE_DelegateInfo);
     
@@ -818,10 +815,12 @@ void AppDomainTimerCallback_Worker(LPVOID ptr)
     LogCall(pMeth,"AppDomainTimerCallback");
 #endif
 
-    MethodDescCallSite(METHOD__TIMER_QUEUE__APPDOMAIN_TIMER_CALLBACK).Call(NULL);
+    ThreadpoolMgr::TimerInfoContext* pTimerInfoContext = (ThreadpoolMgr::TimerInfoContext*)ptr;
+    ARG_SLOT args[] = { PtrToArgSlot(pTimerInfoContext->TimerId) };
+    MethodDescCallSite(METHOD__TIMER_QUEUE__APPDOMAIN_TIMER_CALLBACK).Call(args);
 }
 
-VOID WINAPI AppDomainTimerCallback(PVOID delegateInfo, BOOLEAN timerOrWaitFired)
+VOID WINAPI AppDomainTimerCallback(PVOID callbackState, BOOLEAN timerOrWaitFired)
 {
     Thread* pThread = GetThread();
     if (pThread == NULL)
@@ -840,8 +839,6 @@ VOID WINAPI AppDomainTimerCallback(PVOID delegateInfo, BOOLEAN timerOrWaitFired)
         MODE_ANY;
         GC_TRIGGERS;
         SO_INTOLERANT;
-        
-        PRECONDITION(CheckPointer(delegateInfo));
     }
     CONTRACTL_END;
 
@@ -850,13 +847,14 @@ VOID WINAPI AppDomainTimerCallback(PVOID delegateInfo, BOOLEAN timerOrWaitFired)
 
     GCX_COOP();
 
-    ManagedThreadBase::ThreadPool(((DelegateInfo*)delegateInfo)->m_appDomainId, AppDomainTimerCallback_Worker, NULL);
+    ThreadpoolMgr::TimerInfoContext* pTimerInfoContext = (ThreadpoolMgr::TimerInfoContext*)callbackState;
+    ManagedThreadBase::ThreadPool(pTimerInfoContext->AppDomainId, AppDomainTimerCallback_Worker, pTimerInfoContext);
 
     // We should have released all locks.
     _ASSERTE(g_fEEShutDown || pThread->m_dwLockCount == 0 || pThread->m_fRudeAborted);
 }
 
-HANDLE QCALLTYPE AppDomainTimerNative::CreateAppDomainTimer(INT32 dueTime)
+HANDLE QCALLTYPE AppDomainTimerNative::CreateAppDomainTimer(INT32 dueTime, INT32 timerId)
 {
     QCALL_CONTRACT;
 
@@ -864,35 +862,37 @@ HANDLE QCALLTYPE AppDomainTimerNative::CreateAppDomainTimer(INT32 dueTime)
     BEGIN_QCALL;
 
     _ASSERTE(dueTime >= 0);
+    _ASSERTE(timerId >= 0);
 
     AppDomain* pAppDomain = GetThread()->GetDomain();
     ADID adid = pAppDomain->GetId();
 
-    DelegateInfoHolder delegateInfo = DelegateInfo::MakeDelegateInfo(
-        pAppDomain,
-        NULL,
-        NULL,
-        NULL);
+    ThreadpoolMgr::TimerInfoContext* timerContext = new (nothrow) ThreadpoolMgr::TimerInfoContext();
+    if (timerContext == NULL)
+    {
+        COMPlusThrowOM();
+    }
+
+    timerContext->AppDomainId = adid;
+    timerContext->TimerId = timerId;
 
     BOOL res = ThreadpoolMgr::CreateTimerQueueTimer(
         &hTimer,
         (WAITORTIMERCALLBACK)AppDomainTimerCallback,
-        (PVOID)delegateInfo,
+        (PVOID)timerContext,
         (ULONG)dueTime,
         (ULONG)-1 /* this timer doesn't repeat */,
         0 /* no flags */);
 
     if (!res)
     {
+        delete timerContext;
+
         if (GetLastError() == ERROR_CALL_NOT_IMPLEMENTED)
             COMPlusThrow(kNotSupportedException);
         else
             COMPlusThrowWin32();
     }
-    else
-    {
-        delegateInfo.SuppressRelease();
-    }
 
     END_QCALL;
     return hTimer;

diff --git a/src/vm/comthreadpool.h b/src/vm/comthreadpool.h
index b949885..3e01717 100644 (file)
--- a/src/vm/comthreadpool.h
+++ b/src/vm/comthreadpool.h
@@ -60,7 +60,7 @@ public:
 class AppDomainTimerNative
 {
 public:
-    static HANDLE QCALLTYPE CreateAppDomainTimer(INT32 dueTime);
+    static HANDLE QCALLTYPE CreateAppDomainTimer(INT32 dueTime, INT32 timerId);
     static BOOL QCALLTYPE ChangeAppDomainTimer(HANDLE hTimer, INT32 dueTime);
     static BOOL QCALLTYPE DeleteAppDomainTimer(HANDLE hTimer);
 };

diff --git a/src/vm/mscorlib.h b/src/vm/mscorlib.h
index 4942e0d..88cafea 100644 (file)
--- a/src/vm/mscorlib.h
+++ b/src/vm/mscorlib.h
@@ -933,7 +933,7 @@ DEFINE_CLASS(TP_WAIT_CALLBACK,         Threading,              _ThreadPoolWaitCa
 DEFINE_METHOD(TP_WAIT_CALLBACK,        PERFORM_WAIT_CALLBACK,               PerformWaitCallback,                   SM_RetBool)
 
 DEFINE_CLASS(TIMER_QUEUE,           Threading,                TimerQueue)
-DEFINE_METHOD(TIMER_QUEUE,          APPDOMAIN_TIMER_CALLBACK, AppDomainTimerCallback,   SM_RetVoid)
+DEFINE_METHOD(TIMER_QUEUE,          APPDOMAIN_TIMER_CALLBACK, AppDomainTimerCallback,   SM_Int_RetVoid)
 
 DEFINE_CLASS(TIMESPAN,              System,                 TimeSpan)
 

diff --git a/src/vm/win32threadpool.cpp b/src/vm/win32threadpool.cpp
index 97c020a..a73328b 100644 (file)
--- a/src/vm/win32threadpool.cpp
+++ b/src/vm/win32threadpool.cpp
@@ -4962,9 +4962,7 @@ void ThreadpoolMgr::DeleteTimer(TimerInfo* timerInfo)
     if (timerInfo->Context != NULL)
     {
         GCX_COOP();
-        DelegateInfo *pDelInfo = (DelegateInfo *)timerInfo->Context;
-        pDelInfo->Release();
-        RecycleMemory( pDelInfo, MEMTYPE_DelegateInfo );
+        delete (ThreadpoolMgr::TimerInfoContext*)timerInfo->Context;
     }
 
     if (timerInfo->ExternalEventSafeHandle != NULL)
@@ -4978,7 +4976,7 @@ void ThreadpoolMgr::DeleteTimer(TimerInfo* timerInfo)
 
 // We add TimerInfos from deleted timers into a linked list.
 // A worker thread will later release the handles held by the TimerInfo
-// and recycle them if possible (See DelegateInfo::MakeDelegateInfo)
+// and recycle them if possible.
 void ThreadpoolMgr::QueueTimerInfoForRelease(TimerInfo *pTimerInfo)
 {
     CONTRACTL
@@ -5048,10 +5046,7 @@ void ThreadpoolMgr::FlushQueueOfTimerInfos()
         GCX_COOP();
         if (pCurrTimerInfo->Context != NULL)
         {
-            DelegateInfo *pCurrDelInfo = (DelegateInfo *) pCurrTimerInfo->Context;
-            pCurrDelInfo->Release();
-
-            RecycleMemory( pCurrDelInfo, MEMTYPE_DelegateInfo );
+            delete (ThreadpoolMgr::TimerInfoContext*)pCurrTimerInfo->Context;
         }
 
         if (pCurrTimerInfo->ExternalEventSafeHandle != NULL)

diff --git a/src/vm/win32threadpool.h b/src/vm/win32threadpool.h
index 764c65e..fd5b98f 100644 (file)
--- a/src/vm/win32threadpool.h
+++ b/src/vm/win32threadpool.h
@@ -220,6 +220,11 @@ public:
         MEMTYPE_COUNT           = 3,
     };
 
+    typedef struct {
+        ADID AppDomainId;
+        INT32 TimerId;
+    } TimerInfoContext;
+
     static BOOL Initialize();
 
     static BOOL SetMaxThreadsHelper(DWORD MaxWorkerThreads,