From 731041bbafcd75698de0fa84141fa79f65f8d508 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Sat, 22 Jul 2023 07:34:40 +0000 Subject: [PATCH v10] Optimize WAL insertion lock acquisition and release WAL insertion lock variable insertingAt is currently being read and written with the help of LWLock wait list lock to avoid torn-free reads/writes. This wait list lock can become a point of contention on a highly concurrent write workloads. This commit makes insertingAt a 64-bit atomic variable which provides torn-free reads/writes. Note that the atomic functions pg_atomic_*_u64 simulate atomic operations with a spinlock on platforms without 64-bit atomic variable support. This reduces contention on LWLock wait list lock and improves performance of highly concurrent write workloads - for instance, for a pgbench run with -T900, each transaction generating WAL of size ~256 bytes, more than 3X improvement in TPS is seen - 3.11X @ 512 clients 3.79 @ 768 clients, 3.47 @ 1024 clients, 2.27 @ 2048 clients and 2.77 @ 4096 clients. --- src/backend/access/transam/xlog.c | 4 +-- src/backend/storage/lmgr/lwlock.c | 46 ++++++++++++++----------------- src/include/storage/lwlock.h | 6 ++-- 3 files changed, 26 insertions(+), 30 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 8b0710abe6..75814ac947 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -376,7 +376,7 @@ typedef struct XLogwrtResult typedef struct { LWLock lock; - XLogRecPtr insertingAt; + pg_atomic_uint64 insertingAt; XLogRecPtr lastImportantAt; } WALInsertLock; @@ -4611,7 +4611,7 @@ XLOGShmemInit(void) for (i = 0; i < NUM_XLOGINSERT_LOCKS; i++) { LWLockInitialize(&WALInsertLocks[i].l.lock, LWTRANCHE_WAL_INSERT); - WALInsertLocks[i].l.insertingAt = InvalidXLogRecPtr; + pg_atomic_init_u64(&WALInsertLocks[i].l.insertingAt, InvalidXLogRecPtr); WALInsertLocks[i].l.lastImportantAt = InvalidXLogRecPtr; } diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 01d738f306..ffa865eb28 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -1547,9 +1547,8 @@ LWLockAcquireOrWait(LWLock *lock, LWLockMode mode) * *result is set to true if the lock was free, and false otherwise. */ static bool -LWLockConflictsWithVar(LWLock *lock, - uint64 *valptr, uint64 oldval, uint64 *newval, - bool *result) +LWLockConflictsWithVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, + uint64 *newval, bool *result) { bool mustwait; uint64 value; @@ -1572,13 +1571,10 @@ LWLockConflictsWithVar(LWLock *lock, *result = false; /* - * Read value using the lwlock's wait list lock, as we can't generally - * rely on atomic 64 bit reads/stores. TODO: On platforms with a way to - * do atomic 64 bit reads/writes the spinlock should be optimized away. + * Reading this value atomically is safe even on platforms where uint64 + * cannot be read without observing a torn value. */ - LWLockWaitListLock(lock); - value = *valptr; - LWLockWaitListUnlock(lock); + value = pg_atomic_read_u64(valptr); if (value != oldval) { @@ -1607,7 +1603,8 @@ LWLockConflictsWithVar(LWLock *lock, * in shared mode, returns 'true'. */ bool -LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval) +LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, + uint64 *newval) { PGPROC *proc = MyProc; int extraWaits = 0; @@ -1735,29 +1732,32 @@ LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval) * LWLockUpdateVar - Update a variable and wake up waiters atomically * * Sets *valptr to 'val', and wakes up all processes waiting for us with - * LWLockWaitForVar(). Setting the value and waking up the processes happen - * atomically so that any process calling LWLockWaitForVar() on the same lock - * is guaranteed to see the new value, and act accordingly. + * LWLockWaitForVar(). It first sets the value atomically and then wakes up + * waiting processes so that any process calling LWLockWaitForVar() on the same + * lock is guaranteed to see the new value, and act accordingly. * * The caller must be holding the lock in exclusive mode. */ void -LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val) +LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) { proclist_head wakeup; proclist_mutable_iter iter; PRINT_LWDEBUG("LWLockUpdateVar", lock, LW_EXCLUSIVE); + /* + * Note that pg_atomic_exchange_u64 is a full barrier, so we're guaranteed + * that the variable is updated before waking up waiters. + */ + pg_atomic_exchange_u64(valptr, val); + proclist_init(&wakeup); LWLockWaitListLock(lock); Assert(pg_atomic_read_u32(&lock->state) & LW_VAL_EXCLUSIVE); - /* Update the lock's value */ - *valptr = val; - /* * See if there are any LW_WAIT_UNTIL_FREE waiters that need to be woken * up. They are always in the front of the queue. @@ -1873,17 +1873,13 @@ LWLockRelease(LWLock *lock) * LWLockReleaseClearVar - release a previously acquired lock, reset variable */ void -LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val) +LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) { - LWLockWaitListLock(lock); - /* - * Set the variable's value before releasing the lock, that prevents race - * a race condition wherein a new locker acquires the lock, but hasn't yet - * set the variables value. + * Note that pg_atomic_exchange_u64 is a full barrier, so we're guaranteed + * that the variable is updated before releasing the lock. */ - *valptr = val; - LWLockWaitListUnlock(lock); + pg_atomic_exchange_u64(valptr, val); LWLockRelease(lock); } diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index 34169e5889..d77410bdea 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -129,14 +129,14 @@ extern bool LWLockAcquire(LWLock *lock, LWLockMode mode); extern bool LWLockConditionalAcquire(LWLock *lock, LWLockMode mode); extern bool LWLockAcquireOrWait(LWLock *lock, LWLockMode mode); extern void LWLockRelease(LWLock *lock); -extern void LWLockReleaseClearVar(LWLock *lock, uint64 *valptr, uint64 val); +extern void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val); extern void LWLockReleaseAll(void); extern bool LWLockHeldByMe(LWLock *lock); extern bool LWLockAnyHeldByMe(LWLock *lock, int nlocks, size_t stride); extern bool LWLockHeldByMeInMode(LWLock *lock, LWLockMode mode); -extern bool LWLockWaitForVar(LWLock *lock, uint64 *valptr, uint64 oldval, uint64 *newval); -extern void LWLockUpdateVar(LWLock *lock, uint64 *valptr, uint64 val); +extern bool LWLockWaitForVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 oldval, uint64 *newval); +extern void LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val); extern Size LWLockShmemSize(void); extern void CreateLWLocks(void); -- 2.34.1