Content-Type: multipart/mixed; boundary="------------FV6030Z5TGM2q3RK2JDsT5e2"
Message-ID: <9444dc46-ca47-43ed-9058-89c456316306@postgrespro.ru>
Date: Tue, 10 Dec 2024 15:39:28 +0300
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Content-Language: en-US
To: "pgsql-hackers@lists.postgresql.org" <pgsql-hackers@lists.postgresql.org>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Dilip Kumar <dilipbalaut@gmail.com>, "Andrey M. Borodin"
 <x4mmm@yandex-team.ru>
From: Yura Sokolov <y.sokolov@postgrespro.ru>
Subject: Fix bank selection logic in SLRU
Archived-At: <https://www.postgresql.org/message-id/9444dc46-ca47-43ed-9058-89c456316306%40postgrespro.ru>
Precedence: bulk

This is a multi-part message in MIME format.
--------------FV6030Z5TGM2q3RK2JDsT5e2
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

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->bank_mask;

- final merged version allows arbitrary cache size for every cache
   type, but code for bankno were not fixed.

It is not critical bug, since it doesn't hurt correctness just 
performance. In worst case only one bank will be used.

I attach the patch, that changes SlruCtlData->bank_mask to ->nbanks,
and changes calculation to modulo operation.

     bankno = pageno % ctl->nbanks;

Probably, instead of modulo operation could be used multiplication:

     bankno = ((uint64) murmurhash32(pageno) * ctl->nbanks) >> 32;

But I didn't bother to measure does it pay for or not.

[1] 
https://www.postgresql.org/message-id/flat/CAFiTN-vzDvNz%3DExGXz6gdyjtzGixKSqs0mKHMmaQ8sOSEFZ33A%40mail.gmail.com

Regards,
Yura Sokolov aka funny-falcon
--------------FV6030Z5TGM2q3RK2JDsT5e2
Content-Type: text/x-patch; charset=UTF-8;
 name="v1-0001-Fix-SLRU-bank-selection.patch"
Content-Disposition: attachment;
 filename="v1-0001-Fix-SLRU-bank-selection.patch"
Content-Transfer-Encoding: base64

RnJvbSA0YjQzOGU2N2E3OWQ3N2JmMmNhZjNlNWEwMzg2YmI3NzAwZDMyOWJhIE1vbiBTZXAg
MTcgMDA6MDA6MDAgMjAwMQpGcm9tOiBZdXJhIFNva29sb3YgPHkuc29rb2xvdkBwb3N0Z3Jl
c3Byby5ydT4KRGF0ZTogVHVlLCAxMCBEZWMgMjAyNCAxNTozODowMiArMDMwMApTdWJqZWN0
OiBbUEFUQ0ggdjFdIEZpeCBTTFJVIGJhbmsgc2VsZWN0aW9uLgoKRHVlIHRvIGNvZGUgZXZv
bHV0aW9uLCBuYmFua3MgaXMgbm90IHBvd2VyIG9mIHR3byBhbnkgbW9yZS4gQnV0IHN0aWxs
CmJpbmFyeSAmIHdlcmUgdXNlZCB0byBvYnRhaW4gYmFua25vLgotLS0KIHNyYy9iYWNrZW5k
L2FjY2Vzcy90cmFuc2FtL3NscnUuYyB8IDYgKysrLS0tCiBzcmMvaW5jbHVkZS9hY2Nlc3Mv
c2xydS5oICAgICAgICAgfCA2ICsrKy0tLQogMiBmaWxlcyBjaGFuZ2VkLCA2IGluc2VydGlv
bnMoKyksIDYgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvc3JjL2JhY2tlbmQvYWNjZXNz
L3RyYW5zYW0vc2xydS5jIGIvc3JjL2JhY2tlbmQvYWNjZXNzL3RyYW5zYW0vc2xydS5jCmlu
ZGV4IGU3ZjczYmY0Mjc1Li5hZmVkYjVjMDM5ZiAxMDA2NDQKLS0tIGEvc3JjL2JhY2tlbmQv
YWNjZXNzL3RyYW5zYW0vc2xydS5jCisrKyBiL3NyYy9iYWNrZW5kL2FjY2Vzcy90cmFuc2Ft
L3NscnUuYwpAQCAtMzQzLDcgKzM0Myw3IEBAIFNpbXBsZUxydUluaXQoU2xydUN0bCBjdGws
IGNvbnN0IGNoYXIgKm5hbWUsIGludCBuc2xvdHMsIGludCBubHNucywKIAljdGwtPnNoYXJl
ZCA9IHNoYXJlZDsKIAljdGwtPnN5bmNfaGFuZGxlciA9IHN5bmNfaGFuZGxlcjsKIAljdGwt
Pmxvbmdfc2VnbWVudF9uYW1lcyA9IGxvbmdfc2VnbWVudF9uYW1lczsKLQljdGwtPmJhbmtf
bWFzayA9IChuc2xvdHMgLyBTTFJVX0JBTktfU0laRSkgLSAxOworCWN0bC0+bmJhbmtzID0g
bmJhbmtzOwogCXN0cmxjcHkoY3RsLT5EaXIsIHN1YmRpciwgc2l6ZW9mKGN0bC0+RGlyKSk7
CiB9CiAKQEAgLTYwNiw3ICs2MDYsNyBAQCBTaW1wbGVMcnVSZWFkUGFnZV9SZWFkT25seShT
bHJ1Q3RsIGN0bCwgaW50NjQgcGFnZW5vLCBUcmFuc2FjdGlvbklkIHhpZCkKIHsKIAlTbHJ1
U2hhcmVkCXNoYXJlZCA9IGN0bC0+c2hhcmVkOwogCUxXTG9jawkgICAqYmFua2xvY2sgPSBT
aW1wbGVMcnVHZXRCYW5rTG9jayhjdGwsIHBhZ2Vubyk7Ci0JaW50CQkJYmFua25vID0gcGFn
ZW5vICYgY3RsLT5iYW5rX21hc2s7CisJaW50CQkJYmFua25vID0gcGFnZW5vICUgY3RsLT5u
YmFua3M7CiAJaW50CQkJYmFua3N0YXJ0ID0gYmFua25vICogU0xSVV9CQU5LX1NJWkU7CiAJ
aW50CQkJYmFua2VuZCA9IGJhbmtzdGFydCArIFNMUlVfQkFOS19TSVpFOwogCkBAIC0xMTc3
LDcgKzExNzcsNyBAQCBTbHJ1U2VsZWN0TFJVUGFnZShTbHJ1Q3RsIGN0bCwgaW50NjQgcGFn
ZW5vKQogCQlpbnQJCQliZXN0aW52YWxpZHNsb3QgPSAwOwkvKiBrZWVwIGNvbXBpbGVyIHF1
aWV0ICovCiAJCWludAkJCWJlc3RfaW52YWxpZF9kZWx0YSA9IC0xOwogCQlpbnQ2NAkJYmVz
dF9pbnZhbGlkX3BhZ2VfbnVtYmVyID0gMDsJLyoga2VlcCBjb21waWxlciBxdWlldCAqLwot
CQlpbnQJCQliYW5rbm8gPSBwYWdlbm8gJiBjdGwtPmJhbmtfbWFzazsKKwkJaW50CQkJYmFu
a25vID0gcGFnZW5vICUgY3RsLT5uYmFua3M7CiAJCWludAkJCWJhbmtzdGFydCA9IGJhbmtu
byAqIFNMUlVfQkFOS19TSVpFOwogCQlpbnQJCQliYW5rZW5kID0gYmFua3N0YXJ0ICsgU0xS
VV9CQU5LX1NJWkU7CiAKZGlmZiAtLWdpdCBhL3NyYy9pbmNsdWRlL2FjY2Vzcy9zbHJ1Lmgg
Yi9zcmMvaW5jbHVkZS9hY2Nlc3Mvc2xydS5oCmluZGV4IDk3ZTYxMmNkMTAwLi5mYWJlYTIy
MDI5MCAxMDA2NDQKLS0tIGEvc3JjL2luY2x1ZGUvYWNjZXNzL3NscnUuaAorKysgYi9zcmMv
aW5jbHVkZS9hY2Nlc3Mvc2xydS5oCkBAIC0xMjksOSArMTI5LDkgQEAgdHlwZWRlZiBzdHJ1
Y3QgU2xydUN0bERhdGEKIAlTbHJ1U2hhcmVkCXNoYXJlZDsKIAogCS8qCi0JICogQml0bWFz
ayB0byBkZXRlcm1pbmUgYmFuayBudW1iZXIgZnJvbSBwYWdlIG51bWJlci4KKwkgKiBOdW1i
ZXIgb2YgYmFua3MgdG8gZGV0ZXJtaW5lIGJhbmsgbnVtYmVyIGZyb20gcGFnZSBudW1iZXIu
CiAJICovCi0JYml0czE2CQliYW5rX21hc2s7CisJYml0czE2CQluYmFua3M7CiAKIAkvKgog
CSAqIElmIHRydWUsIHVzZSBsb25nIHNlZ21lbnQgZmlsZSBuYW1lcy4gIE90aGVyd2lzZSwg
dXNlIHNob3J0IGZpbGUgbmFtZXMuCkBAIC0xNzksNyArMTc5LDcgQEAgU2ltcGxlTHJ1R2V0
QmFua0xvY2soU2xydUN0bCBjdGwsIGludDY0IHBhZ2VubykKIHsKIAlpbnQJCQliYW5rbm87
CiAKLQliYW5rbm8gPSBwYWdlbm8gJiBjdGwtPmJhbmtfbWFzazsKKwliYW5rbm8gPSBwYWdl
bm8gJSBjdGwtPm5iYW5rczsKIAlyZXR1cm4gJihjdGwtPnNoYXJlZC0+YmFua19sb2Nrc1ti
YW5rbm9dLmxvY2spOwogfQogCi0tIAoyLjQzLjAKCg==

--------------FV6030Z5TGM2q3RK2JDsT5e2--