]> granicus.if.org Git - postgresql/commitdiff
Reduce size of common allocation header.
authorAndres Freund <andres@anarazel.de>
Tue, 28 Feb 2017 07:32:22 +0000 (23:32 -0800)
committerAndres Freund <andres@anarazel.de>
Wed, 1 Mar 2017 03:42:44 +0000 (19:42 -0800)
The new slab allocator needs different per-allocation information than
the classical aset.c.  The definition in 58b25e981 wasn't sufficiently
careful on 32 platforms with 8 byte alignment, leading to buildfarm
failures.  That's not entirely easy to fix by just adjusting the
definition.

As slab.c doesn't actually need the size part(s) of the common header,
all chunks are equally sized after all, it seems better to instead
reduce the header to the part needed by all allocators, namely which
context an allocation belongs to. That has the advantage of reducing
the overhead of slab allocations, and also allows for more flexibility
in future allocators.

To avoid spreading the logic about accessing a chunk's context around,
centralize it in GetMemoryChunkContext(), which allows to delete a
good number of lines.

A followup commit will revise the mmgr/README portion about
StandardChunkHeader, and more.

Author: Andres Freund
Discussion: https://postgr.es/m/20170228074420.aazv4iw6k562mnxg@alap3.anarazel.de

src/backend/utils/mmgr/aset.c
src/backend/utils/mmgr/mcxt.c
src/backend/utils/mmgr/slab.c
src/include/utils/memutils.h
src/tools/pgindent/typedefs.list

index 8056c00ae49caf526cdf0007887e837af70410bb..b7904f46829f53af0247f86e4b20883450758903 100644 (file)
  */
 
 #define ALLOC_BLOCKHDRSZ       MAXALIGN(sizeof(AllocBlockData))
-#define ALLOC_CHUNKHDRSZ       MAXALIGN(sizeof(AllocChunkData))
-
-/* Portion of ALLOC_CHUNKHDRSZ examined outside aset.c. */
-#define ALLOC_CHUNK_PUBLIC     \
-       (offsetof(AllocChunkData, size) + sizeof(Size))
-
-/* Portion of ALLOC_CHUNKHDRSZ excluding trailing padding. */
-#ifdef MEMORY_CONTEXT_CHECKING
-#define ALLOC_CHUNK_USED       \
-       (offsetof(AllocChunkData, requested_size) + sizeof(Size))
-#else
-#define ALLOC_CHUNK_USED       \
-       (offsetof(AllocChunkData, size) + sizeof(Size))
-#endif
+#define ALLOC_CHUNKHDRSZ       sizeof(struct AllocChunkData)
 
 typedef struct AllocBlockData *AllocBlock;             /* forward reference */
 typedef struct AllocChunkData *AllocChunk;
@@ -169,20 +156,25 @@ typedef struct AllocBlockData
 /*
  * AllocChunk
  *             The prefix of each piece of memory in an AllocBlock
- *
- * NB: this MUST match StandardChunkHeader as defined by utils/memutils.h.
  */
 typedef struct AllocChunkData
 {
-       /* aset is the owning aset if allocated, or the freelist link if free */
-       void       *aset;
        /* size is always the size of the usable space in the chunk */
        Size            size;
 #ifdef MEMORY_CONTEXT_CHECKING
        /* when debugging memory usage, also store actual requested size */
        /* this is zero in a free chunk */
        Size            requested_size;
+#if MAXIMUM_ALIGNOF > 4 && SIZEOF_VOID_P == 4
+       Size            padding;
 #endif
+
+#endif   /* MEMORY_CONTEXT_CHECKING */
+
+       /* aset is the owning aset if allocated, or the freelist link if free */
+       void       *aset;
+
+       /* there must not be any padding to reach a MAXALIGN boundary here! */
 }      AllocChunkData;
 
 /*
@@ -334,6 +326,10 @@ AllocSetContextCreate(MemoryContext parent,
 {
        AllocSet        set;
 
+       StaticAssertStmt(offsetof(AllocChunkData, aset) + sizeof(MemoryContext) ==
+                                        MAXALIGN(sizeof(AllocChunkData)),
+                                        "padding calculation in AllocChunkData is wrong");
+
        /*
         * First, validate allocation parameters.  (If we're going to throw an
         * error, we should do so before the context is created, not after.)  We
@@ -616,13 +612,13 @@ AllocSetAlloc(MemoryContext context, Size size)
                AllocAllocInfo(set, chunk);
 
                /*
-                * Chunk header public fields remain DEFINED.  The requested
-                * allocation itself can be NOACCESS or UNDEFINED; our caller will
-                * soon make it UNDEFINED.  Make extra space at the end of the chunk,
-                * if any, NOACCESS.
+                * Chunk's metadata fields remain DEFINED.  The requested allocation
+                * itself can be NOACCESS or UNDEFINED; our caller will soon make it
+                * UNDEFINED.  Make extra space at the end of the chunk, if any,
+                * NOACCESS.
                 */
-               VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + ALLOC_CHUNK_PUBLIC,
-                                                chunk_size + ALLOC_CHUNKHDRSZ - ALLOC_CHUNK_PUBLIC);
+               VALGRIND_MAKE_MEM_NOACCESS((char *) chunk + ALLOC_CHUNKHDRSZ,
+                                                                  chunk_size - ALLOC_CHUNKHDRSZ);
 
                return AllocChunkGetPointer(chunk);
        }
@@ -709,7 +705,7 @@ AllocSetAlloc(MemoryContext context, Size size)
                                chunk = (AllocChunk) (block->freeptr);
 
                                /* Prepare to initialize the chunk header. */
-                               VALGRIND_MAKE_MEM_UNDEFINED(chunk, ALLOC_CHUNK_USED);
+                               VALGRIND_MAKE_MEM_UNDEFINED(chunk, ALLOC_CHUNKHDRSZ);
 
                                block->freeptr += (availchunk + ALLOC_CHUNKHDRSZ);
                                availspace -= (availchunk + ALLOC_CHUNKHDRSZ);
@@ -799,7 +795,7 @@ AllocSetAlloc(MemoryContext context, Size size)
        chunk = (AllocChunk) (block->freeptr);
 
        /* Prepare to initialize the chunk header. */
-       VALGRIND_MAKE_MEM_UNDEFINED(chunk, ALLOC_CHUNK_USED);
+       VALGRIND_MAKE_MEM_UNDEFINED(chunk, ALLOC_CHUNKHDRSZ);
 
        block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ);
        Assert(block->freeptr <= block->endptr);
index c21dfcb96bac70397113fdd0326ac931f2268e15..07732ba59bcd7ee0c0e3fe6f01ffa802321b38d3 100644 (file)
@@ -389,55 +389,10 @@ MemoryContextAllowInCriticalSection(MemoryContext context, bool allow)
 Size
 GetMemoryChunkSpace(void *pointer)
 {
-       StandardChunkHeader *header;
+       MemoryContext context = GetMemoryChunkContext(pointer);
 
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       Assert(pointer != NULL);
-       Assert(pointer == (void *) MAXALIGN(pointer));
-
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       header = (StandardChunkHeader *)
-               ((char *) pointer - STANDARDCHUNKHEADERSIZE);
-
-       AssertArg(MemoryContextIsValid(header->context));
-
-       return (*header->context->methods->get_chunk_space) (header->context,
-                                                                                                                pointer);
-}
-
-/*
- * GetMemoryChunkContext
- *             Given a currently-allocated chunk, determine the context
- *             it belongs to.
- */
-MemoryContext
-GetMemoryChunkContext(void *pointer)
-{
-       StandardChunkHeader *header;
-
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       Assert(pointer != NULL);
-       Assert(pointer == (void *) MAXALIGN(pointer));
-
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       header = (StandardChunkHeader *)
-               ((char *) pointer - STANDARDCHUNKHEADERSIZE);
-
-       AssertArg(MemoryContextIsValid(header->context));
-
-       return header->context;
+       return (context->methods->get_chunk_space) (context,
+                                                                                               pointer);
 }
 
 /*
@@ -611,23 +566,9 @@ MemoryContextCheck(MemoryContext context)
 bool
 MemoryContextContains(MemoryContext context, void *pointer)
 {
-       StandardChunkHeader *header;
-
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       if (pointer == NULL || pointer != (void *) MAXALIGN(pointer))
-               return false;
+       MemoryContext ptr_context = GetMemoryChunkContext(pointer);
 
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       header = (StandardChunkHeader *)
-               ((char *) pointer - STANDARDCHUNKHEADERSIZE);
-
-       return header->context == context;
+       return ptr_context == context;
 }
 
 /*--------------------
@@ -991,23 +932,7 @@ palloc_extended(Size size, int flags)
 void
 pfree(void *pointer)
 {
-       MemoryContext context;
-
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       Assert(pointer != NULL);
-       Assert(pointer == (void *) MAXALIGN(pointer));
-
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       context = ((StandardChunkHeader *)
-                          ((char *) pointer - STANDARDCHUNKHEADERSIZE))->context;
-
-       AssertArg(MemoryContextIsValid(context));
+       MemoryContext context = GetMemoryChunkContext(pointer);
 
        (*context->methods->free_p) (context, pointer);
        VALGRIND_MEMPOOL_FREE(context, pointer);
@@ -1020,27 +945,12 @@ pfree(void *pointer)
 void *
 repalloc(void *pointer, Size size)
 {
-       MemoryContext context;
+       MemoryContext context = GetMemoryChunkContext(pointer);
        void       *ret;
 
        if (!AllocSizeIsValid(size))
                elog(ERROR, "invalid memory alloc request size %zu", size);
 
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       Assert(pointer != NULL);
-       Assert(pointer == (void *) MAXALIGN(pointer));
-
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       context = ((StandardChunkHeader *)
-                          ((char *) pointer - STANDARDCHUNKHEADERSIZE))->context;
-
-       AssertArg(MemoryContextIsValid(context));
        AssertNotInCriticalSection(context);
 
        /* isReset must be false already */
@@ -1103,27 +1013,12 @@ MemoryContextAllocHuge(MemoryContext context, Size size)
 void *
 repalloc_huge(void *pointer, Size size)
 {
-       MemoryContext context;
+       MemoryContext context = GetMemoryChunkContext(pointer);
        void       *ret;
 
        if (!AllocHugeSizeIsValid(size))
                elog(ERROR, "invalid memory alloc request size %zu", size);
 
-       /*
-        * Try to detect bogus pointers handed to us, poorly though we can.
-        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
-        * allocated chunk.
-        */
-       Assert(pointer != NULL);
-       Assert(pointer == (void *) MAXALIGN(pointer));
-
-       /*
-        * OK, it's probably safe to look at the chunk header.
-        */
-       context = ((StandardChunkHeader *)
-                          ((char *) pointer - STANDARDCHUNKHEADERSIZE))->context;
-
-       AssertArg(MemoryContextIsValid(context));
        AssertNotInCriticalSection(context);
 
        /* isReset must be false already */
index a5e140eef705017f3276d745d9d45fd9062525d5..11a126acf368092d6efaa4cf363174c461fbe884 100644 (file)
 #include "lib/ilist.h"
 
 
-#define SLAB_CHUNKHDRSZ MAXALIGN(sizeof(SlabChunk))
-
-/* Portion of SLAB_CHUNKHDRSZ excluding trailing padding. */
-#define SLAB_CHUNK_USED \
-       (offsetof(SlabChunk, header) + sizeof(StandardChunkHeader))
-
 /*
  * SlabContext is a specialized implementation of MemoryContext.
  */
@@ -103,17 +97,15 @@ typedef struct SlabChunk
 {
        /* block owning this chunk */
        void       *block;
-
-       /* include StandardChunkHeader because mcxt.c expects that */
-       StandardChunkHeader header;
-
+       SlabContext *slab;                      /* owning context */
+       /* there must not be any padding to reach a MAXALIGN boundary here! */
 } SlabChunk;
 
 
 #define SlabPointerGetChunk(ptr)       \
-       ((SlabChunk *)(((char *)(ptr)) - SLAB_CHUNKHDRSZ))
+       ((SlabChunk *)(((char *)(ptr)) - sizeof(SlabChunk)))
 #define SlabChunkGetPointer(chk)       \
-       ((void *)(((char *)(chk)) + SLAB_CHUNKHDRSZ))
+       ((void *)(((char *)(chk)) + sizeof(SlabChunk)))
 #define SlabBlockGetChunk(slab, block, idx) \
        ((SlabChunk *) ((char *) (block) + sizeof(SlabBlock)    \
                                        + (idx * slab->fullChunkSize)))
@@ -198,6 +190,10 @@ SlabContextCreate(MemoryContext parent,
        Size            freelistSize;
        SlabContext *slab;
 
+       StaticAssertStmt(offsetof(SlabChunk, slab) +sizeof(MemoryContext) ==
+                                        MAXALIGN(sizeof(SlabChunk)),
+                                        "padding calculation in SlabChunk is wrong");
+
        /* otherwise the linked list inside freed chunk isn't guaranteed to fit */
        StaticAssertStmt(MAXIMUM_ALIGNOF >= sizeof(int),
                                         "MAXALIGN too small to fit int32");
@@ -207,7 +203,7 @@ SlabContextCreate(MemoryContext parent,
 
        /* Make sure the block can store at least one chunk. */
        if (blockSize - sizeof(SlabBlock) < fullChunkSize)
-               elog(ERROR, "block size %ld for slab is too small for %ld chunks",
+               elog(ERROR, "block size %zu for slab is too small for %zu chunks",
                         blockSize, chunkSize);
 
        /* Compute maximum number of chunks per block */
@@ -333,7 +329,7 @@ SlabAlloc(MemoryContext context, Size size)
 
        /* make sure we only allow correct request size */
        if (size != slab->chunkSize)
-               elog(ERROR, "unexpected alloc chunk size %ld (expected %ld)",
+               elog(ERROR, "unexpected alloc chunk size %zu (expected %zu)",
                         size, slab->chunkSize);
 
        /*
@@ -445,20 +441,21 @@ SlabAlloc(MemoryContext context, Size size)
                slab->minFreeChunks = 0;
 
        /* Prepare to initialize the chunk header. */
-       VALGRIND_MAKE_MEM_UNDEFINED(chunk, SLAB_CHUNK_USED);
+       VALGRIND_MAKE_MEM_UNDEFINED(chunk, sizeof(SlabChunk));
 
        chunk->block = (void *) block;
-
-       chunk->header.context = (MemoryContext) slab;
-       chunk->header.size = MAXALIGN(size);
+       chunk->slab = slab;
 
 #ifdef MEMORY_CONTEXT_CHECKING
-       chunk->header.requested_size = size;
-       VALGRIND_MAKE_MEM_NOACCESS(&chunk->header.requested_size,
-                                                          sizeof(chunk->header.requested_size));
        /* slab mark to catch clobber of "unused" space */
-       if (size < chunk->header.size)
+       if (slab->chunkSize < (slab->fullChunkSize - sizeof(SlabChunk)))
+       {
                set_sentinel(SlabChunkGetPointer(chunk), size);
+               VALGRIND_MAKE_MEM_NOACCESS(((char *) chunk) +
+                                                                  sizeof(SlabChunk) + slab->chunkSize,
+                                                                  slab->fullChunkSize -
+                                                                  (slab->chunkSize + sizeof(SlabChunk)));
+       }
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
        /* fill the allocated space with junk */
@@ -484,11 +481,9 @@ SlabFree(MemoryContext context, void *pointer)
        SlabFreeInfo(slab, chunk);
 
 #ifdef MEMORY_CONTEXT_CHECKING
-       VALGRIND_MAKE_MEM_DEFINED(&chunk->header.requested_size,
-                                                         sizeof(chunk->header.requested_size));
        /* Test for someone scribbling on unused space in chunk */
-       if (chunk->header.requested_size < chunk->header.size)
-               if (!sentinel_ok(pointer, chunk->header.requested_size))
+       if (slab->chunkSize < (slab->fullChunkSize - sizeof(SlabChunk)))
+               if (!sentinel_ok(pointer, slab->chunkSize))
                        elog(WARNING, "detected write past chunk end in %s %p",
                                 slab->header.name, chunk);
 #endif
@@ -507,12 +502,7 @@ SlabFree(MemoryContext context, void *pointer)
 #ifdef CLOBBER_FREED_MEMORY
        /* XXX don't wipe the int32 index, used for block-level freelist */
        wipe_mem((char *) pointer + sizeof(int32),
-                        chunk->header.size - sizeof(int32));
-#endif
-
-#ifdef MEMORY_CONTEXT_CHECKING
-       /* Reset requested_size to 0 in chunks that are on freelist */
-       chunk->header.requested_size = 0;
+                        slab->chunkSize - sizeof(int32));
 #endif
 
        /* remove the block from a freelist */
@@ -590,9 +580,11 @@ SlabRealloc(MemoryContext context, void *pointer, Size size)
 static Size
 SlabGetChunkSpace(MemoryContext context, void *pointer)
 {
-       SlabChunk  *chunk = SlabPointerGetChunk(pointer);
+       SlabContext *slab = castNode(SlabContext, context);
+
+       Assert(slab);
 
-       return chunk->header.size + SLAB_CHUNKHDRSZ;
+       return slab->fullChunkSize;
 }
 
 /*
@@ -742,37 +734,20 @@ SlabCheck(MemoryContext context)
                                {
                                        SlabChunk  *chunk = SlabBlockGetChunk(slab, block, j);
 
-                                       VALGRIND_MAKE_MEM_DEFINED(&chunk->header.requested_size,
-                                                                          sizeof(chunk->header.requested_size));
-
-                                       /* we're in a no-freelist branch */
-                                       VALGRIND_MAKE_MEM_NOACCESS(&chunk->header.requested_size,
-                                                                          sizeof(chunk->header.requested_size));
-
                                        /* chunks have both block and slab pointers, so check both */
                                        if (chunk->block != block)
                                                elog(WARNING, "problem in slab %s: bogus block link in block %p, chunk %p",
                                                         name, block, chunk);
 
-                                       if (chunk->header.context != (MemoryContext) slab)
+                                       if (chunk->slab != slab)
                                                elog(WARNING, "problem in slab %s: bogus slab link in block %p, chunk %p",
                                                         name, block, chunk);
 
-                                       /* now make sure the chunk size is correct */
-                                       if (chunk->header.size != MAXALIGN(slab->chunkSize))
-                                               elog(WARNING, "problem in slab %s: bogus chunk size in block %p, chunk %p",
-                                                        name, block, chunk);
-
-                                       /* now make sure the chunk size is correct */
-                                       if (chunk->header.requested_size != slab->chunkSize)
-                                               elog(WARNING, "problem in slab %s: bogus chunk requested size in block %p, chunk %p",
-                                                        name, block, chunk);
-
                                        /* there might be sentinel (thanks to alignment) */
-                                       if (chunk->header.requested_size < chunk->header.size &&
-                                               !sentinel_ok(chunk, SLAB_CHUNKHDRSZ + chunk->header.requested_size))
-                                               elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p",
-                                                        name, block, chunk);
+                                       if (slab->chunkSize < (slab->fullChunkSize - sizeof(SlabChunk)))
+                                               if (!sentinel_ok(chunk, slab->chunkSize))
+                                                       elog(WARNING, "problem in slab %s: detected write past chunk end in block %p, chunk %p",
+                                                                name, block, chunk);
                                }
                        }
 
index 5223a4da3952f1c1851e69b7f47430f4c5b33e91..58e816d8e97d1bdc6714e0f51340689934ad54c0 100644 (file)
 
 #define AllocHugeSizeIsValid(size)     ((Size) (size) <= MaxAllocHugeSize)
 
-/*
- * All chunks allocated by any memory context manager are required to be
- * preceded by a StandardChunkHeader at a spacing of STANDARDCHUNKHEADERSIZE.
- * A currently-allocated chunk must contain a backpointer to its owning
- * context as well as the allocated size of the chunk.  The backpointer is
- * used by pfree() and repalloc() to find the context to call.  The allocated
- * size is not absolutely essential, but it's expected to be needed by any
- * reasonable implementation.
- */
-typedef struct StandardChunkHeader
-{
-       MemoryContext context;          /* owning context */
-       Size            size;                   /* size of data space allocated in chunk */
-#ifdef MEMORY_CONTEXT_CHECKING
-       /* when debugging memory usage, also store actual requested size */
-       Size            requested_size;
-#endif
-} StandardChunkHeader;
-
-#define STANDARDCHUNKHEADERSIZE  MAXALIGN(sizeof(StandardChunkHeader))
-
 
 /*
  * Standard top-level memory contexts.
@@ -100,7 +79,6 @@ extern void MemoryContextDeleteChildren(MemoryContext context);
 extern void MemoryContextSetParent(MemoryContext context,
                                           MemoryContext new_parent);
 extern Size GetMemoryChunkSpace(void *pointer);
-extern MemoryContext GetMemoryChunkContext(void *pointer);
 extern MemoryContext MemoryContextGetParent(MemoryContext context);
 extern bool MemoryContextIsEmpty(MemoryContext context);
 extern void MemoryContextStats(MemoryContext context);
@@ -113,6 +91,42 @@ extern void MemoryContextCheck(MemoryContext context);
 #endif
 extern bool MemoryContextContains(MemoryContext context, void *pointer);
 
+/*
+ * GetMemoryChunkContext
+ *             Given a currently-allocated chunk, determine the context
+ *             it belongs to.
+ *
+ * All chunks allocated by any memory context manager are required to be
+ * preceded by the corresponding MemoryContext stored, without padding, in the
+ * preceding sizeof(void*) bytes.  A currently-allocated chunk must contain a
+ * backpointer to its owning context.  The backpointer is used by pfree() and
+ * repalloc() to find the context to call.
+ */
+#ifndef FRONTEND
+static inline MemoryContext
+GetMemoryChunkContext(void *pointer)
+{
+       MemoryContext context;
+
+       /*
+        * Try to detect bogus pointers handed to us, poorly though we can.
+        * Presumably, a pointer that isn't MAXALIGNED isn't pointing at an
+        * allocated chunk.
+        */
+       Assert(pointer != NULL);
+       Assert(pointer == (void *) MAXALIGN(pointer));
+
+       /*
+        * OK, it's probably safe to look at the context.
+        */
+       context = *(MemoryContext *) (((char *) pointer) - sizeof(void *));
+
+       AssertArg(MemoryContextIsValid(context));
+
+       return context;
+}
+#endif
+
 /*
  * This routine handles the context-type-independent part of memory
  * context creation.  It's intended to be called from context-type-
index 1fd7ec4256ae6bad400492513cac5be41f8a84d2..6717eccbb0092d019ff06d36555b67408968327a 100644 (file)
@@ -2001,7 +2001,6 @@ SplitLR
 SplitVar
 SplitedPageLayout
 StackElem
-StandardChunkHeader
 StartBlobPtr
 StartBlobsPtr
 StartDataPtr