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

On July 22, 2021 8:38 AM David Rowley <dgrowleyml(at)gmail(dot)com>
> On Thu, 22 Jul 2021 at 12:27, houzj(dot)fnst(at)fujitsu(dot)com <houzj(dot)fnst(at)fujitsu(dot)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