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);
}
