Skip to content

perf(mutex): optimize uncontended lock release - #406

Open
sundeep8967 wants to merge 1 commit into
apache:mainfrom
sundeep8967:fix/403-uncontended-mutex
Open

sundeep8967 wants to merge 1 commit into
apache:mainfrom
sundeep8967:fix/403-uncontended-mutex

Conversation

@sundeep8967

Copy link
Copy Markdown

Description

Avoid acquiring the waiters queue lock in release_with_lock when no waiters are queued.

Previously, MutexGuard::drop and related guards returned permits through release_with_lock, which unconditionally acquired the semaphore's internal waiters mutex lock. Under uncontended reuse, acquiring and releasing this lock on every iteration added significant overhead.

This change introduces an atomic has_waiters: AtomicBool tracker:

  • When a waiter is enqueued in acquired_or_enqueue, has_waiters is marked true.
  • When the waiter queue becomes empty during distribute_permits or waiter unlinking / drop, has_waiters is marked false.
  • release_with_lock inspects has_waiters via 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 to distribute_permits to guarantee wakeup.

Benchmark Results

Ran cargo x bench --bench primitives -- 'mutex::lock::uncontended_reuse' (24,000,000 rounds):

  • Baseline: 5,849 ms median
  • Optimized: 3,792 ms median (~35% reduction in latency / ~1.54x speedup)

Ran cargo x bench --bench primitives -- 'mutex::lock::queued_handoff_cycle':

  • 1 waiter (5,000,000 rounds): 8,035 ms -> 6,797 ms
  • 4 waiters (1,250,000 rounds): 5,067 ms -> 4,775 ms
  • 16 waiters (312,500 rounds): 4,302 ms -> 4,252 ms

Verification

  • cargo x lint passed cleanly (clippy, formatting, taplo, typos, hawkeye, doc lints).
  • Unit tests (cargo test --package asyncband --all-features) and integration test suites (cargo test --package tests-integration) passed.

Fixes #403

Comment thread asyncband/src/internal/semaphore.rs Outdated
Comment on lines 197 to 210
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;
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 orthur2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@sundeep8967

Copy link
Copy Markdown
Author

Thanks @orthur2 for the suggestion! That is a much cleaner architecture, perfectly matching the permit-holding pattern from #412 without needing additional atomic flags or custom internal release methods. I will rebase on main and update Mutex to acquire and release 2 permits out of 3.

Signed-off-by: sundeep8967 <sundeep8967@gmail.com>
@sundeep8967
sundeep8967 force-pushed the fix/403-uncontended-mutex branch from b15247d to 5689a1e Compare November 8, 2026 09:50
@sundeep8967

Copy link
Copy Markdown
Author

Updated: Rebased onto latest main and updated Mutex to acquire and release 2 out of 3 internal permits per @orthur2's guidance, avoiding the queue lock on uncontended release without requiring has_waiters or modifying Semaphore. All tests pass cleanly across unit, integration, and doc tests.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Reduce uncontended Mutex lock and unlock overhead

2 participants