Repository navigation
perf(mutex): optimize uncontended lock release - #406
sundeep8967 wants to merge 1 commit into
Conversation
| if !self.has_waiters.load(Ordering::Acquire) { | ||
| let available = self.available_permits.load(Ordering::Relaxed); | ||
| if self.try_update_permits(available, n).is_ok() { | ||
| // If has_waiters is still false, no waiter could have enqueued without | ||
| // either observing available permits or taking the queue lock. | ||
| if !self.has_waiters.load(Ordering::Acquire) { | ||
| return; | ||
| } | ||
| // If a waiter raced and set has_waiters, fall through to distribute_permits | ||
| // with 0 extra permits so any newly enqueued waiter is serviced. | ||
| self.distribute_permits(0, self.waiters.lock()); | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
This can lose a wakeup. A queuing waiter leaves available_permits at 0 and only sets has_waiters after push_back. An unlock that reads the flag before that can still CAS 0 → 1 once the waiter is queued, so the permit stays in available_permits and nobody wakes the waiter. The distribute_permits(0, ..) fallback doesn't help, since it has nothing to hand out.
orthur2
left a comment
There was a problem hiding this comment.
Thanks for working on this. I'd prefer to fix the lost wakeup by making queuing itself change available_permits, like #412 does for OnceCell. Could Mutex start with 3 permits and have each guard take 2 (acquire(2), try_acquire(2), release(2))? A held lock then leaves one permit free, and the first waiter takes it as it queues, so the existing release can skip the lock while that permit is still there. Then we don't need has_waiters or release_with_lock.
Signed-off-by: sundeep8967 <sundeep8967@gmail.com>
b15247d to
5689a1e
Compare
|
Updated: Rebased onto latest |
Description
Avoid acquiring the
waitersqueue lock inrelease_with_lockwhen no waiters are queued.Previously,
MutexGuard::dropand related guards returned permits throughrelease_with_lock, which unconditionally acquired the semaphore's internalwaitersmutex lock. Under uncontended reuse, acquiring and releasing this lock on every iteration added significant overhead.This change introduces an atomic
has_waiters: AtomicBooltracker:acquired_or_enqueue,has_waitersis markedtrue.distribute_permitsor waiter unlinking / drop,has_waitersis markedfalse.release_with_lockinspectshas_waitersvia Acquire ordering: if no waiters are present, it atomically restores permits via CAS without acquiring the queue lock. If a waiter races and registers, it falls through todistribute_permitsto guarantee wakeup.Benchmark Results
Ran
cargo x bench --bench primitives -- 'mutex::lock::uncontended_reuse'(24,000,000 rounds):Ran
cargo x bench --bench primitives -- 'mutex::lock::queued_handoff_cycle':Verification
cargo x lintpassed cleanly (clippy, formatting, taplo, typos, hawkeye, doc lints).cargo test --package asyncband --all-features) and integration test suites (cargo test --package tests-integration) passed.Fixes #403