Skip to content

Improve ConcurrentStack.TryPopRange performance under contention. #134306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
aw0lid wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Improve ConcurrentStack.TryPopRange performance under contention. #134306

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions ...raries/System.Collections.Concurrent/src/System/Collections/Concurrent/ConcurrentStack.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -586,6 +586,8 @@ private int TryPopCore(int count, out Node? poppedHead)
Node? head;
Node next;
int backoff = 1;
int shift = Numerics.BitOperations.Log2(int.MaxValue / (uint)count);
int checkHeadMask = (1 << shift) - 1;
Comment thread
jkotas marked this conversation as resolved.

@jkotas jkotas Sep 27, 2026

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.

Is there some rationale behind this formula?

If I am reading this correctly, these additional checks are only going to kick for count >30k. For example, when count = 10_000, checkHeadMask is going to be 131071 and so we won't execute any additional checks. So it is surprising that you are able to measure an improvement for 10_000.

@aw0lid aw0lid Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

You're right — for count = 10,000, the formula does not actually introduce any additional _head checks.

The rationale was to make the check interval adaptive to count— checking _head more frequently as range size grows to prevent heavy traversal on a stale chain, while keeping checks infrequent for smaller counts to minimize overhead. I intentionally used a heuristic rather than hardcoded thresholds, derived purely from experimentation.

Regarding the improvements observed at smaller counts (like 10,000), the repeated runs don't show a consistent benefit, so I don't consider them meaningful evidence for the heuristic:

TryPopCount = 10,000 (After):

ParallelThreads Run 1 Run 2 Run 3
0 179.8 μs 198.5 μs 176.7 μs
1 287.0 μs 333.0 μs 278.9 μs
2 620.3 μs 590.9 μs 563.7 μs
5 1,069.2 μs 1,189.5 μs 1,203.9 μs
8 666.9 μs 1,367.8 μs 1,431.1 μs
16 771.1 μs 1,446.7 μs 1,496.8 μs

Baseline (Before):

ParallelThreads Run 1 Run 2 Run 3
0 176.2 μs 175.5 μs 175.8 μs
1 283.0 μs 285.4 μs 282.6 μs
2 664.8 μs 690.0 μs 584.8 μs
5 1,181.9 μs 1,157.8 μs 1,185.0 μs
8 1,438.0 μs 1,215.8 μs 1,193.2 μs
16 1,539.0 μs 1,946.6 μs 1,561.5 μs

I don't consider small-count improvements meaningful evidence for the heuristic; the main value of this PR lies in large ranges under high contention, where stopping stale traversals can prevent severe performance degradation.

while (true)
{
head = _head;
Expand All @@ -605,6 +607,13 @@ private int TryPopCore(int count, out Node? poppedHead)
for (; nodesCount < count && next._next != null; nodesCount++)
{
next = next._next;
if ((nodesCount & checkHeadMask) == 0)
{
if (head != _head)
{
break;
Comment on lines +610 to +614

@jkotas jkotas Sep 27, 2026

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.

Valid feedback

@aw0lid aw0lid Sep 27, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I did some additional experiments around skipping the CAS after detecting that _head has changed. The results are mixed for smaller ranges:

TryPopCount Threads Baseline CAS kept CAS skipped
3 16 24.08 μs 24.46 μs 25.29 μs
10 16 22.67 μs 20.64 μs 21.35 μs
100 16 43.86 μs 45.23 μs 42.65 μs
1,000 16 69.38 μs 68.68 μs 66.66 μs

For 100,000+, skipping the CAS caused a substantial regression under contention, reaching seconds per iteration. These cases also take significantly longer to benchmark.

@tannergooding, I'd be interested in your thoughts on this, and whether there is a way to avoid the CAS instruction in this path.

}
}
}

// Try to swap the new head. If we succeed, break out of the loop.
Expand Down
Loading