From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: James Coleman <jtc331@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Ranier Vilela <ranier.vf@gmail.com>
Cc: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Subject: Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
Date: Mon, 19 Jul 2021 07:50:16 +0200
Message-ID: <2866025.21dbZ4J8xM@aivenronan> (raw)
In-Reply-To: <CAApHDvq-mDyy=BG7S1DWzxS_7cB02m98yzYKH==JPgCqH2p5Vg@mail.gmail.com>
References: <3177670.itZtoPt7T5@aivenronan>
<CAAaqYe8BFurBGFxatWdSnEF-EfS+nVzqk-gmbwdztUtjAk5sZQ@mail.gmail.com>
<CAApHDvq-mDyy=BG7S1DWzxS_7cB02m98yzYKH==JPgCqH2p5Vg@mail.gmail.com>
Le samedi 17 juillet 2021, 10:36:09 CEST David Rowley a écrit :
> On Sat, 17 Jul 2021 at 01:14, James Coleman <jtc331@gmail.com> wrote:
> > The only remaining question I have is whether or not costing needs to
> > change, given the very significant speedup for datum sort.
>
> I'm looking at cost_tuplesort and the only thing that I think might
> make sense would be to adjust how the input_bytes value is calculated.
> For now, that's done with the following function that's used in quite
> a number of places.
>
> static double
> relation_byte_size(double tuples, int width)
> {
> return tuples * (MAXALIGN(width) + MAXALIGN(SizeofHeapTupleHeader));
> }
>
> It seems, at least in the case of Sort, that using SizeofHeapTupleHead
> is just always wrong as it should be SizeofMinimalTupleHeader. I know
> that's also the case for Memoize too. I've not checked the other
> locations.
>
> The only thing I can really see that we might do would be not add the
> MAXALIGN(SizeofHeapTupleHeader) when there's just a single column.
> We'd need to pass down the number of attributes from
> create_sort_path() so we'd know when and when not to add that. I'm not
> saying that we should do this. I'm just saying that I don't really see
> what else we might do.
>
> I can imagine another patch might just want to do a complete overhaul
> of all locations that use relation_byte_size(). There are various
> things that function just does not account for. e.g, the fact that we
> allocate chunks in powers of 2 and that there's a chunk header added
> on. Of course, "width" is just an estimate, so maybe trying to
> calculate something too precisely wouldn't be too wise. However,
> there's a bit of a chicken and the egg problem there as there'd be
> little incentive to improve "width" unless we started making more
> accurate use of the value.
>
> Anyway, none of the above take into account that the Datum sort is
> just a little faster, The only thing that exists in the existing cost
> modal that we could use to adjust the cost of an in memory sort is the
> comparison_cost. The problem there is that the comparison is exactly
> the same in both Datum and Tuple sorts. The only thing that really
> changes between Datum and Tuple sort is the fact that we don't make a
> MinimalTuple when doing a Datum sort. The cost modal, unfortunately,
> does not account for that. That kinda makes me think that we should
> do nothing as if we start to account for making MemoryTuples then
> we'll just penalise Tuple sorts and that might cause someone to be
> upset.
>
Thank you for taking the time to perform that analysis. I agree with you and
tt looks to me that if we were to start accounting for it, we would have to
make the change almost transparent for tuple sorts so that it stays roughly
the same, which is impossible since we don't apply the comparison cost to all
tuples but only to the number of tuples we actually expect to compare.
On the other hand, if we don't change the sorting cost and it just ends up
being faster in some cases I doubt anyone would complain.
--
Ronan Dunklau
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: ronan.dunklau@aiven.io, jtc331@gmail.com, dgrowleyml@gmail.com, ranier.vf@gmail.com, pgsql-hackers@lists.postgresql.org, dilipbalaut@gmail.com
Subject: Re: [PATCH] Use optimized single-datum tuplesort in ExecSort
In-Reply-To: <2866025.21dbZ4J8xM@aivenronan>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox