Skip to content

Commit

Permalink
Make recursive mutexes faster
Browse files Browse the repository at this point in the history
Recursive mutexes now go as fast as normal mutexes. The tradeoff is they
are no longer safe to use in signal handlers. However you can still have
signal safe mutexes if you set your mutex to both recursive and pshared.
You can also make functions that use recursive mutexes signal safe using
sigprocmask to ensure recursion doesn't happen due to any signal handler

The impact of this change is that, on Windows, many functions which edit
the file descriptor table rely on recursive mutexes, e.g. open(). If you
develop your app so it uses pread() and pwrite() then your app should go
very fast when performing a heavily multithreaded and contended workload

For example, when scaling to 40+ cores, *NSYNC mutexes can go as much as
1000x faster (in CPU time) than the naive recursive lock implementation.
Now recursive will use *NSYNC under the hood when it's possible to do so
  • Loading branch information
jart committed Sep 10, 2024
1 parent 58d252f commit 2f48a02
Show file tree
Hide file tree
Showing 37 changed files with 2,684 additions and 2,209 deletions.
2 changes: 2 additions & 0 deletions libc/calls/clock_nanosleep.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@
* @param clock may be
* - `CLOCK_REALTIME`
* - `CLOCK_MONOTONIC`
* - `CLOCK_REALTIME_COARSE` but is likely to sleep negative time
* - `CLOCK_MONTONIC_COARSE` but is likely to sleep negative time
* @param flags can be 0 for relative and `TIMER_ABSTIME` for absolute
* @param req can be a relative or absolute time, depending on `flags`
* @param rem shall be updated with the remainder of unslept time when
Expand Down
42 changes: 41 additions & 1 deletion libc/intrin/pthread_mutex_lock.c
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,37 @@ static errno_t pthread_mutex_lock_recursive(pthread_mutex_t *mutex,
}
}

#if PTHREAD_USE_NSYNC
static errno_t pthread_mutex_lock_recursive_nsync(pthread_mutex_t *mutex,
uint64_t word) {
int me = gettid();
for (;;) {
if (MUTEX_OWNER(word) == me) {
if (MUTEX_TYPE(word) != PTHREAD_MUTEX_ERRORCHECK) {
if (MUTEX_DEPTH(word) < MUTEX_DEPTH_MAX) {
if (atomic_compare_exchange_weak_explicit(
&mutex->_word, &word, MUTEX_INC_DEPTH(word),
memory_order_relaxed, memory_order_relaxed))
return 0;
continue;
} else {
return EAGAIN;
}
} else {
return EDEADLK;
}
}
_weaken(nsync_mu_lock)((nsync_mu *)mutex->_nsyncx);
word = MUTEX_UNLOCK(word);
word = MUTEX_LOCK(word);
word = MUTEX_SET_OWNER(word, me);
mutex->_word = word;
mutex->_pid = __pid;
return 0;
}
}
#endif

static errno_t pthread_mutex_lock_impl(pthread_mutex_t *mutex) {
uint64_t word;

Expand Down Expand Up @@ -141,8 +172,17 @@ static errno_t pthread_mutex_lock_impl(pthread_mutex_t *mutex) {
return 0;
}

// handle recursive and error checking mutexes
// handle recursive and error checking mutexes
#if PTHREAD_USE_NSYNC
if (_weaken(nsync_mu_lock) &&
MUTEX_PSHARED(word) == PTHREAD_PROCESS_PRIVATE) {
return pthread_mutex_lock_recursive_nsync(mutex, word);
} else {
return pthread_mutex_lock_recursive(mutex, word);
}
#else
return pthread_mutex_lock_recursive(mutex, word);
#endif
}

/**
Expand Down
41 changes: 41 additions & 0 deletions libc/intrin/pthread_mutex_trylock.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,38 @@ static errno_t pthread_mutex_trylock_recursive(pthread_mutex_t *mutex,
}
}

static errno_t pthread_mutex_trylock_recursive_nsync(pthread_mutex_t *mutex,
uint64_t word) {
int me = gettid();
for (;;) {
if (MUTEX_OWNER(word) == me) {
if (MUTEX_TYPE(word) != PTHREAD_MUTEX_ERRORCHECK) {
if (MUTEX_DEPTH(word) < MUTEX_DEPTH_MAX) {
if (atomic_compare_exchange_weak_explicit(
&mutex->_word, &word, MUTEX_INC_DEPTH(word),
memory_order_relaxed, memory_order_relaxed))
return 0;
continue;
} else {
return EAGAIN;
}
} else {
return EDEADLK;
}
}
if (_weaken(nsync_mu_trylock)((nsync_mu *)mutex->_nsyncx)) {
word = MUTEX_UNLOCK(word);
word = MUTEX_LOCK(word);
word = MUTEX_SET_OWNER(word, me);
mutex->_word = word;
mutex->_pid = __pid;
return 0;
} else {
return EBUSY;
}
}
}

/**
* Attempts acquiring lock.
*
Expand Down Expand Up @@ -119,5 +151,14 @@ errno_t pthread_mutex_trylock(pthread_mutex_t *mutex) {
}

// handle recursive and error checking mutexes
#if PTHREAD_USE_NSYNC
if (_weaken(nsync_mu_trylock) &&
MUTEX_PSHARED(word) == PTHREAD_PROCESS_PRIVATE) {
return pthread_mutex_trylock_recursive_nsync(mutex, word);
} else {
return pthread_mutex_trylock_recursive(mutex, word);
}
#else
return pthread_mutex_trylock_recursive(mutex, word);
#endif
}
44 changes: 44 additions & 0 deletions libc/intrin/pthread_mutex_unlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
│ PERFORMANCE OF THIS SOFTWARE. │
╚─────────────────────────────────────────────────────────────────────────────*/
#include "libc/calls/calls.h"
#include "libc/calls/state.internal.h"
#include "libc/dce.h"
#include "libc/errno.h"
#include "libc/intrin/atomic.h"
Expand Down Expand Up @@ -69,6 +70,35 @@ static errno_t pthread_mutex_unlock_recursive(pthread_mutex_t *mutex,
}
}

#if PTHREAD_USE_NSYNC
static errno_t pthread_mutex_unlock_recursive_nsync(pthread_mutex_t *mutex,
uint64_t word) {
int me = gettid();
for (;;) {

// we allow unlocking an initialized lock that wasn't locked, but we
// don't allow unlocking a lock held by another thread, or unlocking
// recursive locks from a forked child, since it should be re-init'd
if (MUTEX_OWNER(word) && (MUTEX_OWNER(word) != me || mutex->_pid != __pid))
return EPERM;

// check if this is a nested lock with signal safety
if (MUTEX_DEPTH(word)) {
if (atomic_compare_exchange_strong_explicit(
&mutex->_word, &word, MUTEX_DEC_DEPTH(word), memory_order_relaxed,
memory_order_relaxed))
return 0;
continue;
}

// actually unlock the mutex
mutex->_word = MUTEX_UNLOCK(word);
_weaken(nsync_mu_unlock)((nsync_mu *)mutex->_nsyncx);
return 0;
}
}
#endif

/**
* Releases mutex.
*
Expand All @@ -81,6 +111,11 @@ static errno_t pthread_mutex_unlock_recursive(pthread_mutex_t *mutex,
errno_t pthread_mutex_unlock(pthread_mutex_t *mutex) {
uint64_t word;

if (__vforked) {
LOCKTRACE("skipping pthread_mutex_lock(%t) due to vfork", mutex);
return 0;
}

LOCKTRACE("pthread_mutex_unlock(%t)", mutex);

// get current state of lock
Expand Down Expand Up @@ -111,5 +146,14 @@ errno_t pthread_mutex_unlock(pthread_mutex_t *mutex) {
}

// handle recursive and error checking mutexes
#if PTHREAD_USE_NSYNC
if (_weaken(nsync_mu_unlock) &&
MUTEX_PSHARED(word) == PTHREAD_PROCESS_PRIVATE) {
return pthread_mutex_unlock_recursive_nsync(mutex, word);
} else {
return pthread_mutex_unlock_recursive(mutex, word);
}
#else
return pthread_mutex_unlock_recursive(mutex, word);
#endif
}
1 change: 0 additions & 1 deletion libc/intrin/pthread_mutexattr_settype.c
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
*
* @param type can be one of
* - `PTHREAD_MUTEX_NORMAL`
* - `PTHREAD_MUTEX_DEFAULT`
* - `PTHREAD_MUTEX_RECURSIVE`
* - `PTHREAD_MUTEX_ERRORCHECK`
* @return 0 on success, or error on failure
Expand Down
6 changes: 3 additions & 3 deletions libc/intrin/reservefd.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,10 @@
╚─────────────────────────────────────────────────────────────────────────────*/
#include "libc/calls/internal.h"
#include "libc/calls/state.internal.h"
#include "libc/intrin/fds.h"
#include "libc/intrin/atomic.h"
#include "libc/intrin/cmpxchg.h"
#include "libc/intrin/extend.h"
#include "libc/intrin/fds.h"
#include "libc/macros.h"
#include "libc/runtime/memtrack.internal.h"
#include "libc/str/str.h"
Expand All @@ -47,7 +47,7 @@ int __ensurefds_unlocked(int fd) {

/**
* Grows file descriptor array memory if needed.
* @asyncsignalsafe
* @asyncsignalsafe if signals are blocked
*/
int __ensurefds(int fd) {
__fds_lock();
Expand Down Expand Up @@ -82,7 +82,7 @@ int __reservefd_unlocked(int start) {

/**
* Finds open file descriptor slot.
* @asyncsignalsafe
* @asyncsignalsafe if signals are blocked
*/
int __reservefd(int start) {
int fd;
Expand Down
12 changes: 11 additions & 1 deletion libc/sock/socketpair-nt.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
╚─────────────────────────────────────────────────────────────────────────────*/
#include "libc/calls/internal.h"
#include "libc/calls/state.internal.h"
#include "libc/calls/struct/sigset.internal.h"
#include "libc/calls/syscall_support-nt.internal.h"
#include "libc/nt/createfile.h"
#include "libc/nt/enum/accessmask.h"
Expand All @@ -33,7 +34,8 @@
#include "libc/sysv/errfuns.h"
#ifdef __x86_64__

textwindows int sys_socketpair_nt(int family, int type, int proto, int sv[2]) {
textwindows static int sys_socketpair_nt_impl(int family, int type, int proto,
int sv[2]) {
uint32_t mode;
int64_t hpipe, h1;
char16_t pipename[64];
Expand Down Expand Up @@ -111,4 +113,12 @@ textwindows int sys_socketpair_nt(int family, int type, int proto, int sv[2]) {
return rc;
}

textwindows int sys_socketpair_nt(int family, int type, int proto, int sv[2]) {
int rc;
BLOCK_SIGNALS;
rc = sys_socketpair_nt_impl(family, type, proto, sv);
ALLOW_SIGNALS;
return rc;
}

#endif /* __x86_64__ */
4 changes: 4 additions & 0 deletions libc/thread/pthread_cond_init.c
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
│ TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR │
│ PERFORMANCE OF THIS SOFTWARE. │
╚─────────────────────────────────────────────────────────────────────────────*/
#include "libc/dce.h"
#include "libc/sysv/consts/clock.h"
#include "libc/thread/thread.h"

Expand All @@ -29,8 +30,11 @@ errno_t pthread_cond_init(pthread_cond_t *cond,
const pthread_condattr_t *attr) {
*cond = (pthread_cond_t){0};
if (attr) {
cond->_footek = IsXnuSilicon() || attr->_pshared;
cond->_pshared = attr->_pshared;
cond->_clock = attr->_clock;
} else {
cond->_footek = IsXnuSilicon();
}
return 0;
}
7 changes: 6 additions & 1 deletion libc/thread/pthread_cond_signal.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,14 @@
errno_t pthread_cond_signal(pthread_cond_t *cond) {

#if PTHREAD_USE_NSYNC
// do nothing if pthread_cond_timedwait() hasn't been called yet
// this is because we dont know for certain if nsync is safe
if (!atomic_load_explicit(&cond->_waited, memory_order_acquire))
return 0;

// favor *NSYNC if this is a process private condition variable
// if using Mike Burrows' code isn't possible, use a naive impl
if (!cond->_pshared && !IsXnuSilicon()) {
if (!cond->_footek) {
nsync_cv_signal((nsync_cv *)cond);
return 0;
}
Expand Down
22 changes: 18 additions & 4 deletions libc/thread/pthread_cond_timedwait.c
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "libc/calls/cp.internal.h"
#include "libc/dce.h"
#include "libc/errno.h"
#include "libc/intrin/atomic.h"
#include "libc/sysv/consts/clock.h"
#include "libc/thread/lock.h"
#include "libc/thread/posixthread.internal.h"
Expand Down Expand Up @@ -116,17 +117,30 @@ errno_t pthread_cond_timedwait(pthread_cond_t *cond, pthread_mutex_t *mutex,
MUTEX_OWNER(muword) != gettid())
return EPERM;

// if condition variable is shared then mutex must be too
if (cond->_pshared)
if (MUTEX_PSHARED(muword) != PTHREAD_PROCESS_SHARED)
#if PTHREAD_USE_NSYNC
// the first time pthread_cond_timedwait() is called we learn if the
// associated mutex is normal and private. that means *NSYNC is safe
// this decision is permanent. you can't use a recursive mutex later
if (!atomic_load_explicit(&cond->_waited, memory_order_acquire)) {
if (!cond->_footek)
if (MUTEX_TYPE(muword) != PTHREAD_MUTEX_NORMAL ||
MUTEX_PSHARED(muword) != PTHREAD_PROCESS_PRIVATE)
cond->_footek = true;
atomic_store_explicit(&cond->_waited, true, memory_order_release);
} else if (!cond->_footek) {
if (MUTEX_TYPE(muword) != PTHREAD_MUTEX_NORMAL ||
MUTEX_PSHARED(muword) != PTHREAD_PROCESS_PRIVATE)
return EINVAL;
}
#endif

// now perform the actual wait
errno_t err;
BEGIN_CANCELATION_POINT;
#if PTHREAD_USE_NSYNC
// favor *NSYNC if this is a process private condition variable
// if using Mike Burrows' code isn't possible, use a naive impl
if (!cond->_pshared && !IsXnuSilicon()) {
if (!cond->_footek) {
err = nsync_cv_wait_with_deadline(
(nsync_cv *)cond, (nsync_mu *)mutex, cond->_clock,
abstime ? *abstime : nsync_time_no_deadline, 0);
Expand Down
4 changes: 3 additions & 1 deletion libc/thread/thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@

#define PTHREAD_BARRIER_SERIAL_THREAD 31337

#define PTHREAD_MUTEX_DEFAULT 0
#define PTHREAD_MUTEX_NORMAL 0
#define PTHREAD_MUTEX_RECURSIVE 1
#define PTHREAD_MUTEX_ERRORCHECK 2
Expand Down Expand Up @@ -77,6 +76,7 @@ typedef struct pthread_mutex_s {
};
/* this cleverly overlaps with NSYNC struct Dll *waiters; */
_PTHREAD_ATOMIC(uint64_t) _word;
long _nsyncx[2];
} pthread_mutex_t;

typedef struct pthread_mutexattr_s {
Expand All @@ -95,6 +95,8 @@ typedef struct pthread_cond_s {
uint32_t _nsync;
char _pshared;
char _clock;
char _footek;
_PTHREAD_ATOMIC(char) _waited;
};
};
_PTHREAD_ATOMIC(uint32_t) _sequence;
Expand Down
1 change: 0 additions & 1 deletion test/libc/intrin/pthread_mutex_lock_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ TEST(pthread_mutex_lock, recursive) {
}
ASSERT_EQ(0, pthread_mutex_lock(&lock));
ASSERT_EQ(0, pthread_mutex_unlock(&lock));
ASSERT_EQ(0, pthread_mutex_unlock(&lock));
ASSERT_EQ(0, pthread_mutex_destroy(&lock));
}

Expand Down
2 changes: 1 addition & 1 deletion test/libc/thread/footek_test.c
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#define USE POSIX
#define USE POSIX_RECURSIVE
#define ITERATIONS 100000
#define THREADS 30

Expand Down
Loading

0 comments on commit 2f48a02

Please sign in to comment.