Hi,
While reading the fast-path lock code, I noticed that GetLockStatusData()
checks all slots for every backend to gather fast-path lock data. However,
backends with PID=0 don't hold fast-path locks, right? If so, we can
improve efficiency by having GetLockStatusData() skip those backends early.
Additionally, when GetLockStatusData() checks a backend, it currently
goes through all the slots accross its groups. Each group has 16 slots,
so if a backend has 4 groups (this can change depending on
max_locks_per_transaction),
that means checking 64 slots. Instead, we could refactor the function
to skip groups without registered fast-path locks, improving performance.
Since each set of 16 slots is packed into a uint32 variable
(PGPROC->fpLockBits[i]),
it’s easy to check if a group has any fast-path locks.
I've attached a patch that implements these changes. This refactoring is
especially useful when max_connections and max_locks_per_transaction are
set high, as it reduces unnecessary checks across numerous slots.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
From 2f4c1f51ccd32981a1bfefe04468e630cb0bd63c Mon Sep 17 00:00:00 2001
From: Fujii Masao <fu...@postgresql.org>
Date: Sun, 20 Oct 2024 14:51:07 +0900
Subject: [PATCH v1] Refactor GetLockStatusData() to skip backends/groups
without fast-path locks.
Previously, GetLockStatusData() checked all slots for every backend
to gather fast-path lock data, which could be inefficient. This commit
refactors it by skipping backends with PID=0 (since they don't hold
fast-path locks) and skipping groups with no registered fast-path locks,
improving efficiency.
This refactoring is particularly beneficial, for example when
max_connections and max_locks_per_transaction are set high,
as it reduces unnecessary checks across numerous slots.
---
src/backend/storage/lmgr/lock.c | 67 +++++++++++++++++++--------------
1 file changed, 39 insertions(+), 28 deletions(-)
diff --git a/src/backend/storage/lmgr/lock.c b/src/backend/storage/lmgr/lock.c
index 09a8ac1578..4ea47f2d99 100644
--- a/src/backend/storage/lmgr/lock.c
+++ b/src/backend/storage/lmgr/lock.c
@@ -3731,44 +3731,55 @@ GetLockStatusData(void)
for (i = 0; i < ProcGlobal->allProcCount; ++i)
{
PGPROC *proc = &ProcGlobal->allProcs[i];
- uint32 f;
+
+ /* Skip backends with pid=0, as they don't hold fast-path locks
*/
+ if (proc->pid == 0)
+ continue;
LWLockAcquire(&proc->fpInfoLock, LW_SHARED);
- for (f = 0; f < FP_LOCK_SLOTS_PER_BACKEND; ++f)
+ for (uint32 g = 0; g < FastPathLockGroupsPerBackend; g++)
{
- LockInstanceData *instance;
- uint32 lockbits = FAST_PATH_GET_BITS(proc, f);
-
- /* Skip unallocated slots. */
- if (!lockbits)
+ /* Skip unallocated groups */
+ if (proc->fpLockBits[g] == 0)
continue;
- if (el >= els)
+ for (int j = 0; j < FP_LOCK_SLOTS_PER_GROUP; j++)
{
- els += MaxBackends;
- data->locks = (LockInstanceData *)
- repalloc(data->locks,
sizeof(LockInstanceData) * els);
- }
+ LockInstanceData *instance;
+ uint32 f = FAST_PATH_SLOT(g, j);
+ uint32 lockbits =
FAST_PATH_GET_BITS(proc, f);
- instance = &data->locks[el];
- SET_LOCKTAG_RELATION(instance->locktag,
proc->databaseId,
-
proc->fpRelId[f]);
- instance->holdMask = lockbits <<
FAST_PATH_LOCKNUMBER_OFFSET;
- instance->waitLockMode = NoLock;
- instance->vxid.procNumber = proc->vxid.procNumber;
- instance->vxid.localTransactionId = proc->vxid.lxid;
- instance->pid = proc->pid;
- instance->leaderPid = proc->pid;
- instance->fastpath = true;
+ /* Skip unallocated slots. */
+ if (!lockbits)
+ continue;
- /*
- * Successfully taking fast path lock means there were
no
- * conflicting locks.
- */
- instance->waitStart = 0;
+ if (el >= els)
+ {
+ els += MaxBackends;
+ data->locks = (LockInstanceData *)
+ repalloc(data->locks,
sizeof(LockInstanceData) * els);
+ }
- el++;
+ instance = &data->locks[el];
+ SET_LOCKTAG_RELATION(instance->locktag,
proc->databaseId,
+
proc->fpRelId[f]);
+ instance->holdMask = lockbits <<
FAST_PATH_LOCKNUMBER_OFFSET;
+ instance->waitLockMode = NoLock;
+ instance->vxid.procNumber =
proc->vxid.procNumber;
+ instance->vxid.localTransactionId =
proc->vxid.lxid;
+ instance->pid = proc->pid;
+ instance->leaderPid = proc->pid;
+ instance->fastpath = true;
+
+ /*
+ * Successfully taking fast path lock means
there were no
+ * conflicting locks.
+ */
+ instance->waitStart = 0;
+
+ el++;
+ }
}
if (proc->fpVXIDLock)
--
2.46.2
