On Tue, Jul 25, 2023 at 04:43:16PM +0900, Michael Paquier wrote: > 0001 has been now applied. I have done more tests while looking at > this patch since yesterday and was surprised to see higher TPS numbers > on HEAD with the same tests as previously, and the patch was still > shining with more than 256 clients.
I found this code when searching for callers that use atomic exchanges as atomic writes with barriers (for a separate thread [0]). Can't we use pg_atomic_write_u64() here since the locking functions that follow should serve as barriers? I've attached a patch to demonstrate what I'm thinking. This might be more performant, although maybe less so after commit 64b1fb5. Am I missing something obvious here? If not, I might rerun the benchmarks to see whether it makes any difference. [0] https://www.postgresql.org/message-id/flat/20231110205128.GB1315705%40nathanxps13 -- Nathan Bossart Amazon Web Services: https://aws.amazon.com
diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c index 315a78cda9..b43972bd2e 100644 --- a/src/backend/storage/lmgr/lwlock.c +++ b/src/backend/storage/lmgr/lwlock.c @@ -1752,10 +1752,10 @@ LWLockUpdateVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) 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. + * We rely on the barrier provided by LWLockWaitListLock() to ensure the + * variable is updated before waking up waiters. */ - pg_atomic_exchange_u64(valptr, val); + pg_atomic_write_u64(valptr, val); proclist_init(&wakeup); @@ -1881,10 +1881,10 @@ void LWLockReleaseClearVar(LWLock *lock, pg_atomic_uint64 *valptr, uint64 val) { /* - * Note that pg_atomic_exchange_u64 is a full barrier, so we're guaranteed - * that the variable is updated before releasing the lock. + * We rely on the barrier provided by LWLockRelease() to ensure the + * variable is updated before releasing the lock. */ - pg_atomic_exchange_u64(valptr, val); + pg_atomic_write_u64(valptr, val); LWLockRelease(lock); }