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.

@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.

@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 the update. release_with_lock has no callers left now, and its doc still says a mutex guard holds its only permit. Can you also remove it along with its unit test?

Comment thread CHANGELOG.md

### Improvements

* Reduce the cost of acquiring and releasing an uncontended `Mutex` by avoiding its internal wait-queue lock.

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.

Locking takes the same path as before. Only unlocking skips the queue lock now.

Suggested change
* Reduce the cost of acquiring and releasing an uncontended `Mutex` by avoiding its internal wait-queue lock.
* Reduce the cost of unlocking an uncontended `Mutex` by avoiding its internal wait-queue lock.

@sundeep8967
sundeep8967 force-pushed the fix/403-uncontended-mutex branch from 5689a1e to ba0ef2a Compare November 9, 2026 06:46
@sundeep8967

Copy link
Copy Markdown
Author

Updated: Removed the unused release_with_lock method and its unit test from Semaphore as suggested. Full test suite passing cleanly (56 unit tests, 158 doc tests, 19 compile-fail tests).

Avoid acquiring the waiters queue lock when dropping a MutexGuard
in the uncontended case.

By configuring the internal semaphore to start with 3 permits and
having each MutexGuard acquire and release 2 permits, an uncontended
lock holder leaves 1 spare permit in the semaphore. When a waiter
arrives, it claims this spare permit while enqueuing, ensuring that
the lock holder's release(2) can safely restore permits without
acquiring the waiters queue lock whenever no waiters have enqueued.

Removes unused Semaphore::release_with_lock and its test.

Fixes apache#403
Signed-off-by: sundeep8967 <[email protected]>
@sundeep8967
sundeep8967 force-pushed the fix/403-uncontended-mutex branch from ba0ef2a to 4da8edd Compare November 9, 2026 17:56

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