From 5209ccd60c1fe3c2118bbcb2ad2550c94676b1c2 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilip.kumar@enterprisedb.com>
Date: Thu, 30 Nov 2023 13:41:50 +0530
Subject: [PATCH v12 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      | 44 +++++++++++++++++++++++---
 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              | 15 +++++++++
 src/include/utils/guc_hooks.h          | 11 +++++++
 10 files changed, 141 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/transam/clog.c b/src/backend/access/transam/clog.c
index 6e6b73a877..fc70b91bc9 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
@@ -1029,3 +1030,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 a323fab4ff..10e378f846 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"
 
@@ -1027,3 +1028,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 89e6bafb27..65739b2f9c 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"
 
@@ -3421,3 +3422,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 7a371d9034..ce589493e4 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"
 
 static inline int
 SlruFileName(SlruCtl ctl, char *path, int64 segno)
@@ -284,7 +285,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
@@ -293,6 +297,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
 	ctl->shared = shared;
 	ctl->sync_handler = sync_handler;
 	ctl->long_segment_names = long_segment_names;
+	ctl->bank_mask = (nslots / SLRU_BANK_SIZE) - 1;
 	strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
 }
 
@@ -524,12 +529,18 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 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 &&
@@ -1056,9 +1067,15 @@ SlruSelectLRUPage(SlruCtl ctl, int64 pageno)
 		int			bestinvalidslot = 0;	/* keep compiler quiet */
 		int			best_invalid_delta = -1;
 		int64		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)
@@ -1093,7 +1110,7 @@ SlruSelectLRUPage(SlruCtl ctl, int64 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;
 			int64		this_page_number;
@@ -1666,3 +1683,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 2259f882ef..3f2444a37e 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"
 
 
@@ -383,3 +384,12 @@ SubTransPagePrecedes(int64 page1, int64 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 8b80f75193..87082b8f86 100644
--- a/src/backend/commands/async.c
+++ b/src/backend/commands/async.c
@@ -148,6 +148,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"
@@ -2378,3 +2379,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 02eb2c9822..9175aaabd1 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"
 
@@ -5012,3 +5013,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 75e5725d9c..ef4ed8e8c4 100644
--- a/src/backend/utils/misc/guc_tables.c
+++ b/src/backend/utils/misc/guc_tables.c
@@ -2295,7 +2295,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&multixact_offsets_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_multixact_offsets_buffers, NULL, NULL
 	},
 
 	{
@@ -2306,7 +2306,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&multixact_members_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_multixact_members_buffers, NULL, NULL
 	},
 
 	{
@@ -2317,7 +2317,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,
@@ -2327,7 +2327,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&notify_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_notify_buffers, NULL, NULL
 	},
 
 	{
@@ -2338,7 +2338,7 @@ struct config_int ConfigureNamesInt[] =
 		},
 		&serial_buffers,
 		64, 16, SLRU_MAX_ALLOWED_BUFFERS,
-		NULL, NULL, NULL
+		check_serial_buffers, NULL, NULL
 	},
 
 	{
@@ -2349,7 +2349,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
 	},
 
 	{
@@ -2360,7 +2360,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 be047e3032..bd682d6368 100644
--- a/src/include/access/slru.h
+++ b/src/include/access/slru.h
@@ -17,6 +17,14 @@
 #include "storage/lwlock.h"
 #include "storage/sync.h"
 
+/*
+ * SLRU bank size for slotno hash banks.  Limit the bank size to 16 because we
+ * perform sequential search within a bank (while looking for a target page or
+ * while doing victim buffer search) and if we keep this size big then it may
+ * affect the performance.
+ */
+#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.
@@ -147,6 +155,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;
@@ -184,5 +198,6 @@ extern bool SlruScanDirCbReportPresence(SlruCtl ctl, char *filename,
 										int64 segpage, void *data);
 extern bool SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64 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)

