perf(completion): make terminal waits lock-free - #288
Conversation
8748fa7 to
5e2ac0a
Compare
|
Rebased onto the current |
5e2ac0a to
7010f81
Compare
|
@ryux1 Thanks for your contribution. CI is green now. I also tested 7010f81 locally on Linux (WSL): cargo x test and cargo x check passed, along with the Completion and trait tests on Rust 1.86. Seven additional local tests covering reentrant waker callbacks, panic cleanup, and completion/cancellation races passed on stable, Rust 1.86, and under Miri with four seeds. I didn't find any blocking correctness issues. I haven't independently reproduced the benchmark comparison. The conflict overlaps with the test-helper consolidation in #293. main now imports shared helpers from tests_integration, while this PR adds clone-callback support in the same area of completion_test.rs. Could you preserve the shared-helper refactor and adapt the new clone-callback regression test on top? In particular, the shared assert_completes_without_deadlock now takes only the test closure. We can then recheck the updated version. |
tisonkun
left a comment
There was a problem hiding this comment.
Generally LGTM.
I'm considering two minor improvements. Would test out tomorrow and get this merged.
|
Thanks for your contribution @ryux1 ! |
Closes #246.
Summary
Completionwaits lock-free with oneOnceLock<Option<T>>; pending registration remains cancellable.WakerSet::register, consolidate overlapping lifecycle tests, and cover terminal reads, pending repolls/cancellation, and notification fan-out in benchmarks.Design Notes
An uninitialized cell means pending; a stored
Some(value)means completed and a storedNonemeans abandoned. The single completer detaches the waiter set before initializing the cell, so observing a terminal result also observes waiter detachment. Pending polls and cancellation recheck the cell under the waiter mutex. Waker cloning follows the existing short-lock registration policy; replaced/cancelled wakers are dropped and notifications run after unlocking.Local measurements compare main (
f056962) with this branch on Apple M4 Max, macOS 26.6.2, Rust 1.98.0. Both use the same benchmark harness, excluding observer-container construction and handle cloning from fan-out. Values are medians of six per-run medians in nanoseconds, with run order balanced across main, an intermediate AtomicBool variant, and this implementation.ready_waitabandoned_waitcancel_pendingrepoll_pendingcomplete_then_waitnotify_pendingfanout/2fanout/8fanout/32Terminal reads improve substantially; pending cancellation and repolling pay roughly 0.45 ns and 0.21 ns respectively. Small fan-out differences are noisy. Completion/notification throughput is comparable to the AtomicBool variant, without maintaining a separate terminal flag.
Validation:
cargo +stable x testandcargo +1.86.0 x test(484 tests each),cargo x check,cargo x lint,cargo +stable x bench --no-run, and all 14 Completion integration tests under Miri with strict provenance, symbolic alignment, and seeds0..4.