Re: Use generation context to speed up tuplesorts

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tv(at)fuzzy(dot)cz>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Use generation context to speed up tuplesorts
Date: 2021-08-03 14:10:23
Message-ID: 36829a8a-63d0-c428-89d5-07e49561973e@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 8/2/21 1:17 PM, David Rowley wrote:
> On Sat, 31 Jul 2021 at 14:34, Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
>> I spent a bit of time hacking on the Generation context, adding the two
>> improvements discussed in this thread:
>>
>> 1) internal handling of block sizes, similar to what AllocSet does (it
>> pretty much just copies parts of it)
>>
>> 2) keeper block (we keep one empry block instead of freeing it)
>>
>> 3) I've also added allocChunkLimit, which makes it look a bit more like
>> AllocSet (instead of using just blockSize/8, which does not work too
>> well with dynamic blockSize)
>>
>> I haven't done any extensive tests on it, but it does pass check-world
>> with asserts etc. I haven't touched the comments, those need updating.
>> regards
>
> Thanks for starting work on that. I've only had a quick look, but I
> can have a more detailed look once you've got it more complete.
>

A review would be nice, although it can wait - It'd be interesting to
know if those patches help with the workload(s) you've been looking at.

> For now it does not really look like the keeper block stuff is wired
> up the same way as in aset.c. I'd expect you to be allocating that in
> the same malloc as you're using to allocate the context struct itself
> in GenerationContextCreate().
>

Yes, that difference is natural. The AllocSet works a bit differently,
as it does not release the blocks (except during reset), while the
Generation context frees the blocks. So it seems pointless to use the
same "keeper" block as AllocSet - instead my intention was to keep one
"allocated" block as a cache, which should help with tight pfree/palloc
cycles. Maybe we should not call that "keeper" block?

> Also, likely as a result of the above, minContextSize does not seem to
> be wired up to anything apart from an Assert().
>

Hmm, yeah. This is probably due to copying some of the block-growth and
keeper block code from AllocSet. There should be just init/max block
size, I think.

I did run the same set of benchmarks as for Slab, measuring some usual
allocation patterns. The results for i5-2500k machine are attached (for
the xeon it's almost exactly the same behavior). While running those
tests I realized the last patch is wrong and sets allocChunkLimit=1,
which is bogus and causes significant regression. So here's an updated
version of the patch series too.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
0001-generation-bench-v2.patch text/x-patch 18.9 KB
0002-Generation-grow-blocks-v2.patch text/x-patch 7.5 KB
0003-Generation-keeper-block-v2.patch text/x-patch 4.4 KB
0004-Generation-allocChunkLimit-v2.patch text/x-patch 3.0 KB
generation i5.ods application/vnd.oasis.opendocument.spreadsheet 889.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Gilles Darold 2021-08-03 14:11:24 Re: [PATCH] proposal for regexp_count, regexp_instr, regexp_substr and regexp_replace
Previous Message Pavel Borisov 2021-08-03 14:00:05 Re: Parallel scan with SubTransGetTopmostTransaction assert coredump