Conversation
RingChannel woke its idle consumers through a photon::semaphore, whose signal() holds one per-channel spinlock across try_resume(), which in turn calls prelocked_thread_interrupt() -- so a cross-vCPU wake-up costs an eventfd write syscall inside the critical section. Every consumer returning from wait_defer() re-acquires that same lock, and every recv() miss did two seq_cst RMWs on the shared `idler` counter. All notifications of a channel thus serialize behind one lock whose holder may be preempted while everybody else spins on it, so dispatch throughput fell as vCPUs were added instead of rising. Replace it with a park slot per sleeping consumer, linked into a lock-free idle stack. A producer's fast path is a fence and a relaxed load of the stack top: no store to shared state, no lock. A wake-up is one exchange() of the whole stack plus a thread_interrupt() issued outside of any lock, so N producers can wake N consumers in parallel. Not losing a notification rests on three invariants: - Dekker: the producer fences between its push and its idle() load, while the consumer publishes its slot with a seq_cst RMW and re-checks the queue right afterwards. The two can not miss each other. - Publish-in-defer: prepare_usleep() does not inspect error_number, so an interrupt arriving before the sleep commits would be dropped. The slot is therefore promoted to COMMITTED from the defer callback of thread_usleep_defer(), and only a COMMITTED slot may be interrupted. - Slot lifetime: a slot is a stack frame, so its owner never returns from park() while a claimer might still touch it. The claimer's last write is its CLAIMED store, and the owner waits for it, escalating from pause to thread_yield to sched_yield: if that store has not landed yet, the claimer is not on a CPU, and spinning would only keep it away. The escalation alone cuts hand-off stalls of ~1.5ms (0.3% of all parks, yet 15-33% of the wall time of the synchronous WorkPool cases) down to microseconds. The 100ms self-wake of a parked consumer is kept as a safety net only: it re-checks the queue and re-arms the slot. Correctness does not depend on it, which is what lets the new test disable it. Interleaved A/B on one host, perf_workpool --fires=40000, the sync/StdContext case, median of 4 runs: vCPUs 4 8 16 32 before 40413 18416 9162 6898 QPS after 66126 111856 141728 134764 QPS before 9042 15860 36472 49332 ns/dispatch after 5638 2478 1572 1512 ns/dispatch Cross-vCPU wake-up latency improves 4.6x, burst fan-out 1.2x with 4 consumers and 14x with 32, and the whole benchmark spends 39% less user CPU. Fire-and-forget dispatch on few vCPUs is the one case that regresses (~30% at 4 vCPUs), because the semaphore version hardly ever really slept there; it turns into a 1.5x to 3.4x gain by 16 vCPUs. Add test-ringchannel-notify, which drives the channel with its safety net disabled, so that a lost notification hangs the case and is reported, instead of hiding as a 100ms hiccup. Its teeth were verified by mutation: dropping the re-check after publish, or committing the slot before the sleep instead of from the defer callback, are both caught within a few thousand rounds. Add perf-ringchannel to measure the notification paths in isolation. No public interface changes: send(), recv() and notification_pending() keep their signatures, and notification_pending() keeps its meaning of wake-ups issued but not yet observed. Being header-only, the class does change layout, so consumers must be rebuilt against matching headers, as any photon upgrade already requires.
Coldwings
requested review from
EricHuangqx,
beef9999,
lihuiba and
liulanzheng
and
a lite review from Copilot
and removed request for
Copilot
September 15, 2026 04:10
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RingChannelnotifies its idle consumers through aphoton::semaphore. That pathdoes not scale with the number of vCPUs:
semaphore::signal()holds oneper-channel spinlock across
try_resume(), which callsprelocked_thread_interrupt().For a consumer parked on another vCPU that is a
cancel_wait()eventfd write syscallexecuted inside the critical section.
wait_defer()re-acquires that very same lock.recv()that misses did twoseq_cstRMWs on the sharedidlercounter.So all notifications of a channel serialize behind a single lock whose holder can be
preempted while everybody else spins on it. WorkPool dispatch throughput therefore falls
as vCPUs are added.
Change
A park slot per sleeping consumer, linked into a lock-free idle stack (
ParkStack):shared state, no lock, nothing to contend on while consumers are busy.
exchange()of the whole stack plus athread_interrupt()issuedoutside of any lock, so N producers wake N consumers in parallel.
recv().FlexRingChannelgets the same treatment. The 100ms self-wake is kept as a safety netonly: it re-checks the queue and re-arms the slot, and correctness does not depend on it.
Why no notification is lost
idle()load; the consumerpublishes its slot with a
seq_cstRMW and re-checks the queue right afterwards. Thetwo cannot miss each other ([atomics.order]/4 needs the fence on both sides, hence the
one after
publish()).prepare_usleep()does not inspecterror_number, so aninterrupt arriving before the sleep commits would be dropped. The slot is promoted to
COMMITTEDfrom the defer callback ofthread_usleep_defer(), and only aCOMMITTEDslot may be interrupted. A claimer that finds it not yet committed leaves the wake-up to
the owner's own defer callback.
park()while a claimer may still touch it. The claimer's last write is its
CLAIMEDstore; theowner waits for it, escalating pause →
thread_yield→sched_yield, because if thestore has not landed the claimer is not on a CPU and spinning only keeps it away. That
escalation alone cuts hand-off stalls of ~1.5ms (0.3% of parks, but 15-33% of the wall
time of the sync WorkPool cases) down to microseconds.
Numbers
Interleaved A/B of two binaries on the same host (each round runs both, so machine drift
cannot favour either side).
perf_workpool --fires=40000, sync/StdContext case, median of 4:The sync/PhotonContext case has the same shape (38214 → 53454 QPS at 4 vCPUs,
23268 → 136678 at 16). Notice the direction: before, the curve falls from 8 vCPUs on;
after, it rises until the host runs out of cores (11 here) and then flattens.
perf-ringchannel, median of 4, N producers × N consumers:The whole
perf_workpoolrun also spends 39% less user CPU (2.96s → 1.85s) and 24%less wall time.
Honest about the costs:
cause: the semaphore version hardly ever really slept there — leftover
m_counttokensmade
wait()return immediately — so it bought throughput by burning CPU. This turnsinto a 1.5x-3.4x gain by 16 vCPUs.
send+recvon one channel is ~20% slower per round trip (publish CAS plusthe CLAIMING/CLAIMED hand-off). That shape means a thread notifying itself, which no real
user does.
Tests
test-ringchannel-notify: six cases (strict ping-pong, ping-pong racing the safetynet, MPMC burst, single producer against 8 consumers, external
thread_interruptduringa park, same-vCPU pairing). They run the channel with the safety net disabled, so a
lost notification hangs the case and an OS-thread watchdog reports it, instead of hiding
as a 100ms hiccup. A producer paces itself against an empty queue so that every item has
to pay the full notification path.
publish()iscaught within ~30k rounds; committing the slot before the sleep instead of from the defer
callback within ~800.
perf-ringchannel(not registered with ctest) for the numbers above.test-lockfree,test-executor-*(includingtest-executor-burst-drain, which assertsno notification accumulates over a burst),
test-go-channel,test-workpool-fanoutallpass.
Backport
Recommended for at least
release/0.9(labelledneed-backportso the cascade cancarry it further if maintainers want 0.8 too). Rationale: this is a scalability fix for
anything built on
WorkPool/Executorwith 8+ vCPUs, and it changes no interface thataffects source compatibility —
send(),recv()andnotification_pending()keep theirsignatures, and
notification_pending()keeps its meaning of "wake-ups issued but not yetobserved". The class is header-only, so its layout does change and consumers must be
rebuilt against matching headers, which any photon upgrade already requires.