From a9169f0200fc57e01cbd216bbd41c9ea3a79a7b0 Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 3 Nov 2017 17:21:59 -0400
Subject: [PATCH] Avoid looping through line pointers twice in
 PageRepairFragmentation().

There doesn't seem to be any good reason to do the filling of the
itemidbase[] array separately from the first traversal of the pointers.
It's certainly not a win if there are any line pointers with storage,
and even if there aren't, this change doesn't insert code into the part
of the first loop that will be traversed in that case.  So let's just
merge the two loops.

Yura Sokolov, reviewed by Claudio Freire

Discussion: https://postgr.es/m/e49befcc6f1d7099834c6fdf5c675a60@postgrespro.ru
---
 src/backend/storage/page/bufpage.c | 56 ++++++++++++++----------------
 1 file changed, 26 insertions(+), 30 deletions(-)

diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c
index 41642eb59c4..b6aa2af818a 100644
--- a/src/backend/storage/page/bufpage.c
+++ b/src/backend/storage/page/bufpage.c
@@ -481,6 +481,8 @@ PageRepairFragmentation(Page page)
 	Offset		pd_lower = ((PageHeader) page)->pd_lower;
 	Offset		pd_upper = ((PageHeader) page)->pd_upper;
 	Offset		pd_special = ((PageHeader) page)->pd_special;
+	itemIdSortData itemidbase[MaxHeapTuplesPerPage];
+	itemIdSort	itemidptr;
 	ItemId		lp;
 	int			nline,
 				nstorage,
@@ -505,45 +507,23 @@ PageRepairFragmentation(Page page)
 				 errmsg("corrupted page pointers: lower = %u, upper = %u, special = %u",
 						pd_lower, pd_upper, pd_special)));
 
+	/*
+	 * Run through the line pointer array and collect data about live items.
+	 */
 	nline = PageGetMaxOffsetNumber(page);
-	nunused = nstorage = 0;
+	itemidptr = itemidbase;
+	nunused = totallen = 0;
 	for (i = FirstOffsetNumber; i <= nline; i++)
 	{
 		lp = PageGetItemId(page, i);
 		if (ItemIdIsUsed(lp))
 		{
-			if (ItemIdHasStorage(lp))
-				nstorage++;
-		}
-		else
-		{
-			/* Unused entries should have lp_len = 0, but make sure */
-			ItemIdSetUnused(lp);
-			nunused++;
-		}
-	}
-
-	if (nstorage == 0)
-	{
-		/* Page is completely empty, so just reset it quickly */
-		((PageHeader) page)->pd_upper = pd_special;
-	}
-	else
-	{
-		/* Need to compact the page the hard way */
-		itemIdSortData itemidbase[MaxHeapTuplesPerPage];
-		itemIdSort	itemidptr = itemidbase;
-
-		totallen = 0;
-		for (i = 0; i < nline; i++)
-		{
-			lp = PageGetItemId(page, i + 1);
 			if (ItemIdHasStorage(lp))
 			{
-				itemidptr->offsetindex = i;
+				itemidptr->offsetindex = i - 1;
 				itemidptr->itemoff = ItemIdGetOffset(lp);
-				if (itemidptr->itemoff < (int) pd_upper ||
-					itemidptr->itemoff >= (int) pd_special)
+				if (unlikely(itemidptr->itemoff < (int) pd_upper ||
+							 itemidptr->itemoff >= (int) pd_special))
 					ereport(ERROR,
 							(errcode(ERRCODE_DATA_CORRUPTED),
 							 errmsg("corrupted item pointer: %u",
@@ -553,7 +533,23 @@ PageRepairFragmentation(Page page)
 				itemidptr++;
 			}
 		}
+		else
+		{
+			/* Unused entries should have lp_len = 0, but make sure */
+			ItemIdSetUnused(lp);
+			nunused++;
+		}
+	}
 
+	nstorage = itemidptr - itemidbase;
+	if (nstorage == 0)
+	{
+		/* Page is completely empty, so just reset it quickly */
+		((PageHeader) page)->pd_upper = pd_special;
+	}
+	else
+	{
+		/* Need to compact the page the hard way */
 		if (totallen > (Size) (pd_special - pd_lower))
 			ereport(ERROR,
 					(errcode(ERRCODE_DATA_CORRUPTED),