Skip to content

Increase spinning/polling aggressiveness in the thread pool in low-saturation scenarios - #132765

Merged
VSadov merged 5 commits into
dotnet:mainfrom
VSadov:tpSpPr
Aug 28, 2026
Merged

Increase spinning/polling aggressiveness in the thread pool in low-saturation scenarios#132765
VSadov merged 5 commits into
dotnet:mainfrom
VSadov:tpSpPr

Conversation

@VSadov

@VSadovVSadov commented Aug 25, 2026

Copy link
Copy Markdown
Member

We made some changes in the threadpool to reduce spinning. In particular to reduce fruitless spinning - when a worker thread scanned through the work queue and found no work whatsoever. We would park such thread as a matter of throttling pointless scanning. The change helped in high saturation scenarios as reducing spurious scans reduces waste and lets other threads do useful work.

Unfortunately, in some low-saturation scenarios those spurious scans were load bearing.
In such scenarios some redundancy in terms of spurious scans must be tolerated to provide good latency.

If we park workers too aggressively when we do not have many workers in the first place we will need to rely on waking them up to serve incoming requests. In a bursty case this could be a noticeable regression. In bursty ping-pong kind of scenario, if this happens on both the app and the client ends, the result could be amplified further.

Here we are tuning the heuristic that parks threads after spurious scans to be enabled only when we have more than 2/3 of the proc count workers.

There could be better ways to make use of this signal selectively and we should explore further.
This is a simple enough change that we can do for net11.

The change also increases allowed spin time and lowers the max delay between polls to cap the impact on latency from longer spin, if such happens.
(it makes sense to have per iteration cap lower than the total, we had it the other way)

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @VSadov
See info in area-owners.md if you want to be subscribed.

@VSadov

VSadov commented Aug 26, 2026

Copy link
Copy Markdown
MemberAuthor

One example of a benchmark sensitive to this is websockets:

crank --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/scenarios/websocket.benchmarks.yml --config https://raw.githubusercontent.com/aspnet/Benchmarks/main/build/ci.profile.yml --scenario websocket --profile gold-lin-app --profile gold-load-load --variable scenario=echo --variable size=1000 --variable compress=false --application.options.collectCounters true --load.options.collectCounters true --load.framework net11.0 --application.framework net11.0

Too aggressive parking at low saturation results in a catastrophic regression. This PR reverts it all with some extra gain.

=== net10

| Max RPS | 802,092 |
| Requests | 12,031,782 |

=== baseline

| Max RPS | 221,248 |
| Requests | 3,120,884 |

=== this PR

| Max RPS | 1,043,637 |
| Requests | 15,654,929 |

CC: @BrennanConroy

@EgorBo

Copy link
Copy Markdown
Member

@EgorBot orchard -linux_arm -linux_amd

@VSadov
VSadov marked this pull request as ready for review August 26, 2026 16:05
CopilotAI lite review requested due to automatic review settings August 26, 2026 16:05
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adjusts thread-pool latency heuristics by making worker threads less likely to park immediately after a spurious dispatch when the pool is lightly saturated, while also tuning spin/backoff behavior in the low-level semaphore/backoff helpers to reduce latency impact.

Changes:

  • Add a configurable threshold (SpuriousDispatchNoSpinThreshold) so “no-spin” parking after spurious dispatch only happens when enough other workers remain processing work.
  • Increase the LowLevelLifoSemaphore default spin limit and ensure additional waiters can be woken to consume remaining signals.
  • Reduce the maximum per-iteration exponential backoff in Backoff.Exponential to cap delay per retry.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

FileDescription
src/libraries/System.Private.CoreLib/src/System/Threading/PortableThreadPool.WorkerThread.csAdds spurious-dispatch no-spin gating based on remaining processing workers and a new config-controlled threshold.
src/libraries/System.Private.CoreLib/src/System/Threading/LowLevelLifoSemaphore.csIncreases default spin budget and adds logic to wake additional waiters when signals remain.
src/libraries/System.Private.CoreLib/src/System/Threading/Backoff.csLowers exponential backoff cap to reduce worst-case per-iteration spin delay.

CopilotAI review requested due to automatic review settings August 27, 2026 16:22

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

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

  • Local variable DefaultSpuriousDispatchNoSpinThreshold is not a constant but uses PascalCase, which is inconsistent with the surrounding style (locals are camelCase; PascalCase is used for constants like DefaultThreadsToKeepAlive). Renaming helps readability and avoids making it look like a constant.
 // 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);

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

  • The comment describes the backoff cap in microseconds, but Thread.SpinWait is calibrated and hardware-dependent, so the time-based range is more precise than the code can guarantee. Consider describing the cap in terms of SpinWait iterations and optionally mention the time equivalence as approximate.
 // To protect against degenerate cases we will cap the per-iteration wait to 2.2–4.4 microseconds.
private const uint MaxExponentialBackoffBits = 7;

@VSadov

VSadov commented Aug 27, 2026

Copy link
Copy Markdown
MemberAuthor

JSON benchmark on CB200 (32 cores VM),
8 interleaved A/B attempts.

==== default/saturated case (512 connections 32 client threads)

virtually no change

mean rpssdp50p99
pr2,345,02639,8760.163.20
base2,346,07583,8550.163.06

==== low-saturation scenario (32 connections, 8 client threads)

improvement

rpssdp50p75p99cpuTP threads
pr409,8542,1400.0750.0780.08712.4%31
base395,2512,6660.0770.0820.1026.8%33

CopilotAI review requested due to automatic review settings August 27, 2026 21:23

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

@VSadov
VSadov requested a review from eduardo-vpAugust 27, 2026 21:59
@VSadov

Copy link
Copy Markdown
MemberAuthor

The "AllSubsets" filures appear unrelated to this PR. I see them in other PRs too.

@eduardo-vpeduardo-vp left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@VSadov
VSadov enabled auto-merge (squash) August 28, 2026 20:23
@VSadov

Copy link
Copy Markdown
MemberAuthor

Thanks!

@VSadov

Copy link
Copy Markdown
MemberAuthor

/ba-g all the failures are either #132830 or #132831

@VSadov
VSadov merged commit 61f59a1 into dotnet:mainAug 28, 2026
134 of 137 checks passed
@VSadov
VSadov deleted the tpSpPr branch August 28, 2026 21:09
@VSadov

Copy link
Copy Markdown
MemberAuthor

/backport to release/11.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/11.0 (link to workflow run)

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@VSadov@EgorBo@eduardo-vp