On Mon, Dec 11, 2023 at 10:42 AM Dilip Kumar <dilipbal...@gmail.com> wrote:
> On Thu, Nov 30, 2023 at 3:30 PM Dilip Kumar <dilipbal...@gmail.com> wrote:
> >
> > On Wed, Nov 29, 2023 at 4:58 PM Dilip Kumar <dilipbal...@gmail.com>
> wrote:
>
> Here is the updated patch based on some comments by tender wang (those
> comments were sent only to me)
>
few nitpicks:
+
+ /*
+ * 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;
...
...
+ int bankno = pageno & ctl->bank_mask;
I am a bit uncomfortable seeing it as a mask, why can't it be simply a
number
of banks (num_banks) and get the bank number through modulus op (pageno %
num_banks) instead of bitwise & operation (pageno & ctl->bank_mask) which
is a
bit difficult to read compared to modulus op which is quite simple,
straightforward and much common practice in hashing.
Are there any advantages of using & over % ?
Also, a few places in 0002 and 0003 patch, need the bank number, it is
better
to have a macro for that.
---
extern bool SlruScanDirCbDeleteAll(SlruCtl ctl, char *filename, int64
segpage,
void *data);
-
+extern bool check_slru_buffers(const char *name, int *newval);
#endif /* SLRU_H */
Add an empty line after the declaration, in 0002 patch.
---
-TransactionIdSetStatusBit(TransactionId xid, XidStatus status, XLogRecPtr
lsn, int slotno)
+TransactionIdSetStatusBit(TransactionId xid, XidStatus status, XLogRecPtr
lsn,
+ int slotno)
Unrelated change for 0003 patch.
---
Regards,
Amul
