[release/11.0] Increase spinning/polling aggressiveness in the thread pool in low-saturation scenarios (#132908)

Backport of #132765 to release/11.0

/cc @VSadov

## Customer Impact

- [ ] Customer reported
- [x] Found internally

Too aggressive parking of threadpool threads in response to spurious
wake ups may result in having too few active workers in bursty low
saturation scenarios and cause massive regressions.

Some tests like Websockets show nearly 4x less RPS compared to net10.

## Regression

- [x] Yes
- [ ] No

Introduced in #128606
The change resultied in improvements in high saturation
throughput-sensitive scenarios. It was later discovered that in low
staturation latency-sensitive scenarios it resulted in regressions.

## Testing

The fix was validated with various benchmarks both high-saturation and
low, on both x64 and arm.

- Websockets
- JSON (high, low, mid connection and client thread count)
- Orchard
- Nuget restore RPS tests

## Risk

Low. 

This is an intentionally small change. The new parking policy will only
be enabled when the threadpool has more than 2/3 proc count workers.

Co-authored-by: Vladimir Sadov <vsadov@microsoft.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

Author: 3 people

src/libraries/System.Private.CoreLib/src/System/Threading/Backoff.cs

@@ -15,8 +15,8 @@ internal static class Backoff
         // the exponential backoff will generally be not more than 2X worse than the perfect guess and
         // will do a lot less attempts than a simple retry. On multiprocessor machine fruitless attempts
         // will cause unnecessary sharing of the contended state which may make modifying the state more expensive.
-        // To protect against degenerate cases we will cap the per-iteration wait to 1-2 thousand spinwaits.
-        private const uint MaxExponentialBackoffBits = 10;
+        // To protect against degenerate cases we will cap the per-iteration wait to 2.2–4.4 microseconds.
+        private const uint MaxExponentialBackoffBits = 7;
 
         internal static unsafe int Exponential(uint attempt)
         {

src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelLifoSemaphore.cs

@@ -17,8 +17,10 @@ internal sealed partial class LowLevelLifoSemaphore
     {
         // The spin count is chosen to be in the range of typical thread wake latency and some additional overhead,
         // all assuming a single spin is calibrated to around 35 nanoseconds.
-        // The thread wake latency commonly measures at 2-10 microsecond (year 2026) and unlikely to drastically change.
-        private const int DefaultSemaphoreSpinCountLimit = 256;
+        // The thread wake latency commonly measures at ~10 microseconds (year 2026) and is unlikely to drastically change.
+        // But since the wakes are LIFO, the spin needs to survive additional overhead (we will need to take a lock, unlink the thread).
+        // So we limit the spin to about 35 microseconds.
+        private const int DefaultSemaphoreSpinCountLimit = 1024;
         // The cooldown roughly serves as detection that the thread did not spend time being blocked.
         // If it woke in under 4 microseconds, it was likely a fast/trivial wake without blocking.
         private const int DefaultWakeCooldown = 4;
@@ -148,8 +150,9 @@ public bool WaitNoSpin(int timeoutMs)
         }
 
         // If we have signals and have waiters, we need to make sure at least one is waking.
-        // We wake one waiter at a time. If it finds work it will ask for workers and that can wake more waiters
-        // if other workers do not consume the additional signals.
+        // We wake one waiter at a time. If it finds a signal it will wake another worker, unless other workers consume
+        // the additional signals first.
+
         // It is generally unusual to have > 1 signal. That only happens when the count of desired workers had a forced change.
         // In any case, we would prefer that extra signals be consumed by active workers, but must guarantee that signals
         // are consumed eventually thus we release waiters one by one.
@@ -257,6 +260,11 @@ private bool WaitAsWaiter(int timeoutMs)
                         if (counts.SignalCount != 0)
                         {
                             // success
+
+                            // If there are remaining signals, wake another waiter to ensure signals are eventually consumed.
+                            // In a saturated pool there may be little new semaphore traffic, and we'd otherwise keep
+                            // sleeping workers counted as running for too long.
+                            MaybeWakeWaiter(newCounts);
                             return true;
                         }
 

src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.cs

@@ -16,6 +16,8 @@ private static partial class WorkerThread
         {
             private static readonly short ThreadsToKeepAlive = DetermineThreadsToKeepAlive();
 
+            private static readonly short SpuriousDispatchNoSpinThreshold = DetermineSpuriousDispatchNoSpinThreshold();
+
             // This value represents an assumption of how much uncommitted stack space a worker thread may use in the future.
             // Used in calculations to estimate when to throttle the rate of thread injection to reduce the possibility of
             // preexisting threads from running out of memory when using new stack space in low-memory situations.
@@ -36,6 +38,23 @@ private static short DetermineThreadsToKeepAlive()
                 return threadsToKeepAlive >= -1 ? threadsToKeepAlive : DefaultThreadsToKeepAlive;
             }
 
+            private static short DetermineSpuriousDispatchNoSpinThreshold()
+            {
+                // default to 2/3 of proc count.
+                // At more than this working threads we start parking threads after a spurious dispatch.
+                short DefaultSpuriousDispatchNoSpinThreshold = (short)(Environment.ProcessorCount * 2 / 3);
+
+                // When a worker is invited to dispatch work items but finds none, it may park without spinning first.
+                // That is only preferable while more than this number of workers are still processing work and can take
+                // the next request. Set to 0 to park without spinning when at least one other worker is still processing work.
+                short threshold =
+                    AppContextConfigHelper.GetInt16Config(
+                        "System.Threading.ThreadPool.SpuriousDispatchNoSpinThreshold",
+                        "DOTNET_ThreadPool_SpuriousDispatchNoSpinThreshold",
+                        DefaultSpuriousDispatchNoSpinThreshold);
+                return threshold >= 0 ? threshold : DefaultSpuriousDispatchNoSpinThreshold;
+            }
+
             /// <summary>
             /// Semaphore for controlling how many threads are currently working.
             /// </summary>
@@ -122,7 +141,8 @@ private static void WorkerThreadStart()
             // returns true if the worker should Wait without spinning.
             private static bool WorkerDoWork(PortableThreadPool threadPoolInstance)
             {
-                bool noSpin;
+                bool spurious;
+                short numProcessingWork;
 
                 do
                 {
@@ -137,8 +157,8 @@ private static bool WorkerDoWork(PortableThreadPool threadPoolInstance)
                         switch (ThreadPoolWorkQueue.Dispatch())
                         {
                             case ThreadPoolWorkQueue.DispatchResult.Spurious:
-                                // We were invited but found no work. This is counterproductive. We should park.
-                                noSpin = true;
+                                // We were invited but found no work. This is counterproductive. We may want to park.
+                                spurious = true;
                                 break;
 
                             case ThreadPoolWorkQueue.DispatchResult.ShouldStop:
@@ -149,25 +169,28 @@ private static bool WorkerDoWork(PortableThreadPool threadPoolInstance)
                             default:
                                 // We did some work, but then there was nothing to do.
                                 // Spin a bit before parking in case we are invited back.
-                                noSpin = false;
+                                spurious = false;
                                 break;
                         }
                     }
                     else
                     {
                         // Not a common case. This can happen when worker goal was increased and invited extra threads.
                         // We will spin in case there is work for all and another request will soon follow.
-                        noSpin = false;
+                        spurious = false;
                     }
 
                     // We could not find more work in the queue and will try to stop being active.
                     // One caveat - in Saturated state we have seen a thread request but could not signal for a worker
                     // to come and see to it. Thus in Saturated state, one thread will clear the state and will come
                     // back for another try to clear the thread request and do Dispatch - without consuming a signal.
                     // See `TryIncrementProcessingWork` for details about Saturated state.
-                } while (!TryRemoveWorkingWorker(threadPoolInstance));
+                } while (!TryRemoveWorkingWorker(threadPoolInstance, out numProcessingWork));
 
-                return noSpin;
+                // Parking right away after a spurious dispatch is only worthwhile while other workers remain
+                // processing work and can take the next request. When few workers are left, the next request is
+                // likely to come to this thread, so it is cheaper to spin and stay available.
+                return spurious && numProcessingWork > SpuriousDispatchNoSpinThreshold;
             }
 
             // returns true if the worker is shutting down
@@ -234,9 +257,10 @@ private static bool ShouldExitWorker(PortableThreadPool threadPoolInstance, LowL
             /// Tries to reduce the number of working workers by one.
             /// If we are in a Saturated state, clears the state instead and returns false.
             /// Returns true if number of active threads was actually reduced.
+            /// <paramref name="numProcessingWork"/> receives the resulting number of workers processing work.
             /// See `TryDecrementProcessingWork` for details about Saturated state.
             /// </summary>
-            private static bool TryRemoveWorkingWorker(PortableThreadPool threadPoolInstance)
+            private static bool TryRemoveWorkingWorker(PortableThreadPool threadPoolInstance, out short numProcessingWork)
             {
                 uint collisionCount = 0;
                 while (true)
@@ -246,6 +270,7 @@ private static bool TryRemoveWorkingWorker(PortableThreadPool threadPoolInstance
                     bool decremented = newCounts.TryDecrementProcessingWork();
                     if (threadPoolInstance._separated.counts.InterlockedCompareExchange(newCounts, oldCounts) == oldCounts)
                     {
+                        numProcessingWork = newCounts.NumProcessingWork;
                         return decremented;
                     }