mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use doubly-linked block lists in aset.c to reduce large-chunk overhead.

Browse Source 
Large chunks (those too large for any palloc freelist) are managed as
separate blocks.  Formerly, realloc'ing or pfree'ing such a chunk required
O(N) time in a context with N blocks, since we had to traipse down the
singly-linked block list to locate the block's predecessor before we could
fix the list links.  This can result in O(N^2) runtime in situations where
large numbers of such chunks are manipulated within one context.  Cases
like that were not foreseen in the original design of aset.c, and indeed
didn't arise until fairly recently.  But such problems can now occur in
reorderbuffer.c and in hash joining, both of which make repeated large
requests without scaling up their request size as they do so, and which
will free their requests in not-necessarily-LIFO order.

To fix, change the block list from singly-linked to doubly-linked.
This adds another 4 or 8 bytes to ALLOC_BLOCKHDRSZ, but that doesn't
seem like unacceptable overhead, since aset.c's blocks are normally
8K or more, and never less than 1K in current practice.

In passing, get rid of some redundant AllocChunkGetPointer() calls in
AllocSetRealloc (the compiler might be smart enough to optimize these
away anyway, but no need to assume that) and improve AllocSetCheck's
checking of block header fields.

Back-patch to 9.4 where reorderbuffer.c appeared.  We could take this
further back, but currently there's no evidence that it would be useful.

Discussion: https://postgr.es/m/CAMkU=1x1hvue1XYrZoWk_omG0Ja5nBvTdvgrOeVkkeqs71CV8g@mail.gmail.com
This commit is contained in:
Tom Lane 2017-03-08 12:21:12 -05:00
parent f35742ccb7
commit ff97741bc8
1 changed files with 66 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

108
src/backend/utils/mmgr/aset.c
View File

@ -148,7 +148,8 @@ typedef AllocSetContext *AllocSet;
typedef struct AllocBlockData typedef struct AllocBlockData
{ {
AllocSet aset; /* aset that owns this block */ AllocSet aset; /* aset that owns this block */
AllocBlock next; /* next block in aset's blocks list */ AllocBlock prev; /* prev block in aset's blocks list, if any */
AllocBlock next; /* next block in aset's blocks list, if any */
char *freeptr; /* start of free space in this block */ char *freeptr; /* start of free space in this block */
char *endptr; /* end of space in this block */ char *endptr; /* end of space in this block */
} AllocBlockData; } AllocBlockData;
@ -407,7 +408,10 @@ AllocSetContextCreate(MemoryContext parent,
block->aset = set; block->aset = set;
block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ; block->freeptr = ((char *) block) + ALLOC_BLOCKHDRSZ;
block->endptr = ((char *) block) + blksize; block->endptr = ((char *) block) + blksize;
block->prev = NULL;
block->next = set->blocks; block->next = set->blocks;
if (block->next)
block->next->prev = block;
set->blocks = block; set->blocks = block;
/* Mark block as not to be released at reset time */ /* Mark block as not to be released at reset time */
set->keeper = block; set->keeper = block;
@ -489,6 +493,7 @@ AllocSetReset(MemoryContext context)
VALGRIND_MAKE_MEM_NOACCESS(datastart, block->freeptr - datastart); VALGRIND_MAKE_MEM_NOACCESS(datastart, block->freeptr - datastart);
#endif #endif
block->freeptr = datastart; block->freeptr = datastart;
block->prev = NULL;
block->next = NULL; block->next = NULL;
} }
else else
@ -595,16 +600,20 @@ AllocSetAlloc(MemoryContext context, Size size)
#endif #endif
/* /*
* Stick the new block underneath the active allocation block, so that * Stick the new block underneath the active allocation block, if any,
* we don't lose the use of the space remaining therein. * so that we don't lose the use of the space remaining therein.
*/ */
if (set->blocks != NULL) if (set->blocks != NULL)
{ {
block->prev = set->blocks;
block->next = set->blocks->next; block->next = set->blocks->next;
if (block->next)
block->next->prev = block;
set->blocks->next = block; set->blocks->next = block;
} }
else else
{ {
block->prev = NULL;
block->next = NULL; block->next = NULL;
set->blocks = block; set->blocks = block;
} }
@ -785,7 +794,10 @@ AllocSetAlloc(MemoryContext context, Size size)
VALGRIND_MAKE_MEM_NOACCESS(block->freeptr, VALGRIND_MAKE_MEM_NOACCESS(block->freeptr,
blksize - ALLOC_BLOCKHDRSZ); blksize - ALLOC_BLOCKHDRSZ);
block->prev = NULL;
block->next = set->blocks; block->next = set->blocks;
if (block->next)
block->next->prev = block;
set->blocks = block; set->blocks = block;
} }
@ -845,29 +857,28 @@ AllocSetFree(MemoryContext context, void *pointer)
{ {
/* /*
* Big chunks are certain to have been allocated as single-chunk * Big chunks are certain to have been allocated as single-chunk
* blocks. Find the containing block and return it to malloc(). * blocks. Just unlink that block and return it to malloc().
*/ */
AllocBlock block = set->blocks; AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ);
AllocBlock prevblock = NULL;
while (block != NULL) /*
{ * Try to verify that we have a sane block pointer: it should
if (chunk == (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ)) * reference the correct aset, and freeptr and endptr should point
break; * just past the chunk.
prevblock = block; */
block = block->next; if (block->aset != set ||
} block->freeptr != block->endptr ||
if (block == NULL) block->freeptr != ((char *) block) +
(chunk->size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ))
elog(ERROR, "could not find block containing chunk %p", chunk); elog(ERROR, "could not find block containing chunk %p", chunk);
/* let's just make sure chunk is the only one in the block */
Assert(block->freeptr == ((char *) block) +
(chunk->size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ));
/* OK, remove block from aset's list and free it */ /* OK, remove block from aset's list and free it */
if (prevblock == NULL) if (block->prev)
set->blocks = block->next; block->prev->next = block->next;
else else
prevblock->next = block->next; set->blocks = block->next;
if (block->next)
block->next->prev = block->prev;
#ifdef CLOBBER_FREED_MEMORY #ifdef CLOBBER_FREED_MEMORY
wipe_mem(block, block->freeptr - ((char *) block)); wipe_mem(block, block->freeptr - ((char *) block));
#endif #endif
@ -973,27 +984,24 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
if (oldsize > set->allocChunkLimit) if (oldsize > set->allocChunkLimit)
{ {
/* /*
* The chunk must have been allocated as a single-chunk block. Find * The chunk must have been allocated as a single-chunk block. Use
* the containing block and use realloc() to make it bigger with * realloc() to make the containing block bigger with minimum space
* minimum space wastage. * wastage.
*/ */
AllocBlock block = set->blocks; AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ);
AllocBlock prevblock = NULL;
Size chksize; Size chksize;
Size blksize; Size blksize;
while (block != NULL) /*
{ * Try to verify that we have a sane block pointer: it should
if (chunk == (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ)) * reference the correct aset, and freeptr and endptr should point
break; * just past the chunk.
prevblock = block; */
block = block->next; if (block->aset != set ||
} block->freeptr != block->endptr ||
if (block == NULL) block->freeptr != ((char *) block) +
(chunk->size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ))
elog(ERROR, "could not find block containing chunk %p", chunk); elog(ERROR, "could not find block containing chunk %p", chunk);
/* let's just make sure chunk is the only one in the block */
Assert(block->freeptr == ((char *) block) +
(chunk->size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ));
/* Do the realloc */ /* Do the realloc */
chksize = MAXALIGN(size); chksize = MAXALIGN(size);
@ -1006,10 +1014,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
/* Update pointers since block has likely been moved */ /* Update pointers since block has likely been moved */
chunk = (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ); chunk = (AllocChunk) (((char *) block) + ALLOC_BLOCKHDRSZ);
pointer = AllocChunkGetPointer(chunk); pointer = AllocChunkGetPointer(chunk);
if (prevblock == NULL) if (block->prev)
set->blocks = block; block->prev->next = block;
else else
prevblock->next = block; set->blocks = block;
if (block->next)
block->next->prev = block;
chunk->size = chksize; chunk->size = chksize;
#ifdef MEMORY_CONTEXT_CHECKING #ifdef MEMORY_CONTEXT_CHECKING
@ -1033,7 +1043,7 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
/* set mark to catch clobber of "unused" space */ /* set mark to catch clobber of "unused" space */
if (size < chunk->size) if (size < chunk->size)
set_sentinel(AllocChunkGetPointer(chunk), size); set_sentinel(pointer, size);
#else /* !MEMORY_CONTEXT_CHECKING */ #else /* !MEMORY_CONTEXT_CHECKING */
/* /*
@ -1046,7 +1056,8 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
/* Make any trailing alignment padding NOACCESS. */ /* Make any trailing alignment padding NOACCESS. */
VALGRIND_MAKE_MEM_NOACCESS((char *) pointer + size, chksize - size); VALGRIND_MAKE_MEM_NOACCESS((char *) pointer + size, chksize - size);
return AllocChunkGetPointer(chunk);
return pointer;
} }
else else
{ {
@ -1200,9 +1211,12 @@ AllocSetCheck(MemoryContext context)
{ {
AllocSet set = (AllocSet) context; AllocSet set = (AllocSet) context;
char *name = set->header.name; char *name = set->header.name;
AllocBlock prevblock;
AllocBlock block; AllocBlock block;
for (block = set->blocks; block != NULL; block = block->next) for (prevblock = NULL, block = set->blocks;
block != NULL;
prevblock = block, block = block->next)
{ {
char *bpoz = ((char *) block) + ALLOC_BLOCKHDRSZ; char *bpoz = ((char *) block) + ALLOC_BLOCKHDRSZ;
long blk_used = block->freeptr - bpoz; long blk_used = block->freeptr - bpoz;
@ -1219,6 +1233,16 @@ AllocSetCheck(MemoryContext context)
name, block); name, block);
} }
/*
* Check block header fields
*/
if (block->aset != set ||
block->prev != prevblock ||
block->freeptr < bpoz ||
block->freeptr > block->endptr)
elog(WARNING, "problem in alloc set %s: corrupt header in block %p",
name, block);
/* /*
* Chunk walker * Chunk walker
*/ */