Re: Memory Accounting

On Sun, Sep 29, 2019 at 12:12:49AM +0200, Tomas Vondra wrote:
>On Thu, Sep 26, 2019 at 01:36:46PM -0700, Jeff Davis wrote:
>>On Thu, 2019-09-26 at 21:22 +0200, Tomas Vondra wrote:
>>>It's worth mentioning that those bechmarks (I'm assuming we're
>>>talking
>>>about the numbers Rober shared in [1]) were done on patches that used
>>>the eager accounting approach (i.e. walking all parent contexts and
>>>updating the accounting for them).
>>>
>>>I'm pretty sure the current "lazy accounting" patches don't have that
>>>issue, so unless someone objects and/or can show numbers
>>>demonstrating
>>>I'wrong I'll stick to my plan to get this committed soon.
>>
>>That was my conclusion, as well.
>>
>
>I was about to commit the patch, but during the final review I've
>noticed two places that I think are bugs:
>
>1) aset.c (AllocSetDelete)
>--------------------------
>
>#ifdef CLOBBER_FREED_MEMORY
> wipe_mem(block, block->freeptr - ((char *) block));
>#endif
>
> if (block != set->keeper)
> {
> context->mem_allocated -= block->endptr - ((char *) block);
> free(block);
> }
>
>2) generation.c (GenerationReset)
>---------------------------------
>
>#ifdef CLOBBER_FREED_MEMORY
> wipe_mem(block, block->blksize);
>#endif
> context->mem_allocated -= block->blksize;
>
>
>Notice that when CLOBBER_FREED_MEMORY is defined, the code first calls
>wipe_mem and then accesses fields of the (wiped) block. Interesringly
>enough, the regression tests don't seem to exercise these bits - I've
>tried adding elog(ERROR) and it still passes. For (2) that's not very
>surprising because Generation context is only really used in logical
>decoding (and we don't delete the context I think). Not sure about (1)
>but it might be because AllocSetReset does the right thing and only
>leaves behind the keeper block.
>
>I'm pretty sure a custom function calling the contexts explicitly would
>fall over, but I haven't tried.
>

Oh, and one more thing - this probably needs to add at least some basic
explanation of the accounting to src/backend/mmgr/README.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services