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

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 6e4a3434394ea1806465a96c99fd2829d4a2df7e..071ee304306831e83e9c31dc21d4f46efe36b3db 100644 (file)
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -1084,62 +1084,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;
@@ -1153,11 +1103,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;
                block = (AllocBlock) realloc(block, blksize);
                if (block == NULL)
@@ -1182,17 +1140,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;
 
@@ -1217,15 +1179,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.)