On Fri, Mar 12, 2021 at 12:31 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > ProcSendSignal(pid) searches the ProcArray for the given pid and then > sets that backend's procLatch. It has only two users: UnpinBuffer() > and ReleasePredicateLocks(). In both cases, we could just as easily > have recorded the pgprocno instead, avoiding the locking and the > searching. We'd also be able to drop some special book-keeping for > the startup process, whose pid can't be found via the ProcArray.
Rebased.
From 5c2d78110d5efb61f40cf741948a4b247c0bddaf Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Thu, 11 Mar 2021 23:09:11 +1300 Subject: [PATCH v2] Optimize ProcSendSignal(). Instead of referring to target processes by pid, use pgprocno so that we don't have to scan the ProcArray and keep track of the startup process. Discussion: https://postgr.es/m/CA%2BhUKGLYRyDaneEwz5Uya_OgFLMx5BgJfkQSD%3Dq9HmwsfRRb-w%40mail.gmail.com --- src/backend/access/transam/xlog.c | 1 - src/backend/storage/buffer/buf_init.c | 3 +- src/backend/storage/buffer/bufmgr.c | 10 ++--- src/backend/storage/lmgr/predicate.c | 5 ++- src/backend/storage/lmgr/proc.c | 49 ++--------------------- src/include/storage/buf_internals.h | 2 +- src/include/storage/predicate_internals.h | 1 + src/include/storage/proc.h | 6 +-- 8 files changed, 17 insertions(+), 60 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 441a9124cd..6a394f3583 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -7281,7 +7281,6 @@ StartupXLOG(void) */ if (ArchiveRecoveryRequested && IsUnderPostmaster) { - PublishStartupProcessInformation(); EnableSyncRequestForwarding(); SendPostmasterSignal(PMSIGNAL_RECOVERY_STARTED); bgwriterLaunched = true; diff --git a/src/backend/storage/buffer/buf_init.c b/src/backend/storage/buffer/buf_init.c index a299be1043..b9a83c0e9b 100644 --- a/src/backend/storage/buffer/buf_init.c +++ b/src/backend/storage/buffer/buf_init.c @@ -16,6 +16,7 @@ #include "storage/buf_internals.h" #include "storage/bufmgr.h" +#include "storage/proc.h" BufferDescPadded *BufferDescriptors; char *BufferBlocks; @@ -118,7 +119,7 @@ InitBufferPool(void) CLEAR_BUFFERTAG(buf->tag); pg_atomic_init_u32(&buf->state, 0); - buf->wait_backend_pid = 0; + buf->wait_backend_pgprocno = INVALID_PGPROCNO; buf->buf_id = i; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 4b296a22c4..ab3506f05b 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -1896,11 +1896,11 @@ UnpinBuffer(BufferDesc *buf, bool fixOwner) BUF_STATE_GET_REFCOUNT(buf_state) == 1) { /* we just released the last pin other than the waiter's */ - int wait_backend_pid = buf->wait_backend_pid; + int wait_backend_pgprocno = buf->wait_backend_pgprocno; buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); - ProcSendSignal(wait_backend_pid); + ProcSendSignal(wait_backend_pgprocno); } else UnlockBufHdr(buf, buf_state); @@ -4007,7 +4007,7 @@ UnlockBuffers(void) * got a cancel/die interrupt before getting the signal. */ if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - buf->wait_backend_pid == MyProcPid) + buf->wait_backend_pgprocno == MyProc->pgprocno) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(buf, buf_state); @@ -4143,7 +4143,7 @@ LockBufferForCleanup(Buffer buffer) LockBuffer(buffer, BUFFER_LOCK_UNLOCK); elog(ERROR, "multiple backends attempting to wait for pincount 1"); } - bufHdr->wait_backend_pid = MyProcPid; + bufHdr->wait_backend_pgprocno = MyProc->pgprocno; PinCountWaitBuf = bufHdr; buf_state |= BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); @@ -4214,7 +4214,7 @@ LockBufferForCleanup(Buffer buffer) */ buf_state = LockBufHdr(bufHdr); if ((buf_state & BM_PIN_COUNT_WAITER) != 0 && - bufHdr->wait_backend_pid == MyProcPid) + bufHdr->wait_backend_pgprocno == MyProc->pgprocno) buf_state &= ~BM_PIN_COUNT_WAITER; UnlockBufHdr(bufHdr, buf_state); diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c index d493aeef0f..c151e0c30c 100644 --- a/src/backend/storage/lmgr/predicate.c +++ b/src/backend/storage/lmgr/predicate.c @@ -1876,6 +1876,7 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot, sxact->finishedBefore = InvalidTransactionId; sxact->xmin = snapshot->xmin; sxact->pid = MyProcPid; + sxact->pgprocno = MyProc->pgprocno; SHMQueueInit(&(sxact->predicateLocks)); SHMQueueElemInit(&(sxact->finishedLink)); sxact->flags = 0; @@ -3669,7 +3670,7 @@ ReleasePredicateLocks(bool isCommit, bool isReadOnlySafe) */ if (SxactIsDeferrableWaiting(roXact) && (SxactIsROUnsafe(roXact) || SxactIsROSafe(roXact))) - ProcSendSignal(roXact->pid); + ProcSendSignal(roXact->pgprocno); possibleUnsafeConflict = nextConflict; } @@ -5006,6 +5007,7 @@ PostPrepare_PredicateLocks(TransactionId xid) Assert(SxactIsPrepared(MySerializableXact)); MySerializableXact->pid = 0; + MySerializableXact->pgprocno = INVALID_PGPROCNO; hash_destroy(LocalPredicateLockHash); LocalPredicateLockHash = NULL; @@ -5081,6 +5083,7 @@ predicatelock_twophase_recover(TransactionId xid, uint16 info, sxact->vxid.backendId = InvalidBackendId; sxact->vxid.localTransactionId = (LocalTransactionId) xid; sxact->pid = 0; + sxact->pgprocno = INVALID_PGPROCNO; /* a prepared xact hasn't committed yet */ sxact->prepareSeqNo = RecoverySerCommitSeqNo; diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 2575ea1ca0..a8934fdae8 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -177,8 +177,6 @@ InitProcGlobal(void) ProcGlobal->autovacFreeProcs = NULL; ProcGlobal->bgworkerFreeProcs = NULL; ProcGlobal->walsenderFreeProcs = NULL; - ProcGlobal->startupProc = NULL; - ProcGlobal->startupProcPid = 0; ProcGlobal->startupBufferPinWaitBufId = -1; ProcGlobal->walwriterLatch = NULL; ProcGlobal->checkpointerLatch = NULL; @@ -624,21 +622,6 @@ InitAuxiliaryProcess(void) on_shmem_exit(AuxiliaryProcKill, Int32GetDatum(proctype)); } -/* - * Record the PID and PGPROC structures for the Startup process, for use in - * ProcSendSignal(). See comments there for further explanation. - */ -void -PublishStartupProcessInformation(void) -{ - SpinLockAcquire(ProcStructLock); - - ProcGlobal->startupProc = MyProc; - ProcGlobal->startupProcPid = MyProcPid; - - SpinLockRelease(ProcStructLock); -} - /* * Used from bufmgr to share the value of the buffer that Startup waits on, * or to reset the value to "not waiting" (-1). This allows processing @@ -1903,38 +1886,12 @@ ProcWaitForSignal(uint32 wait_event_info) } /* - * ProcSendSignal - send a signal to a backend identified by PID + * ProcSendSignal - send a signal to a backend identified by pgprocno */ void -ProcSendSignal(int pid) +ProcSendSignal(int pgprocno) { - PGPROC *proc = NULL; - - if (RecoveryInProgress()) - { - SpinLockAcquire(ProcStructLock); - - /* - * Check to see whether it is the Startup process we wish to signal. - * This call is made by the buffer manager when it wishes to wake up a - * process that has been waiting for a pin in so it can obtain a - * cleanup lock using LockBufferForCleanup(). Startup is not a normal - * backend, so BackendPidGetProc() will not return any pid at all. So - * we remember the information for this special case. - */ - if (pid == ProcGlobal->startupProcPid) - proc = ProcGlobal->startupProc; - - SpinLockRelease(ProcStructLock); - } - - if (proc == NULL) - proc = BackendPidGetProc(pid); - - if (proc != NULL) - { - SetLatch(&proc->procLatch); - } + SetLatch(&ProcGlobal->allProcs[pgprocno].procLatch); } /* diff --git a/src/include/storage/buf_internals.h b/src/include/storage/buf_internals.h index 33fcaf5c9a..9b634616e4 100644 --- a/src/include/storage/buf_internals.h +++ b/src/include/storage/buf_internals.h @@ -187,7 +187,7 @@ typedef struct BufferDesc /* state of the tag, containing flags, refcount and usagecount */ pg_atomic_uint32 state; - int wait_backend_pid; /* backend PID of pin-count waiter */ + int wait_backend_pgprocno; /* backend of pin-count waiter */ int freeNext; /* link in freelist chain */ LWLock content_lock; /* to lock access to buffer contents */ } BufferDesc; diff --git a/src/include/storage/predicate_internals.h b/src/include/storage/predicate_internals.h index 104f560d38..f154b3c3b8 100644 --- a/src/include/storage/predicate_internals.h +++ b/src/include/storage/predicate_internals.h @@ -113,6 +113,7 @@ typedef struct SERIALIZABLEXACT TransactionId xmin; /* the transaction's snapshot xmin */ uint32 flags; /* OR'd combination of values defined below */ int pid; /* pid of associated process */ + int pgprocno; /* pgprocno of associated process */ } SERIALIZABLEXACT; #define SXACT_FLAG_COMMITTED 0x00000001 /* already committed */ diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index be67d8a861..7c1e2efe30 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -352,9 +352,6 @@ typedef struct PROC_HDR Latch *checkpointerLatch; /* Current shared estimate of appropriate spins_per_delay value */ int spins_per_delay; - /* The proc of the Startup process, since not in ProcArray */ - PGPROC *startupProc; - int startupProcPid; /* Buffer id of the buffer that Startup process waits for pin on, or -1 */ int startupBufferPinWaitBufId; } PROC_HDR; @@ -395,7 +392,6 @@ extern void InitProcess(void); extern void InitProcessPhase2(void); extern void InitAuxiliaryProcess(void); -extern void PublishStartupProcessInformation(void); extern void SetStartupBufferPinWaitBufId(int bufid); extern int GetStartupBufferPinWaitBufId(void); @@ -411,7 +407,7 @@ extern bool IsWaitingForLock(void); extern void LockErrorCleanup(void); extern void ProcWaitForSignal(uint32 wait_event_info); -extern void ProcSendSignal(int pid); +extern void ProcSendSignal(int pgprocno); extern PGPROC *AuxiliaryPidGetProc(int pid); -- 2.30.2
