Re: Fix bank selection logic in SLRU

2025-01-08 Thread Alvaro Herrera
On 2024-Dec-27, Andrey Borodin wrote: > > On 19 Dec 2024, at 20:48, Yura Sokolov wrote: > > > > Here's version with type change bits16 -> uint16 > > Thanks! This version looks good to me. I’ll mark the CF entry as RfC. Thank you, I have pushed this. -- Álvaro Herrera 48°01'N 7°

Re: Fix bank selection logic in SLRU

2024-12-27 Thread Andrey Borodin
> On 19 Dec 2024, at 20:48, Yura Sokolov wrote: > > Here's version with type change bits16 -> uint16 Thanks! This version looks good to me. I’ll mark the CF entry as RfC. Best regards, Andrey Borodin.

Re: Fix bank selection logic in SLRU

2024-12-19 Thread Yura Sokolov
19.12.2024 15:10, Andrey M. Borodin wrote: On 19 Dec 2024, at 15:37, Yura Sokolov wrote: So, there's no more than 8192 banks at the moment. OK, but still current type indicates bitwise usage, while struct member is used as a number. Ok, I agree. Here's version with type change bits16 ->

Re: Fix bank selection logic in SLRU

2024-12-19 Thread Andrey M. Borodin
> On 19 Dec 2024, at 15:37, Yura Sokolov wrote: > > So, there's no more than 8192 banks at the moment. OK, but still current type indicates bitwise usage, while struct member is used as a number. Best regards, Andrey Borodin.

Re: Fix bank selection logic in SLRU

2024-12-19 Thread Yura Sokolov
19.12.2024 13:10, Andrey Borodin пишет: On 19 Dec 2024, at 15:01, Yura Sokolov wrote: - `&` takes 0.69ns - `mult-rec` takes 2.94ns - `%` takes 3.24ns. Thanks, Yura, for benchmarks and off-list conversation. I’ve reproduced similar numbers on my Apple M2. I agree that additional 3-4ns are n

Re: Fix bank selection logic in SLRU

2024-12-19 Thread Andrey Borodin
> On 19 Dec 2024, at 15:01, Yura Sokolov wrote: > > - `&` takes 0.69ns > - `mult-rec` takes 2.94ns > - `%` takes 3.24ns. Thanks, Yura, for benchmarks and off-list conversation. I’ve reproduced similar numbers on my Apple M2. I agree that additional 3-4ns are negligible in case of SLRU access.

Re: Fix bank selection logic in SLRU

2024-12-19 Thread Yura Sokolov
10.12.2024 17:07, Dilip Kumar wrote: On Tue, 10 Dec 2024 at 6:32 PM, Andrey M. Borodin > wrote: > On 10 Dec 2024, at 15:39, Yura Sokolov mailto:y.soko...@postgrespro.ru>> wrote: > > It is not critical bug, since it doesn't hurt correctness just p

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Andrey M. Borodin
> On 10 Dec 2024, at 16:58, Dilip Kumar wrote: > > slru buffers are in multiple of 16(bank size) Yes, my example with 64 buffers is incorrect. The worst case scenario is when user configures 80, 144, 528 or 1040 buffers, but only two banks (32 buffers) will be used. Best regards, Andrey Bo

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Dilip Kumar
On Tue, 10 Dec 2024 at 6:32 PM, Andrey M. Borodin wrote: > > > > On 10 Dec 2024, at 15:39, Yura Sokolov wrote: > > > > It is not critical bug, since it doesn't hurt correctness just > performance. In worst case only one bank will be used. > > Ugh... yeah. IMO the problem is that we do not have p

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Dilip Kumar
On Tue, 10 Dec 2024 at 7:30 PM, Robert Haas wrote: > On Tue, Dec 10, 2024 at 8:58 AM Dilip Kumar wrote: > >> Bank selection code assumes that number of buffers is power of 2. > >> If the number of buffers is not power of 2 - only subset of buffers > will be used. In worst case, e.g. 65 buffers,

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Robert Haas
On Tue, Dec 10, 2024 at 8:58 AM Dilip Kumar wrote: >> Bank selection code assumes that number of buffers is power of 2. >> If the number of buffers is not power of 2 - only subset of buffers will be >> used. In worst case, e.g. 65 buffers, everything will be buffered only in >> bank 64. > > But

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Dilip Kumar
On Tue, 10 Dec 2024 at 7:24 PM, Andrey M. Borodin wrote: > > > > On 10 Dec 2024, at 16:26, Dilip Kumar wrote: > > > > IIUC, we do check that it should be in multiple of bank size (i.e.) > > which is multiple of 2, right? Am I missing something? > > Bank selection code assumes that number of buf

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Andrey M. Borodin
> On 10 Dec 2024, at 16:26, Dilip Kumar wrote: > > IIUC, we do check that it should be in multiple of bank size (i.e.) > which is multiple of 2, right? Am I missing something? Bank selection code assumes that number of buffers is power of 2. If the number of buffers is not power of 2 - only

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Dilip Kumar
On Tue, Dec 10, 2024 at 6:32 PM Andrey M. Borodin wrote: > > > > > On 10 Dec 2024, at 15:39, Yura Sokolov wrote: > > > > It is not critical bug, since it doesn't hurt correctness just performance. > > In worst case only one bank will be used. > > Ugh... yeah. IMO the problem is that we do not ha

Re: Fix bank selection logic in SLRU

2024-12-10 Thread Andrey M. Borodin
> On 10 Dec 2024, at 15:39, Yura Sokolov wrote: > > It is not critical bug, since it doesn't hurt correctness just performance. > In worst case only one bank will be used. Ugh... yeah. IMO the problem is that we do not have protection that rejects values that are not power of 2. If other va

Fix bank selection logic in SLRU

2024-12-10 Thread Yura Sokolov
Good day, hackers. Due to long discussion about SLRU size configuration [1] and code evolution, non-serious bug were introduced: - intermediate versions assumed cache size is always power of 2, - therefore to determine bank simple binary-and with mask were used: bankno = pageno & ctl->ban