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

On 4/7/24 23:09, Andres Freund wrote:
> 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
>

Yup, changing it to this:

#define KeeperBlock(set) ((BumpBlock *) ((char *) (set) +
MAXALIGN(sizeof(BumpContext))))

fixes the issue for me.

regards

--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company