Re: define pg_structiszero(addr, s, r)

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: define pg_structiszero(addr, s, r)
Date: 2024-11-05 05:59:46
Message-ID: Zym0UvNM+fswQsIZ@ip-10-97-1-34.eu-west-3.compute.internal
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On Tue, Nov 05, 2024 at 01:31:58PM +0900, Michael Paquier wrote:
> 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
> > > <bertranddrouvot(dot)pg(at)gmail(dot)com> 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.

I'm not sure to get this one. Is it more clear in the code comments that
we can start multiple bytes comparison once p is aligned?

> > - 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.

Yeah, agree, removed in v2 attached.

Regards,

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

Attachment Content-Type Size
v2-0001-Optimize-pg_memory_is_all_zeros.patch text/x-diff 3.3 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Gurjeet Singh 2024-11-05 06:00:10 Re: doc: pgevent.dll location
Previous Message Michael Paquier 2024-11-05 05:50:06 Re: Consider pipeline implicit transaction as a transaction block