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

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.

David