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
