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

From David Rowley
Subject Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
Date
Msg-id CAApHDvq6LZP5PhL=nch0paYY-6u4mpP=bwdhcJcWj5NbxE8X4g@mail.gmail.com
Whole thread Raw
In response to RE: [PATCH] Use optimized single-datum tuplesort in ExecSort  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
Responses RE: [PATCH] Use optimized single-datum tuplesort in ExecSort
List 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



pgsql-hackers by date:

Previous
From: "houzj.fnst@fujitsu.com"
Date:
Subject: RE: Added schema level support for publication.
Next
From: "houzj.fnst@fujitsu.com"
Date:
Subject: RE: Added schema level support for publication.