public inbox for [email protected]  
help / color / mirror / Atom feed
From: Andres Freund <[email protected]>
Subject: [PATCH v1 06/14] heapam: Add batch mode mvcc check and use it in page mode
Date: Thu, 17 Oct 2024 13:16:36 -0400

There are two reasons for doing so:

1) It is generally faster to perform checks in a batched fashion and making
   sequential scans faster is nice.

2) We would like to stop setting hint bits while pages are being written
   out. The necessary locking becomes visible for page mode scans if done for
   every tuple. With batching the overhead can be amortized to only happen
   once per page.

There are substantial further optimization opportunities along these
lines:

- Right now HeapTupleSatisfiesMVCCBatch() simply uses the single-tuple
  HeapTupleSatisfiesMVCC(), relying on the compiler to inline it. We could
  instead write an explicitly optimized version that avoids repeated xid
  tests.

- Introduce batched version of the serializability test

- Introduce batched version of HeapTupleSatisfiesVacuum

Author:
Reviewed-by:
Discussion: https://postgr.es/m/
Backpatch:
---
 src/include/access/heapam.h                 |  28 +++++
 src/backend/access/heap/heapam.c            | 110 ++++++++++++++++----
 src/backend/access/heap/heapam_visibility.c |  50 +++++++++
 src/tools/pgindent/typedefs.list            |   1 +
 4 files changed, 166 insertions(+), 23 deletions(-)

diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 96cf82f97b7..d4a790251cc 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -417,6 +417,34 @@ extern bool HeapTupleHeaderIsOnlyLocked(HeapTupleHeader tuple);
 extern bool HeapTupleIsSurelyDead(HeapTuple htup,
 								  struct GlobalVisState *vistest);
 
+/*
+ * FIXME: define to be removed
+ *
+ * Without this I see worse performance. But it's a bit ugly, so I thought
+ * it'd be useful to leave a way in for others to experiment with this.
+ */
+#define BATCHMVCC_FEWER_ARGS
+
+#ifdef BATCHMVCC_FEWER_ARGS
+typedef struct BatchMVCCState
+{
+	HeapTupleData tuples[MaxHeapTuplesPerPage];
+	bool		visible[MaxHeapTuplesPerPage];
+} BatchMVCCState;
+#endif
+
+extern int	HeapTupleSatisfiesMVCCBatch(Snapshot snapshot, Buffer buffer,
+										int ntups,
+#ifdef BATCHMVCC_FEWER_ARGS
+										BatchMVCCState *batchmvcc,
+#else
+										HeapTupleData *tuples,
+										bool *visible,
+#endif
+										OffsetNumber *vistuples_dense);
+
+
+
 /*
  * To avoid leaking too much knowledge about reorderbuffer implementation
  * details this is implemented in reorderbuffer.c not heapam_visibility.c
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index d0cb4b1e29b..99656a1aac5 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -437,42 +437,106 @@ page_collect_tuples(HeapScanDesc scan, Snapshot snapshot,
 					BlockNumber block, int lines,
 					bool all_visible, bool check_serializable)
 {
+	Oid			relid = RelationGetRelid(scan->rs_base.rs_rd);
+#ifdef BATCHMVCC_FEWER_ARGS
+	BatchMVCCState batchmvcc;
+#else
+	HeapTupleData tuples[MaxHeapTuplesPerPage];
+	bool		visible[MaxHeapTuplesPerPage];
+#endif
 	int			ntup = 0;
-	OffsetNumber lineoff;
+	int			nvis = 0;
 
-	for (lineoff = FirstOffsetNumber; lineoff <= lines; lineoff++)
+	/* page at a time should have been disabled otherwise */
+	Assert(IsMVCCSnapshot(snapshot));
+
+	/* first find all tuples on the page */
+	for (OffsetNumber lineoff = FirstOffsetNumber; lineoff <= lines; lineoff++)
 	{
 		ItemId		lpp = PageGetItemId(page, lineoff);
-		HeapTupleData loctup;
-		bool		valid;
+		HeapTuple	tup;
 
-		if (!ItemIdIsNormal(lpp))
+		if (unlikely(!ItemIdIsNormal(lpp)))
 			continue;
 
-		loctup.t_data = (HeapTupleHeader) PageGetItem(page, lpp);
-		loctup.t_len = ItemIdGetLength(lpp);
-		loctup.t_tableOid = RelationGetRelid(scan->rs_base.rs_rd);
-		ItemPointerSet(&(loctup.t_self), block, lineoff);
-
-		if (all_visible)
-			valid = true;
-		else
-			valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer);
-
-		if (check_serializable)
-			HeapCheckForSerializableConflictOut(valid, scan->rs_base.rs_rd,
-												&loctup, buffer, snapshot);
-
-		if (valid)
+		/*
+		 * If the page is not all-visible or we need to check serializability,
+		 * maintain enough state to be able to refind the tuple efficiently,
+		 * without again needing to extract it from the page.
+		 */
+		if (!all_visible || check_serializable)
 		{
-			scan->rs_vistuples[ntup] = lineoff;
-			ntup++;
+#ifdef BATCHMVCC_FEWER_ARGS
+			tup = &batchmvcc.tuples[ntup];
+#else
+			tup = &tuples[ntup];
+#endif
+
+			tup->t_data = (HeapTupleHeader) PageGetItem(page, lpp);
+			tup->t_len = ItemIdGetLength(lpp);
+			tup->t_tableOid = relid;
+			ItemPointerSet(&(tup->t_self), block, lineoff);
 		}
+
+		/*
+		 * If the page is all visible, these fields won'otherwise wont be
+		 * populated in loop below.
+		 */
+		if (all_visible)
+		{
+			if (check_serializable)
+			{
+#ifdef BATCHMVCC_FEWER_ARGS
+				batchmvcc.visible[ntup] = true;
+#else
+				visible[ntup] = true;
+#endif
+			}
+			scan->rs_vistuples[ntup] = lineoff;
+		}
+
+		ntup++;
 	}
 
 	Assert(ntup <= MaxHeapTuplesPerPage);
 
-	return ntup;
+	/* unless the page is all visible, test visibility for all tuples one go */
+	if (all_visible)
+		nvis = ntup;
+	else
+		nvis = HeapTupleSatisfiesMVCCBatch(snapshot, buffer,
+										   ntup,
+#ifdef BATCHMVCC_FEWER_ARGS
+										   &batchmvcc,
+#else
+										   tuples, visible,
+#endif
+										   scan->rs_vistuples
+			);
+
+	/*
+	 * So far we don't have batch API for testing serializabilty, so do so
+	 * one-by-one.
+	 */
+	if (check_serializable)
+	{
+		for (int i = 0; i < ntup; i++)
+		{
+#ifdef BATCHMVCC_FEWER_ARGS
+			HeapCheckForSerializableConflictOut(batchmvcc.visible[i],
+												scan->rs_base.rs_rd,
+												&batchmvcc.tuples[i],
+												buffer, snapshot);
+#else
+			HeapCheckForSerializableConflictOut(visible[i],
+												scan->rs_base.rs_rd,
+												&tuples[i],
+												buffer, snapshot);
+#endif
+		}
+	}
+
+	return nvis;
 }
 
 /*
diff --git a/src/backend/access/heap/heapam_visibility.c b/src/backend/access/heap/heapam_visibility.c
index b7aa8bb7a52..4baafc01a2c 100644
--- a/src/backend/access/heap/heapam_visibility.c
+++ b/src/backend/access/heap/heapam_visibility.c
@@ -1575,6 +1575,56 @@ HeapTupleSatisfiesHistoricMVCC(HeapTuple htup, Snapshot snapshot,
 		return true;
 }
 
+/*
+ * Perform HeaptupleSatisfiesMVCC() on each passed in tuple. This is more
+ * efficient than doing HeapTupleSatisfiesMVCC() one-by-one.
+ *
+ * To be checked tuples are passed via BatchMVCCState->tuples. Each tuple's
+ * visibility is set in batchmvcc->visible[]. In addition, ->vistuples_dense
+ * is set to contain the offsets of visible tuples.
+ *
+ * Returns the number of visible tuples.
+ */
+int
+HeapTupleSatisfiesMVCCBatch(Snapshot snapshot, Buffer buffer,
+							int ntups,
+#ifdef BATCHMVCC_FEWER_ARGS
+							BatchMVCCState *batchmvcc,
+#else
+							HeapTupleData *tuples,
+							bool *visible,
+#endif
+							OffsetNumber *vistuples_dense)
+{
+	int			nvis = 0;
+
+	Assert(IsMVCCSnapshot(snapshot));
+
+	for (int i = 0; i < ntups; i++)
+	{
+		bool		valid;
+#ifdef BATCHMVCC_FEWER_ARGS
+		HeapTuple	tup = &batchmvcc->tuples[i];
+#else
+		HeapTuple	tup = &tuples[i];
+#endif
+
+		valid = HeapTupleSatisfiesMVCC(tup, snapshot, buffer);
+#ifdef BATCHMVCC_FEWER_ARGS
+		batchmvcc->visible[i] = valid;
+#else
+		visible[i] = valid;
+#endif
+		if (likely(valid))
+		{
+			vistuples_dense[nvis] = tup->t_self.ip_posid;
+			nvis++;
+		}
+	}
+
+	return nvis;
+}
+
 /*
  * HeapTupleSatisfiesVisibility
  *		True iff heap tuple satisfies a time qual.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 171a7dd5d2b..15c7673465b 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -244,6 +244,7 @@ Barrier
 BaseBackupCmd
 BaseBackupTargetHandle
 BaseBackupTargetType
+BatchMVCCState
 BeginDirectModify_function
 BeginForeignInsert_function
 BeginForeignModify_function
-- 
2.46.0.519.g2e7b89e038


--gpbnw6zzq3spol54
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0007-bufmgr-Make-it-easier-to-change-number-of-buffer-.patch"



view thread (13+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected]
  Subject: Re: [PATCH v1 06/14] heapam: Add batch mode mvcc check and use it in page mode
  In-Reply-To: <no-message-id-621744@localhost>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox