On Thu, Sep 19, 2024 at 4:57 AM Bertrand Drouvot <bertranddrouvot...@gmail.com> wrote: > > Hi, > > On Wed, Sep 18, 2024 at 10:03:21AM +0200, Peter Eisentraut wrote: > > On 18.09.24 06:16, Bertrand Drouvot wrote: > > > +#define pg_structiszero(addr, s, r) > > > \ > > > + do { > > > \ > > > + /* We assume this initializes to zeroes */ > > > \ > > > + static const s all_zeroes; > > > \ > > > + r = (memcmp(addr, &all_zeroes, sizeof(all_zeroes)) == 0); > > > \ > > > + } while (0) > > > > Thanks for the feedback. > > > This assumption is kind of the problem, isn't it? Because, you can't assume > > that. And the existing code is arguably kind of wrong. But moreover, this > > macro also assumes that the "addr" argument has no random padding bits. > > > > In the existing code, you can maybe make a local analysis that the code is > > working correctly, although I'm not actually sure. > > I think it is but will give it a closer look. > > > But if you are > > repackaging this as a general macro under a general-sounding name, then the > > requirements should be more stringent. > > Agree. That said in v2 ([1]), it has been renamed to pgstat_entry_all_zeros(). > > I think that I will: > > 1/ take a closer look regarding the existing assumption > 2/ if 1/ outcome is fine, then add more detailed comments around > pgstat_entry_all_zeros() to make sure it's not used outside of the existing > context > > Sounds good to you? > > [1]: > https://www.postgresql.org/message-id/ZuqHLCdZXtEsbyb/%40ip-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 >
Hi, if this is performance critical then FWIW my understanding is that memcmp can outperform simple loop checking, and my hacky testing seemed to confirm that. See https://godbolt.org/z/GWY1ob9bE static inline bool is_all_zeros2(const char *p, size_t len) { static size_t nz = 0; static const char *z = NULL; if (nz < len) { if (z) free((void *)z); nz = len; z = (const char *)calloc(1, nz); } return memcmp(p, z, len) == 0; } ~~ Executor x86-64 gcc 14.2 (C, Editor #1) Program stdout Iterate 1000000 times... check zeros using loop -- elapsed=0.041196s check zeros using memcmp -- elapsed=0.016407s ====== Kind Regards, Peter Smith. Fujitsu Australia