Hi,
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.
A related idea, saving space in BufferDesc but having to do slightly
more expensive work, would be for UnpinBuffer() to reuse the new
condition variable instead of ProcSendSignal().
From b2f3bf47e11f572a32f24945d10ffb9a7bc47b1f 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] 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.
---
src/backend/access/transam/xlog.c | 1 -
src/backend/storage/buffer/buf_init.c | 2 +-
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, 16 insertions(+), 60 deletions(-)
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 18af3d4120..5ff9bbda27 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -7225,7 +7225,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..9ae17cf4da 100644
--- a/src/backend/storage/buffer/buf_init.c
+++ b/src/backend/storage/buffer/buf_init.c
@@ -118,7 +118,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 4c1d5eceb4..84b3669d8d 100644
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -1818,11 +1818,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);
@@ -3923,7 +3923,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);
@@ -4059,7 +4059,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);
@@ -4130,7 +4130,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 897045ee27..b7c10c73d9 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;
@@ -618,21 +616,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
@@ -1894,38 +1877,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 a777cb64a1..d298ab8a14 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.1
