From 139f5d67266c94f96f3e88b056c297307efacb32 Mon Sep 17 00:00:00 2001 From: Alexandre Felipe Date: Mon, 17 Aug 2026 18:41:39 +0100 Subject: [PATCH 2/5] LWLock fast paths inline LWLockAcquire(LWLock* , LWLockMode) +- extern LWLockAcquireShared(LWLock*) | +- fast LWLockAcquireCommon(LWLock*, LW_SHARED) | +- fast LWLockAttemptLock(LWLock*, LW_SHARED) | +- ... slow path +- extern LWLockACquireExclusive(LWLock*) +- fast LWLockAcquireCommon(LWLock*, LW_EXCLUSIVE) +- fast LWLockAttemptLock(LWLock*, LW_EXCLUSIVE) +- ... slow path LWLock tracking only stores (LWLock*), the mode can be inferred at any time from the LWLock state. since at any time either the number of exclusive locks or the number of shared locks on a LWLock must be zero. If a lock is currently held, one of them will be non-zero. Is reading the mode from the lock state in the hot path a concern? This patch provides a LWLockRelease(LWLock, LWLockMode) that uses an architecture similar to LWLockAcquire. inline LWLockReleaseMode(LWLock* , LWLockMode) +- extern LWLockReleaseShared(LWLock*) | +- if(lock is at the top of held_lwlocks) | | fast LWLockReleaseCommon(LWLock*, LW_SHARED) | +- ... slow path +- extern LWLockReleaseExclusive(LWLock*) +- if(lock is at the top of held_lwlocks) | fast LWLockReleaseCommon(LWLock*, LW_EXCLUSIVE) +- ... slow path Is the top of the stack check in the hot path a concern? This patch provides a LWLockReleaseLast(LWLock*, LWLockMode), that only checks for the existence of a lock. inline LWLockReleaseMode(LWLock* , LWLockMode) +- extern LWLockReleaseShared(LWLock*) | +- fast LWLockReleaseCommon(LWLock*, LW_SHARED) +- extern LWLockReleaseExclusive(LWLock*) +- fast LWLockReleaseCommon(LWLock*, LW_EXCLUSIVE) But this can easily be removed if it raises safety concerns. Example results ==> running lwlock/query.sql (n=128 rounds=1000000) Output format is aligned. op | avg | min | q1 | med | q3 | max | std -------------+-------+------+-------+-------+-------+--------+------ spin-lock | 14.75 | 7.81 | 12.70 | 15.63 | 16.28 | 68.69 | 4.28 LWLock-ex | 6.96 | 5.86 | 6.51 | 6.52 | 6.84 | 93.75 | 3.98 LWLock-sh | 6.86 | 5.86 | 6.19 | 6.52 | 6.84 | 130.21 | 4.01 lw-ex-mode | 6.82 | 5.86 | 6.19 | 6.51 | 6.84 | 103.20 | 3.95 lw-ex-last | 6.77 | 5.86 | 6.19 | 6.51 | 6.84 | 116.53 | 3.65 lw-sh-mode | 6.69 | 5.86 | 6.19 | 6.51 | 6.84 | 29.95 | 1.41 lw-sh-last | 6.96 | 5.86 | 6.19 | 6.51 | 6.84 | 320.96 | 9.98 LWLock-cond | 6.01 | 5.20 | 5.53 | 5.86 | 5.86 | 17.25 | 1.04 nop | 0.54 | 0.00 | 0.33 | 0.33 | 0.66 | 2.93 | 0.30 --- src/backend/storage/lmgr/lwlock.c | 233 +++++++++++++++++---- src/include/storage/lwlock.h | 34 ++- src/test/modules/microbench/lwlock/bench.c | 36 ++++ 3 files changed, 262 insertions(+), 41 deletions(-) diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 82a1d4d2e26..f572b4a4f84 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -1146,8 +1146,8 @@ LWLockDequeueSelf(LWLock *lock) * * Side effect: cancel/die interrupts are held off until lock release. */ -bool -LWLockAcquire(LWLock *lock, LWLockMode mode) +static pg_always_inline bool +LWLockAcquireCommon(LWLock *lock, LWLockMode mode) { PGPROC *proc = MyProc; bool result = true; @@ -1310,6 +1310,30 @@ LWLockAcquire(LWLock *lock, LWLockMode mode) return result; } +/* + * LWLockAcquireExclusive - acquire an exclusive lock + * + * extern function specialised for LW_SHARED at compile time + * can be called via LWLockAcquire(lock, LW_EXCLUSIVE) + */ +extern bool +LWLockAcquireExclusive(LWLock *lock) +{ + return LWLockAcquireCommon(lock, LW_EXCLUSIVE); +} + +/* + * LWLockAcquireShared - acquire a shared lock + * + * Extern function specialised for LW_SHARED at compile time. + * can be called via LWLockAcquire(lock, LW_SHARED) + */ +extern bool +LWLockAcquireShared(LWLock *lock) +{ + return LWLockAcquireCommon(lock, LW_SHARED); +} + /* * LWLockConditionalAcquire - acquire a lightweight lock in the specified mode * @@ -1757,36 +1781,28 @@ LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) /* - * LWLockRelease - release a previously acquired lock + * Cold path for waking waiters - kept out of line to enable shrink-wrapping. + */ +static pg_noinline void +LWLockReleaseWakeWaiters(LWLock *lock) +{ + /* XXX: remove before commit? */ + LOG_LWDEBUG("LWLockRelease", lock, "releasing waiters"); + LWLockWakeup(lock); + RESUME_INTERRUPTS(); +} + +/* + * LWLockReleaseInternal - core release logic, always inlined for constant folding. * * NB: This will leave lock->owner pointing to the current backend (if * LOCK_DEBUG is set). This is somewhat intentional, as it makes it easier to * debug cases of missing wakeups during lock release. */ -void -LWLockRelease(LWLock *lock) +static pg_always_inline void +LWLockReleaseInternal(LWLock *lock, LWLockMode mode) { - LWLockMode mode; uint32 oldstate; - bool check_waiters; - int i; - - /* - * Remove lock from list of locks held. Usually, but not always, it will - * be the latest-acquired lock; so search array backwards. - */ - for (i = num_held_lwlocks; --i >= 0;) - if (lock == held_lwlocks[i].lock) - break; - - if (i < 0) - elog(ERROR, "lock %s is not held", T_NAME(lock)); - - mode = held_lwlocks[i].mode; - - num_held_lwlocks--; - for (; i < num_held_lwlocks; i++) - held_lwlocks[i] = held_lwlocks[i + 1]; PRINT_LWDEBUG("LWLockRelease", lock, mode); @@ -1807,30 +1823,167 @@ LWLockRelease(LWLock *lock) /* * Check if we're still waiting for backends to get scheduled, if so, - * don't wake them up again. + * don't wake them up again. As waking up waiters requires the spinlock + * to be acquired, only do so if necessary. */ - if ((oldstate & LW_FLAG_HAS_WAITERS) && - !(oldstate & LW_FLAG_WAKE_IN_PROGRESS) && - (oldstate & LW_LOCK_MASK) == 0) - check_waiters = true; - else - check_waiters = false; + if (unlikely((oldstate & LW_FLAG_HAS_WAITERS) && + !(oldstate & LW_FLAG_WAKE_IN_PROGRESS) && + (oldstate & LW_LOCK_MASK) == 0)) + { + LWLockReleaseWakeWaiters(lock); + return; + } + + /* + * Now okay to allow cancel/die interrupts. + */ + RESUME_INTERRUPTS(); +} + +/* + * LWLockRelease - generic lock release + * + * extern function without compile-time mode specialisation + */ +extern void +LWLockRelease(LWLock *lock) +{ + int i = num_held_lwlocks - 1; + LWLockMode mode; /* - * As waking up waiters requires the spinlock to be acquired, only do so - * if necessary. + * Fast path: check if this is the most recently acquired lock. */ - if (check_waiters) + if (likely(i >= 0 && held_lwlocks[i].lock == lock)) { - /* XXX: remove before commit? */ - LOG_LWDEBUG("LWLockRelease", lock, "releasing waiters"); - LWLockWakeup(lock); + mode = held_lwlocks[i].mode; + num_held_lwlocks = i; + LWLockReleaseInternal(lock, mode); + return; } /* - * Now okay to allow cancel/die interrupts. + * Slow path: search array backwards to find the lock. */ - RESUME_INTERRUPTS(); + + for (i = num_held_lwlocks; --i >= 0;) + if (lock == held_lwlocks[i].lock) + break; + + if (i < 0) + elog(ERROR, "lock %s is not held", T_NAME(lock)); + + mode = held_lwlocks[i].mode; + + num_held_lwlocks--; + for (; i < num_held_lwlocks; i++) + held_lwlocks[i] = held_lwlocks[i + 1]; + + LWLockReleaseInternal(lock, mode); +} + +/* + * LWLockReleaseCommon - release a lock with known mode + * + * Potentially faster than LWLockRelease, doesn't have to + * read the mode and logic can be simplified if passed mode + * is a compile time constant. + * + * Inlined in LWLockReleaseExclusive and LWLockRelelaseShared + */ +static pg_always_inline void +LWLockReleaseCommon(LWLock *lock, LWLockMode mode) +{ + int i = num_held_lwlocks - 1; + + if (likely(i >= 0 && held_lwlocks[i].lock == lock)) + { + Assert(held_lwlocks[i].mode == mode); + num_held_lwlocks = i; + LWLockReleaseInternal(lock, mode); + return; + } + + + for (;i >= 0; --i) + if (lock == held_lwlocks[i].lock) + break; + + if (i < 0) + elog(ERROR, "lock %s is not held", T_NAME(lock)); + + Assert(held_lwlocks[i].mode == mode); + + num_held_lwlocks--; + for (; i < num_held_lwlocks; i++) + held_lwlocks[i] = held_lwlocks[i + 1]; + + LWLockReleaseInternal(lock, mode); +} +/* + * LWLockReleaseExclusive + * + * Extern function specialised for LW_EXCLUSIVE at compile time + * Can be called via LWLockReleaseMode(LW_EXCLUSIVE) + */ +extern void +LWLockReleaseExclusive(LWLock *lock) +{ + LWLockReleaseCommon(lock, LW_EXCLUSIVE); +} + +/* + * LWLockReleaseShared + * + * Extern function specialised for LW_SHARED at compile time + * Can be called via LWLockReleaseMode(LW_SHARED) + */ +extern void +LWLockReleaseShared(LWLock *lock) +{ + LWLockReleaseCommon(lock, LW_SHARED); +} + + +/* + * LWLockReleaseLastCommon - release a lock at the top of the stack + * + * Static function to be inlined in LWLockReleaseLastShared + * and LWLockReleaseLastExclusive by the compiler. + */ +static pg_always_inline void +LWLockReleaseLastCommon(LWLock *lock, LWLockMode mode) +{ + Assert(num_held_lwlocks > 0); + Assert(held_lwlocks[num_held_lwlocks - 1].lock == lock); + Assert(held_lwlocks[num_held_lwlocks - 1].mode == mode); + + num_held_lwlocks--; + LWLockReleaseInternal(lock, mode); +} + +/* + * LWLockReleaseLastExclusive: + * + * extern function specialised for LW_EXCLUSIVE mode at compile time + * Can be called via LWLockReleaseLast(LW_EXCLUSIVE) + */ +extern void +LWLockReleaseLastExclusive(LWLock *lock) +{ + LWLockReleaseLastCommon(lock, LW_EXCLUSIVE); +} + +/* + * LWLockReleaseLastShared: LW_SHARED mode LWLock release + * + * extern function specialized for LW_SHARED mode at compile time + * Can be called via LWLockReleaseLast(LW_EXCLUSIVE) + */ +extern void +LWLockReleaseLastShared(LWLock *lock) +{ + LWLockReleaseLastCommon(lock, LW_SHARED); } /* diff --git a/src/include/storage/lwlock.h b/src/include/storage/lwlock.h index efa5b427e9f..aa113b7ddb2 100644 --- a/src/include/storage/lwlock.h +++ b/src/include/storage/lwlock.h @@ -113,10 +113,42 @@ typedef enum LWLockMode extern PGDLLIMPORT bool Trace_lwlocks; #endif -extern bool LWLockAcquire(LWLock *lock, LWLockMode mode); +extern bool LWLockAcquireExclusive(LWLock *lock); +extern bool LWLockAcquireShared(LWLock *lock); + +inline bool LWLockAcquire(LWLock *lock, LWLockMode mode) +{ + if(mode == LW_EXCLUSIVE) + return LWLockAcquireExclusive(lock); + else + return LWLockAcquireShared(lock); +} + extern bool LWLockConditionalAcquire(LWLock *lock, LWLockMode mode); extern bool LWLockAcquireOrWait(LWLock *lock, LWLockMode mode); extern void LWLockRelease(LWLock *lock); + +extern void LWLockReleaseExclusive(LWLock *lock); +extern void LWLockReleaseShared(LWLock *lock); +inline void LWLockReleaseMode(LWLock *lock, LWLockMode mode) +{ + if(mode == LW_EXCLUSIVE) + LWLockReleaseExclusive(lock); + else + LWLockReleaseShared(lock); +} + +extern void LWLockReleaseLastExclusive(LWLock *lock); +extern void LWLockReleaseLastShared(LWLock *lock); +inline void LWLockReleaseLast(LWLock *lock, LWLockMode mode) +{ + if(mode == LW_EXCLUSIVE) + LWLockReleaseLastExclusive(lock); + else + LWLockReleaseLastShared(lock); +} + + extern void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val); extern void LWLockReleaseAll(void); extern bool LWLockHeldByMe(LWLock *lock); diff --git a/src/test/modules/microbench/lwlock/bench.c b/src/test/modules/microbench/lwlock/bench.c index 33d75770a0d..d15939801d7 100644 --- a/src/test/modules/microbench/lwlock/bench.c +++ b/src/test/modules/microbench/lwlock/bench.c @@ -99,6 +99,42 @@ bench_lwlock(PG_FUNCTION_ARGS) } END_TIMING; + BEGIN_TIMING("lw-ex-mode", n) + { + LWLock *lock = locks[i]; + + LWLockAcquire(lock, LW_EXCLUSIVE); + LWLockReleaseMode(lock, LW_EXCLUSIVE); + } + END_TIMING; + + BEGIN_TIMING("lw-ex-last", n) + { + LWLock *lock = locks[i]; + + LWLockAcquire(lock, LW_EXCLUSIVE); + LWLockReleaseLast(lock, LW_EXCLUSIVE); + } + END_TIMING; + + BEGIN_TIMING("lw-sh-mode", n) + { + LWLock *lock = locks[i]; + + LWLockAcquire(lock, LW_SHARED); + LWLockReleaseMode(lock, LW_SHARED); + } + END_TIMING; + + BEGIN_TIMING("lw-sh-last", n) + { + LWLock *lock = locks[i]; + + LWLockAcquire(lock, LW_SHARED); + LWLockReleaseLast(lock, LW_SHARED); + } + END_TIMING; + BEGIN_TIMING("LWLock-cond", n) { LWLock *lock = locks[i]; -- 2.53.0