v8-0002-Divide-SLRU-buffers-into-banks.patch

From d537021f65f8104f327ebdd2f1820abdd2e075af Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilip.kumar@enterprisedb.com>
Date: Fri, 17 Nov 2023 10:24:41 +0530
Subject: [PATCH v8 2/3] Divide SLRU buffers into banks

As we have made slru buffer pool configurable, we want to
eliminate linear search within whole SLRU buffer pool.  To
do so we divide SLRU buffers into banks. Each bank holds 16
buffers. Each SLRU pageno may reside only in one bank.
Adjacent pagenos reside in different banks. Along with this
also ensure that the number of slru buffers are given in
multiples of bank size.

Andrey M. Borodin and Dilip Kumar based on fedback by Alvaro Herrera
---
 src/backend/access/transam/clog.c      | 10 ++++++
 src/backend/access/transam/commit_ts.c | 10 ++++++
 src/backend/access/transam/multixact.c | 19 +++++++++++
 src/backend/access/transam/slru.c      | 45 ++++++++++++++++++++++----
 src/backend/access/transam/subtrans.c  | 10 ++++++
 src/backend/commands/async.c           | 10 ++++++
 src/backend/storage/lmgr/predicate.c   | 10 ++++++
 src/backend/utils/misc/guc_tables.c    | 14 ++++----
 src/include/access/slru.h              | 13 +++++++-
 src/include/utils/guc_hooks.h          | 11 +++++++
 10 files changed, 138 insertions(+), 14 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 8237b40aa6..44008222da 100644
--- a/src/backend/access/transam/clog.c
+++ b/src/backend/access/transam/clog.c
@@ -43,6 +43,7 @@
 #include "pgstat.h"
 #include "storage/proc.h"
 #include "storage/sync.h"
+#include "utils/guc_hooks.h"
 
 /*
  * Defines for CLOG page sizes.  A page is the same BLCKSZ as is used
@@ -1019,3 +1020,12 @@ clogsyncfiletag(const FileTag *ftag, char *path)
 {
 	return SlruSyncFileTag(XactCtl, ftag, path);
 }
+
+/*
+ * GUC check_hook for xact_buffers
+ */
+bool
+check_xact_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("xact_buffers", newval);
+}
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index 9ba5ae6534..96810959ab 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -33,6 +33,7 @@
 #include "pg_trace.h"
 #include "storage/shmem.h"
 #include "utils/builtins.h"
+#include "utils/guc_hooks.h"
 #include "utils/snapmgr.h"
 #include "utils/timestamp.h"
 
@@ -1017,3 +1018,12 @@ committssyncfiletag(const FileTag *ftag, char *path)
 {
 	return SlruSyncFileTag(CommitTsCtl, ftag, path);
 }
+
+/*
+ * GUC check_hook for commit_ts_buffers
+ */
+bool
+check_commit_ts_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("commit_ts_buffers", newval);
+}
\ No newline at end of file
diff --git a/src/backend/access/transam/multixact.c b/src/backend/access/transam/multixact.c
index 62709fcd07..77511c6342 100644
--- a/src/backend/access/transam/multixact.c
+++ b/src/backend/access/transam/multixact.c
@@ -88,6 +88,7 @@
 #include "storage/proc.h"
 #include "storage/procarray.h"
 #include "utils/builtins.h"
+#include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 #include "utils/snapmgr.h"
 
@@ -3419,3 +3420,21 @@ multixactmemberssyncfiletag(const FileTag *ftag, char *path)
 {
 	return SlruSyncFileTag(MultiXactMemberCtl, ftag, path);
 }
+
+/*
+ * GUC check_hook for multixact_offsets_buffers
+ */
+bool
+check_multixact_offsets_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("multixact_offsets_buffers", newval);
+}
+
+/*
+ * GUC check_hook for multixact_members_buffers
+ */
+bool
+check_multixact_members_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("multixact_members_buffers", newval);
+}
\ No newline at end of file
diff --git a/src/backend/access/transam/slru.c b/src/backend/access/transam/slru.c
index 9ed24e1185..b0d90a4bd2 100644
--- a/src/backend/access/transam/slru.c
+++ b/src/backend/access/transam/slru.c
@@ -59,6 +59,7 @@
 #include "pgstat.h"
 #include "storage/fd.h"
 #include "storage/shmem.h"
+#include "utils/guc_hooks.h"
 
 #define SlruFileName(ctl, path, seg) \
 	snprintf(path, MAXPGPATH, "%s/%04X", (ctl)->Dir, seg)
@@ -134,7 +135,6 @@ typedef enum
 static SlruErrorCause slru_errcause;
 static int	slru_errno;
 
-
 static void SimpleLruZeroLSNs(SlruCtl ctl, int slotno);
 static void SimpleLruWaitIO(SlruCtl ctl, int slotno);
 static void SlruInternalWritePage(SlruCtl ctl, int slotno, SlruWriteAll fdata);
@@ -258,7 +258,10 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 		Assert(ptr - (char *) shared <= SimpleLruShmemSize(nslots, nlsns));
 	}
 	else
+	{
 		Assert(found);
+		Assert(shared->num_slots == nslots);
+	}
 
 	/*
 	 * Initialize the unshared control struct, including directory path. We
@@ -266,6 +269,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	 */
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
+	ctl->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -497,12 +501,18 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int pageno, TransactionId xid)
 {
 	SlruShared	shared = ctl->shared;
 	int			slotno;
+	int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+	int			bankend = bankstart + SLRU_BANK_SIZE;
 
 	/* Try to find the page while holding only shared lock */
 	LWLockAcquire(shared->ControlLock, LW_SHARED);
 
-	/* See if page is already in a buffer */
-	for (slotno = 0; slotno < shared->num_slots; slotno++)
+	/*
+	 * See if the page is already in a buffer pool.  The buffer pool is
+	 * divided into banks of buffers and each pageno may reside only in one
+	 * bank so limit the search within the bank.
+	 */
+	for (slotno = bankstart; slotno < bankend; slotno++)
 	{
 		if (shared->page_number[slotno] == pageno &&
 			shared->page_status[slotno] != SLRU_PAGE_EMPTY &&
@@ -1029,9 +1039,15 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		int			bestinvalidslot = 0;	/* keep compiler quiet */
 		int			best_invalid_delta = -1;
 		int			best_invalid_page_number = 0;	/* keep compiler quiet */
+		int			bankstart = (pageno & ctl->bank_mask) * SLRU_BANK_SIZE;
+		int			bankend = bankstart + SLRU_BANK_SIZE;
 
-		/* See if page already has a buffer assigned */
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		/*
+		 * See if the page is already in a buffer pool.  The buffer pool is
+		 * divided into banks of buffers and each pageno may reside only in one
+		 * bank so limit the search within the bank.
+		 */
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			if (shared->page_number[slotno] == pageno &&
 				shared->page_status[slotno] != SLRU_PAGE_EMPTY)
@@ -1066,7 +1082,7 @@ SlruSelectLRUPage(SlruCtl ctl, int pageno)
 		 * multiple pages with the same lru_count.
 		 */
 		cur_count = (shared->cur_lru_count)++;
-		for (slotno = 0; slotno < shared->num_slots; slotno++)
+		for (slotno = bankstart; slotno < bankend; slotno++)
 		{
 			int			this_delta;
 			int			this_page_number;
@@ -1613,3 +1629,20 @@ SlruSyncFileTag(SlruCtl ctl, const FileTag *ftag, char *path)
 	errno = save_errno;
 	return result;
 }
+
+/*
+ * Helper function for GUC check_hook to check whether slru buffers are in
+ * multiples of SLRU_BANK_SIZE.
+ */
+bool
+check_slru_buffers(const char *name, int *newval)
+{
+	/* Value upper and lower hard limits are inclusive */
+	if (*newval % SLRU_BANK_SIZE == 0)
+		return true;
+
+	/* Value does not fall within any allowable range */
+	GUC_check_errdetail("\"%s\" must be in multiple of %d", name,
+						SLRU_BANK_SIZE);
+	return false;
+}
diff --git a/src/backend/access/transam/subtrans.c b/src/backend/access/transam/subtrans.c
index 0dd48f40f3..923e706535 100644
--- a/src/backend/access/transam/subtrans.c
+++ b/src/backend/access/transam/subtrans.c
@@ -33,6 +33,7 @@
 #include "access/transam.h"
 #include "miscadmin.h"
 #include "pg_trace.h"
+#include "utils/guc_hooks.h"
 #include "utils/snapmgr.h"
 
 
@@ -373,3 +374,12 @@ SubTransPagePrecedes(int page1, int page2)
 	return (TransactionIdPrecedes(xid1, xid2) &&
 			TransactionIdPrecedes(xid1, xid2 + SUBTRANS_XACTS_PER_PAGE - 1));
 }
+
+/*
+ * GUC check_hook for subtrans_buffers
+ */
+bool
+check_subtrans_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("subtrans_buffers", newval);
+}
diff --git a/src/backend/commands/async.c b/src/backend/commands/async.c
index 4bdbbe5cc0..98449cbdde 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -149,6 +149,7 @@
 #include "storage/sinval.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
+#include "utils/guc_hooks.h"
 #include "utils/memutils.h"
 #include "utils/ps_status.h"
 #include "utils/snapmgr.h"
@@ -2444,3 +2445,12 @@ ClearPendingActionsAndNotifies(void)
 	pendingActions = NULL;
 	pendingNotifies = NULL;
 }
+
+/*
+ * GUC check_hook for notify_buffers
+ */
+bool
+check_notify_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("notify_buffers", newval);
+}
diff --git a/src/backend/storage/lmgr/predicate.c b/src/backend/storage/lmgr/predicate.c
index 18ea18316d..e4903c67ec 100644
--- a/src/backend/storage/lmgr/predicate.c
+++ b/src/backend/storage/lmgr/predicate.c
@@ -208,6 +208,7 @@
 #include "storage/predicate_internals.h"
 #include "storage/proc.h"
 #include "storage/procarray.h"
+#include "utils/guc_hooks.h"
 #include "utils/rel.h"
 #include "utils/snapmgr.h"
 
@@ -5011,3 +5012,12 @@ AttachSerializableXact(SerializableXactHandle handle)
 	if (MySerializableXact != InvalidSerializableXact)
 		CreateLocalPredicateLockHash();
 }
+
+/*
+ * GUC check_hook for serial_buffers
+ */
+bool
+check_serial_buffers(int *newval, void **extra, GucSource source)
+{
+	return check_slru_buffers("serial_buffers", newval);
+}
diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c
index c1345dab98..8649b066a8 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2296,7 +2296,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&multixact_offsets_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_multixact_offsets_buffers, NULL, NULL
 	},
 
 	{
@@ -2307,7 +2307,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&multixact_members_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_multixact_members_buffers, NULL, NULL
 	},
 
 	{
@@ -2318,7 +2318,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&subtrans_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_subtrans_buffers, NULL, NULL
 	},
 	{
 		{"notify_buffers", PGC_POSTMASTER, RESOURCES_MEM,
@@ -2328,7 +2328,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&notify_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_notify_buffers, NULL, NULL
 	},
 
 	{
@@ -2339,7 +2339,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&serial_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_serial_buffers, NULL, NULL
 	},
 
 	{
@@ -2350,7 +2350,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&xact_buffers,
 		64, 0, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, show_xact_buffers
+		check_xact_buffers, NULL, show_xact_buffers
 	},
 
 	{
@@ -2361,7 +2361,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&commit_ts_buffers,
 		64, 0, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, show_commit_ts_buffers
+		check_commit_ts_buffers, NULL, show_commit_ts_buffers
 	},
 
 	{
diff --git a/src/include/access/slru.h b/src/include/access/slru.h
index c0d37e3eb3..8f20b66776 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -17,6 +17,11 @@
 #include "storage/lwlock.h"
 #include "storage/sync.h"
 
+/*
+ * SLRU bank size for slotno hash banks
+ */
+#define SLRU_BANK_SIZE		16
+
 /*
  * To avoid overflowing internal arithmetic and the size_t data type, the
  * number of buffers should not exceed this number.
@@ -139,6 +144,12 @@ typedef struct SlruCtlData
 	 * it's always the same, it doesn't need to be in shared memory.
 	 */
 	char		Dir[64];
+
+	/*
+	 * Mask for slotno banks, considering 1GB SLRU buffer pool size and the
+	 * SLRU_BANK_SIZE bits16 should be sufficient for the bank mask.
+	 */
+	bits16		bank_mask;
 } SlruCtlData;
 
 typedef SlruCtlData *SlruCtl;
@@ -175,5 +186,5 @@ extern bool SlruScanDirCbReportPresence(SlruCtl ctl, char *filename,
 										int segpage, void *data);
 extern bool SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int segpage,
 								   void *data);
-
+extern bool check_slru_buffers(const char *name, int *newval);
 #endif							/* SLRU_H */
diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h
index 7b95acf36e..0edd59f867 100644
--- a/src/include/utils/guc_hooks.h
+++ b/src/include/utils/guc_hooks.h
@@ -130,6 +130,17 @@ extern bool check_ssl(bool *newval, void **extra, GucSource source);
 extern bool check_stage_log_stats(bool *newval, void **extra, GucSource source);
 extern bool check_synchronous_standby_names(char **newval, void **extra,
 											GucSource source);
+extern bool check_multixact_offsets_buffers(int *newval, void **extra,
+											GucSource source);
+extern bool check_multixact_members_buffers(int *newval, void **extra,
+											GucSource source);
+extern bool check_subtrans_buffers(int *newval, void **extra,
+								   GucSource source);
+extern bool check_notify_buffers(int *newval, void **extra, GucSource source);
+extern bool check_serial_buffers(int *newval, void **extra, GucSource source);
+extern bool check_xact_buffers(int *newval, void **extra, GucSource source);
+extern bool check_commit_ts_buffers(int *newval, void **extra,
+									GucSource source);
 extern void assign_synchronous_standby_names(const char *newval, void *extra);
 extern void assign_synchronous_commit(int newval, void *extra);
 extern void assign_syslog_facility(int newval, void *extra);
-- 
2.39.2 (Apple Git-143)