Remove some unnecessary spinning (#21437)
authorStephen Toub <stoub@microsoft.com>
Sat, 8 Dec 2018 02:14:44 +0000 (21:14 -0500)
committerJan Kotas <jkotas@microsoft.com>
Sat, 8 Dec 2018 02:14:44 +0000 (18:14 -0800)
Most of the use of SpinWait in CoreLib involves waiting for some short-lived operation to complete on another thread, in which case the spinning thread should backoff as it's unable to make forward progress until the other operation completes.  In a few cases, however, SpinWait is being used just around CompareExchange operations, such that at least one thread running this code path is guaranteed to make forward progress, and the backoff in the spinning doesn't actually help (in theory it could help to reduce contention if lots of threads were all trying to CompareExchange concurrently, but in such cases you'd actually want more randomized backoff, as otherwise it's likely all the threads would re-attempt at around the same time and similarly re-encounter contention).

src/System.Private.CoreLib/shared/System/Collections/Concurrent/ConcurrentQueueSegment.cs
src/System.Private.CoreLib/src/System/Threading/Tasks/Task.cs

index c706fae..85224a5 100644 (file)
@@ -112,15 +112,15 @@ namespace System.Collections.Concurrent
                 _frozenForEnqueues = true;
 
                 // Increase the tail by FreezeOffset, spinning until we're successful in doing so.
-                var spinner = new SpinWait();
+                int tail = _headAndTail.Tail;
                 while (true)
                 {
-                    int tail = Volatile.Read(ref _headAndTail.Tail);
-                    if (Interlocked.CompareExchange(ref _headAndTail.Tail, tail + FreezeOffset, tail) == tail)
+                    int oldTail = Interlocked.CompareExchange(ref _headAndTail.Tail, tail + FreezeOffset, tail);
+                    if (oldTail == tail)
                     {
                         break;
                     }
-                    spinner.SpinOnce();
+                    tail = oldTail;
                 }
             }
         }
index 4b9ccc6..b9b60dd 100644 (file)
@@ -721,31 +721,31 @@ namespace System.Threading.Tasks
 
         private bool AtomicStateUpdateSlow(int newBits, int illegalBits)
         {
-            var sw = new SpinWait();
+            int flags = m_stateFlags;
             do
             {
-                int oldFlags = m_stateFlags;
-                if ((oldFlags & illegalBits) != 0) return false;
-                if (Interlocked.CompareExchange(ref m_stateFlags, oldFlags | newBits, oldFlags) == oldFlags)
+                if ((flags & illegalBits) != 0) return false;
+                int oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags | newBits, flags);
+                if (oldFlags == flags)
                 {
                     return true;
                 }
-                sw.SpinOnce();
+                flags = oldFlags;
             } while (true);
         }
 
         internal bool AtomicStateUpdate(int newBits, int illegalBits, ref int oldFlags)
         {
-            SpinWait sw = new SpinWait();
+            int flags = oldFlags = m_stateFlags;
             do
             {
-                oldFlags = m_stateFlags;
-                if ((oldFlags & illegalBits) != 0) return false;
-                if (Interlocked.CompareExchange(ref m_stateFlags, oldFlags | newBits, oldFlags) == oldFlags)
+                if ((flags & illegalBits) != 0) return false;
+                oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags | newBits, flags);
+                if (oldFlags == flags)
                 {
                     return true;
                 }
-                sw.SpinOnce();
+                flags = oldFlags;
             } while (true);
         }
 
@@ -772,13 +772,12 @@ namespace System.Threading.Tasks
             else
             {
                 // Atomically clear the END_AWAIT_NOTIFICATION bit
-                SpinWait sw = new SpinWait();
+                int flags = m_stateFlags;
                 while (true)
                 {
-                    int oldFlags = m_stateFlags;
-                    int newFlags = oldFlags & (~TASK_STATE_WAIT_COMPLETION_NOTIFICATION);
-                    if (Interlocked.CompareExchange(ref m_stateFlags, newFlags, oldFlags) == oldFlags) break;
-                    sw.SpinOnce();
+                    int oldFlags = Interlocked.CompareExchange(ref m_stateFlags, flags & (~TASK_STATE_WAIT_COMPLETION_NOTIFICATION), flags);
+                    if (oldFlags == flags) break;
+                    flags = oldFlags;
                 }
             }
         }