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

Reply via email to