From c477f3e449d1a7322c2453e9cd9d6b710ae91577 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 3 Oct 2019 13:56:26 -0400 Subject: [PATCH] Allow repalloc() to give back space when a large chunk is downsized. Up to now, if you resized a large (>8K) palloc chunk down to a smaller size, aset.c made no attempt to return any space to the malloc pool. That's unpleasant if a really large allocation is resized to a significantly smaller size. I think no such cases existed when this code was designed, and I'm not sure whether they're common even yet, but an upcoming fix to encoding conversion will certainly create such cases. Therefore, fix AllocSetRealloc so that it gives realloc() a chance to do something with the block. This doesn't noticeably increase complexity, we mostly just have to change the order in which the cases are considered. Back-patch to all supported branches. Discussion: https://postgr.es/m/20190816181418.GA898@alvherre.pgsql Discussion: https://postgr.es/m/3614.1569359690@sss.pgh.pa.us --- src/backend/utils/mmgr/aset.c | 142 ++++++++++++++++++---------------- 1 file changed, 77 insertions(+), 65 deletions(-) diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c index 3bbcb1c894..ea23ac2ccb 100644 --- a/src/backend/utils/mmgr/aset.c +++ b/src/backend/utils/mmgr/aset.c @@ -1110,62 +1110,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) set->header.name, chunk); #endif - /* - * Chunk sizes are aligned to power of 2 in AllocSetAlloc(). Maybe the - * allocated area already is >= the new size. (In particular, we always - * fall out here if the requested size is a decrease.) - */ - if (oldsize >= size) - { -#ifdef MEMORY_CONTEXT_CHECKING - Size oldrequest = chunk->requested_size; - -#ifdef RANDOMIZE_ALLOCATED_MEMORY - /* We can only fill the extra space if we know the prior request */ - if (size > oldrequest) - randomize_mem((char *) pointer + oldrequest, - size - oldrequest); -#endif - - chunk->requested_size = size; - - /* - * If this is an increase, mark any newly-available part UNDEFINED. - * Otherwise, mark the obsolete part NOACCESS. - */ - if (size > oldrequest) - VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + oldrequest, - size - oldrequest); - else - VALGRIND_MAKE_MEM_NOACCESS((char *) pointer + size, - oldsize - size); - - /* set mark to catch clobber of "unused" space */ - if (size < oldsize) - set_sentinel(pointer, size); -#else /* !MEMORY_CONTEXT_CHECKING */ - - /* - * We don't have the information to determine whether we're growing - * the old request or shrinking it, so we conservatively mark the - * entire new allocation DEFINED. - */ - VALGRIND_MAKE_MEM_NOACCESS(pointer, oldsize); - VALGRIND_MAKE_MEM_DEFINED(pointer, size); -#endif - - /* Disallow external access to private part of chunk header. */ - VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOCCHUNK_PRIVATE_LEN); - - return pointer; - } - if (oldsize > set->allocChunkLimit) { /* * The chunk must have been allocated as a single-chunk block. Use - * realloc() to make the containing block bigger with minimum space - * wastage. + * realloc() to make the containing block bigger, or smaller, with + * minimum space wastage. */ AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ); Size chksize; @@ -1180,11 +1130,19 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) if (block->aset != set || block->freeptr != block->endptr || block->freeptr != ((char *) block) + - (chunk->size + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ)) + (oldsize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ)) elog(ERROR, "could not find block containing chunk %p", chunk); + /* + * Even if the new request is less than set->allocChunkLimit, we stick + * with the single-chunk block approach. Therefore we need + * chunk->size to be bigger than set->allocChunkLimit, so we don't get + * confused about the chunk's status in future calls. + */ + chksize = Max(size, set->allocChunkLimit + 1); + chksize = MAXALIGN(chksize); + /* Do the realloc */ - chksize = MAXALIGN(size); blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; oldblksize = block->endptr - ((char *)block); @@ -1214,17 +1172,21 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) #ifdef MEMORY_CONTEXT_CHECKING #ifdef RANDOMIZE_ALLOCATED_MEMORY /* We can only fill the extra space if we know the prior request */ - randomize_mem((char *) pointer + chunk->requested_size, - size - chunk->requested_size); + if (size > chunk->requested_size) + randomize_mem((char *) pointer + chunk->requested_size, + size - chunk->requested_size); #endif /* - * realloc() (or randomize_mem()) will have left the newly-allocated + * realloc() (or randomize_mem()) will have left any newly-allocated * part UNDEFINED, but we may need to adjust trailing bytes from the * old allocation. */ - VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, - oldsize - chunk->requested_size); +#ifdef USE_VALGRIND + if (oldsize > chunk->requested_size) + VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, + oldsize - chunk->requested_size); +#endif chunk->requested_size = size; @@ -1249,15 +1211,65 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) return pointer; } + + /* + * Chunk sizes are aligned to power of 2 in AllocSetAlloc(). Maybe the + * allocated area already is >= the new size. (In particular, we will + * fall out here if the requested size is a decrease.) + */ + else if (oldsize >= size) + { +#ifdef MEMORY_CONTEXT_CHECKING + Size oldrequest = chunk->requested_size; + +#ifdef RANDOMIZE_ALLOCATED_MEMORY + /* We can only fill the extra space if we know the prior request */ + if (size > oldrequest) + randomize_mem((char *) pointer + oldrequest, + size - oldrequest); +#endif + + chunk->requested_size = size; + + /* + * If this is an increase, mark any newly-available part UNDEFINED. + * Otherwise, mark the obsolete part NOACCESS. + */ + if (size > oldrequest) + VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + oldrequest, + size - oldrequest); + else + VALGRIND_MAKE_MEM_NOACCESS((char *) pointer + size, + oldsize - size); + + /* set mark to catch clobber of "unused" space */ + if (size < oldsize) + set_sentinel(pointer, size); +#else /* !MEMORY_CONTEXT_CHECKING */ + + /* + * We don't have the information to determine whether we're growing + * the old request or shrinking it, so we conservatively mark the + * entire new allocation DEFINED. + */ + VALGRIND_MAKE_MEM_NOACCESS(pointer, oldsize); + VALGRIND_MAKE_MEM_DEFINED(pointer, size); +#endif + + /* Disallow external access to private part of chunk header. */ + VALGRIND_MAKE_MEM_NOACCESS(chunk, ALLOCCHUNK_PRIVATE_LEN); + + return pointer; + } else { /* - * Small-chunk case. We just do this by brute force, ie, allocate a - * new chunk and copy the data. Since we know the existing data isn't - * huge, this won't involve any great memcpy expense, so it's not - * worth being smarter. (At one time we tried to avoid memcpy when it - * was possible to enlarge the chunk in-place, but that turns out to - * misbehave unpleasantly for repeated cycles of + * Enlarge-a-small-chunk case. We just do this by brute force, ie, + * allocate a new chunk and copy the data. Since we know the existing + * data isn't huge, this won't involve any great memcpy expense, so + * it's not worth being smarter. (At one time we tried to avoid + * memcpy when it was possible to enlarge the chunk in-place, but that + * turns out to misbehave unpleasantly for repeated cycles of * palloc/repalloc/pfree: the eventually freed chunks go into the * wrong freelist for the next initial palloc request, and so we leak * memory indefinitely. See pgsql-hackers archives for 2007-08-11.) -- 2.40.0