Re: Add bump memory context type and use it for tuplesorts

On Fri, 26 Jan 2024 at 01:29, Matthias van de Meent
<boekewurm+postgres(at)gmail(dot)com> wrote:
> >> + allocSize = MAXALIGN(sizeof(BumpContext)) + Bump_BLOCKHDRSZ +
> >> + if (minContextSize != 0)
> >> + allocSize = Max(allocSize, minContextSize);
> >> + else
> >> + allocSize = Max(allocSize, initBlockSize);
>
> Shouldn't this be the following, considering the meaning of "initBlockSize"?

No, we want to make the blocksize exactly initBlockSize if we can. Not
initBlockSize plus all the header stuff. We do it that way for all
the other contexts and I agree that it's a good idea as it keeps the
malloc request sizes powers of 2.

> >> + * BumpFree
> >> + * Unsupported.
> >> [...]
> >> + * BumpRealloc
> >> + * Unsupported.
>
> Rather than the error, can't we make this a no-op (potentially
> optionally, or in a different memory context?)

Unfortunately not. There are no MemoryChunks on bump chunks so we've
no way to determine the context type a given pointer belongs to. I've
left the MemoryChunk on there for debug builds so we can get the
ERRORs to allow us to fix the broken code that is pfreeing these
chunks.

> I understand that allowing pfree/repalloc in bump contexts requires
> each allocation to have a MemoryChunk prefix in overhead, but I think
> it's still a valid use case to have a very low overhead allocator with
> no-op deallocator (except context reset). Do you have performance
> comparison results between with and without the overhead of
> MemoryChunk?

Oh right, you've taken this into account. I was hoping not to have
the headers otherwise the only gains we see over using generation.c is
that of the allocation function being faster.

I certainly did do benchmarks in [1] and saw the 338% increase due to
the reduction in memory. That massive jump happened by accident as
the sort on tenk1 went from not fitting into default 4MB work_mem to
fitting in, so what I happened to measure there was the difference of
spilling to disk and not. The same could happen for this case, so the
overhead of having the chunk headers really depends on what the test
is. Probably, "potentially large" is likely a good way to describe the
overhead of having chunk headers. However, to a lesser extent, there
will be a difference for large sorts as we'll be able to fit more
tuples per batch and do fewer batches. The smaller the tuple, the
more that will be noticeable as the chunk header is a larger portion
of the overall allocation with those.

David

[1] https://postgr.es/m/CAApHDvoH4ASzsAOyHcxkuY01Qf++8JJ0paw+03dk+W25tQEcNQ@mail.gmail.com