From: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
---|---|
To: | David Rowley <dgrowleyml(at)gmail(dot)com> |
Cc: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Add bump memory context type and use it for tuplesorts |
Date: | 2023-07-26 00:11:49 |
Message-ID: | 20230726001149.GB2992317@nathanxps13 |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Tue, Jun 27, 2023 at 09:19:26PM +1200, David Rowley wrote:
> Because of all of what is mentioned above about the current state of
> tuplesort, there does not really seem to be much need to have chunk
> headers in memory we allocate for tuples at all. Not having these
> saves us a further 8 bytes per tuple.
>
> In the attached patch, I've added a bump memory allocator which
> allocates chunks without and chunk header. This means the chunks
> cannot be pfree'd or realloc'd. That seems fine for the use case for
> storing tuples in tuplesort. I've coded bump.c in such a way that when
> built with MEMORY_CONTEXT_CHECKING, we *do* have chunk headers. That
> should allow us to pick up any bugs that are introduced by any code
> which accidentally tries to pfree a bump.c chunk.
This is a neat idea.
> In terms of performance of tuplesort, there's a small (~5-6%)
> performance gain. Not as much as I'd hoped, but I'm also doing a bit
> of other work on tuplesort to make it more efficient in terms of CPU,
> so I suspect the cache efficiency improvements might be more
> pronounced after those.
Nice.
> One thing that might need more thought is that we're running a bit low
> on MemoryContextMethodIDs. I had to use an empty slot that has a bit
> pattern like glibc malloc'd chunks sized 128kB. Maybe it's worth
> freeing up a bit from the block offset in MemoryChunk. This is
> currently 30 bits allowing 1GB offset, but these offsets are always
> MAXALIGNED, so we could free up a couple of bits since those 2
> lowest-order bits will always be 0 anyway.
I think it'd be okay to steal those bits. AFAICT it'd complicate the
macros in memutils_memorychunk.h a bit, but that doesn't seem like such a
terrible price to pay to allow us to keep avoiding the glibc bit patterns.
> + if (base->sortopt & TUPLESORT_ALLOWBOUNDED)
> + tuplen = GetMemoryChunkSpace(tuple);
> + else
> + tuplen = MAXALIGN(tuple->t_len);
nitpick: I see this repeated in a few places, and I wonder if it might
deserve a comment.
I haven't had a chance to try out your benchmark, but I'm hoping to do so
in the near future.
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
From | Date | Subject | |
---|---|---|---|
Next Message | Nathan Bossart | 2023-07-26 00:16:56 | Re: [PATCH] Add function to_oct |
Previous Message | Tom Lane | 2023-07-26 00:10:06 | Buildfarm animal grassquit is failing |