From 462ba6909e3a39053a817bdc013d4b38a79cfd2a Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Fri, 13 Sep 2024 06:25:27 -0700 Subject: [PATCH] Speed up unnamed POSIX semaphores When sem_wait() used its futexes it would always use process shared mode which can be problematic on platforms like Windows, where that causes it to use the slow futex polyfill. Now when sem_init() is called in private mode that'll be passed along so we can use a faster WaitOnAddress() call --- libc/thread/sem_destroy.c | 25 +++++++++++++-------- libc/thread/sem_init.c | 22 +++++++++++------- libc/thread/sem_post.c | 2 +- libc/thread/sem_timedwait.c | 22 +++++++++--------- test/posix/unnamed_semaphore_test.c | 35 +++++++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 30 deletions(-) create mode 100644 test/posix/unnamed_semaphore_test.c diff --git a/libc/thread/sem_destroy.c b/libc/thread/sem_destroy.c index fb0e3c3562b..053295c1b91 100644 --- a/libc/thread/sem_destroy.c +++ b/libc/thread/sem_destroy.c @@ -18,6 +18,7 @@ ╚─────────────────────────────────────────────────────────────────────────────*/ #include "libc/assert.h" #include "libc/intrin/atomic.h" +#include "libc/intrin/strace.h" #include "libc/limits.h" #include "libc/sysv/errfuns.h" #include "libc/thread/semaphore.h" @@ -40,14 +41,20 @@ * @raise EBUSY if `sem` has waiters */ int sem_destroy(sem_t *sem) { - int waiters; + int rc, waiters; npassert(sem->sem_magic != SEM_MAGIC_NAMED); - if (sem->sem_magic != SEM_MAGIC_UNNAMED) - return einval(); - waiters = atomic_load_explicit(&sem->sem_waiters, memory_order_relaxed); - unassert(waiters >= 0); - if (waiters) - return ebusy(); - atomic_store_explicit(&sem->sem_value, INT_MIN, memory_order_relaxed); - return 0; + if (sem->sem_magic != SEM_MAGIC_UNNAMED) { + rc = einval(); + } else { + waiters = atomic_load_explicit(&sem->sem_waiters, memory_order_relaxed); + unassert(waiters >= 0); + if (waiters) { + rc = ebusy(); + } else { + atomic_store_explicit(&sem->sem_value, INT_MIN, memory_order_relaxed); + rc = 0; + } + } + STRACE("sem_destroy(%p) → %d% m", sem, rc); + return rc; } diff --git a/libc/thread/sem_init.c b/libc/thread/sem_init.c index e86f1831340..da976752c8f 100644 --- a/libc/thread/sem_init.c +++ b/libc/thread/sem_init.c @@ -19,6 +19,7 @@ #include "libc/calls/calls.h" #include "libc/dce.h" #include "libc/intrin/atomic.h" +#include "libc/intrin/strace.h" #include "libc/limits.h" #include "libc/sysv/errfuns.h" #include "libc/thread/semaphore.h" @@ -37,12 +38,17 @@ * @raise EINVAL if `value` exceeds `SEM_VALUE_MAX` */ int sem_init(sem_t *sem, int pshared, unsigned value) { - if (value > SEM_VALUE_MAX) - return einval(); - sem->sem_magic = SEM_MAGIC_UNNAMED; - atomic_store_explicit(&sem->sem_value, value, memory_order_relaxed); - sem->sem_pshared = !!pshared; - sem->sem_pid = getpid(); - sem->sem_waiters = 0; - return 0; + int rc; + if (value > SEM_VALUE_MAX) { + rc = einval(); + } else { + sem->sem_magic = SEM_MAGIC_UNNAMED; + atomic_store_explicit(&sem->sem_value, value, memory_order_relaxed); + sem->sem_pshared = !!pshared; + sem->sem_pid = getpid(); + sem->sem_waiters = 0; + rc = 0; + } + STRACE("sem_init(%p, %hhhd, %u) → %d% m", sem, pshared, value, rc); + return rc; } diff --git a/libc/thread/sem_post.c b/libc/thread/sem_post.c index 83fab34099a..80c164c9e80 100644 --- a/libc/thread/sem_post.c +++ b/libc/thread/sem_post.c @@ -46,7 +46,7 @@ int sem_post(sem_t *sem) { old = atomic_fetch_add_explicit(&sem->sem_value, 1, memory_order_acq_rel); unassert(old > INT_MIN); if (old >= 0) { - wakeups = nsync_futex_wake_(&sem->sem_value, 1, true); + wakeups = nsync_futex_wake_(&sem->sem_value, 1, sem->sem_pshared); npassert(wakeups >= 0); rc = 0; } else { diff --git a/libc/thread/sem_timedwait.c b/libc/thread/sem_timedwait.c index 7ac0f3acc59..be046ce6e65 100644 --- a/libc/thread/sem_timedwait.c +++ b/libc/thread/sem_timedwait.c @@ -59,7 +59,7 @@ static void sem_timedwait_cleanup(void *arg) { * @cancelationpoint */ int sem_timedwait(sem_t *sem, const struct timespec *abstime) { - int i, v, rc, e = errno; + int v, rc, e = errno; #if 0 if (IsXnuSilicon() && sem->sem_magic == SEM_MAGIC_KERNEL) { @@ -103,16 +103,13 @@ int sem_timedwait(sem_t *sem, const struct timespec *abstime) { } #endif - for (i = 0; i < 7; ++i) { - rc = sem_trywait(sem); - if (!rc) { - return rc; - } else if (errno == EAGAIN) { - errno = e; - sem_delay(i); - } else { - return rc; - } + rc = sem_trywait(sem); + if (!rc) { + return rc; + } else if (errno == EAGAIN) { + errno = e; + } else { + return rc; } BEGIN_CANCELATION_POINT; @@ -122,7 +119,8 @@ int sem_timedwait(sem_t *sem, const struct timespec *abstime) { do { if (!(v = atomic_load_explicit(&sem->sem_value, memory_order_relaxed))) { - rc = nsync_futex_wait_(&sem->sem_value, v, true, CLOCK_REALTIME, abstime); + rc = nsync_futex_wait_(&sem->sem_value, v, sem->sem_pshared, + CLOCK_REALTIME, abstime); if (rc == -EINTR || rc == -ECANCELED) { errno = -rc; rc = -1; diff --git a/test/posix/unnamed_semaphore_test.c b/test/posix/unnamed_semaphore_test.c new file mode 100644 index 00000000000..f406f82b307 --- /dev/null +++ b/test/posix/unnamed_semaphore_test.c @@ -0,0 +1,35 @@ +#include +#include + +#define THREADS 10 +#define ITERATIONS 100000 + +int g_count; +sem_t g_sem; + +void *worker(void *arg) { + for (int i = 0; i < ITERATIONS; ++i) { + if (sem_wait(&g_sem)) + exit(6); + ++g_count; + if (sem_post(&g_sem)) + exit(7); + } + return 0; +} + +int main(int argc, char *argv[]) { + pthread_t th[THREADS]; + if (sem_init(&g_sem, 0, 1)) + return 1; + for (int i = 0; i < THREADS; ++i) + if (pthread_create(&th[i], 0, worker, 0)) + return 2; + for (int i = 0; i < THREADS; ++i) + if (pthread_join(th[i], 0)) + return 3; + if (g_count != THREADS * ITERATIONS) + return 4; + if (sem_destroy(&g_sem)) + return 5; +}