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

Matthias van de Meent <boekewurm+postgres@gmail.com>

From: Matthias van de Meent <boekewurm+postgres@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Andres Freund <andres@anarazel.de>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-07T00:34:30Z
Lists: pgsql-hackers
On Sun, 7 Apr 2024, 01:59 David Rowley, <dgrowleyml@gmail.com> wrote:

> On Sun, 7 Apr 2024 at 05:45, Matthias van de Meent
> <boekewurm+postgres@gmail.com> wrote:
> > Malloc's docs specify the minimum chunk size at 4*sizeof(void*) and
> itself uses , so using powers of 2 for chunks would indeed fail to detect
> 1s in the 4th bit. I suspect you'll get different results when you check
> the allocation patterns of multiples of 8 bytes, starting from 40,
> especially on 32-bit arm (where MALLOC_ALIGNMENT is 8 bytes, rather than
> the 16 bytes on i386 and 64-bit architectures, assuming  [0] is accurate)


I'd hazard a guess that
> there are more instances of Postgres running on Windows today than on
> 32-bit CPUs and we don't seem too worried about the bit-patterns used
> for Windows.
>

Yeah, that is something I had some thoughts about too, but didn't check if
there was historical context around. I don't think it's worth bothering
right now though.

>> Another reason not to make it 5 bits is that I believe that would make
> >> the mcxt_methods[] array 2304 bytes rather than 576 bytes.  4 bits
> >> makes it 1152 bytes, if I'm counting correctly.
> >
> >
> > I don't think I understand why this would be relevant when only 5 of the
> contexts are actually in use (thus in caches). Is that size concern about
> TLB entries then?
>
> It's a static const array. I don't want to bloat the binary with
> something we'll likely never need.  If we one day need it, we can
> reserve another bit using the same overlapping method.
>

Fair points.

>> I revised the patch to simplify hdrmask logic.  This started with me
> >> having trouble finding the best set of words to document that the
> >> offset is "half the bytes between the chunk and block".  So, instead
> >> of doing that, I've just made it so these two fields effectively
> >> overlap. The lowest bit of the block offset is the same bit as the
> >> high bit of what MemoryChunkGetValue returns.
> >
> >
> > Works for me, I suppose.
>
> hmm. I don't detect much enthusiasm for it.
>

I had a tiring day leaving me short on enthousiasm, after which I realised
there were some things to this patch that would need fixing.

I could've worded this better, but nothing against this code.

-Matthias

Commits

  1. Update mmgr's README to mention BumpContext

  2. Push dedicated BumpBlocks to the tail of the blocks list

  3. Improve test coverage in bump.c

  4. Fix incorrect KeeperBlock macro in bump.c

  5. Use bump memory context for tuplesorts

  6. Introduce a bump memory allocator

  7. Enlarge bit-space for MemoryContextMethodID