On Mon, 2014-11-17 at 18:04 +0100, Andres Freund wrote: > That's quite possibly one culprit for the slowdown. Right now one > AllocSetContext struct fits precisely into three cachelines. After your > change it doesn't anymore.
Thank you! I made the same mistake as Tomas: I saw that MemoryContextData went to 64 bytes and thought "that should be fine" while ignoring AllocSetContext. I did some tests in the past between a version of my patch that made MemoryContextData 72 bytes and one that made it 64 bytes, but I may have neglected to test against the original 56 byte version. I'm still not able to test to see if this is the real problem, but it seems best to eliminate it anyway. > Consider not counting memory in bytes but blocks and adding it directly > after the NodeTag. That'd leave the size the same as right now. I can also just move isReset there, and keep mem_allocated as a uint64. That way, if I find later that I want to track the aggregated value for the child contexts as well, I can split it into two uint32s. I'll hold off any any such optimizations until I see some numbers from HashAgg though. Attached new version. Regards, Jeff Davis
*** a/src/backend/utils/mmgr/aset.c --- b/src/backend/utils/mmgr/aset.c *************** *** 438,451 **** AllocSetContextCreate(MemoryContext parent, Size initBlockSize, Size maxBlockSize) { ! AllocSet context; /* Do the type-independent part of context creation */ ! context = (AllocSet) MemoryContextCreate(T_AllocSetContext, ! sizeof(AllocSetContext), ! &AllocSetMethods, ! parent, ! name); /* * Make sure alloc parameters are reasonable, and save them. --- 438,454 ---- Size initBlockSize, Size maxBlockSize) { ! AllocSet set; ! MemoryContext context; /* Do the type-independent part of context creation */ ! context = MemoryContextCreate(T_AllocSetContext, ! sizeof(AllocSetContext), ! &AllocSetMethods, ! parent, ! name); ! ! set = (AllocSet) context; /* * Make sure alloc parameters are reasonable, and save them. *************** *** 459,467 **** AllocSetContextCreate(MemoryContext parent, if (maxBlockSize < initBlockSize) maxBlockSize = initBlockSize; Assert(AllocHugeSizeIsValid(maxBlockSize)); /* must be safe to double */ ! context->initBlockSize = initBlockSize; ! context->maxBlockSize = maxBlockSize; ! context->nextBlockSize = initBlockSize; /* * Compute the allocation chunk size limit for this context. It can't be --- 462,470 ---- if (maxBlockSize < initBlockSize) maxBlockSize = initBlockSize; Assert(AllocHugeSizeIsValid(maxBlockSize)); /* must be safe to double */ ! set->initBlockSize = initBlockSize; ! set->maxBlockSize = maxBlockSize; ! set->nextBlockSize = initBlockSize; /* * Compute the allocation chunk size limit for this context. It can't be *************** *** 477,486 **** AllocSetContextCreate(MemoryContext parent, * and actually-allocated sizes of any chunk must be on the same side of * the limit, else we get confused about whether the chunk is "big". */ ! context->allocChunkLimit = ALLOC_CHUNK_LIMIT; ! while ((Size) (context->allocChunkLimit + ALLOC_CHUNKHDRSZ) > (Size) ((maxBlockSize - ALLOC_BLOCKHDRSZ) / ALLOC_CHUNK_FRACTION)) ! context->allocChunkLimit >>= 1; /* * Grab always-allocated space, if requested --- 480,489 ---- * and actually-allocated sizes of any chunk must be on the same side of * the limit, else we get confused about whether the chunk is "big". */ ! set->allocChunkLimit = ALLOC_CHUNK_LIMIT; ! while ((Size) (set->allocChunkLimit + ALLOC_CHUNKHDRSZ) > (Size) ((maxBlockSize - ALLOC_BLOCKHDRSZ) / ALLOC_CHUNK_FRACTION)) ! set->allocChunkLimit >>= 1; /* * Grab always-allocated space, if requested *************** *** 500,519 **** AllocSetContextCreate(MemoryContext parent, errdetail("Failed while creating memory context \"%s\".", name))); } ! block->aset = context; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; block->endptr = ((char *) block) + blksize; ! block->next = context->blocks; ! context->blocks = block; /* Mark block as not to be released at reset time */ ! context->keeper = block; /* Mark unallocated space NOACCESS; leave the block header alone. */ VALGRIND_MAKE_MEM_NOACCESS(block->freeptr, blksize - ALLOC_BLOCKHDRSZ); } ! return (MemoryContext) context; } /* --- 503,525 ---- errdetail("Failed while creating memory context \"%s\".", name))); } ! ! context->mem_allocated += blksize; ! ! block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; block->endptr = ((char *) block) + blksize; ! block->next = set->blocks; ! set->blocks = block; /* Mark block as not to be released at reset time */ ! set->keeper = block; /* Mark unallocated space NOACCESS; leave the block header alone. */ VALGRIND_MAKE_MEM_NOACCESS(block->freeptr, blksize - ALLOC_BLOCKHDRSZ); } ! return context; } /* *************** *** 590,595 **** AllocSetReset(MemoryContext context) --- 596,603 ---- else { /* Normal case, release the block */ + context->mem_allocated -= block->endptr - ((char*) block); + #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); #endif *************** *** 632,637 **** AllocSetDelete(MemoryContext context) --- 640,647 ---- { AllocBlock next = block->next; + context->mem_allocated -= block->endptr - ((char *) block); + #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); #endif *************** *** 678,683 **** AllocSetAlloc(MemoryContext context, Size size) --- 688,696 ---- errmsg("out of memory"), errdetail("Failed on request of size %zu.", size))); } + + context->mem_allocated += blksize; + block->aset = set; block->freeptr = block->endptr = ((char *) block) + blksize; *************** *** 873,878 **** AllocSetAlloc(MemoryContext context, Size size) --- 886,893 ---- errdetail("Failed on request of size %zu.", size))); } + context->mem_allocated += blksize; + block->aset = set; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; block->endptr = ((char *) block) + blksize; *************** *** 976,981 **** AllocSetFree(MemoryContext context, void *pointer) --- 991,999 ---- set->blocks = block->next; else prevblock->next = block->next; + + context->mem_allocated -= block->endptr - ((char*) block); + #ifdef CLOBBER_FREED_MEMORY wipe_mem(block, block->freeptr - ((char *) block)); #endif *************** *** 1088,1093 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size) --- 1106,1112 ---- AllocBlock prevblock = NULL; Size chksize; Size blksize; + Size oldblksize; while (block != NULL) { *************** *** 1105,1110 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size) --- 1124,1131 ---- /* Do the realloc */ chksize = MAXALIGN(size); blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; + oldblksize = block->endptr - ((char *)block); + block = (AllocBlock) realloc(block, blksize); if (block == NULL) { *************** *** 1114,1119 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size) --- 1135,1142 ---- errmsg("out of memory"), errdetail("Failed on request of size %zu.", size))); } + context->mem_allocated += blksize - oldblksize; + block->freeptr = block->endptr = ((char *) block) + blksize; /* Update pointers since block has likely been moved */ *** a/src/backend/utils/mmgr/mcxt.c --- b/src/backend/utils/mmgr/mcxt.c *************** *** 417,422 **** MemoryContextIsEmpty(MemoryContext context) --- 417,445 ---- } /* + * Find the memory allocated to blocks for this memory context. If recurse is + * true, also include children. + */ + uint64 + MemoryContextMemAllocated(MemoryContext context, bool recurse) + { + uint64 total = context->mem_allocated; + + AssertArg(MemoryContextIsValid(context)); + + if (recurse) + { + MemoryContext child = context->firstchild; + for (child = context->firstchild; + child != NULL; + child = child->nextchild) + total += MemoryContextMemAllocated(child, true); + } + + return total; + } + + /* * MemoryContextStats * Print statistics about the named context and all its descendants. * *************** *** 576,581 **** MemoryContextCreate(NodeTag tag, Size size, --- 599,605 ---- node->firstchild = NULL; node->nextchild = NULL; node->isReset = true; + node->mem_allocated = 0; node->name = ((char *) node) + size; strcpy(node->name, name); *** a/src/include/nodes/memnodes.h --- b/src/include/nodes/memnodes.h *************** *** 54,65 **** typedef struct MemoryContextMethods typedef struct MemoryContextData { NodeTag type; /* identifies exact kind of context */ MemoryContextMethods *methods; /* virtual function table */ MemoryContext parent; /* NULL if no parent (toplevel context) */ MemoryContext firstchild; /* head of linked list of children */ MemoryContext nextchild; /* next child of same parent */ char *name; /* context name (just for debugging) */ ! bool isReset; /* T = no space alloced since last reset */ #ifdef USE_ASSERT_CHECKING bool allowInCritSection; /* allow palloc in critical section */ #endif --- 54,66 ---- typedef struct MemoryContextData { NodeTag type; /* identifies exact kind of context */ + bool isReset; /* T = no space alloced since last reset */ MemoryContextMethods *methods; /* virtual function table */ MemoryContext parent; /* NULL if no parent (toplevel context) */ MemoryContext firstchild; /* head of linked list of children */ MemoryContext nextchild; /* next child of same parent */ char *name; /* context name (just for debugging) */ ! uint64 mem_allocated; /* track memory allocated for this context */ #ifdef USE_ASSERT_CHECKING bool allowInCritSection; /* allow palloc in critical section */ #endif *** a/src/include/utils/memutils.h --- b/src/include/utils/memutils.h *************** *** 100,105 **** extern Size GetMemoryChunkSpace(void *pointer); --- 100,106 ---- extern MemoryContext GetMemoryChunkContext(void *pointer); extern MemoryContext MemoryContextGetParent(MemoryContext context); extern bool MemoryContextIsEmpty(MemoryContext context); + extern uint64 MemoryContextMemAllocated(MemoryContext context, bool recurse); extern void MemoryContextStats(MemoryContext context); extern void MemoryContextAllowInCriticalSection(MemoryContext context, bool allow); *************** *** 131,136 **** extern MemoryContext AllocSetContextCreate(MemoryContext parent, --- 132,138 ---- Size initBlockSize, Size maxBlockSize); + /* * Recommended default alloc parameters, suitable for "ordinary" contexts * that might hold quite a lot of data.
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers