Skip to content

Commit

Permalink
Improve performance of unwaited condvar_wake_one()/all()
Browse files Browse the repository at this point in the history
Previously, condvar_wake_one()/all() took the condvar's internal lock
before testing if anyone is waiting; A condvar_wake when nobody was
waiting was mutex_lock()+mutex_unlock() time (on my machine, 26 ns)
when there is no contention, but much much higher (involving a context
switch) when several CPUs are trying condvar_wake concurrently.

In this patch, we first test if the queue head is null before
acquiring the lock, and only acquire the lock if it isn't.
Now the condvar_wake-on-an-empty-queue micro-benchmark (see next patch)
takes less than 3ns - regardless of how many CPUs are doing it
concurrently.

Note that the queue head we test is NOT atomic, and we do not
use any memory fences. If we read the queue head and see there 0,
it is safe to decide nobody is waiting and do nothing. But if we
read the queue head and see != 0, we can't do anything with the
value we read - it might be only half-set (if the pointer is not
atomic on this architecture) or be set but the value it points
to is not (we didn't use a memory fence to enforce any ordering).
So if we see the head is != 0, we need to acquire the lock (which
also imposes the required memory visibility and ordering) and try
again.
  • Loading branch information
nyh committed Jul 18, 2013
1 parent 93aeb06 commit 3509b19
Showing 1 changed file with 11 additions and 0 deletions.
11 changes: 11 additions & 0 deletions core/condvar.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ int condvar_wait(condvar_t *condvar, mutex_t* user_mutex, sched::timer* tmr)

void condvar_wake_one(condvar_t *condvar)
{
// To make wake with no waiters faster, and avoid unnecessary contention
// in that case, first check the queue head outside the lock. If it is not
// empty, we still need to take the lock, and re-read the head.
if (!condvar->waiters_fifo.oldest) {
return;
}

mutex_lock(&condvar->m);
struct ccondvar_waiter *wr = condvar->waiters_fifo.oldest;
if (wr) {
Expand All @@ -82,6 +89,10 @@ void condvar_wake_one(condvar_t *condvar)

void condvar_wake_all(condvar_t *condvar)
{
if (!condvar->waiters_fifo.oldest) {
return;
}

mutex_lock(&condvar->m);
struct ccondvar_waiter *wr = condvar->waiters_fifo.oldest;
while (wr) {
Expand Down

0 comments on commit 3509b19

Please sign in to comment.