Minor refactoring in heap_page_prune

hlinnaka · hlinnaka · commit 9917e79d995c · 2024-04-01T12:07:30.000+03:00
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
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
@@ -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,22 +313,20 @@ 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);
 		if (!ItemIdIsUsed(itemid))
 			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,30 +448,28 @@ 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.
 	 */
 	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);
 	}
