Commit c477f3e4 authored by Tom Lane's avatar Tom Lane

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
parent b7a1c553
...@@ -1110,62 +1110,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) ...@@ -1110,62 +1110,12 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
set->header.name, chunk); set->header.name, chunk);
#endif #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) if (oldsize > set->allocChunkLimit)
{ {
/* /*
* The chunk must have been allocated as a single-chunk block. Use * The chunk must have been allocated as a single-chunk block. Use
* realloc() to make the containing block bigger with minimum space * realloc() to make the containing block bigger, or smaller, with
* wastage. * minimum space wastage.
*/ */
AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ); AllocBlock block = (AllocBlock) (((char *) chunk) - ALLOC_BLOCKHDRSZ);
Size chksize; Size chksize;
...@@ -1180,11 +1130,19 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) ...@@ -1180,11 +1130,19 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
if (block->aset != set || if (block->aset != set ||
block->freeptr != block->endptr || block->freeptr != block->endptr ||
block->freeptr != ((char *) block) + 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); 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 */ /* Do the realloc */
chksize = MAXALIGN(size);
blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ; blksize = chksize + ALLOC_BLOCKHDRSZ + ALLOC_CHUNKHDRSZ;
oldblksize = block->endptr - ((char *)block); oldblksize = block->endptr - ((char *)block);
...@@ -1214,17 +1172,21 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) ...@@ -1214,17 +1172,21 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
#ifdef MEMORY_CONTEXT_CHECKING #ifdef MEMORY_CONTEXT_CHECKING
#ifdef RANDOMIZE_ALLOCATED_MEMORY #ifdef RANDOMIZE_ALLOCATED_MEMORY
/* We can only fill the extra space if we know the prior request */ /* We can only fill the extra space if we know the prior request */
randomize_mem((char *) pointer + chunk->requested_size, if (size > chunk->requested_size)
size - chunk->requested_size); randomize_mem((char *) pointer + chunk->requested_size,
size - chunk->requested_size);
#endif #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 * part UNDEFINED, but we may need to adjust trailing bytes from the
* old allocation. * old allocation.
*/ */
VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size, #ifdef USE_VALGRIND
oldsize - chunk->requested_size); if (oldsize > chunk->requested_size)
VALGRIND_MAKE_MEM_UNDEFINED((char *) pointer + chunk->requested_size,
oldsize - chunk->requested_size);
#endif
chunk->requested_size = size; chunk->requested_size = size;
...@@ -1249,15 +1211,65 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size) ...@@ -1249,15 +1211,65 @@ AllocSetRealloc(MemoryContext context, void *pointer, Size size)
return pointer; 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 else
{ {
/* /*
* Small-chunk case. We just do this by brute force, ie, allocate a * Enlarge-a-small-chunk case. We just do this by brute force, ie,
* new chunk and copy the data. Since we know the existing data isn't * allocate a new chunk and copy the data. Since we know the existing
* huge, this won't involve any great memcpy expense, so it's not * data isn't huge, this won't involve any great memcpy expense, so
* worth being smarter. (At one time we tried to avoid memcpy when it * it's not worth being smarter. (At one time we tried to avoid
* was possible to enlarge the chunk in-place, but that turns out to * memcpy when it was possible to enlarge the chunk in-place, but that
* misbehave unpleasantly for repeated cycles of * turns out to misbehave unpleasantly for repeated cycles of
* palloc/repalloc/pfree: the eventually freed chunks go into the * palloc/repalloc/pfree: the eventually freed chunks go into the
* wrong freelist for the next initial palloc request, and so we leak * wrong freelist for the next initial palloc request, and so we leak
* memory indefinitely. See pgsql-hackers archives for 2007-08-11.) * memory indefinitely. See pgsql-hackers archives for 2007-08-11.)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment