This patch tracks memory usage (at the block level) for all memory
contexts. Individual palloc()s aren't tracked; only the new blocks
allocated to the memory context with malloc().

It also adds a new function, MemoryContextMemAllocated() which can
either retrieve the total allocated for the context, or it can recurse
and sum up the allocations for all subcontexts as well.

This is intended to be used by HashAgg in an upcoming patch that will
bound the memory and spill to disk.

Previous discussion here:

http://www.postgresql.org/message-id/1407012053.15301.53.camel@jeff-desktop

Previous concerns:

* There was a slowdown reported of around 1-3% (depending on the exact
version of the patch) on an IBM power machine when doing an index
rebuild. The results were fairly noisy for me, but it seemed to hold up.
See http://www.postgresql.org/message-id/CA
+Tgmobnu7XEn1gRdXnFo37P79bF=qLt46=37ajp3cro9db...@mail.gmail.com 
* Adding a struct field to MemoryContextData may be bad for the CPU
caching behavior, and may be the cause of the above slowdown.
* Previous versions of the patch updated the parent contexts'
allocations as well, which avoided the need to recurse when querying the
total allocation. That seemed to penalize cases that didn't need to
track the allocation. We discussed trying to "opt-in" to this behavior,
but it seemed more awkward than helpful. Now, the patch only updates the
allocation of the current context, and querying means recursing through
the child contexts.
* There was a concern that, if MemoryContextMemAllocated needs to
recurse to the child contexts, it will be too slow for HashAgg of
array_agg, which creates a child context per group. That was solved with
http://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=b419865a814abbca12bdd6eef6a3d5ed67f432e1

My general answer to the performance concerns is that they aren't a
reason to block this patch, unless someone has a suggestion about how to
fix them. Adding one field to a struct and a few arithmetic operations
for each malloc() or realloc() seems reasonable to me.

The current state, where HashAgg just blows up the memory, is just not
reasonable, and we need to track the memory to fix that problem. Others
have also mentioned that we might want to use this mechanism to track
memory for other operators, like Sort or HashJoin, which might be
simpler and more accurate.

Regards,
        Jeff Davis

*** a/src/backend/utils/mmgr/aset.c
--- b/src/backend/utils/mmgr/aset.c
***************
*** 500,505 **** AllocSetContextCreate(MemoryContext parent,
--- 500,508 ----
  					 errdetail("Failed while creating memory context \"%s\".",
  							   name)));
  		}
+ 
+ 		((MemoryContext) set)->mem_allocated += blksize;
+ 
  		block->aset = set;
  		block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
  		block->endptr = ((char *) block) + blksize;
***************
*** 590,595 **** AllocSetReset(MemoryContext context)
--- 593,600 ----
  		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)
--- 637,644 ----
  	{
  		AllocBlock	next = block->next;
  
+ 		context->mem_allocated -= block->endptr - ((char *) block);
+ 
  #ifdef CLOBBER_FREED_MEMORY
  		wipe_mem(block, block->freeptr - ((char *) block));
  #endif
***************
*** 672,677 **** AllocSetAlloc(MemoryContext context, Size size)
--- 679,687 ----
  		block = (AllocBlock) malloc(blksize);
  		if (block == NULL)
  			return NULL;
+ 
+ 		context->mem_allocated += blksize;
+ 
  		block->aset = set;
  		block->freeptr = block->endptr = ((char *) block) + blksize;
  
***************
*** 861,866 **** AllocSetAlloc(MemoryContext context, Size size)
--- 871,878 ----
  		if (block == NULL)
  			return NULL;
  
+ 		context->mem_allocated += blksize;
+ 
  		block->aset = set;
  		block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
  		block->endptr = ((char *) block) + blksize;
***************
*** 964,969 **** AllocSetFree(MemoryContext context, void *pointer)
--- 976,984 ----
  			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
***************
*** 1077,1082 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size)
--- 1092,1098 ----
  		AllocBlock	prevblock = NULL;
  		Size		chksize;
  		Size		blksize;
+ 		Size		oldblksize;
  
  		while (block != NULL)
  		{
***************
*** 1094,1102 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size)
--- 1110,1123 ----
  		/* 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)
  			return NULL;
+ 
+ 		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
***************
*** 477,482 **** MemoryContextIsEmpty(MemoryContext context)
--- 477,505 ----
  }
  
  /*
+  * Find the memory allocated to blocks for this memory context. If recurse is
+  * true, also include children.
+  */
+ int64
+ MemoryContextMemAllocated(MemoryContext context, bool recurse)
+ {
+ 	int64 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.
   *
***************
*** 636,641 **** MemoryContextCreate(NodeTag tag, Size size,
--- 659,665 ----
  	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
***************
*** 63,68 **** typedef struct MemoryContextData
--- 63,69 ----
  	MemoryContext nextchild;	/* next child of same parent */
  	char	   *name;			/* context name (just for debugging) */
  	MemoryContextCallback *reset_cbs;	/* list of reset/delete callbacks */
+ 	int64		mem_allocated;	/* track memory allocated for this context */
  } MemoryContextData;
  
  /* utils/palloc.h contains typedef struct MemoryContextData *MemoryContext */
*** a/src/include/utils/memutils.h
--- b/src/include/utils/memutils.h
***************
*** 103,108 **** extern Size GetMemoryChunkSpace(void *pointer);
--- 103,109 ----
  extern MemoryContext GetMemoryChunkContext(void *pointer);
  extern MemoryContext MemoryContextGetParent(MemoryContext context);
  extern bool MemoryContextIsEmpty(MemoryContext context);
+ extern int64 MemoryContextMemAllocated(MemoryContext context, bool recurse);
  extern void MemoryContextStats(MemoryContext context);
  extern void MemoryContextAllowInCriticalSection(MemoryContext context,
  									bool allow);
-- 
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