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

From: Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Ronan Dunklau <ronan(dot)dunklau(at)aiven(dot)io>, James Coleman <jtc331(at)gmail(dot)com>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>
Subject: Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
Date: 2021-07-13 12:44:03
Message-ID: CAEudQApxw9UvM=NjPFd3=NGwdsOpQjpyFR_OSt0ofwSDpSY1qw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Em ter., 13 de jul. de 2021 às 09:24, David Rowley <dgrowleyml(at)gmail(dot)com>
escreveu:

> On Wed, 14 Jul 2021 at 00:06, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com> wrote:
> >
> > Em ter., 13 de jul. de 2021 às 04:19, Ronan Dunklau <
> ronan(dot)dunklau(at)aiven(dot)io> escreveu:
> >> I would be
> >> surprised the check adds that much to the whole execution though.
> >
> > I think this branch is a misprediction.
>
> It could be. I wondered that myself when I saw Ronan's results were
> better than mine for 2,4 and 7. However, I think Ronan had quite a
> bit of noise in his results as there's no reason for the speedup in
> tests 2,4 and 7.

> > In most cases is it not datumSort?
>
> who knows. Maybe someone's workload always requires the datum sort.
>
> > That's why I would like to use unlikely.
>
> We really only use unlikely() in cases where we want to move code out
> of line to a cold area because it's really never executed under normal
> circumstances. We tend to do that for ERROR cases as we don't ever
> really want to optimise for errors. We also sometimes do it when some
> function has a branch to initialise something during the first call.
> The case in question here does not fit for either of those two cases.
>
Hum, I understand the usage cases now.
Thanks for the hint.

>
> > IMO all the tests should all be to verify past behavior first.
>
> I'm not quire sure what you mean there.
>
I'm saying we could help the branch by keeping the same testing logic as
before and not reversing it.
Attached is a version to demonstrate this, I don't pretend to be v7.

I couldn't find a good phrase to the contrary:
"are we *not* using the single value optimization ?"

I don't have time to take the tests right now.

regards,
Ranier Vilela

Attachment Content-Type Size
allow_single_datum_sort.patch application/octet-stream 4.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jeevan Ladhe 2021-07-13 13:27:00 Re: [PATCH] improve the pg_upgrade error message
Previous Message David Rowley 2021-07-13 12:24:03 Re: [PATCH] Use optimized single-datum tuplesort in ExecSort