Re: define pg_structiszero(addr, s, r)

Japin Li <japinli@hotmail.com>

From: Japin Li <japinli@hotmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>, Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Peter Smith <smithpb2250@gmail.com>, Peter Eisentraut <peter@eisentraut.org>, Heikki Linnakangas <hlinnaka@iki.fi>, pgsql-hackers@lists.postgresql.org
Date: 2024-11-01T15:01:13Z
Lists: pgsql-hackers
On Fri, 01 Nov 2024 at 21:47, David Rowley <dgrowleyml@gmail.com> wrote:
> On Fri, 1 Nov 2024 at 20:49, Michael Paquier <michael@paquier.xyz> wrote:
>>
>> On Fri, Nov 01, 2024 at 07:44:22AM +0000, Bertrand Drouvot wrote:
>> > Worth to add a comment as to why pg_memory_is_all_zeros() should not
>> > be used here?
>>
>> I would not add one in bufpage.c, documenting that where
>> pg_memory_is_all_zeros() is defined may be more adapted.
>
> The thought of having to write a comment to warn people not to use it
> for performance-critical things makes me think it might be better just
> to write a more optimal version of the function so we don't need to
> warn people. I looked around at the callers of the function I saw the
> following numbers of bytes being used for the length: 8192 (the one in
> question), 88, 32 and 112.
>
> I don't know how performance-critical the final three of those are,
> but I imagine all apart from the 32-byte one might be better with a
> non-inlined and more optimised version of the function. The problem
> with inlining the optimised version is that it's more code to inline.
>
+1

Is there a possible overflow?

+	const char *end = &p[len];

How about use MAXALIGN64 macro here?

When handling the aligned, is it possible to handle multiple values
(such as 4 or 8) in one iteration?

It might be faster. However, I'm not tested.


-- 
Regrads,
Japin Li



Commits

  1. Use pg_memory_is_all_zeros() in PageIsVerifiedExtended()

  2. Optimize pg_memory_is_all_zeros() in memutils.h

  3. Remove use of pg_memory_is_all_zeros() in bufpage.c

  4. Add pg_memory_is_all_zeros() in memutils.h