On Fri, 2014-08-29 at 10:12 -0400, Tom Lane wrote:
> What about removing the callback per se and just keeping the argument,
> as it were. That is, a MemoryContext has a field of type "size_t *"
> that is normally NULL, but if set to non-NULL, then we increment the
> pointed-to value for pallocs and decrement for pfrees.
I like that idea; patch attached. Two differences:
* At block level, not chunk level.
* I use a uint64, because a size_t is only guaranteed to hold one
allocation, and we need to sum up many allocations.
When unused, it does still appear to have a little overhead in Robert's
test on his power machine. It seems to be between a 0.5-1.0% regression.
There's not much extra code on that path, just a branch, pointer
dereference, and an addition; so I don't think it will get much cheaper
than it is.
This regression seems harder to reproduce on my workstation, or perhaps
it's just noisier.
> One thought in either case is that we don't have to touch the API for
> MemoryContextCreate: rather, the feature can be enabled in a separate
> call after making the context.
That seems fairly awkward to me because the pointer needs to be
inherited from the parent context when not specified. I left the extra
API call in.
The inheritance is awkward anyway, though. If you create a tracked
context as a child of an already-tracked context, allocations in the
newer one won't count against the original. I don't see a way around
that without introducing even more performance problems.
If this patch is unacceptable, my only remaining idea is to add the
memory only for the current context with no inheritance (thereby
eliminating the branch, also). That will require HashAgg to traverse all
the child contexts to check whether the memory limit has been exceeded.
As Tomas pointed out, that could be a lot of work in the case of
array_agg with many groups.
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,482 ----
Size initBlockSize,
Size maxBlockSize)
{
! /* inherit 'track_mem' from parent context, if available */
! uint64 *track_mem = (parent == NULL) ? NULL : parent->track_mem;
!
! return AllocSetContextCreateTracked(parent, name, minContextSize,
! initBlockSize, maxBlockSize,
! track_mem);
! }
!
! /*
! * AllocSetContextCreateTracked
! * Create a new AllocSet context, tracking total memory usage.
! *
! * parent: parent context, or NULL if top-level context
! * name: name of context (for debugging --- string will be copied)
! * minContextSize: minimum context size
! * initBlockSize: initial allocation block size
! * maxBlockSize: maximum allocation block size
! * track_mem: caller-supplied variable to use for memory tracking
! */
! MemoryContext
! AllocSetContextCreateTracked(MemoryContext parent,
! const char *name,
! Size minContextSize,
! Size initBlockSize,
! Size maxBlockSize,
! uint64 *track_mem)
! {
! AllocSet set;
! MemoryContext context;
/* Do the type-independent part of context creation */
! context = MemoryContextCreate(T_AllocSetContext,
! sizeof(AllocSetContext),
! &AllocSetMethods,
! parent,
! name,
! track_mem);
!
! 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
--- 490,498 ----
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
--- 508,517 ----
* 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;
}
/*
--- 531,554 ----
errdetail("Failed while creating memory context \"%s\".",
name)));
}
!
! if (context->track_mem != NULL)
! *context->track_mem += 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)
--- 625,633 ----
else
{
/* Normal case, release the block */
+ if (context->track_mem != NULL)
+ *context->track_mem -= block->endptr - ((char*) block);
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
***************
*** 632,637 **** AllocSetDelete(MemoryContext context)
--- 670,678 ----
{
AllocBlock next = block->next;
+ if (context->track_mem != NULL)
+ *context->track_mem -= block->endptr - ((char *) block);
+
#ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block));
#endif
***************
*** 678,683 **** AllocSetAlloc(MemoryContext context, Size size)
--- 719,728 ----
errmsg("out of memory"),
errdetail("Failed on request of size %zu.", size)));
}
+
+ if (context->track_mem != NULL)
+ *context->track_mem += blksize;
+
block->aset = set;
block->freeptr = block->endptr = ((char *) block) + blksize;
***************
*** 873,878 **** AllocSetAlloc(MemoryContext context, Size size)
--- 918,926 ----
errdetail("Failed on request of size %zu.", size)));
}
+ if (context->track_mem != NULL)
+ *context->track_mem += blksize;
+
block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize;
***************
*** 976,981 **** AllocSetFree(MemoryContext context, void *pointer)
--- 1024,1033 ----
set->blocks = block->next;
else
prevblock->next = block->next;
+
+ if (context->track_mem != NULL)
+ *context->track_mem -= 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)
--- 1140,1146 ----
AllocBlock prevblock = NULL;
Size chksize;
Size blksize;
+ Size oldblksize;
while (block != NULL)
{
***************
*** 1105,1110 **** AllocSetRealloc(MemoryContext context, void *pointer, Size size)
--- 1158,1165 ----
/* 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)
--- 1169,1177 ----
errmsg("out of memory"),
errdetail("Failed on request of size %zu.", size)));
}
+ if (context->track_mem != NULL)
+ *context->track_mem += 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
***************
*** 546,552 **** MemoryContext
MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name)
{
MemoryContext node;
Size needed = size + strlen(name) + 1;
--- 546,553 ----
MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name,
! uint64 *track_mem)
{
MemoryContext node;
Size needed = size + strlen(name) + 1;
***************
*** 576,581 **** MemoryContextCreate(NodeTag tag, Size size,
--- 577,583 ----
node->firstchild = NULL;
node->nextchild = NULL;
node->isReset = true;
+ node->track_mem = track_mem;
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 *track_mem; /* track memory usage for caller */
#ifdef USE_ASSERT_CHECKING
bool allowInCritSection; /* allow palloc in critical section */
#endif
*** a/src/include/utils/memutils.h
--- b/src/include/utils/memutils.h
***************
*** 117,123 **** extern bool MemoryContextContains(MemoryContext context, void *pointer);
extern MemoryContext MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name);
/*
--- 117,124 ----
extern MemoryContext MemoryContextCreate(NodeTag tag, Size size,
MemoryContextMethods *methods,
MemoryContext parent,
! const char *name,
! uint64 *track_mem);
/*
***************
*** 130,135 **** extern MemoryContext AllocSetContextCreate(MemoryContext parent,
--- 131,142 ----
Size minContextSize,
Size initBlockSize,
Size maxBlockSize);
+ extern MemoryContext AllocSetContextCreateTracked(MemoryContext parent,
+ const char *name,
+ Size minContextSize,
+ Size initBlockSize,
+ Size maxBlockSize,
+ uint64 *total_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
