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

David Rowley <dgrowleyml@gmail.com>

From: David Rowley <dgrowleyml@gmail.com>
To: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
Cc: Ronan Dunklau <ronan.dunklau@aiven.io>, Ranier Vilela <ranier.vf@gmail.com>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>, Dilip Kumar <dilipbalaut@gmail.com>, James Coleman <jtc331@gmail.com>
Date: 2021-07-22T00:38:25Z
Lists: pgsql-hackers
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.

David



Commits

  1. Make nodeSort.c use Datum sorts for single column sorts