Improve GetSnapshotData() performance by avoiding indirection for vacuumFlags access.
authorAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
committerAndres Freund <[email protected]>
Tue, 7 Apr 2020 04:28:55 +0000 (21:28 -0700)
src/backend/access/transam/twophase.c
src/backend/commands/analyze.c
src/backend/commands/vacuum.c
src/backend/replication/logical/logical.c
src/backend/replication/slot.c
src/backend/storage/ipc/procarray.c
src/backend/storage/lmgr/deadlock.c
src/backend/storage/lmgr/proc.c
src/include/storage/proc.h

index 8103c5cb71f64d507fb2c7c89330c543212b2df2..06b61605649a06fbb0cde07b0401dbabd7e03ad7 100644 (file)
@@ -466,7 +466,7 @@ MarkAsPreparingGuts(GlobalTransaction gxact, TransactionId xid, const char *gid,
    proc->xidCopy = xid;
    Assert(proc->xmin == InvalidTransactionId);
    proc->delayChkpt = false;
-   pgxact->vacuumFlags = 0;
+   proc->vacuumFlagsCopy = 0;
    proc->pid = 0;
    proc->backendId = InvalidBackendId;
    proc->databaseId = databaseid;
index 7b75945c4a90d8e63554e8a2b00937e93e4fa775..94fc5b635317ee5f51157ae949a8ac3ec873834f 100644 (file)
@@ -250,7 +250,8 @@ analyze_rel(Oid relid, RangeVar *relation,
     * OK, let's do it.  First let other backends know I'm in ANALYZE.
     */
    LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-   MyPgXact->vacuumFlags |= PROC_IN_ANALYZE;
+   MyProc->vacuumFlagsCopy |= PROC_IN_ANALYZE;
+   ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy;
    LWLockRelease(ProcArrayLock);
    pgstat_progress_start_command(PROGRESS_COMMAND_ANALYZE,
                                  RelationGetRelid(onerel));
@@ -285,7 +286,8 @@ analyze_rel(Oid relid, RangeVar *relation,
     * because the vacuum flag is cleared by the end-of-xact code.
     */
    LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-   MyPgXact->vacuumFlags &= ~PROC_IN_ANALYZE;
+   MyProc->vacuumFlagsCopy &= ~PROC_IN_ANALYZE;
+   ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy;
    LWLockRelease(ProcArrayLock);
 }
 
index eb4fd28e06e9e9283d9072a4860ca102e27182b2..8937a9964887a68977ff424ea725c542577f5195 100644 (file)
@@ -1730,9 +1730,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
         * which is probably Not Good.
         */
        LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-       MyPgXact->vacuumFlags |= PROC_IN_VACUUM;
+       MyProc->vacuumFlagsCopy |= PROC_IN_VACUUM;
        if (params->is_wraparound)
-           MyPgXact->vacuumFlags |= PROC_VACUUM_FOR_WRAPAROUND;
+           MyProc->vacuumFlagsCopy |= PROC_VACUUM_FOR_WRAPAROUND;
+       ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy;
        LWLockRelease(ProcArrayLock);
    }
 
index 5adf253583b1cfc7dc4e0586afd834a1f4bf68bd..d5a28821e14796f366d4ec2fd08f24115666dd51 100644 (file)
@@ -163,7 +163,8 @@ StartupDecodingContext(List *output_plugin_options,
    if (!IsTransactionOrTransactionBlock())
    {
        LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-       MyPgXact->vacuumFlags |= PROC_IN_LOGICAL_DECODING;
+       MyProc->vacuumFlagsCopy |= PROC_IN_LOGICAL_DECODING;
+       ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy;
        LWLockRelease(ProcArrayLock);
    }
 
index 47851ec4c1a9945db2f8366d634ccc0fbd16d81e..0dcc6a90e798b4467ce44db864a72373804a5b50 100644 (file)
@@ -468,7 +468,8 @@ ReplicationSlotRelease(void)
 
    /* might not have been set when we've been a plain slot */
    LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
-   MyPgXact->vacuumFlags &= ~PROC_IN_LOGICAL_DECODING;
+   MyProc->vacuumFlagsCopy &= ~PROC_IN_LOGICAL_DECODING;
+   ProcGlobal->vacuumFlags[MyProc->pgxactoff] = MyProc->vacuumFlagsCopy;
    LWLockRelease(ProcArrayLock);
 }
 
index 4386831ff563ff7c5e2e39bb8c4a8af35feea489..ee571459c1a055653b1fea28473f8dae9a9571c7 100644 (file)
@@ -431,9 +431,12 @@ ProcArrayAdd(PGPROC *proc)
            (arrayP->numProcs - index) * sizeof(*arrayP->pgprocnos));
    memmove(&ProcGlobal->xids[index + 1], &ProcGlobal->xids[index],
            (arrayP->numProcs - index) * sizeof(*ProcGlobal->xids));
+   memmove(&ProcGlobal->vacuumFlags[index + 1], &ProcGlobal->vacuumFlags[index],
+           (arrayP->numProcs - index) * sizeof(*ProcGlobal->vacuumFlags));
 
    arrayP->pgprocnos[index] = proc->pgprocno;
    ProcGlobal->xids[index] = proc->xidCopy;
+   ProcGlobal->vacuumFlags[index] = proc->vacuumFlagsCopy;
 
    arrayP->numProcs++;
 
@@ -497,6 +500,7 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
    }
 
    Assert(TransactionIdIsValid(ProcGlobal->xids[proc->pgxactoff] == 0));
+   ProcGlobal->vacuumFlags[proc->pgxactoff] = 0;
 
    for (index = 0; index < arrayP->numProcs; index++)
    {
@@ -507,6 +511,8 @@ ProcArrayRemove(PGPROC *proc, TransactionId latestXid)
                    (arrayP->numProcs - index - 1) * sizeof(*arrayP->pgprocnos));
            memmove(&ProcGlobal->xids[index], &ProcGlobal->xids[index + 1],
                    (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->xids));
+           memmove(&ProcGlobal->vacuumFlags[index], &ProcGlobal->vacuumFlags[index + 1],
+                   (arrayP->numProcs - index - 1) * sizeof(*ProcGlobal->vacuumFlags));
 
            arrayP->pgprocnos[arrayP->numProcs - 1] = -1;   /* for debugging */
            arrayP->numProcs--;
@@ -585,14 +591,23 @@ ProcArrayEndTransaction(PGPROC *proc, TransactionId latestXid)
        Assert(!TransactionIdIsValid(proc->xidCopy));
 
        proc->lxid = InvalidLocalTransactionId;
-       /* must be cleared with xid/xmin: */
-       pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
        proc->xmin = InvalidTransactionId;
        proc->delayChkpt = false; /* be sure this is cleared in abort */
        proc->recoveryConflictPending = false;
 
        Assert(pgxact->nxids == 0);
        Assert(pgxact->overflowed == false);
+
+       /* must be cleared with xid/xmin: */
+       /* avoid unnecessarily dirtying shared cachelines */
+       if (proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK)
+       {
+           Assert(!LWLockHeldByMe(ProcArrayLock));
+           LWLockAcquire(ProcArrayLock, LW_SHARED);
+           proc->vacuumFlagsCopy &= ~PROC_VACUUM_STATE_MASK;
+           ProcGlobal->vacuumFlags[proc->pgxactoff] = proc->vacuumFlagsCopy;
+           LWLockRelease(ProcArrayLock);
+       }
    }
 }
 
@@ -613,12 +628,18 @@ ProcArrayEndTransactionInternal(PGPROC *proc, PGXACT *pgxact,
    ProcGlobal->xids[pgxactoff] = InvalidTransactionId;
    proc->xidCopy = InvalidTransactionId;
    proc->lxid = InvalidLocalTransactionId;
-   /* must be cleared with xid/xmin: */
-   pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
    proc->xmin = InvalidTransactionId;
    proc->delayChkpt = false; /* be sure this is cleared in abort */
    proc->recoveryConflictPending = false;
 
+   /* must be cleared with xid/xmin: */
+   /* avoid unnecessarily dirtying shared cachelines */
+   if (proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK)
+   {
+       proc->vacuumFlagsCopy &= ~PROC_VACUUM_STATE_MASK;
+       ProcGlobal->vacuumFlags[proc->pgxactoff] = proc->vacuumFlagsCopy;
+   }
+
    /* Clear the subtransaction-XID cache too while holding the lock */
    pgxact->nxids = 0;
    pgxact->overflowed = false;
@@ -778,9 +799,8 @@ ProcArrayClearTransaction(PGPROC *proc)
    proc->xmin = InvalidTransactionId;
    proc->recoveryConflictPending = false;
 
-   /* redundant, but just in case */
-   pgxact->vacuumFlags &= ~PROC_VACUUM_STATE_MASK;
-   proc->delayChkpt = false;
+   Assert(!(proc->vacuumFlagsCopy & PROC_VACUUM_STATE_MASK));
+   Assert(!proc->delayChkpt);
 
    /* Clear the subtransaction-XID cache too */
    pgxact->nxids = 0;
@@ -1523,7 +1543,7 @@ ComputeTransactionHorizons(ComputedHorizons *h)
    {
        int         pgprocno = arrayP->pgprocnos[index];
        PGPROC     *proc = &allProcs[pgprocno];
-       PGXACT     *pgxact = &allPgXact[pgprocno];
+       int8        vacuumFlags = ProcGlobal->vacuumFlags[index];
        TransactionId xid;
        TransactionId xmin;
 
@@ -1540,10 +1560,6 @@ ComputeTransactionHorizons(ComputedHorizons *h)
         */
        xmin = TransactionIdOlder(xmin, xid);
 
-       /* if neither is set, this proc doesn't influence the horizon */
-       if (!TransactionIdIsValid(xmin))
-           continue;
-
        /*
         * Don't ignore any procs when determining the accessible
         * horizon. While slots always should ensure logical decoding backends
@@ -1558,7 +1574,7 @@ ComputeTransactionHorizons(ComputedHorizons *h)
         * removed, as long as pg_subtrans is not truncated) or doing logical
         * decoding (which manages xmin separately, check below).
         */
-       if (pgxact->vacuumFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING))
+       if (vacuumFlags & (PROC_IN_VACUUM | PROC_IN_LOGICAL_DECODING))
            continue;
 
        /* shared tables need to take backends in all database into account */
@@ -1877,6 +1893,7 @@ GetSnapshotData(Snapshot snapshot)
        size_t      numProcs = arrayP->numProcs;
        TransactionId *xip = snapshot->xip;
        int         workspace_count = 0;
+       uint8      *allVacuumFlags = ProcGlobal->vacuumFlags;
 
        /*
         * First collect set of pgxactoff/xids that need to be included in the
@@ -1925,7 +1942,7 @@ GetSnapshotData(Snapshot snapshot)
            TransactionId xid = snapshot_workspace_xid[i];
            int         pgprocno = arrayP->pgprocnos[pgxactoff];
            PGXACT     *pgxact = &allPgXact[pgprocno];
-           uint8       vacuumFlags = pgxact->vacuumFlags;
+           uint8       vacuumFlags = allVacuumFlags[pgxactoff];
            int         nsubxids;
 
            Assert(allProcs[arrayP->pgprocnos[pgxactoff]].pgxactoff == pgxactoff);
@@ -2183,11 +2200,11 @@ ProcArrayInstallImportedXmin(TransactionId xmin,
    {
        int         pgprocno = arrayP->pgprocnos[index];
        PGPROC     *proc = &allProcs[pgprocno];
-       PGXACT     *pgxact = &allPgXact[pgprocno];
+       int         vacuumFlags = ProcGlobal->vacuumFlags[index];
        TransactionId xid;
 
        /* Ignore procs running LAZY VACUUM */
-       if (pgxact->vacuumFlags & PROC_IN_VACUUM)
+       if (vacuumFlags & PROC_IN_VACUUM)
            continue;
 
        /* We are only interested in the specific virtual transaction. */
@@ -2876,12 +2893,12 @@ GetCurrentVirtualXIDs(TransactionId limitXmin, bool excludeXmin0,
    {
        int         pgprocno = arrayP->pgprocnos[index];
        PGPROC     *proc = &allProcs[pgprocno];
-       PGXACT     *pgxact = &allPgXact[pgprocno];
+       uint8       vacuumFlags = ProcGlobal->vacuumFlags[index];
 
        if (proc == MyProc)
            continue;
 
-       if (excludeVacuum & pgxact->vacuumFlags)
+       if (excludeVacuum & vacuumFlags)
            continue;
 
        if (allDbs || proc->databaseId == MyDatabaseId)
@@ -3296,7 +3313,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
        {
            int         pgprocno = arrayP->pgprocnos[index];
            PGPROC     *proc = &allProcs[pgprocno];
-           PGXACT     *pgxact = &allPgXact[pgprocno];
+           uint8       vacuumFlags = ProcGlobal->vacuumFlags[index];
 
            if (proc->databaseId != databaseId)
                continue;
@@ -3310,7 +3327,7 @@ CountOtherDBBackends(Oid databaseId, int *nbackends, int *nprepared)
            else
            {
                (*nbackends)++;
-               if ((pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) &&
+               if ((vacuumFlags & PROC_IS_AUTOVACUUM) &&
                    nautovacs < MAXAUTOVACPIDS)
                    autovac_pids[nautovacs++] = proc->pid;
            }
index beedc7947db945915e712f39f33deabce35d5303..70e653bd3c9192057f05193067754f9f94129172 100644 (file)
@@ -544,7 +544,6 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
 {
    PGPROC     *proc;
    LOCK       *lock = checkProc->waitLock;
-   PGXACT     *pgxact;
    PROCLOCK   *proclock;
    SHM_QUEUE  *procLocks;
    LockMethod  lockMethodTable;
@@ -582,7 +581,6 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
        PGPROC     *leader;
 
        proc = proclock->tag.myProc;
-       pgxact = &ProcGlobal->allPgXact[proc->pgprocno];
        leader = proc->lockGroupLeader == NULL ? proc : proc->lockGroupLeader;
 
        /* A proc never blocks itself or any other lock group member */
@@ -628,9 +626,11 @@ FindLockCycleRecurseMember(PGPROC *checkProc,
                     * problems (which needs to read a different vacuumFlag
                     * bit), but we don't do that here to avoid grabbing
                     * ProcArrayLock.
+                    *
+                    * XXX: That's why this is using vacuumFlagsCopy.
                     */
                    if (checkProc == MyProc &&
-                       pgxact->vacuumFlags & PROC_IS_AUTOVACUUM)
+                       proc->vacuumFlagsCopy & PROC_IS_AUTOVACUUM)
                        blocking_autovacuum_proc = proc;
 
                    /* We're done looking at this proclock */
index 4dc588223c12385ee37d669315cac895c462c45a..00f26d93c1a45e0276d82dc97e94e2fca91600e6 100644 (file)
@@ -113,6 +113,7 @@ ProcGlobalShmemSize(void)
    size = add_size(size, mul_size(NUM_AUXILIARY_PROCS, sizeof(PGXACT)));
    size = add_size(size, mul_size(max_prepared_xacts, sizeof(PGXACT)));
    size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->xids)));
+   size = add_size(size, mul_size(TotalProcs, sizeof(*ProcGlobal->vacuumFlags)));
 
    return size;
 }
@@ -230,6 +231,8 @@ InitProcGlobal(void)
    // XXX: Pad to cacheline (or even page?)!
    ProcGlobal->xids = (TransactionId *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->xids));
    MemSet(ProcGlobal->xids, 0, TotalProcs * sizeof(*ProcGlobal->xids));
+   ProcGlobal->vacuumFlags = (uint8 *) ShmemAlloc(TotalProcs * sizeof(*ProcGlobal->vacuumFlags));
+   MemSet(ProcGlobal->vacuumFlags, 0, TotalProcs * sizeof(*ProcGlobal->vacuumFlags));
 
    for (i = 0; i < TotalProcs; i++)
    {
@@ -412,10 +415,10 @@ InitProcess(void)
    MyProc->tempNamespaceId = InvalidOid;
    MyProc->isBackgroundWorker = IsBackgroundWorker;
    MyProc->delayChkpt = false;
-   MyPgXact->vacuumFlags = 0;
+   MyProc->vacuumFlagsCopy = 0;
    /* NB -- autovac launcher intentionally does not set IS_AUTOVACUUM */
    if (IsAutoVacuumWorkerProcess())
-       MyPgXact->vacuumFlags |= PROC_IS_AUTOVACUUM;
+       MyProc->vacuumFlagsCopy |= PROC_IS_AUTOVACUUM;
    MyProc->lwWaiting = false;
    MyProc->lwWaitMode = 0;
    MyProc->waitLock = NULL;
@@ -594,7 +597,7 @@ InitAuxiliaryProcess(void)
    MyProc->tempNamespaceId = InvalidOid;
    MyProc->isBackgroundWorker = IsBackgroundWorker;
    MyProc->delayChkpt = false;
-   MyPgXact->vacuumFlags = 0;
+   MyProc->vacuumFlagsCopy = 0;
    MyProc->lwWaiting = false;
    MyProc->lwWaitMode = 0;
    MyProc->waitLock = NULL;
@@ -1330,7 +1333,7 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
        if (deadlock_state == DS_BLOCKED_BY_AUTOVACUUM && allow_autovacuum_cancel)
        {
            PGPROC     *autovac = GetBlockingAutoVacuumPgproc();
-           PGXACT     *autovac_pgxact = &ProcGlobal->allPgXact[autovac->pgprocno];
+           uint8       vacuumFlags;
 
            LWLockAcquire(ProcArrayLock, LW_EXCLUSIVE);
 
@@ -1338,8 +1341,9 @@ ProcSleep(LOCALLOCK *locallock, LockMethod lockMethodTable)
             * Only do it if the worker is not working to protect against Xid
             * wraparound.
             */
-           if ((autovac_pgxact->vacuumFlags & PROC_IS_AUTOVACUUM) &&
-               !(autovac_pgxact->vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND))
+           vacuumFlags = ProcGlobal->vacuumFlags[proc->pgxactoff];
+           if ((vacuumFlags & PROC_IS_AUTOVACUUM) &&
+               !(vacuumFlags & PROC_VACUUM_FOR_WRAPAROUND))
            {
                int         pid = autovac->pid;
                StringInfoData locktagbuf;
index a6ba96b558c013fe3ea9f1355b29689e240bb8fe..e765eeec92a073f300daa06e3b80bbd4482edd96 100644 (file)
@@ -41,7 +41,7 @@ struct XidCache
 };
 
 /*
- * Flags for PGXACT->vacuumFlags
+ * Flags for ProcGlobal->vacuumFlags[]
  */
 #define        PROC_IS_AUTOVACUUM  0x01    /* is it an autovac worker? */
 #define        PROC_IN_VACUUM      0x02    /* currently running lazy vacuum */
@@ -105,6 +105,7 @@ struct PGPROC
     * accessed without holding ProcArrayLock.
     */
    TransactionId xidCopy;
+   uint8       vacuumFlagsCopy;
 
    LocalTransactionId lxid;    /* local id of top-level transaction currently
                                 * being executed by this proc, if running;
@@ -228,7 +229,6 @@ extern PGDLLIMPORT struct PGXACT *MyPgXact;
  */
 typedef struct PGXACT
 {
-   uint8       vacuumFlags;    /* vacuum-related flags, see above */
    bool        overflowed;
 
    uint8       nxids;
@@ -266,6 +266,11 @@ typedef struct PROC_HDR
     */
    TransactionId *xids;
 
+   /*
+    * Vacuum flags. See PROC_* above.
+    */
+   uint8      *vacuumFlags;
+
    /* Length of allProcs array */
    uint32      allProcCount;
    /* Head of list of free PGPROC structures */