On Sat, 2014-11-15 at 21:36 +0000, Simon Riggs wrote:
> Do I understand correctly that we are trying to account for exact
> memory usage at palloc/pfree time? Why??
Not palloc chunks, only tracking at the level of allocated blocks (that
we allocate with malloc).
It was a surprise to me that accounting at that level would have any
measurable impact, but Robert found a reasonable case on a POWER machine
that degraded a couple percent. I wasn't able to reproduce it
consistently on x86.
> Or alternatively, can't we just sample the allocations to reduce the overhead?
Not sure quite what you mean by "sample", but it sounds like something
along those lines would work.
Attached is a patch that does something very simple: only tracks blocks
held in the current context, with no inheritance or anything like it.
This reduces it to a couple arithmetic instructions added to the
alloc/dealloc path, so hopefully that removes the general performance
concern raised by Robert[1].
To calculate the total memory used, I included a function
MemoryContextMemAllocated() that walks the memory context and its
children recursively.
Of course, I was originally trying to avoid that, because it moves the
problem to HashAgg. For each group, it will need to execute
MemoryContextMemAllocated() to see if work_mem has been exceeded. It
will have to visit a couple contexts, or perhaps many (in the case of
array_agg, which creates one per group), which could be a measurable
regression for HashAgg.
But if that does turn out to be a problem, I think it's solvable. First,
I could micro-optimize the function by making it iterative rather than
recursive, to save on function call overhead. Second, I could offer a
way to prevent the HTAB from creating its own context, which would be
one less context to visit. And if those don't work, perhaps I could
resort to a sampling method of some kind, as you allude to above.
Regards,
Jeff Davis
[1] I'm fairly sure I tested something very similar on Robert's POWER
machine a while ago, and it was fine. But I think it's offline or moved
now, so I can't verify the results. If this patch still somehow turns
out to be a 1%+ regression on any reasonable test, I don't know what to
say.
*** 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
***************
*** 60,65 **** typedef struct MemoryContextData
--- 60,66 ----
MemoryContext nextchild; /* next child of same parent */
char *name; /* context name (just for debugging) */
bool isReset; /* T = no space alloced since last reset */
+ 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
