From: Ronan Dunklau <ronan.dunklau@aiven.io>
To: Julien Rouhaud <rjuju123@gmail.com>, David Rowley <dgrowleyml@gmail.com>
Cc: Tomas Vondra <tomas.vondra@enterprisedb.com>,
 PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>,
 Andres Freund <andres@anarazel.de>, Tomas Vondra <tv@fuzzy.cz>
Subject: Re: Use generation context to speed up tuplesorts
Date: Thu, 20 Jan 2022 08:36:46 +0100
Message-ID: <4377612.LvFx2qVVIh@aivenronan>
Organization: aiven
In-Reply-To: 
 <CAApHDvrbe7rwS_pYnT=L7amt0Wp9MtgFLfg2Qwu8Mv1G9tEiow@mail.gmail.com>
References: 
 <CAApHDvoH4ASzsAOyHcxkuY01Qf++8JJ0paw+03dk+W25tQEcNQ@mail.gmail.com>
 <20220118064539.o3gc257z2vavb66e@jrouhaud>
 <CAApHDvrbe7rwS_pYnT=L7amt0Wp9MtgFLfg2Qwu8Mv1G9tEiow@mail.gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="iso-8859-1"
Archived-At: 
 <https://www.postgresql.org/message-id/4377612.LvFx2qVVIh%40aivenronan>
Precedence: bulk

Le jeudi 20 janvier 2022, 02:31:15 CET David Rowley a =E9crit :
> On Tue, 18 Jan 2022 at 19:45, Julien Rouhaud <rjuju123@gmail.com> wrote:
> > I'm also a bit confused about which patch(es) should actually be review=
ed
> > in that thread.  My understanding is that the original patch might not =
be
> > relevant anymore but I might be wrong.  The CF entry should probably be
> > updated to clarify that, with an annotation/ title change / author upda=
te
> > or something.
> >=20
> > In the meantime I will switch the entry to Waiting on Author.
>=20
> I think what's going on here is a bit confusing. There's quite a few
> patches on here that aim to do very different things.
>=20
> The patch I originally proposed was just to make tuplesort use
> generation memory contexts. This helps speed up tuplesort because
> generation contexts allow palloc() to be faster than the standard
> allocator. Additionally, there's no power-of-2 wastage with generation
> contexts like there is with the standard allocator. This helps fit
> more tuples on fewer CPU cache lines.
>=20
> I believe Andres then mentioned that the fixed block size for
> generation contexts might become a problem. With the standard
> allocator the actual size of the underlying malloc() grows up until a
> maximum size.  With generation contexts this is always the same, so we
> could end up with a very large number of blocks which means lots of
> small mallocs which could end up slow.  Tomas then posted a series of
> patches to address this.
>=20
> I then posted another patch that has the planner make a choice on the
> size of the blocks to use for the generation context based on the
> tuple width and number of tuple estimates. My hope there was to
> improve performance further by making a better choice for how large to
> make the blocks in the first place.  This reduces the need for Tomas'
> patch, but does not eliminate the need for it.
>=20
> As of now, I still believe we'll need Tomas' patches to allow the
> block size to grow up to a maximum size.  I think those patches are
> likely needed before we think about making tuplesort use generation
> contexts.  The reason I believe this is that we don't always have good
> estimates for the number of tuples we're going to sort.  nodeSort.c is
> fairly easy as it just fetches tuples once and then sorts them.  The
> use of tuplesort for nodeIncrementalSort.c is much more complex as
> we'll likely be performing a series of smaller sorts which are much
> harder to get size estimates for. This means there's likely no magic
> block size that we can use for the generation context that's used to
> store the tuples in tuplesort.  This is also the case for order by
> aggregates and probably most other usages of tuplesort.
>=20

You left out the discussion I started about glibc's malloc specific behavio=
ur.

I tried to demonstrate that a big part of the performance gain you were see=
ing=20
were not in fact due to our allocator by itself, but by the way different b=
lock=20
sizes allocations interact with this malloc implementation and how it handl=
es=20
releasing memory back to the system. I also tried to show that we can give=
=20
DBAs more control over how much memory to "waste" as the price for faster=20
allocations.

I agree that the generation context memory manager is useful in the tupleso=
rt=20
context, if only for the fact that we fall back to disk less quickly due to=
=20
the reduced wastage of memory, but I would be wary of drawing conclusions t=
oo=20
quickly based on a specific malloc implementation which shows threshold eff=
ects,=20
which are compounded by our growing block sizes code in general.

I have on my TODO list to run the same kind of benchmarks using jemalloc, t=
o=20
better isolate the performance gains expected from the generation allocator=
=20
itself from the side effect of avoiding glibc's pattern of releasing memory=
=20
back to the kernel too quickly.=20

Julien, sorry for the confusion  of adding that discussion and those patche=
s=20
to the same thread, but I don't really see a way of dissociating those two=
=20
aspects.

=2D-=20
Ronan Dunklau