Re: define pg_structiszero(addr, s, r)

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>, Ranier Vilela <ranier.vf@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-06T04:44:58Z
Lists: pgsql-hackers
On Wed, Nov 06, 2024 at 03:53:48PM +1300, David Rowley wrote:
> I'm not sure if I'm clear on what works for you.  The latest patch I
> saw did 1 size_t per iteration. Are you saying we should do just
> size_t per loop? or we should form the code in a way that allows the
> compiler to use SIMD instructions?

Oh, sorry.  I thought that you wanted to keep the size_t checks as
done in [1] anyway.

But your suggestion is different, and instructions like xmmword would
be enough to show up as you group more the checks 8 at a time:
bool
pg_memory_is_all_zeros_size_t_times_8(const void *ptr, size_t len)
{
    const char *p = (const char *) ptr;
    const char *end = &p[len];
    const char *aligned_end = (const char *) ((uintptr_t) end & (~(sizeof(size_t) - 1)));

    while (((uintptr_t) p & (sizeof(size_t) - 1)) != 0)
    {
        if (p == end)
	    return true;
	if (*p++ != 0)
	    return false;
    }
    for (; p < aligned_end - (sizeof(size_t) * 7); p += sizeof(size_t) * 8)
    {
        if (((size_t *) p)[0] != 0 |
	    ((size_t *) p)[1] != 0 |
	    ((size_t *) p)[2] != 0 |
	    ((size_t *) p)[3] != 0 |
	    ((size_t *) p)[4] != 0 |
	    ((size_t *) p)[5] != 0 |
	    ((size_t *) p)[6] != 0 |
	    ((size_t *) p)[7] != 0)
	    return false;
    }
    while (p < end)
    {
        if (*p++ != 0)
	   return false;
    }
    return true;
}

That's smart for large areas to cover.  The patch should document why
we are doing it this way.  This should have a few more parenthesis in
the second loop, or -Wparentheses would complain.

Should the last loop check only 1 byte at a time or should this stuff
include one more step before the last one you wrote to do a couple of
checks with size_t?  That may matter for areas small enough (len <
sizeof(size_t) * 8) causing the second step to not be taken, but large
enough (len > sizeof(size_t)) to apply a couple of size_t checks per
loop. 

[1]: https://www.postgresql.org/message-id/Zym0UvNM+fswQsIZ@ip-10-97-1-34.eu-west-3.compute.internal
--
Michael

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