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

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: David Rowley <dgrowleyml@gmail.com>, John Naylor <johncnaylorls@gmail.com>, Matthias van de Meent <boekewurm+postgres@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2024-04-07T21:09:24Z
Lists: pgsql-hackers
Hi,

On 2024-04-07 22:35:47 +0200, Tomas Vondra wrote:
> I haven't investigated, but I'd considering it works on 64-bit, I guess
> it's not considering alignment somewhere. I can dig more if needed.

I think I may the problem:


#define KeeperBlock(set) ((BumpBlock *) ((char *) (set) + sizeof(BumpContext)))
#define IsKeeperBlock(set, blk) (KeeperBlock(set) == (blk))

BumpContextCreate():
...
	/* Fill in the initial block's block header */
	block = (BumpBlock *) (((char *) set) + MAXALIGN(sizeof(BumpContext)));
	/* determine the block size and initialize it */
	firstBlockSize = allocSize - MAXALIGN(sizeof(BumpContext));
	BumpBlockInit(set, block, firstBlockSize);
...
	((MemoryContext) set)->mem_allocated = allocSize;

void
BumpCheck(MemoryContext context)
...
		if (IsKeeperBlock(bump, block))
			total_allocated += block->endptr - (char *) bump;
...

I suspect that KeeperBlock() isn't returning true, because IsKeeperBlock misses
the MAXALIGN(). I think that about fits with:

> #4  0x008f0088 in BumpCheck (context=0x131e330) at bump.c:808
> 808             Assert(total_allocated == context->mem_allocated);
> (gdb) p total_allocated
> $1 = 8120
> (gdb) p context->mem_allocated
> $2 = 8192


Greetings,

Andres Freund



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