Make use of pg_memory_is_all_zeros() in more places

Started by Bertrand Drouvotover 1 year ago4 messageshackers
Jump to latest
#1Bertrand Drouvot
bertranddrouvot.pg@gmail.com

Hi hackers,

While searching for memcmp() calls in "*stat*.c" files (due to [1]/messages/by-id/Z1hNLvcPgVLPxCoc@ip-10-97-1-34.eu-west-3.compute.internal), it appeared
that we could $SUBJECT. Please find attached a patch doing so.

While at it, I did a quick check on all the memcmp() calls (even those not in
"*stat*.c" files) and it looks to me that there is no others that could be replaced
with pg_memory_is_all_zeros().

[1]: /messages/by-id/Z1hNLvcPgVLPxCoc@ip-10-97-1-34.eu-west-3.compute.internal

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

v1-0001-Make-use-of-pg_memory_is_all_zeros-in-more-places.patchtext/x-diff; charset=us-asciiDownload+6-13
#2Michael Paquier
michael@paquier.xyz
In reply to: Bertrand Drouvot (#1)
Re: Make use of pg_memory_is_all_zeros() in more places

On Tue, Dec 10, 2024 at 02:18:33PM +0000, Bertrand Drouvot wrote:

While searching for memcmp() calls in "*stat*.c" files (due to [1]), it appeared
that we could $SUBJECT. Please find attached a patch doing so.

-            SockAddr    zero_clientaddr;
-            memset(&zero_clientaddr, 0, sizeof(zero_clientaddr));
-            if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
-                       sizeof(zero_clientaddr)) == 0)
+            if (pg_memory_is_all_zeros(&(beentry->st_clientaddr),
+                                       sizeof(beentry->st_clientaddr)))

It also means that you're removing these variables used only for the
all-zero comparisons. Makes sense to me.
--
Michael

#3Bertrand Drouvot
bertranddrouvot.pg@gmail.com
In reply to: Michael Paquier (#2)
Re: Make use of pg_memory_is_all_zeros() in more places

Hi,

On Wed, Dec 11, 2024 at 03:03:34PM +0900, Michael Paquier wrote:

On Tue, Dec 10, 2024 at 02:18:33PM +0000, Bertrand Drouvot wrote:

While searching for memcmp() calls in "*stat*.c" files (due to [1]), it appeared
that we could $SUBJECT. Please find attached a patch doing so.

-            SockAddr    zero_clientaddr;
-            memset(&zero_clientaddr, 0, sizeof(zero_clientaddr));
-            if (memcmp(&(beentry->st_clientaddr), &zero_clientaddr,
-                       sizeof(zero_clientaddr)) == 0)
+            if (pg_memory_is_all_zeros(&(beentry->st_clientaddr),
+                                       sizeof(beentry->st_clientaddr)))

It also means that you're removing these variables used only for the
all-zero comparisons. Makes sense to me.

Yeap and also given its size (136 bytes), it looks like that it could provide
some performance benefits too (see check_SockAddr_is_zeroes.c attached):

$ gcc --version
gcc (Ubuntu 11.4.0-1ubuntu1~22.04) 11.4.0
Copyright (C) 2021 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ gcc -O2 check_SockAddr_is_zeroes.c -o check_SockAddr_is_zeroes; ./check_SockAddr_is_zeroes
memcmp: done in 21460 nanoseconds
pg_memory_is_all_zeros: done in 1307 nanoseconds (16.4193 times faster than memcmp)

$ /usr/local/gcc-14.1.0/bin/gcc-14.1.0 -O2 check_SockAddr_is_zeroes.c -o check_SockAddr_is_zeroes; ./check_SockAddr_is_zeroes
memcmp: done in 21012 nanoseconds
pg_memory_is_all_zeros: done in 4039 nanoseconds (5.20228 times faster than memcmp)

That's not a hot path but still interesting to see.

Regards,

--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com

Attachments:

check_SockAddr_is_zeroes.ctext/x-csrc; charset=us-asciiDownload
#4Nathan Bossart
nathandbossart@gmail.com
In reply to: Bertrand Drouvot (#3)
Re: Make use of pg_memory_is_all_zeros() in more places

Committed.

--
nathan