From 5fc4579e34fc417129e697542b4ac71c93523d1c Mon Sep 17 00:00:00 2001 From: Dilip Kumar Date: Thu, 30 Nov 2023 13:41:50 +0530 Subject: [PATCH v13 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 5d96195c53..7d349d2213 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 27aab51162..41337471e2 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 1957845f58..f8eceeac30 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 9ac4790f16..211527b075 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 6059999a3c..82243c2728 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 20a4dfec2a..9059c0a202 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 7fc34720bf..10c51e2883 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 e56c14b78f..8aad05eaf5 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2319,7 +2319,7 @@ struct config_int ConfigureNamesInt[] = }, &multixact_offsets_buffers, 64, 16, SLRU_MAX_ALLOWED_BUFFERS, - NULL, NULL, NULL + check_multixact_offsets_buffers, NULL, NULL }, { @@ -2330,7 +2330,7 @@ struct config_int ConfigureNamesInt[] = }, &multixact_members_buffers, 64, 16, SLRU_MAX_ALLOWED_BUFFERS, - NULL, NULL, NULL + check_multixact_members_buffers, NULL, NULL }, { @@ -2341,7 +2341,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, @@ -2351,7 +2351,7 @@ struct config_int ConfigureNamesInt[] = }, ¬ify_buffers, 64, 16, SLRU_MAX_ALLOWED_BUFFERS, - NULL, NULL, NULL + check_notify_buffers, NULL, NULL }, { @@ -2362,7 +2362,7 @@ struct config_int ConfigureNamesInt[] = }, &serial_buffers, 64, 16, SLRU_MAX_ALLOWED_BUFFERS, - NULL, NULL, NULL + check_serial_buffers, NULL, NULL }, { @@ -2373,7 +2373,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 }, { @@ -2384,7 +2384,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 72b30bba7f..2b74e11d42 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 a7bcb6b42a..f458da88ac 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)