On Wed, 2014-08-06 at 11:43 -0400, Robert Haas wrote:
> Comparing the median times, that's about a 3% regression. For this
> particular case, we might be able to recapture that by replacing the
> bespoke memory-tracking logic in tuplesort.c with use of this new
> facility. I'm not sure whether there are other cases that we might
> also want to test; I think stuff that runs all on the server side is
> likely to show up problems more clearly than pgbench. Maybe a
> PL/pgsql loop that does something allocation-intensive on each
> iteration, for example, like parsing a big JSON document.
I wasn't able to reproduce your results on my machine. At -s 300, with
maintenance_work_mem set high enough to do internal sort, it took about
40s and I heard some disk activity, so I didn't think it was a valid
result. I went down to -s 150, and it took around 5.3s on both master
and memory-accounting.
Either way, it's better to be conservative. Attached is a version of the
patch with opt-in memory usage tracking. Child contexts inherit the
setting from their parent.
Regards,
Jeff Davis
*** a/src/backend/utils/mmgr/aset.c
--- b/src/backend/utils/mmgr/aset.c
***************
*** 242,247 **** typedef struct AllocChunkData
--- 242,249 ----
#define AllocChunkGetPointer(chk) \
((AllocPointer)(((char *)(chk)) + ALLOC_CHUNKHDRSZ))
+ static void update_allocation(MemoryContext context, int64 size);
+
/*
* These functions implement the MemoryContext API for AllocSet contexts.
*/
***************
*** 430,435 **** randomize_mem(char *ptr, size_t size)
--- 432,440 ----
* minContextSize: minimum context size
* initBlockSize: initial allocation block size
* maxBlockSize: maximum allocation block size
+ *
+ * The flag determining whether this context tracks memory usage is inherited
+ * from the parent context.
*/
MemoryContext
AllocSetContextCreate(MemoryContext parent,
***************
*** 438,443 **** AllocSetContextCreate(MemoryContext parent,
--- 443,468 ----
Size initBlockSize,
Size maxBlockSize)
{
+ return AllocSetContextCreateTracked(
+ parent, name, minContextSize, initBlockSize, maxBlockSize,
+ (parent == NULL) ? false : parent->track_mem);
+ }
+
+ /*
+ * AllocSetContextCreateTracked
+ * Create a new AllocSet context.
+ *
+ * Implementation for AllocSetContextCreate, but also allows the caller to
+ * specify whether memory usage should be tracked or not.
+ */
+ MemoryContext
+ AllocSetContextCreateTracked(MemoryContext parent,
+ const char *name,
+ Size minContextSize,
+ Size initBlockSize,
+ Size maxBlockSize,
+ bool track_mem)
+ {
AllocSet context;
/* Do the type-independent part of context creation */
***************
*** 445,451 **** AllocSetContextCreate(MemoryContext parent,
sizeof(AllocSetContext),
&AllocSetMethods,
parent,
! name);
/*
* Make sure alloc parameters are reasonable, and save them.
--- 470,477 ----
sizeof(AllocSetContext),
&AllocSetMethods,
parent,
! name,
! track_mem);
/*
* Make sure alloc parameters are reasonable, and save them.
***************
*** 500,505 **** AllocSetContextCreate(MemoryContext parent,
--- 526,534 ----
errdetail("Failed while creating memory context \"%s\".",
name)));
}
+
+ update_allocation((MemoryContext) context, blksize);
+
block->aset = context;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize;
***************
*** 590,595 **** AllocSetReset(MemoryContext context)
--- 619,625 ----
else
{
/* Normal case, release the block */
+ update_allocation(context, -(block->endptr - ((char*) block)));
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
***************
*** 632,637 **** AllocSetDelete(MemoryContext context)
--- 662,668 ----
{
AllocBlock next = block->next;
+ update_allocation(context, -(block->endptr - ((char*) block)));
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
***************
*** 678,683 **** AllocSetAlloc(MemoryContext context, Size size)
--- 709,717 ----
errmsg("out of memory"),
errdetail("Failed on request of size %zu.", size)));
}
+
+ update_allocation(context, blksize);
+
block->aset = set;
block->freeptr = block->endptr = ((char *) block) + blksize;
***************
*** 873,878 **** AllocSetAlloc(MemoryContext context, Size size)
--- 907,914 ----
errdetail("Failed on request of size %zu.", size)));
}
+ update_allocation(context, blksize);
+
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize;
***************
*** 976,981 **** AllocSetFree(MemoryContext context, void *pointer)
--- 1012,1018 ----
set->blocks = block->next;
else
prevblock->next = block->next;
+ update_allocation(context, -(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)
--- 1125,1131 ----
AllocBlock prevblock = NULL;
Size chksize;
Size blksize;
+ Size oldblksize;
while (block != NULL)
{
***************
*** 1105,1110 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size)
--- 1143,1150 ----
/* 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)
--- 1154,1160 ----
errmsg("out of memory"),
errdetail("Failed on request of size %zu.", size)));
}
+ update_allocation(context, blksize - oldblksize);
block->freeptr = block->endptr = ((char *) block) + blksize;
/* Update pointers since block has likely been moved */
***************
*** 1277,1282 **** AllocSetStats(MemoryContext context, int level)
--- 1318,1350 ----
}
+ /*
+ * update_allocation
+ *
+ * Track newly-allocated or newly-freed memory (freed memory should be
+ * negative).
+ */
+ static void
+ update_allocation(MemoryContext context, int64 size)
+ {
+ MemoryContext parent;
+
+ if (!context->track_mem)
+ return;
+
+ context->self_allocated += size;
+
+ for (parent = context; parent != NULL; parent = parent->parent)
+ {
+ if (!parent->track_mem)
+ break;
+
+ parent->total_allocated += size;
+ Assert(parent->self_allocated >= 0);
+ Assert(parent->total_allocated >= 0);
+ }
+ }
+
#ifdef MEMORY_CONTEXT_CHECKING
/*
*** a/src/backend/utils/mmgr/mcxt.c
--- b/src/backend/utils/mmgr/mcxt.c
***************
*** 324,329 **** MemoryContextAllowInCriticalSection(MemoryContext context, bool allow)
--- 324,346 ----
}
/*
+ * MemoryContextGetAllocated
+ *
+ * Return memory allocated by the system to this context. If total is true,
+ * include child contexts. Context must have track_mem set.
+ */
+ int64
+ MemoryContextGetAllocated(MemoryContext context, bool total)
+ {
+ Assert(context->track_mem);
+
+ if (total)
+ return context->total_allocated;
+ else
+ return context->self_allocated;
+ }
+
+ /*
* GetMemoryChunkSpace
* Given a currently-allocated chunk, determine the total space
* it occupies (including all memory-allocation overhead).
***************
*** 546,552 **** MemoryContext
MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name)
{
MemoryContext node;
Size needed = size + strlen(name) + 1;
--- 563,570 ----
MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name,
! bool track_mem)
{
MemoryContext node;
Size needed = size + strlen(name) + 1;
***************
*** 576,581 **** MemoryContextCreate(NodeTag tag, Size size,
--- 594,602 ----
node->firstchild = NULL;
node->nextchild = NULL;
node->isReset = true;
+ node->track_mem = track_mem;
+ node->total_allocated = 0;
+ node->self_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,68 ----
MemoryContext nextchild; /* next child of same parent */
char *name; /* context name (just for debugging) */
bool isReset; /* T = no space alloced since last reset */
+ bool track_mem; /* whether to track memory usage */
+ int64 total_allocated; /* including child contexts */
+ int64 self_allocated; /* not including child contexts */
#ifdef USE_ASSERT_CHECKING
bool allowInCritSection; /* allow palloc in critical section */
#endif
*** a/src/include/utils/memutils.h
--- b/src/include/utils/memutils.h
***************
*** 96,101 **** extern void MemoryContextDeleteChildren(MemoryContext context);
--- 96,102 ----
extern void MemoryContextResetAndDeleteChildren(MemoryContext context);
extern void MemoryContextSetParent(MemoryContext context,
MemoryContext new_parent);
+ extern int64 MemoryContextGetAllocated(MemoryContext context, bool total);
extern Size GetMemoryChunkSpace(void *pointer);
extern MemoryContext GetMemoryChunkContext(void *pointer);
extern MemoryContext MemoryContextGetParent(MemoryContext context);
***************
*** 117,123 **** extern bool MemoryContextContains(MemoryContext context, void *pointer);
extern MemoryContext MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name);
/*
--- 118,125 ----
extern MemoryContext MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name,
! bool track_mem);
/*
***************
*** 130,135 **** extern MemoryContext AllocSetContextCreate(MemoryContext parent,
--- 132,143 ----
Size minContextSize,
Size initBlockSize,
Size maxBlockSize);
+ extern MemoryContext AllocSetContextCreateTracked(MemoryContext parent,
+ const char *name,
+ Size minContextSize,
+ Size initBlockSize,
+ Size maxBlockSize,
+ bool track_mem);
/*
* Recommended default alloc parameters, suitable for "ordinary" contexts
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
