public inbox for [email protected]
help / color / mirror / Atom feedFrom: Fujii Masao <[email protected]>
To: [email protected]
Subject: Refactor GetLockStatusData() by skipping unused backends and groups
Date: Mon, 21 Oct 2024 09:19:49 +0900
Message-ID: <[email protected]> (raw)
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 <[email protected]>
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
Attachments:
[text/plain] v1-0001-Refactor-GetLockStatusData-to-skip-backends-group.patch (3.4K, ../[email protected]/2-v1-0001-Refactor-GetLockStatusData-to-skip-backends-group.patch)
download | inline diff:
From 2f4c1f51ccd32981a1bfefe04468e630cb0bd63c Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
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
view thread (2+ messages)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: Refactor GetLockStatusData() by skipping unused backends and groups
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox