From d72cebf13f9866112309883f72a382fc2cb57e17 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <heikki.linnakangas@iki.fi>
Date: Sun, 17 Mar 2024 23:08:42 +0200
Subject: [PATCH v3heikki 3/4] Move work to the first loop

It seems more efficient and more straightforward to do freezing in the
first loop. When it was part of the 2nd loop, the 2nd loop needed to
do more work (PageGetItemId() and related checks) for tuples that were
already processed as part of an earlier chain, while in the 1st loop
that work is already done.
---
 src/backend/access/heap/pruneheap.c | 141 ++++++++++------------------
 1 file changed, 52 insertions(+), 89 deletions(-)

diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index b3573bb628b..3541628799a 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -78,9 +78,6 @@ static int	heap_prune_chain(Buffer buffer,
 							 PruneState *prstate, PruneFreezeResult *presult);
 
 static inline HTSV_Result htsv_get_valid_status(int status);
-static void prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, PruneState *prstate,
-									   HeapPageFreeze *pagefrz, HeapTupleFreeze *frozen,
-									   PruneFreezeResult *presult);
 static void heap_prune_record_prunable(PruneState *prstate, TransactionId xid);
 static void heap_prune_record_redirect(PruneState *prstate,
 									   OffsetNumber offnum, OffsetNumber rdoffnum,
@@ -322,6 +319,16 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	/* for recovery conflicts */
 	presult->frz_conflict_horizon = InvalidTransactionId;
 
+	/*
+	 * We will update the VM after pruning, collecting LP_DEAD items, and
+	 * freezing tuples. Keep track of whether or not the page is all_visible
+	 * and all_frozen and use this information to update the VM. all_visible
+	 * implies lpdead_items == 0, but don't trust all_frozen result unless
+	 * all_visible is also set to true.
+	 */
+	/* HEIKKI: the caller updates the VM? not us */
+	presult->all_frozen = true;
+
 	/* For advancing relfrozenxid and relminmxid */
 	presult->new_relfrozenxid = InvalidTransactionId;
 	presult->new_relminmxid = InvalidMultiXactId;
@@ -493,6 +500,42 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 				elog(ERROR, "unexpected HeapTupleSatisfiesVacuum result");
 				break;
 		}
+
+		if (prstate.htsv[offnum] != HEAPTUPLE_DEAD)
+		{
+			/*
+			 * Deliberately don't set hastup for LP_DEAD items.  We make the
+			 * soft assumption that any LP_DEAD items encountered here will
+			 * become LP_UNUSED later on, before count_nondeletable_pages is
+			 * reached.  If we don't make this assumption then rel truncation
+			 * will only happen every other VACUUM, at most.  Besides, VACUUM
+			 * must treat hastup/nonempty_pages as provisional no matter how
+			 * LP_DEAD items are handled (handled here, or handled later on).
+			 */
+			presult->hastup = true;
+
+			/* Since we're not removing this tuple, consider freezing it */
+			if (pagefrz)
+			{
+				bool		totally_frozen;
+
+				if ((heap_prepare_freeze_tuple(htup, pagefrz,
+											   &frozen[presult->nfrozen],
+											   &totally_frozen)))
+				{
+					/* Save prepared freeze plan for later */
+					frozen[presult->nfrozen++].offset = offnum;
+				}
+
+				/*
+				 * If any tuple isn't either totally frozen already or eligible to become
+				 * totally frozen (according to its freeze plan), then the page definitely
+				 * cannot be set all-frozen in the visibility map later on.
+				 */
+				if (!totally_frozen)
+					presult->all_frozen = false;
+			}
+		}
 	}
 
 	/*
@@ -517,61 +560,29 @@ heap_page_prune_and_freeze(Relation relation, Buffer buffer,
 	 */
 	presult->all_visible_except_removable = presult->all_visible;
 
-	/*
-	 * We will update the VM after pruning, collecting LP_DEAD items, and
-	 * freezing tuples. Keep track of whether or not the page is all_visible
-	 * and all_frozen and use this information to update the VM. all_visible
-	 * implies lpdead_items == 0, but don't trust all_frozen result unless
-	 * all_visible is also set to true.
-	 */
-	presult->all_frozen = true;
-
-	/* Scan the page */
+	/* Scan the page for hot chains */
 	for (offnum = FirstOffsetNumber;
 		 offnum <= maxoff;
 		 offnum = OffsetNumberNext(offnum))
 	{
 		ItemId		itemid;
 
-		/* see preceding loop */
-		if (off_loc)
-			*off_loc = offnum;
-
-		if (pagefrz)
-			prune_prepare_freeze_tuple(page, offnum, &prstate,
-									   pagefrz, frozen, presult);
-
-		itemid = PageGetItemId(page, offnum);
-
-		if (ItemIdIsNormal(itemid) &&
-			prstate.htsv[offnum] != HEAPTUPLE_DEAD)
-		{
-			Assert(prstate.htsv[offnum] != -1);
-
-			/*
-			 * Deliberately don't set hastup for LP_DEAD items.  We make the
-			 * soft assumption that any LP_DEAD items encountered here will
-			 * become LP_UNUSED later on, before count_nondeletable_pages is
-			 * reached.  If we don't make this assumption then rel truncation
-			 * will only happen every other VACUUM, at most.  Besides, VACUUM
-			 * must treat hastup/nonempty_pages as provisional no matter how
-			 * LP_DEAD items are handled (handled here, or handled later on).
-			 */
-			presult->hastup = true;
-		}
-
 		/* Ignore items already processed as part of an earlier chain */
 		if (prstate.marked[offnum])
 			continue;
 
+		/* see preceding loop */
+		if (off_loc)
+			*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,
 											  &prstate, presult);
-
 	}
 
 	/* Clear the offset information once we have processed the given page. */
@@ -1217,54 +1228,6 @@ heap_prune_chain(Buffer buffer, OffsetNumber rootoffnum,
 	return ndeleted;
 }
 
-/*
- * While pruning, before actually executing pruning and updating the line
- * pointers, we may consider freezing tuples referred to by LP_NORMAL line
- * pointers whose visibility status is not HEAPTUPLE_DEAD. That is to say, we
- * want to consider freezing normal tuples which will not be removed.
-*/
-static void
-prune_prepare_freeze_tuple(Page page, OffsetNumber offnum, PruneState *prstate,
-						   HeapPageFreeze *pagefrz,
-						   HeapTupleFreeze *frozen,
-						   PruneFreezeResult *presult)
-{
-	bool		totally_frozen;
-	HeapTupleHeader htup;
-	ItemId		itemid;
-
-	Assert(pagefrz);
-
-	itemid = PageGetItemId(page, offnum);
-
-	if (!ItemIdIsNormal(itemid))
-		return;
-
-	/* We do not consider freezing tuples which will be removed. */
-	if (prstate->htsv[offnum] == HEAPTUPLE_DEAD ||
-		prstate->htsv[offnum] == -1)
-		return;
-
-	htup = (HeapTupleHeader) PageGetItem(page, itemid);
-
-	/* Tuple with storage -- consider need to freeze */
-	if ((heap_prepare_freeze_tuple(htup, pagefrz,
-								   &frozen[presult->nfrozen],
-								   &totally_frozen)))
-	{
-		/* Save prepared freeze plan for later */
-		frozen[presult->nfrozen++].offset = offnum;
-	}
-
-	/*
-	 * If any tuple isn't either totally frozen already or eligible to become
-	 * totally frozen (according to its freeze plan), then the page definitely
-	 * cannot be set all-frozen in the visibility map later on
-	 */
-	if (!totally_frozen)
-		presult->all_frozen = false;
-}
-
 /* Record lowest soon-prunable XID */
 static void
 heap_prune_record_prunable(PruneState *prstate, TransactionId xid)
-- 
2.39.2

