RE: [PATCH] Use optimized single-datum tuplesort in ExecSort

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