database/postgres
database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-04-27 22:56:53 +03:00
Code

Fix SLRU bank selection code

Browse Source 
The originally submitted code (using bit masking) was correct when the
number of slots was restricted to be a power of two -- but that
limitation was removed during development that led to commit
53c2a97a9266, which made the bank selection code incorrect.  This led to
always using a smaller number of banks than available.  Change said code
to use integer modulo instead, which works correctly with an arbitrary
number of banks.

It's likely that we could improve on this to avoid runtime use of
integer division.  But with this change we're, at least, not wasting
memory on unused banks, and more banks mean less contention, which is
likely to have a much higher performance impact than a single
instruction's latency.

Author: Yura Sokolov <y.sokolov@postgrespro.ru>
Reviewed-by: Andrey Borodin <x4mmm@yandex-team.ru>
Discussion: https://postgr.es/m/9444dc46-ca47-43ed-9058-89c456316306@postgrespro.ru
This commit is contained in:
Álvaro Herrera 2025-01-09 07:33:52 +01:00
parent 970b97eeb8
commit 69ab446514
No known key found for this signature in database
GPG Key ID: 1C20ACB9D5C564AE
2 changed files with 6 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
src/backend/access/transam/slru.c
View File

@ -343,7 +343,7 @@ SimpleLruInit(SlruCtl ctl, const char *name, int nslots, int nlsns,
ctl->shared = shared; ctl->shared = shared;
ctl->sync_handler = sync_handler; ctl->sync_handler = sync_handler;
ctl->long_segment_names = long_segment_names; ctl->long_segment_names = long_segment_names;
ctl->bank_mask = (nslots / SLRU_BANK_SIZE) - 1; ctl->nbanks = nbanks;
strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir)); strlcpy(ctl->Dir, subdir, sizeof(ctl->Dir));
} }
@ -606,7 +606,7 @@ SimpleLruReadPage_ReadOnly(SlruCtl ctl, int64 pageno, TransactionId xid)
{ {
SlruShared shared = ctl->shared; SlruShared shared = ctl->shared;
LWLock *banklock = SimpleLruGetBankLock(ctl, pageno); LWLock *banklock = SimpleLruGetBankLock(ctl, pageno);
int bankno = pageno & ctl->bank_mask; int bankno = pageno % ctl->nbanks;
int bankstart = bankno * SLRU_BANK_SIZE; int bankstart = bankno * SLRU_BANK_SIZE;
int bankend = bankstart + SLRU_BANK_SIZE; int bankend = bankstart + SLRU_BANK_SIZE;
@ -1180,7 +1180,7 @@ SlruSelectLRUPage(SlruCtl ctl, int64 pageno)
int bestinvalidslot = 0; /* keep compiler quiet */ int bestinvalidslot = 0; /* keep compiler quiet */
int best_invalid_delta = -1; int best_invalid_delta = -1;
int64 best_invalid_page_number = 0; /* keep compiler quiet */ int64 best_invalid_page_number = 0; /* keep compiler quiet */
int bankno = pageno & ctl->bank_mask; int bankno = pageno % ctl->nbanks;
int bankstart = bankno * SLRU_BANK_SIZE; int bankstart = bankno * SLRU_BANK_SIZE;
int bankend = bankstart + SLRU_BANK_SIZE; int bankend = bankstart + SLRU_BANK_SIZE;

9
src/include/access/slru.h
View File

@ -128,10 +128,8 @@ typedef struct SlruCtlData
{ {
SlruShared shared; SlruShared shared;
/* /* Number of banks in this SLRU. */
* Bitmask to determine bank number from page number. uint16 nbanks;
*/
bits16 bank_mask;
/* /*
* If true, use long segment file names. Otherwise, use short file names. * If true, use long segment file names. Otherwise, use short file names.
@ -163,7 +161,6 @@ typedef struct SlruCtlData
* it's always the same, it doesn't need to be in shared memory. * it's always the same, it doesn't need to be in shared memory.
*/ */
char Dir[64]; char Dir[64];
} SlruCtlData; } SlruCtlData;
typedef SlruCtlData *SlruCtl; typedef SlruCtlData *SlruCtl;
@ -179,7 +176,7 @@ SimpleLruGetBankLock(SlruCtl ctl, int64 pageno)
{ {
int bankno; int bankno;
bankno = pageno & ctl->bank_mask; bankno = pageno % ctl->nbanks;
return &(ctl->shared->bank_locks[bankno].lock); return &(ctl->shared->bank_locks[bankno].lock);
} }