public inbox for [email protected]
help / color / mirror / Atom feedFrom: Michael Paquier <[email protected]>
To: Bertrand Drouvot <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: [email protected]
Subject: Re: define pg_structiszero(addr, s, r)
Date: Tue, 5 Nov 2024 13:31:58 +0900
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <CAApHDvrXzPAr3FxoBuB7b3D-okNoNA2jxLun1rW8Yw5wkbqusw@mail.gmail.com>
<ZyR2Tzdmj9c6E/[email protected]>
<CAApHDvrEDWPEyWQF7UhAt+8JR6w2eVpv1xP21FqFH+c=8Lb3JA@mail.gmail.com>
<[email protected]>
<[email protected]>
<[email protected]>
<CAApHDvqULM7KhreSa8PU2bshrguOaRdJsfjKCTvDkByYRFLaMg@mail.gmail.com>
<[email protected]>
<CAApHDvoU1SfD6rWjGp2tpLd+VGg_TfXhPJgCSNY5Z5LULXiv1A@mail.gmail.com>
<[email protected]>
On Mon, Nov 04, 2024 at 05:17:54PM +0000, Bertrand Drouvot wrote:
> Hi,
>
> On Tue, Nov 05, 2024 at 12:24:48AM +1300, David Rowley wrote:
> > On Sat, 2 Nov 2024 at 01:50, Bertrand Drouvot
> > <[email protected]> wrote:
> > >
> > > On Fri, Nov 01, 2024 at 09:47:05PM +1300, David Rowley wrote:
> > > > I've attached what I thought a more optimal version might look like in
> > > > case anyone thinks making it better is a good idea.
> > > >
> > >
> > > Thanks for the proposal!
> > >
> > > I like the idea, I think that's worth to add a few comments, something like:
> >
> > I'm happy if you want to pick this up and continue working on it.
>
> Sure, please find attached v1, the changes are:
>
> - switch from "const char" to "const unsigned char" (could have been done in the
> current version of pg_memory_is_all_zeros() though)
> - added some comments
+ * The test is divided into three phases for efficiency:
+ * - Initial alignment (byte per byte comparison)
+ * - Multiple bytes comparison at once
+ * - Remaining bytes (byte per byte comparison)
It does not look like this insists enough on the alignment part of the
optization? A MAXALIGN'd size would use only size_t comparisons, and
a pointer aligned would do no byte comparisons.
> - adding an Assert for ptr != 0
I'm not sure that the Assert() addition is a good idea. That could
get hot very easily depending on the caller, even if for assert
builds we don't care much about the performance, that could lead to
some paths being a lot slower.
On Fri, 01 Nov 2024 at 21:47, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> 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.
These three don't matter in terms of performance. The cycles spent
for all-zero checks of the checkpointer and bgwriter are nothing
compared to the concurrent I/O activity they handle, and the sizes are
small compared to the 8k pages. The flush callback for relation
pgstats happens once at a given interval (see around
PGSTAT_[MIN|MAX]_INTERVAL).
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../[email protected]/2-signature.asc)
download
view thread (36+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: define pg_structiszero(addr, s, r)
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox