Re: Fix bank selection logic in SLRU

Yura Sokolov <y.sokolov@postgrespro.ru>

From: Yura Sokolov <y.sokolov@postgrespro.ru>
To: Dilip Kumar <dilipbalaut@gmail.com>, "Andrey M. Borodin" <x4mmm@yandex-team.ru>
Cc: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>, Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: 2024-12-19T10:01:07Z
Lists: pgsql-hackers
10.12.2024 17:07, Dilip Kumar wrote:
> On Tue, 10 Dec 2024 at 6:32 PM, Andrey M. Borodin <x4mmm@yandex-team.ru 
> <mailto:x4mmm@yandex-team.ru>> wrote:
> 
> 
> 
>      > On 10 Dec 2024, at 15:39, Yura Sokolov <y.sokolov@postgrespro.ru
>     <mailto:y.sokolov@postgrespro.ru>> 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 values given system operates as if there are
>     2^(popcount(n)-1) banks. So if we just round down value to nearest
>     power of 2 - we will help incorrectly configured systems to use
>     proper amount of memory and keep performance of properly configured
>     systems.
> 
> 
> +1
> 
> 
> 
>     IMO doing modulo is not necessary. And hash function is pure waste
>     of CPU cycles.
> 
> 
> I agree

I did some measurement "divide-modulo" vs "modulo using multiplication by
reciprocal" vs "simple binary and" using simple C program [0].
(Note: loop is made to be dependent on previous iteration result so no
parallel computation happens).

Results on Ryzen 7 5825U:

     $ time ./div 100000000 15 3 # binary and
     real    0m0,943s
     $ time ./div 100000000 15 1 # multiply by reciprocal
     real    0m3,123s
     $ time ./div 100000000 15 0 # just plain `%`
     real    0m4,540s

It means:
- `&` takes 0.69ns
- `mult-rec` takes 2.94ns
- `%` takes 3.24ns.

I believe, compared to further memory accesses it could be count as
negligible.

(Certainly, it could be worse on some older processors. But I still doubt
it will be measurably worse on top of all other things SLRU does.)

[0] https://gist.github.com/funny-falcon/173923b4fea7ffdf9e02595a0f99aa74


Regards,
Yura Sokolov aka funny-falcon



Commits

  1. Fix SLRU bank selection code