From: | Peter Smith <smithpb2250(at)gmail(dot)com> |
---|---|
To: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
Cc: | Peter Eisentraut <peter(at)eisentraut(dot)org>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Re: define pg_structiszero(addr, s, r) |
Date: | 2024-10-28 22:36:34 |
Message-ID: | CAHut+PvHmWiPyqiDRnD7FYsc4QskXpKEZAH3Z8Ahd_GSnRXWrw@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Sep 19, 2024 at 4:57 AM Bertrand Drouvot
<bertranddrouvot(dot)pg(at)gmail(dot)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
From | Date | Subject | |
---|---|---|---|
Next Message | David Rowley | 2024-10-28 22:54:01 | Re: EXPLAIN IndexOnlyScan shows disabled when enable_indexonlyscan=on |
Previous Message | David E. Wheeler | 2024-10-28 22:19:43 | Re: RFC: Extension Packaging & Lookup |