]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
Minor refactoring in heap_page_prune
authorHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 1 Apr 2024 09:07:30 +0000 (12:07 +0300)
committerHeikki Linnakangas <heikki.linnakangas@iki.fi>
Mon, 1 Apr 2024 09:07:30 +0000 (12:07 +0300)
Pass 'page', 'blockno' and 'maxoff' to heap_prune_chain() as
arguments, so that it doesn't need to fetch them from the buffer. This
saves a few cycles per chain.

Remove the "if (off_loc != NULL)" checks, and require the caller to
pass a non-NULL 'off_loc'. Pass a pointer to a dummy local variable
when it's not needed. Those checks are cheap, but it's still better to
avoid them in the per-chain loops when we can do so easily.

The CPU time saving from these changes are hardly measurable, but
fewer instructions is good anyway, so why not. I spotted the potential
for these while reviewing Melanie Plageman's patch set to combine
prune and freeze records.

Discussion: https://www.postgresql.org/message-id/CAAKRu_abm2tHhrc0QSQa%3D%3DsHe%3DVA1%3Doz1dJMQYUOKuHmu%2B9Xrg%40mail.gmail.com

src/backend/access/heap/pruneheap.c

index ef816c2fa9c947e07a3fac7d1bd3267f34f0143b..f92eef98e8d1bf6261dc1dbd9b02005bbae6e73d 100644 (file)
@@ -57,10 +57,8 @@ typedef struct
 static HTSV_Result heap_prune_satisfies_vacuum(PruneState *prstate,
                                                                                           HeapTuple tup,
                                                                                           Buffer buffer);
-static int     heap_prune_chain(Buffer buffer,
-                                                        OffsetNumber rootoffnum,
-                                                        int8 *htsv,
-                                                        PruneState *prstate);
+static int     heap_prune_chain(Page page, BlockNumber blockno, OffsetNumber maxoff,
+                                                        OffsetNumber rootoffnum, int8 *htsv, PruneState *prstate);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
                                                                           OffsetNumber offnum, OffsetNumber rdoffnum);
@@ -145,6 +143,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
                 */
                if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
                {
+                       OffsetNumber dummy_off_loc;
                        PruneResult presult;
 
                        /*
@@ -153,7 +152,7 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
                         * that during on-access pruning with the current implementation.
                         */
                        heap_page_prune(relation, buffer, vistest, false,
-                                                       &presult, PRUNE_ON_ACCESS, NULL);
+                                                       &presult, PRUNE_ON_ACCESS, &dummy_off_loc);
 
                        /*
                         * Report the number of tuples reclaimed to pgstats.  This is
@@ -296,8 +295,7 @@ heap_page_prune(Relation relation, Buffer buffer,
                 * Set the offset number so that we can display it along with any
                 * error that occurred while processing this tuple.
                 */
-               if (off_loc)
-                       *off_loc = offnum;
+               *off_loc = offnum;
 
                presult->htsv[offnum] = heap_prune_satisfies_vacuum(&prstate, &tup,
                                                                                                                        buffer);
@@ -315,8 +313,7 @@ heap_page_prune(Relation relation, Buffer buffer,
                        continue;
 
                /* see preceding loop */
-               if (off_loc)
-                       *off_loc = offnum;
+               *off_loc = offnum;
 
                /* Nothing to do if slot is empty */
                itemid = PageGetItemId(page, offnum);
@@ -324,13 +321,12 @@ heap_page_prune(Relation relation, Buffer buffer,
                        continue;
 
                /* Process this item or chain of items */
-               presult->ndeleted += heap_prune_chain(buffer, offnum,
+               presult->ndeleted += heap_prune_chain(page, blockno, maxoff, offnum,
                                                                                          presult->htsv, &prstate);
        }
 
        /* Clear the offset information once we have processed the given page. */
-       if (off_loc)
-               *off_loc = InvalidOffsetNumber;
+       *off_loc = InvalidOffsetNumber;
 
        /* Any error while applying the changes is critical */
        START_CRIT_SECTION();
@@ -452,22 +448,20 @@ heap_prune_satisfies_vacuum(PruneState *prstate, HeapTuple tup, Buffer buffer)
  * Returns the number of tuples (to be) deleted from the page.
  */
 static int
-heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
-                                int8 *htsv, PruneState *prstate)
+heap_prune_chain(Page page, BlockNumber blockno, OffsetNumber maxoff,
+                                OffsetNumber rootoffnum, int8 *htsv, PruneState *prstate)
 {
        int                     ndeleted = 0;
-       Page            dp = (Page) BufferGetPage(buffer);
        TransactionId priorXmax = InvalidTransactionId;
        ItemId          rootlp;
        HeapTupleHeader htup;
        OffsetNumber latestdead = InvalidOffsetNumber,
-                               maxoff = PageGetMaxOffsetNumber(dp),
                                offnum;
        OffsetNumber chainitems[MaxHeapTuplesPerPage];
        int                     nchain = 0,
                                i;
 
-       rootlp = PageGetItemId(dp, rootoffnum);
+       rootlp = PageGetItemId(page, rootoffnum);
 
        /*
         * If it's a heap-only tuple, then it is not the start of a HOT chain.
@@ -475,7 +469,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
        if (ItemIdIsNormal(rootlp))
        {
                Assert(htsv[rootoffnum] != -1);
-               htup = (HeapTupleHeader) PageGetItem(dp, rootlp);
+               htup = (HeapTupleHeader) PageGetItem(page, rootlp);
 
                if (HeapTupleHeaderIsHeapOnly(htup))
                {
@@ -536,7 +530,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
                if (prstate->marked[offnum])
                        break;
 
-               lp = PageGetItemId(dp, offnum);
+               lp = PageGetItemId(page, offnum);
 
                /* Unused item obviously isn't part of the chain */
                if (!ItemIdIsUsed(lp))
@@ -575,7 +569,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
                }
 
                Assert(ItemIdIsNormal(lp));
-               htup = (HeapTupleHeader) PageGetItem(dp, lp);
+               htup = (HeapTupleHeader) PageGetItem(page, lp);
 
                /*
                 * Check the tuple XMIN against prior XMAX, if any
@@ -666,8 +660,7 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
                /*
                 * Advance to next chain member.
                 */
-               Assert(ItemPointerGetBlockNumber(&htup->t_ctid) ==
-                          BufferGetBlockNumber(buffer));
+               Assert(ItemPointerGetBlockNumber(&htup->t_ctid) == blockno);
                offnum = ItemPointerGetOffsetNumber(&htup->t_ctid);
                priorXmax = HeapTupleHeaderGetUpdateXid(htup);
        }