RE: [PATCH] Use optimized single-datum tuplesort in ExecSort - Mailing list pgsql-hackers

From houzj.fnst@fujitsu.com
Subject RE: [PATCH] Use optimized single-datum tuplesort in ExecSort
Date
Msg-id OS0PR01MB57168410EF53D32035C97CA894E49@OS0PR01MB5716.jpnprd01.prod.outlook.com
Whole thread Raw
In response to Re: [PATCH] Use optimized single-datum tuplesort in ExecSort  (David Rowley <dgrowleyml@gmail.com>)
List pgsql-hackers
On July 22, 2021 8:38 AM David Rowley <dgrowleyml@gmail.com>
> On Thu, 22 Jul 2021 at 12:27, houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com>
> wrote:
> > The above seems can be shorter like the following ?
> >
> > for (;;)
> > {
> >         slot = ExecProcNode(outerNode);
> >         if (TupIsNull(slot))
> >                 break;
> >         if (node->datumSort)
> >         {
> >                 slot_getsomeattrs(slot, 1);
> >                 tuplesort_putdatum(tuplesortstate,
> >                                         slot->tts_values[0],
> >                                         slot->tts_isnull[0]);
> >         }
> >         else
> >                 tuplesort_puttupleslot(tuplesortstate, slot); }
> 
> I don't think that's a good change.  It puts the branch inside the loop the pulls
> all tuples from the subplan.  Given the loop is likely to be very hot combined
> with the fact that it's so simple, I'd much rather have two separate loops to
> keep the extra branch outside the loop.  It's true the branch predictor is likely
> to get the prediction correct on each iteration, but unless the compiler
> rewrites this into two loops then the comparison and jump must be done per
> loop.

Ah, you are right, I missed that. Thanks for the explanation.

Best regards,
houzj

pgsql-hackers by date:

Previous
From: "houzj.fnst@fujitsu.com"
Date:
Subject: RE: Added schema level support for publication.
Next
From: Bryn Llewellyn
Date:
Subject: Re: Have I found an interval arithmetic bug?