Hi!
On 16.01.2025 04:36, Tom Lane wrote:
"Anton A. Melnikov" <a.melni...@postgrespro.ru> writes:
Seems it is possible to exclude much less code from checking
under valgrind and get the same result by replacing the only
function call pg_rightmost_one_pos64() with a valgrind-safe
code. See the attached patch, please.
There is no place anywhere in our code base where we hide unsafe
code from valgrind rather than fixing said code. This does not
seem like a place to start such an ugly practice. Performance
does not trump everything else.
Thanks for remark. Agreed.
I'd be inclined to just remove the pg_rightmost_one_pos64 call
in favor of the other coding you suggest.
Here is a patch like that.
With the best wishes,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
From bac26e0c7ee900246d454dd98de45b3ecefc15ec Mon Sep 17 00:00:00 2001
From: "Anton A. Melnikov" <a.melni...@postgrespro.ru>
Date: Thu, 16 Jan 2025 02:56:52 +0300
Subject: [PATCH] Use valgrind-safe code to find a number of rightmost zero
bytes.
---
src/include/common/hashfn_unstable.h | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/src/include/common/hashfn_unstable.h b/src/include/common/hashfn_unstable.h
index a09c0b7d5ce..fa9b954c6b9 100644
--- a/src/include/common/hashfn_unstable.h
+++ b/src/include/common/hashfn_unstable.h
@@ -299,15 +299,21 @@ fasthash_accum_cstring_aligned(fasthash_state *hs, const char *str)
if (firstbyte64(chunk) != 0)
{
- size_t remainder;
+ size_t remainder = 0;
uint64 mask;
/*
- * The byte corresponding to the NUL will be 0x80, so the rightmost
- * bit position will be in the range 15, 23, ..., 63. Turn this into
- * byte position by dividing by 8.
+ * The byte corresponding to the NUL terminator will be the rightmost 0x80.
+ * All zero bytes to the right of it correspond to the tail of the string.
+ * It remains to count them.
*/
- remainder = pg_rightmost_one_pos64(zero_byte_low) / BITS_PER_BYTE;
+ while ((zero_byte_low & 0xFF) == 0)
+ {
+ zero_byte_low >>= 8;
+ ++remainder;
+ }
+
+ Assert(remainder != 0);
/*
* Create a mask for the remaining bytes so we can combine them into
--
2.48.1
