public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v2] Narrow the scope of the variable outputstr in logicalrep_write_tuple 2+ messages / 2 participants [nested] [flat]
* [PATCH v2] Narrow the scope of the variable outputstr in logicalrep_write_tuple @ 2021-01-18 08:04 Japin Li <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Japin Li @ 2021-01-18 08:04 UTC (permalink / raw) --- src/backend/replication/logical/proto.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c index 62275ebabe..f2c85cabb5 100644 --- a/src/backend/replication/logical/proto.c +++ b/src/backend/replication/logical/proto.c @@ -493,7 +493,6 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple, bool binar HeapTuple typtup; Form_pg_type typclass; Form_pg_attribute att = TupleDescAttr(desc, i); - char *outputstr; if (att->attisdropped || att->attgenerated) continue; @@ -537,6 +536,8 @@ logicalrep_write_tuple(StringInfo out, Relation rel, HeapTuple tuple, bool binar } else { + char *outputstr; + pq_sendbyte(out, LOGICALREP_COLUMN_TEXT); outputstr = OidOutputFunctionCall(typclass->typoutput, values[i]); pq_sendcountedtext(out, outputstr, strlen(outputstr), false); -- 2.30.0 --=-=-=-- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Refactor GetLockStatusData() by skipping unused backends and groups @ 2024-10-21 00:19 Fujii Masao <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Fujii Masao @ 2024-10-21 00:19 UTC (permalink / raw) To: pgsql-hackers 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 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-10-21 00:19 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2021-01-18 08:04 [PATCH v2] Narrow the scope of the variable outputstr in logicalrep_write_tuple Japin Li <[email protected]> 2024-10-21 00:19 Refactor GetLockStatusData() by skipping unused backends and groups Fujii Masao <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox