Remove superfluous 'pgprocno' field from PGPROC

It was always just the index of the PGPROC entry from the beginning of
the proc array. Introduce a macro to compute it from the pointer
instead.

Reviewed-by: Andres Freund
Discussion: https://www.postgresql.org/message-id/8171f1aa-496f-46a6-afc3-c46fe7a9b407@iki.fi

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 06fc2989bab189bc4227a90e4c29d0852ed15ab8..97f7434da348dca7a4809813267dde831d4c773d 100644 (file)
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -425,6 +425,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
 {
    volatile PROC_HDR *procglobal = ProcGlobal;
    PGPROC     *proc = MyProc;
+   int         pgprocno = MyProcNumber;
    uint32      nextidx;
    uint32      wakeidx;
 
@@ -458,7 +459,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
         * less efficiently.
         */
        if (nextidx != INVALID_PGPROCNO &&
-           ProcGlobal->allProcs[nextidx].clogGroupMemberPage != proc->clogGroupMemberPage)
+           GetPGProcByNumber(nextidx)->clogGroupMemberPage != proc->clogGroupMemberPage)
        {
            /*
             * Ensure that this proc is not a member of any clog group that
@@ -473,7 +474,7 @@ TransactionGroupUpdateXidStatus(TransactionId xid, XidStatus status,
 
        if (pg_atomic_compare_exchange_u32(&procglobal->clogGroupFirst,
                                           &nextidx,
-                                          (uint32) proc->pgprocno))
+                                          (uint32) pgprocno))
            break;
    }
 

diff --git a/src/backend/access/transam/twophase.c b/src/backend/access/transam/twophase.c
index 8426458f7f5d0c48cdc71af5a8a01421fb071a1a..234c8d08ebc0e7149720f5c5d44d38d02e88570b 100644 (file)
--- a/src/backend/access/transam/twophase.c
+++ b/src/backend/access/transam/twophase.c
@@ -284,7 +284,7 @@ TwoPhaseShmemInit(void)
            TwoPhaseState->freeGXacts = &gxacts[i];
 
            /* associate it with a PGPROC assigned by InitProcGlobal */
-           gxacts[i].pgprocno = PreparedXactProcs[i].pgprocno;
+           gxacts[i].pgprocno = GetNumberFromPGProc(&PreparedXactProcs[i]);
 
            /*
             * Assign a unique ID for each dummy proc, so that the range of
@@ -461,7 +461,6 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
 
    /* Initialize the PGPROC entry */
    MemSet(proc, 0, sizeof(PGPROC));
-   proc->pgprocno = gxact->pgprocno;
    dlist_node_init(&proc->links);
    proc->waitStatus = PROC_WAIT_STATUS_OK;
    if (LocalTransactionIdIsValid(MyProc->lxid))
@@ -780,7 +779,7 @@ pg_prepared_xact(PG_FUNCTION_ARGS)
    while (status->array != NULL && status->currIdx < status->ngxacts)
    {
        GlobalTransaction gxact = &status->array[status->currIdx++];
-       PGPROC     *proc = &ProcGlobal->allProcs[gxact->pgprocno];
+       PGPROC     *proc = GetPGProcByNumber(gxact->pgprocno);
        Datum       values[5] = {0};
        bool        nulls[5] = {0};
        HeapTuple   tuple;
@@ -935,7 +934,7 @@ TwoPhaseGetDummyProc(TransactionId xid, bool lock_held)
 {
    GlobalTransaction gxact = TwoPhaseGetGXact(xid, lock_held);
 
-   return &ProcGlobal->allProcs[gxact->pgprocno];
+   return GetPGProcByNumber(gxact->pgprocno);
 }
 
 /************************************************************************/
@@ -1080,7 +1079,7 @@ save_state_data(const void *data, uint32 len)
 void
 StartPrepare(GlobalTransaction gxact)
 {
-   PGPROC     *proc = &ProcGlobal->allProcs[gxact->pgprocno];
+   PGPROC     *proc = GetPGProcByNumber(gxact->pgprocno);
    TransactionId xid = gxact->xid;
    TwoPhaseFileHeader hdr;
    TransactionId *children;
@@ -1539,7 +1538,7 @@ FinishPreparedTransaction(const char *gid, bool isCommit)
     * try to commit the same GID at once.
     */
    gxact = LockGXact(gid, GetUserId());
-   proc = &ProcGlobal->allProcs[gxact->pgprocno];
+   proc = GetPGProcByNumber(gxact->pgprocno);
    xid = gxact->xid;
 
    /*

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 50c347a6795313cfae387abc110c856c5a37f1bf..c1162d55bff8cc97f0e62984c5c350465dcbbe44 100644 (file)
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -1379,7 +1379,7 @@ WALInsertLockAcquire(void)
    static int  lockToTry = -1;
 
    if (lockToTry == -1)
-       lockToTry = MyProc->pgprocno % NUM_XLOGINSERT_LOCKS;
+       lockToTry = MyProcNumber % NUM_XLOGINSERT_LOCKS;
    MyLockNo = lockToTry;
 
    /*

diff --git a/src/backend/postmaster/bgwriter.c b/src/backend/postmaster/bgwriter.c
index f11ce27084eeac095a72ae08fbc8c2adad683db0..6364b16261fe323df019e33af6d3b8420914ba9b 100644 (file)
--- a/src/backend/postmaster/bgwriter.c
+++ b/src/backend/postmaster/bgwriter.c
@@ -326,7 +326,7 @@ BackgroundWriterMain(void)
        if (rc == WL_TIMEOUT && can_hibernate && prev_hibernate)
        {
            /* Ask for notification at next buffer allocation */
-           StrategyNotifyBgWriter(MyProc->pgprocno);
+           StrategyNotifyBgWriter(MyProcNumber);
            /* Sleep ... */
            (void) WaitLatch(MyLatch,
                             WL_LATCH_SET | WL_TIMEOUT | WL_EXIT_ON_PM_DEATH,

diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 02814cd2c8fceefd7cbe495dc9139a3445f05bd4..9c18e4b3efbc698b71925d474e14adbc75e1d00c 100644 (file)
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -242,7 +242,7 @@ PgArchiverMain(void)
     * Advertise our pgprocno so that backends can use our latch to wake us up
     * while we're sleeping.
     */
-   PgArch->pgprocno = MyProc->pgprocno;
+   PgArch->pgprocno = MyProcNumber;
 
    /* Create workspace for pgarch_readyXlog() */
    arch_files = palloc(sizeof(struct arch_files_state));

diff --git a/src/backend/postmaster/walsummarizer.c b/src/backend/postmaster/walsummarizer.c
index e85d4970347b41ddb8d30a08a693412a4c64f9b6..f295eff32f476fdd114522235551126f3b6891ef 100644 (file)
--- a/src/backend/postmaster/walsummarizer.c
+++ b/src/backend/postmaster/walsummarizer.c
@@ -248,7 +248,7 @@ WalSummarizerMain(void)
    /* Advertise ourselves. */
    on_shmem_exit(WalSummarizerShutdown, (Datum) 0);
    LWLockAcquire(WALSummarizerLock, LW_EXCLUSIVE);
-   WalSummarizerCtl->summarizer_pgprocno = MyProc->pgprocno;
+   WalSummarizerCtl->summarizer_pgprocno = MyProcNumber;
    LWLockRelease(WALSummarizerLock);
 
    /* Create and switch to a memory context that we can reset on error. */

diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
index 07575ef31299abdc70562ae50e983fe112bcf5d8..bdf89bbc4dc79e64e66b91362e067859f7a67adb 100644 (file)
--- a/src/backend/storage/buffer/bufmgr.c
+++ b/src/backend/storage/buffer/bufmgr.c
@@ -4780,7 +4780,7 @@ UnlockBuffers(void)
         * got a cancel/die interrupt before getting the signal.
         */
        if ((buf_state & BM_PIN_COUNT_WAITER) != 0 &&
-           buf->wait_backend_pgprocno == MyProc->pgprocno)
+           buf->wait_backend_pgprocno == MyProcNumber)
            buf_state &= ~BM_PIN_COUNT_WAITER;
 
        UnlockBufHdr(buf, buf_state);
@@ -4930,7 +4930,7 @@ LockBufferForCleanup(Buffer buffer)
            LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
            elog(ERROR, "multiple backends attempting to wait for pincount 1");
        }
-       bufHdr->wait_backend_pgprocno = MyProc->pgprocno;
+       bufHdr->wait_backend_pgprocno = MyProcNumber;
        PinCountWaitBuf = bufHdr;
        buf_state |= BM_PIN_COUNT_WAITER;
        UnlockBufHdr(bufHdr, buf_state);
@@ -4994,7 +4994,7 @@ LockBufferForCleanup(Buffer buffer)
         */
        buf_state = LockBufHdr(bufHdr);
        if ((buf_state & BM_PIN_COUNT_WAITER) != 0 &&
-           bufHdr->wait_backend_pgprocno == MyProc->pgprocno)
+           bufHdr->wait_backend_pgprocno == MyProcNumber)
            buf_state &= ~BM_PIN_COUNT_WAITER;
        UnlockBufHdr(bufHdr, buf_state);
 

diff --git a/src/backend/storage/ipc/procarray.c b/src/backend/storage/ipc/procarray.c
index ee2d7f8585a8d01301af23423922c8a5c197724e..dd329a86ef4d57ba2304ac78ad52d9ef53b7e906 100644 (file)
--- a/src/backend/storage/ipc/procarray.c
+++ b/src/backend/storage/ipc/procarray.c
@@ -468,6 +468,7 @@ CreateSharedProcArray(void)
 void
 ProcArrayAdd(PGPROC *proc)
 {
+   int         pgprocno = GetNumberFromPGProc(proc);
    ProcArrayStruct *arrayP = procArray;
    int         index;
    int         movecount;
@@ -499,13 +500,13 @@ ProcArrayAdd(PGPROC *proc)
     */
    for (index = 0; index < arrayP->numProcs; index++)
    {
-       int         procno PG_USED_FOR_ASSERTS_ONLY = arrayP->pgprocnos[index];
+       int         this_procno = arrayP->pgprocnos[index];
 
-       Assert(procno >= 0 && procno < (arrayP->maxProcs + NUM_AUXILIARY_PROCS));
-       Assert(allProcs[procno].pgxactoff == index);
+       Assert(this_procno >= 0 && this_procno < (arrayP->maxProcs + NUM_AUXILIARY_PROCS));
+       Assert(allProcs[this_procno].pgxactoff == index);
 
        /* If we have found our right position in the array, break */
-       if (arrayP->pgprocnos[index] > proc->pgprocno)
+       if (this_procno > pgprocno)
            break;
    }
 
@@ -523,7 +524,7 @@ ProcArrayAdd(PGPROC *proc)
            &ProcGlobal->statusFlags[index],
            movecount * sizeof(*ProcGlobal->statusFlags));
 
-   arrayP->pgprocnos[index] = proc->pgprocno;
+   arrayP->pgprocnos[index] = GetNumberFromPGProc(proc);
    proc->pgxactoff = index;
    ProcGlobal->xids[index] = proc->xid;
    ProcGlobal->subxidStates[index] = proc->subxidStatus;
@@ -791,6 +792,7 @@ ProcArrayEndTransactionInternal(PGPROC *proc, TransactionId latestXid)
 static void
 ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
 {
+   int         pgprocno = GetNumberFromPGProc(proc);
    PROC_HDR   *procglobal = ProcGlobal;
    uint32      nextidx;
    uint32      wakeidx;
@@ -808,7 +810,7 @@ ProcArrayGroupClearXid(PGPROC *proc, TransactionId latestXid)
 
        if (pg_atomic_compare_exchange_u32(&procglobal->procArrayGroupFirst,
                                           &nextidx,
-                                          (uint32) proc->pgprocno))
+                                          (uint32) pgprocno))
            break;
    }
 

diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c
index e2d6d6852207e2a950adf9484b65575aa301affd..10fdae19dcc07386ee2f31f0843b791f6e50d53c 100644 (file)
--- a/src/backend/storage/lmgr/condition_variable.c
+++ b/src/backend/storage/lmgr/condition_variable.c
@@ -57,7 +57,7 @@ ConditionVariableInit(ConditionVariable *cv)
 void
 ConditionVariablePrepareToSleep(ConditionVariable *cv)
 {
-   int         pgprocno = MyProc->pgprocno;
+   int         pgprocno = MyProcNumber;
 
    /*
     * If some other sleep is already prepared, cancel it; this is necessary
@@ -181,10 +181,10 @@ ConditionVariableTimedSleep(ConditionVariable *cv, long timeout,
         * guarantee not to return spuriously, we'll avoid this obvious case.
         */
        SpinLockAcquire(&cv->mutex);
-       if (!proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink))
+       if (!proclist_contains(&cv->wakeup, MyProcNumber, cvWaitLink))
        {
            done = true;
-           proclist_push_tail(&cv->wakeup, MyProc->pgprocno, cvWaitLink);
+           proclist_push_tail(&cv->wakeup, MyProcNumber, cvWaitLink);
        }
        SpinLockRelease(&cv->mutex);
 
@@ -236,8 +236,8 @@ ConditionVariableCancelSleep(void)
        return false;
 
    SpinLockAcquire(&cv->mutex);
-   if (proclist_contains(&cv->wakeup, MyProc->pgprocno, cvWaitLink))
-       proclist_delete(&cv->wakeup, MyProc->pgprocno, cvWaitLink);
+   if (proclist_contains(&cv->wakeup, MyProcNumber, cvWaitLink))
+       proclist_delete(&cv->wakeup, MyProcNumber, cvWaitLink);
    else
        signaled = true;
    SpinLockRelease(&cv->mutex);
@@ -281,7 +281,7 @@ ConditionVariableSignal(ConditionVariable *cv)
 void
 ConditionVariableBroadcast(ConditionVariable *cv)
 {
-   int         pgprocno = MyProc->pgprocno;
+   int         pgprocno = MyProcNumber;
    PGPROC     *proc = NULL;
    bool        have_sentinel = false;
 

diff --git a/src/backend/storage/lmgr/lwlock.c b/src/backend/storage/lmgr/lwlock.c
index 71677cf031709fd304b0ca55bed28f0e127d9ad4..997857679ede1cd89948d45cca8bda3af299c6a2 100644 (file)
--- a/src/backend/storage/lmgr/lwlock.c
+++ b/src/backend/storage/lmgr/lwlock.c
@@ -1056,9 +1056,9 @@ LWLockQueueSelf(LWLock *lock, LWLockMode mode)
 
    /* LW_WAIT_UNTIL_FREE waiters are always at the front of the queue */
    if (mode == LW_WAIT_UNTIL_FREE)
-       proclist_push_head(&lock->waiters, MyProc->pgprocno, lwWaitLink);
+       proclist_push_head(&lock->waiters, MyProcNumber, lwWaitLink);
    else
-       proclist_push_tail(&lock->waiters, MyProc->pgprocno, lwWaitLink);
+       proclist_push_tail(&lock->waiters, MyProcNumber, lwWaitLink);
 
    /* Can release the mutex now */
    LWLockWaitListUnlock(lock);
@@ -1097,7 +1097,7 @@ LWLockDequeueSelf(LWLock *lock)
     */
    on_waitlist = MyProc->lwWaiting == LW_WS_WAITING;
    if (on_waitlist)
-       proclist_delete(&lock->waiters, MyProc->pgprocno, lwWaitLink);
+       proclist_delete(&lock->waiters, MyProcNumber, lwWaitLink);
 
    if (proclist_is_empty(&lock->waiters) &&
        (pg_atomic_read_u32(&lock->state) & LW_FLAG_HAS_WAITERS) != 0)

diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index eed63a05ed837f4c819baf9765353eabc3241ac0..d62060d58c831437d48c2521f0db2860661c9c67 100644 (file)
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -1830,7 +1830,7 @@ GetSerializableTransactionSnapshotInt(Snapshot snapshot,
    sxact->finishedBefore = InvalidTransactionId;
    sxact->xmin = snapshot->xmin;
    sxact->pid = MyProcPid;
-   sxact->pgprocno = MyProc->pgprocno;
+   sxact->pgprocno = MyProcNumber;
    dlist_init(&sxact->predicateLocks);
    dlist_node_init(&sxact->finishedLink);
    sxact->flags = 0;

diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 1afcbfc052cefa4e2555c3c49d66f6ac9f8a6df6..4aec4a3c5f426c9a08c3b8203d8c3b156c4ea29b 100644 (file)
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -65,6 +65,7 @@ bool      log_lock_waits = false;
 
 /* Pointer to this process's PGPROC struct, if any */
 PGPROC    *MyProc = NULL;
+int            MyProcNumber = INVALID_PGPROCNO;
 
 /*
  * This spinlock protects the freelist of recycled PGPROC structures.
@@ -228,7 +229,6 @@ InitProcGlobal(void)
            InitSharedLatch(&(proc->procLatch));
            LWLockInitialize(&(proc->fpInfoLock), LWTRANCHE_LOCK_FASTPATH);
        }
-       proc->pgprocno = i;
 
        /*
         * Newly created PGPROCs for normal backends, autovacuum and bgworkers
@@ -353,6 +353,7 @@ InitProcess(void)
                (errcode(ERRCODE_TOO_MANY_CONNECTIONS),
                 errmsg("sorry, too many clients already")));
    }
+   MyProcNumber = GetNumberFromPGProc(MyProc);
 
    /*
     * Cross-check that the PGPROC is of the type we expect; if this were not
@@ -566,6 +567,8 @@ InitAuxiliaryProcess(void)
 
    SpinLockRelease(ProcStructLock);
 
+   MyProcNumber = GetNumberFromPGProc(MyProc);
+
    /*
     * Initialize all fields of MyProc, except for those previously
     * initialized by InitProcGlobal.
@@ -907,6 +910,7 @@ ProcKill(int code, Datum arg)
 
    proc = MyProc;
    MyProc = NULL;
+   MyProcNumber = INVALID_PGPROCNO;
    DisownLatch(&proc->procLatch);
 
    procgloballist = proc->procgloballist;
@@ -978,6 +982,7 @@ AuxiliaryProcKill(int code, Datum arg)
 
    proc = MyProc;
    MyProc = NULL;
+   MyProcNumber = INVALID_PGPROCNO;
    DisownLatch(&proc->procLatch);
 
    SpinLockAcquire(ProcStructLock);
@@ -1903,10 +1908,9 @@ BecomeLockGroupMember(PGPROC *leader, int pid)
 
    /*
     * Get lock protecting the group fields.  Note LockHashPartitionLockByProc
-    * accesses leader->pgprocno in a PGPROC that might be free.  This is safe
-    * because all PGPROCs' pgprocno fields are set during shared memory
-    * initialization and never change thereafter; so we will acquire the
-    * correct lock even if the leader PGPROC is in process of being recycled.
+    * calculates the proc number based on the PGPROC slot without looking at
+    * its contents, so we will acquire the correct lock even if the leader
+    * PGPROC is in process of being recycled.
     */
    leader_lwlock = LockHashPartitionLockByProc(leader);
    LWLockAcquire(leader_lwlock, LW_EXCLUSIVE);

diff --git a/src/include/storage/lock.h b/src/include/storage/lock.h
index 00679624f7d8729a1f0c1185ac5776b7f2bbe4cd..ed6071f328669a9ffb9b523a75c89e717a3124f3 100644 (file)
--- a/src/include/storage/lock.h
+++ b/src/include/storage/lock.h
@@ -540,7 +540,7 @@ typedef enum
  * used for a given lock group is determined by the group leader's pgprocno.
  */
 #define LockHashPartitionLockByProc(leader_pgproc) \
-   LockHashPartitionLock((leader_pgproc)->pgprocno)
+   LockHashPartitionLock(GetNumberFromPGProc(leader_pgproc))
 
 /*
  * function prototypes

diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h
index 20d6fa652dc690a3770e57abdfcffe6f0713dbf1..4453c6df87716bd169d084a783ff1d752d7db783 100644 (file)
--- a/src/include/storage/proc.h
+++ b/src/include/storage/proc.h
@@ -194,11 +194,6 @@ struct PGPROC
    int         pgxactoff;      /* offset into various ProcGlobal->arrays with
                                 * data mirrored from this PGPROC */
 
-   int         pgprocno;       /* Number of this PGPROC in
-                                * ProcGlobal->allProcs array. This is set
-                                * once by InitProcGlobal().
-                                * ProcGlobal->allProcs[n].pgprocno == n */
-
    /* These fields are zero while a backend is still starting up: */
    BackendId   backendId;      /* This backend's backend ID (if assigned) */
    Oid         databaseId;     /* OID of database this backend is using */
@@ -307,6 +302,7 @@ struct PGPROC
 
 
 extern PGDLLIMPORT PGPROC *MyProc;
+extern PGDLLIMPORT int MyProcNumber;   /* same as GetNumberFromPGProc(MyProc) */
 
 /*
  * There is one ProcGlobal struct for the whole database cluster.
@@ -410,8 +406,9 @@ extern PGDLLIMPORT PROC_HDR *ProcGlobal;
 
 extern PGDLLIMPORT PGPROC *PreparedXactProcs;
 
-/* Accessor for PGPROC given a pgprocno. */
+/* Accessor for PGPROC given a pgprocno, and vice versa. */
 #define GetPGProcByNumber(n) (&ProcGlobal->allProcs[(n)])
+#define GetNumberFromPGProc(proc) ((proc) - &ProcGlobal->allProcs[0])
 
 /*
  * We set aside some extra PGPROC structures for auxiliary processes,