On Tue, 2014-08-19 at 12:54 +0200, Tomas Vondra wrote:
> The use-case for this is tracking a chosen subtree of contexts - e.g.
> aggcontext and below, so I'd expect the tracked subtrees to be relatively
> shallow. Am I right?
Right.
> My fear is that by removing the inheritance bit, we'll hurt cases with a
> lot of child contexts. For example, array_agg currently creates a separate
> context for each group - what happens if you have 100k groups and do
> MemoryContextGetAllocated? I guess iterating over 100k groups is not free.
Good point. We don't want to make checking the allocated space into an
expensive operation, because then we will be forced to call it less
frequently, which implies that we'd be sloppier about actually fitting
in work_mem.
> Wouldn't the solution with inheritance and propagating the accounting info
> to the parent actually better? Or maybe allowing both, having two flags
> when creating a context instead of one?
That seems overly complicated. We only have one driving use case, so two
orthogonal options sounds excessive. Perhaps one option if we can't
solve the performance problem and we need to isolate the changes to
hashagg.
> Also, do we really need to track allocated bytes - couldn't we track
> kilobytes or something and use smaller data types to get below the 64B?
Good idea.
I attached a patch that uses two uint32 fields so that it doesn't
increase the size of MemoryContextData, and it tracks memory usage for
all contexts. I was unable to detect any performance regression vs.
master, but on my machine the results are noisy.
It would be easier to resolve the performance concern if I could
reliably get the results Robert is getting. I think I was able to
reproduce the regression with the old patch, but the results were still
noisy.
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, size_t size);
+
/*
* These functions implement the MemoryContext API for AllocSet contexts.
*/
***************
*** 250,256 **** static void AllocSetFree(MemoryContext context, void *pointer);
static void *AllocSetRealloc(MemoryContext context, void *pointer, Size size);
static void AllocSetInit(MemoryContext context);
static void AllocSetReset(MemoryContext context);
! static void AllocSetDelete(MemoryContext context);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
static void AllocSetStats(MemoryContext context, int level);
--- 252,258 ----
static void *AllocSetRealloc(MemoryContext context, void *pointer, Size size);
static void AllocSetInit(MemoryContext context);
static void AllocSetReset(MemoryContext context);
! static void AllocSetDelete(MemoryContext context, MemoryContext parent);
static Size AllocSetGetChunkSpace(MemoryContext context, void *pointer);
static bool AllocSetIsEmpty(MemoryContext context);
static void AllocSetStats(MemoryContext context, int level);
***************
*** 500,505 **** AllocSetContextCreate(MemoryContext parent,
--- 502,510 ----
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)
--- 595,601 ----
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
***************
*** 611,617 **** AllocSetReset(MemoryContext context)
* But note we are not responsible for deleting the context node itself.
*/
static void
! AllocSetDelete(MemoryContext context)
{
AllocSet set = (AllocSet) context;
AllocBlock block = set->blocks;
--- 617,623 ----
* But note we are not responsible for deleting the context node itself.
*/
static void
! AllocSetDelete(MemoryContext context, MemoryContext parent)
{
AllocSet set = (AllocSet) context;
AllocBlock block = set->blocks;
***************
*** 623,628 **** AllocSetDelete(MemoryContext context)
--- 629,644 ----
AllocSetCheck(context);
#endif
+ /*
+ * Parent is already unlinked from context, so can't use
+ * update_allocation().
+ */
+ while (parent != NULL)
+ {
+ parent->total_allocated -= context->total_allocated;
+ parent = parent->parent;
+ }
+
/* Make it look empty, just in case... */
MemSetAligned(set->freelist, 0, sizeof(set->freelist));
set->blocks = NULL;
***************
*** 678,683 **** AllocSetAlloc(MemoryContext context, Size size)
--- 694,702 ----
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)
--- 892,899 ----
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)
--- 997,1003 ----
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)
--- 1110,1116 ----
AllocBlock prevblock = NULL;
Size chksize;
Size blksize;
+ Size oldblksize;
while (block != NULL)
{
***************
*** 1105,1110 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size)
--- 1128,1135 ----
/* 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)
--- 1139,1145 ----
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)
--- 1303,1333 ----
}
+ /*
+ * update_allocation
+ *
+ * Track newly-allocated or newly-freed memory (freed memory should be
+ * negative).
+ */
+ static void
+ update_allocation(MemoryContext context, size_t size_bytes)
+ {
+ MemoryContext parent;
+ uint32 size_kb = size_bytes/1024;
+
+ /* the maximum memory we can track is INT32_MAX * 1024 */
+ Assert(size_bytes/1024 <= UINT32_MAX);
+
+ context->self_allocated += size_kb;
+
+ for (parent = context; parent != NULL; parent = parent->parent)
+ {
+ parent->total_allocated += size_kb;
+ 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
***************
*** 187,192 **** MemoryContextResetChildren(MemoryContext context)
--- 187,194 ----
void
MemoryContextDelete(MemoryContext context)
{
+ MemoryContext parent = context->parent;
+
AssertArg(MemoryContextIsValid(context));
/* We had better not be deleting TopMemoryContext ... */
Assert(context != TopMemoryContext);
***************
*** 202,208 **** MemoryContextDelete(MemoryContext context)
*/
MemoryContextSetParent(context, NULL);
! (*context->methods->delete_context) (context);
VALGRIND_DESTROY_MEMPOOL(context);
pfree(context);
}
--- 204,211 ----
*/
MemoryContextSetParent(context, NULL);
! /* pass the parent in case it's needed, however */
! (*context->methods->delete_context) (context, parent);
VALGRIND_DESTROY_MEMPOOL(context);
pfree(context);
}
***************
*** 324,329 **** MemoryContextAllowInCriticalSection(MemoryContext context, bool allow)
--- 327,347 ----
}
/*
+ * 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)
+ {
+ if (total)
+ return (int64)context->total_allocated * 1024L;
+ else
+ return (int64)context->self_allocated * 1024L;
+ }
+
+ /*
* GetMemoryChunkSpace
* Given a currently-allocated chunk, determine the total space
* it occupies (including all memory-allocation overhead).
***************
*** 576,581 **** MemoryContextCreate(NodeTag tag, Size size,
--- 594,601 ----
node->firstchild = NULL;
node->nextchild = NULL;
node->isReset = true;
+ 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
***************
*** 41,47 **** typedef struct MemoryContextMethods
void *(*realloc) (MemoryContext context, void *pointer, Size size);
void (*init) (MemoryContext context);
void (*reset) (MemoryContext context);
! void (*delete_context) (MemoryContext context);
Size (*get_chunk_space) (MemoryContext context, void *pointer);
bool (*is_empty) (MemoryContext context);
void (*stats) (MemoryContext context, int level);
--- 41,48 ----
void *(*realloc) (MemoryContext context, void *pointer, Size size);
void (*init) (MemoryContext context);
void (*reset) (MemoryContext context);
! void (*delete_context) (MemoryContext context,
! MemoryContext parent);
Size (*get_chunk_space) (MemoryContext context, void *pointer);
bool (*is_empty) (MemoryContext context);
void (*stats) (MemoryContext context, int level);
***************
*** 59,64 **** typedef struct MemoryContextData
--- 60,67 ----
MemoryContext firstchild; /* head of linked list of children */
MemoryContext nextchild; /* next child of same parent */
char *name; /* context name (just for debugging) */
+ uint32 total_allocated; /* kB; including child contexts */
+ uint32 self_allocated; /* kB; not including child contexts */
bool isReset; /* T = no space alloced since last reset */
#ifdef USE_ASSERT_CHECKING
bool allowInCritSection; /* allow palloc in critical section */
*** 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);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
