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
