From a59c3200dd127feb0cb09a055250ff6401aee1aa Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Mon, 27 Feb 2017 23:32:22 -0800
Subject: [PATCH] Remove StandardChunkHeader for Slab's benefit.

---
 src/backend/utils/mmgr/aset.c    |  51 ++++++++---------
 src/backend/utils/mmgr/mcxt.c    | 121 +++------------------------------------
 src/backend/utils/mmgr/slab.c    |  81 ++++++++------------------
 src/include/utils/memutils.h     |  56 +++++++++++-------
 src/tools/pgindent/typedefs.list |   1 -
 5 files changed, 92 insertions(+), 218 deletions(-)

diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 8056c00ae4..31228ade19 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -97,20 +97,7 @@
  */
 
 #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,27 @@ 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 +328,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 +614,14 @@ 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 context 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,
+								   chunk_size + ALLOC_CHUNKHDRSZ);
+		VALGRIND_MAKE_MEM_DEFINED(chunk->aset, sizeof(MemoryContext));
 
 		return AllocChunkGetPointer(chunk);
 	}
@@ -709,7 +708,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, sizeof(AllocChunkData));
 
 				block->freeptr += (availchunk + ALLOC_CHUNKHDRSZ);
 				availspace -= (availchunk + ALLOC_CHUNKHDRSZ);
@@ -799,7 +798,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, sizeof(AllocChunkData));
 
 	block->freeptr += (chunk_size + ALLOC_CHUNKHDRSZ);
 	Assert(block->freeptr <= block->endptr);
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index 6ad0bb47b6..80b41b0e65 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -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;
+	MemoryContext	ptr_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.
-	 */
-	if (pointer == NULL || pointer != (void *) MAXALIGN(pointer))
-		return false;
-
-	/*
-	 * 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 */
diff --git a/src/backend/utils/mmgr/slab.c b/src/backend/utils/mmgr/slab.c
index a5e140eef7..99956fc279 100644
--- a/src/backend/utils/mmgr/slab.c
+++ b/src/backend/utils/mmgr/slab.c
@@ -57,12 +57,6 @@
 #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,19 +441,14 @@ 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);
 #endif
 #ifdef RANDOMIZE_ALLOCATED_MEMORY
@@ -484,11 +475,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 +496,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 +574,11 @@ SlabRealloc(MemoryContext context, void *pointer, Size size)
 static Size
 SlabGetChunkSpace(MemoryContext context, void *pointer)
 {
-	SlabChunk  *chunk = SlabPointerGetChunk(pointer);
+	SlabContext *slab = castNode(SlabContext, context);
 
-	return chunk->header.size + SLAB_CHUNKHDRSZ;
+	Assert(slab);
+
+	return slab->fullChunkSize;
 }
 
 /*
@@ -742,37 +728,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);
 				}
 			}
 
diff --git a/src/include/utils/memutils.h b/src/include/utils/memutils.h
index 5223a4da39..930e469858 100644
--- a/src/include/utils/memutils.h
+++ b/src/include/utils/memutils.h
@@ -45,27 +45,6 @@
 
 #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);
@@ -114,6 +92,40 @@ extern void MemoryContextCheck(MemoryContext context);
 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.
+ */
+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;
+}
+
+/*
  * This routine handles the context-type-independent part of memory
  * context creation.  It's intended to be called from context-type-
  * specific creation routines, and noplace else.
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index 1fd7ec4256..6717eccbb0 100644
--- a/src/tools/pgindent/typedefs.list
+++ b/src/tools/pgindent/typedefs.list
@@ -2001,7 +2001,6 @@ SplitLR
 SplitVar
 SplitedPageLayout
 StackElem
-StandardChunkHeader
 StartBlobPtr
 StartBlobsPtr
 StartDataPtr
-- 
2.11.0.22.g8d7a455.dirty

