rt: improve spawn_blocking scalability with sharded queue

[alex]

The blocking pool's task queue was protected by a single mutex, causing severe contention when many threads spawn blocking tasks concurrently. This resulted in nearly linear degradation: 16 concurrent threads took ~18x longer than a single thread.

Replace the single-mutex queue with a sharded queue that distributes tasks across 16 lock-protected shards. The implementation adapts to concurrency levels by using fewer shards when thread count is low, maintaining cache locality while avoiding contention at scale.

Benchmark results (spawning 100 batches of 16 tasks per thread):

Concurrency	Before	After	Improvement
1 thread	13.3ms	17.8ms	+34%
2 threads	26.0ms	20.1ms	-23%
4 threads	45.4ms	27.5ms	-39%
8 threads	111.5ms	20.3ms	-82%
16 threads	247.8ms	22.4ms	-91%

The slight overhead at 1 thread is due to the sharded infrastructure, but this is acceptable given the dramatic improvement at higher concurrency where the original design suffered from lock contention.

(Notwithstanding that this shows as a commit from claude, every line is human reviewed. If there's a mistake, it's Alex's fault.)

Closes #2528.

[alex]

(FreeBSD failures look unrelated.)

[martin-g]

Please rebase to latest master to get the fix for the FreeBSD failures.

[ADD-SP]

With the Release ordering, compiler might reorder the self.shards[index].push(task) and fetch_max, which means that the .push(task) might be sequenced-before the fetch_max.

[alex]

I think you're right. (I hate atomic orderings :-/) AcqRel I think is what we want.

[Darksonn]

This is still wrong. Consider the following scenario:

Thread A                   Thread B            Thread C
                                               preferred_shard = 0
                           preferred_shard = 1
max_shard_pushed = 0
shards[0].push(_)
condvar.notify_one()
                           wakes up ...
                           max_shard = 0
max_shard_pushed = 1
shards[1].push(_)
condvar.notify_one()
                                               wakes up ...
                                               max_shard = 1
                                               shards[0].pop() = Some
                           shards[0].pop() = None

In this case, Thread B does not check shards[1] because it read max_shard with the value of zero. This means that two tasks were spawned, but only one gets picked up.

Well, I guess in principle Thread C will see the second task after it finishes executing the first one.

[alex]

Hmm, I think it's always the case that if a push happens concurrently with a pop that the pop might miss it, and we'll have to "fall back" to catching it in the wait_for_task loop.

I think in principle we could address this one by reloading max_shard_pushed, but of course you can still have a race condition.

In the scenario you've got here, what would happen is that after thread B returns None from pop, it'll go wait_for_task and then the task will get picked up.

[Darksonn]

Hmm, I think it's always the case that if a push happens concurrently with a pop that the pop might miss it, and we'll have to "fall back" to catching it in the wait_for_task loop.

Your notify_one() call ensures that for every push(), there will be a subsequent call to pop() that is not concurrent and hence guaranteed to see the pushed message. So there's at least one thread that's guaranteed to pick up each message.

If we imagine that the max_shard_pushed logic was removed, then Thread B would in fact be guaranteed to see the message in shards[1].

• shards[1].push(_) on A happens-before shards[0].pop() = Some on thread C because thread C is the thread waken up by the second notify_one() call.
• shards[0].pop() = Some on thread C happens-before shards[0].pop() = None on thread B, since otherwise thread B would have gotten Some when calling pop().
• After shards[0].pop() = None, thread B would attempt to call shards[1].pop()

So by this logic, the shards[1].pop() call would in fact happen after shards[1].push(_), and is hence guaranteed to see the message that was pushed.

[ADD-SP]

Here, we attempt to acquire the shard lock while holding the condvar lock, this is the nested locking pattern. In general, we should avoid this pattern as it is error-prone.

[alex]

Hmm, is there a preferred pattern to avoid the nested locking?

In this case we want to ensure that when we wait for a notification, there wasn't already a task that's made pending concurrently.

[Darksonn]

Nested locking is fine as long as locks are always taken in the same order.

[alex]

(I don't think the netlify failures are related)

[ADD-SP]

Sorry for the late review. I'd like to take some time to think about the nested locking issue. We may have a better choice, or not.

[alex]

No problem -- rebase was to pick up a fix for the netlify failures.

Let me know if there's other experiments it'd be useful for me to try.

[Darksonn]

This logic seems error-prone and likely to lead to missed tasks. Does it actually matter for your benchmark?

[alex]

It matters at N_THREADS=1 -- I don't personally care about that case at all, if we're ok with a small pessimization there (10% iirc?), I'd be delighted to delete this max shard logic and just always use a fixed number.

[alex]

Does this sound ok to you? I'd love to delete it because its responsible for a lot of the complexity.

[alex]

I've gone ahead and dropped this behavior.