From 60b5c203ee2396cb72508cbf7d76d112f196df14 Mon Sep 17 00:00:00 2001
From: Thomas Munro <thomas.munro@gmail.com>
Date: Thu, 4 Jul 2019 10:13:44 +1200
Subject: [PATCH] Use compiler alignment hints for palloc and memset.

---
 src/backend/postmaster/pgstat.c      |   2 +-
 src/backend/storage/ipc/procsignal.c |   2 +-
 src/backend/utils/adt/numeric.c      |   2 +-
 src/backend/utils/mmgr/aset.c        |   4 +-
 src/backend/utils/mmgr/mcxt.c        | 117 +--------------------------
 src/common/fe_memutils.c             |   6 --
 src/include/c.h                      | 105 ++++--------------------
 src/include/nodes/nodes.h            |   4 +-
 src/include/utils/palloc.h           |  44 +++++-----
 9 files changed, 50 insertions(+), 236 deletions(-)

diff --git a/src/backend/postmaster/pgstat.c b/src/backend/postmaster/pgstat.c
index b4f2b28b51..09dd0912e3 100644
--- a/src/backend/postmaster/pgstat.c
+++ b/src/backend/postmaster/pgstat.c
@@ -3206,7 +3206,7 @@ pgstat_progress_start_command(ProgressCommandType cmdtype, Oid relid)
 	PGSTAT_BEGIN_WRITE_ACTIVITY(beentry);
 	beentry->st_progress_command = cmdtype;
 	beentry->st_progress_command_target = relid;
-	MemSet(&beentry->st_progress_param, 0, sizeof(beentry->st_progress_param));
+	MemSet((void *) &beentry->st_progress_param, 0, sizeof(beentry->st_progress_param));
 	PGSTAT_END_WRITE_ACTIVITY(beentry);
 }
 
diff --git a/src/backend/storage/ipc/procsignal.c b/src/backend/storage/ipc/procsignal.c
index 7605b2c367..ea9c23c0b3 100644
--- a/src/backend/storage/ipc/procsignal.c
+++ b/src/backend/storage/ipc/procsignal.c
@@ -116,7 +116,7 @@ ProcSignalInit(int pss_idx)
 			 MyProcPid, pss_idx);
 
 	/* Clear out any leftover signal reasons */
-	MemSet(slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
+	MemSet((void *) slot->pss_signalFlags, 0, NUM_PROCSIGNALS * sizeof(sig_atomic_t));
 
 	/* Mark slot with my PID */
 	slot->pss_pid = MyProcPid;
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 201784bbf6..d324124d9a 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -451,7 +451,7 @@ static void dump_var(const char *str, NumericVar *var);
 			 pfree(buf); \
 	} while (0)
 
-#define init_var(v)		MemSetAligned(v, 0, sizeof(NumericVar))
+#define init_var(v)		MemSet(v, 0, sizeof(NumericVar))
 
 #define NUMERIC_DIGITS(num) (NUMERIC_HEADER_IS_SHORT(num) ? \
 	(num)->choice.n_short.n_data : (num)->choice.n_long.n_data)
diff --git a/src/backend/utils/mmgr/aset.c b/src/backend/utils/mmgr/aset.c
index 6e4a343439..d20dedb9e5 100644
--- a/src/backend/utils/mmgr/aset.c
+++ b/src/backend/utils/mmgr/aset.c
@@ -508,7 +508,7 @@ AllocSetContextCreateInternal(MemoryContext parent,
 	set->keeper = block;
 
 	/* Finish filling in aset-specific parts of the context header */
-	MemSetAligned(set->freelist, 0, sizeof(set->freelist));
+	MemSet(set->freelist, 0, sizeof(set->freelist));
 
 	set->initBlockSize = initBlockSize;
 	set->maxBlockSize = maxBlockSize;
@@ -575,7 +575,7 @@ AllocSetReset(MemoryContext context)
 #endif
 
 	/* Clear chunk freelists */
-	MemSetAligned(set->freelist, 0, sizeof(set->freelist));
+	MemSet(set->freelist, 0, sizeof(set->freelist));
 
 	block = set->blocks;
 
diff --git a/src/backend/utils/mmgr/mcxt.c b/src/backend/utils/mmgr/mcxt.c
index b07be12236..1bac9f72a9 100644
--- a/src/backend/utils/mmgr/mcxt.c
+++ b/src/backend/utils/mmgr/mcxt.c
@@ -803,82 +803,6 @@ MemoryContextAlloc(MemoryContext context, Size size)
 	return ret;
 }
 
-/*
- * MemoryContextAllocZero
- *		Like MemoryContextAlloc, but clears allocated memory
- *
- *	We could just call MemoryContextAlloc then clear the memory, but this
- *	is a very common combination, so we provide the combined operation.
- */
-void *
-MemoryContextAllocZero(MemoryContext context, Size size)
-{
-	void	   *ret;
-
-	AssertArg(MemoryContextIsValid(context));
-	AssertNotInCriticalSection(context);
-
-	if (!AllocSizeIsValid(size))
-		elog(ERROR, "invalid memory alloc request size %zu", size);
-
-	context->isReset = false;
-
-	ret = context->methods->alloc(context, size);
-	if (unlikely(ret == NULL))
-	{
-		MemoryContextStats(TopMemoryContext);
-		ereport(ERROR,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory"),
-				 errdetail("Failed on request of size %zu in memory context \"%s\".",
-						   size, context->name)));
-	}
-
-	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
-
-	MemSetAligned(ret, 0, size);
-
-	return ret;
-}
-
-/*
- * MemoryContextAllocZeroAligned
- *		MemoryContextAllocZero where length is suitable for MemSetLoop
- *
- *	This might seem overly specialized, but it's not because newNode()
- *	is so often called with compile-time-constant sizes.
- */
-void *
-MemoryContextAllocZeroAligned(MemoryContext context, Size size)
-{
-	void	   *ret;
-
-	AssertArg(MemoryContextIsValid(context));
-	AssertNotInCriticalSection(context);
-
-	if (!AllocSizeIsValid(size))
-		elog(ERROR, "invalid memory alloc request size %zu", size);
-
-	context->isReset = false;
-
-	ret = context->methods->alloc(context, size);
-	if (unlikely(ret == NULL))
-	{
-		MemoryContextStats(TopMemoryContext);
-		ereport(ERROR,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory"),
-				 errdetail("Failed on request of size %zu in memory context \"%s\".",
-						   size, context->name)));
-	}
-
-	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
-
-	MemSetLoop(ret, 0, size);
-
-	return ret;
-}
-
 /*
  * MemoryContextAllocExtended
  *		Allocate space within the specified context using the given flags.
@@ -897,7 +821,7 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
 
 	context->isReset = false;
 
-	ret = context->methods->alloc(context, size);
+	ret = pg_assume_aligned(context->methods->alloc(context, size), sizeof(long));
 	if (unlikely(ret == NULL))
 	{
 		if ((flags & MCXT_ALLOC_NO_OOM) == 0)
@@ -915,7 +839,7 @@ MemoryContextAllocExtended(MemoryContext context, Size size, int flags)
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
-		MemSetAligned(ret, 0, size);
+		memset(ret, 0, size);
 
 	return ret;
 }
@@ -951,39 +875,6 @@ palloc(Size size)
 	return ret;
 }
 
-void *
-palloc0(Size size)
-{
-	/* duplicates MemoryContextAllocZero to avoid increased overhead */
-	void	   *ret;
-	MemoryContext context = CurrentMemoryContext;
-
-	AssertArg(MemoryContextIsValid(context));
-	AssertNotInCriticalSection(context);
-
-	if (!AllocSizeIsValid(size))
-		elog(ERROR, "invalid memory alloc request size %zu", size);
-
-	context->isReset = false;
-
-	ret = context->methods->alloc(context, size);
-	if (unlikely(ret == NULL))
-	{
-		MemoryContextStats(TopMemoryContext);
-		ereport(ERROR,
-				(errcode(ERRCODE_OUT_OF_MEMORY),
-				 errmsg("out of memory"),
-				 errdetail("Failed on request of size %zu in memory context \"%s\".",
-						   size, context->name)));
-	}
-
-	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
-
-	MemSetAligned(ret, 0, size);
-
-	return ret;
-}
-
 void *
 palloc_extended(Size size, int flags)
 {
@@ -1000,7 +891,7 @@ palloc_extended(Size size, int flags)
 
 	context->isReset = false;
 
-	ret = context->methods->alloc(context, size);
+	ret = pg_assume_aligned(context->methods->alloc(context, size), sizeof(long));
 	if (unlikely(ret == NULL))
 	{
 		if ((flags & MCXT_ALLOC_NO_OOM) == 0)
@@ -1018,7 +909,7 @@ palloc_extended(Size size, int flags)
 	VALGRIND_MEMPOOL_ALLOC(context, ret, size);
 
 	if ((flags & MCXT_ALLOC_ZERO) != 0)
-		MemSetAligned(ret, 0, size);
+		MemSet(ret, 0, size);
 
 	return ret;
 }
diff --git a/src/common/fe_memutils.c b/src/common/fe_memutils.c
index ce99b4f4da..5b3cf862d8 100644
--- a/src/common/fe_memutils.c
+++ b/src/common/fe_memutils.c
@@ -118,12 +118,6 @@ palloc(Size size)
 	return pg_malloc_internal(size, 0);
 }
 
-void *
-palloc0(Size size)
-{
-	return pg_malloc_internal(size, MCXT_ALLOC_ZERO);
-}
-
 void *
 palloc_extended(Size size, int flags)
 {
diff --git a/src/include/c.h b/src/include/c.h
index 2a082afab1..d9cfb96b58 100644
--- a/src/include/c.h
+++ b/src/include/c.h
@@ -193,6 +193,18 @@
 #define pg_unreachable() abort()
 #endif
 
+/*
+ * Hint that a function always returns pointers to memory with given alignment,
+ * or that a given pointer has a given alignment.
+ */
+#if defined(__GNUC__) && (GCC_VERSION >= 40900 || defined(__clang__))
+#define pg_attribute_assume_aligned(x) __attribute__((assume_aligned(x)))
+#define pg_assume_aligned(p, x) __builtin_assume_aligned((p), (x))
+#else
+#define pg_attribute_assume_aligned(x)
+#define pg_assume_aligned(p, x) (p)
+#endif
+
 /*
  * Hints to the compiler about the likelihood of a branch. Both likely() and
  * unlikely() return the boolean value of the contained expression.
@@ -924,98 +936,11 @@ extern void ExceptionalCondition(const char *conditionName,
 		} \
 	} while (0)
 
-
-/* Get a bit mask of the bits set in non-long aligned addresses */
-#define LONG_ALIGN_MASK (sizeof(long) - 1)
-
-/*
- * MemSet
- *	Exactly the same as standard library function memset(), but considerably
- *	faster for zeroing small word-aligned structures (such as parsetree nodes).
- *	This has to be a macro because the main point is to avoid function-call
- *	overhead.   However, we have also found that the loop is faster than
- *	native libc memset() on some platforms, even those with assembler
- *	memset() functions.  More research needs to be done, perhaps with
- *	MEMSET_LOOP_LIMIT tests in configure.
- */
-#define MemSet(start, val, len) \
-	do \
-	{ \
-		/* must be void* because we don't know if it is integer aligned yet */ \
-		void   *_vstart = (void *) (start); \
-		int		_val = (val); \
-		Size	_len = (len); \
-\
-		if ((((uintptr_t) _vstart) & LONG_ALIGN_MASK) == 0 && \
-			(_len & LONG_ALIGN_MASK) == 0 && \
-			_val == 0 && \
-			_len <= MEMSET_LOOP_LIMIT && \
-			/* \
-			 *	If MEMSET_LOOP_LIMIT == 0, optimizer should find \
-			 *	the whole "if" false at compile time. \
-			 */ \
-			MEMSET_LOOP_LIMIT != 0) \
-		{ \
-			long *_start = (long *) _vstart; \
-			long *_stop = (long *) ((char *) _start + _len); \
-			while (_start < _stop) \
-				*_start++ = 0; \
-		} \
-		else \
-			memset(_vstart, _val, _len); \
-	} while (0)
-
 /*
- * MemSetAligned is the same as MemSet except it omits the test to see if
- * "start" is word-aligned.  This is okay to use if the caller knows a-priori
- * that the pointer is suitably aligned (typically, because he just got it
- * from palloc(), which always delivers a max-aligned pointer).
+ * Previously we had our own implementation of memset, but now we rely on
+ * the compiler to inline when appropriate with alignment analysis.
  */
-#define MemSetAligned(start, val, len) \
-	do \
-	{ \
-		long   *_start = (long *) (start); \
-		int		_val = (val); \
-		Size	_len = (len); \
-\
-		if ((_len & LONG_ALIGN_MASK) == 0 && \
-			_val == 0 && \
-			_len <= MEMSET_LOOP_LIMIT && \
-			MEMSET_LOOP_LIMIT != 0) \
-		{ \
-			long *_stop = (long *) ((char *) _start + _len); \
-			while (_start < _stop) \
-				*_start++ = 0; \
-		} \
-		else \
-			memset(_start, _val, _len); \
-	} while (0)
-
-
-/*
- * MemSetTest/MemSetLoop are a variant version that allow all the tests in
- * MemSet to be done at compile time in cases where "val" and "len" are
- * constants *and* we know the "start" pointer must be word-aligned.
- * If MemSetTest succeeds, then it is okay to use MemSetLoop, otherwise use
- * MemSetAligned.  Beware of multiple evaluations of the arguments when using
- * this approach.
- */
-#define MemSetTest(val, len) \
-	( ((len) & LONG_ALIGN_MASK) == 0 && \
-	(len) <= MEMSET_LOOP_LIMIT && \
-	MEMSET_LOOP_LIMIT != 0 && \
-	(val) == 0 )
-
-#define MemSetLoop(start, val, len) \
-	do \
-	{ \
-		long * _start = (long *) (start); \
-		long * _stop = (long *) ((char *) _start + (Size) (len)); \
-	\
-		while (_start < _stop) \
-			*_start++ = 0; \
-	} while (0)
-
+#define MemSet(a, b, c) memset((a), (b), (c))
 
 /* ----------------------------------------------------------------
  *				Section 8:	random stuff
diff --git a/src/include/nodes/nodes.h b/src/include/nodes/nodes.h
index 4e2fb39105..3c2bb93fdd 100644
--- a/src/include/nodes/nodes.h
+++ b/src/include/nodes/nodes.h
@@ -545,7 +545,7 @@ typedef struct Node
 #define newNode(size, tag) \
 ({	Node   *_result; \
 	AssertMacro((size) >= sizeof(Node));		/* need the tag, at least */ \
-	_result = (Node *) palloc0fast(size); \
+	_result = (Node *) palloc0(size); \
 	_result->type = (tag); \
 	_result; \
 })
@@ -562,7 +562,7 @@ extern PGDLLIMPORT Node *newNodeMacroHolder;
 #define newNode(size, tag) \
 ( \
 	AssertMacro((size) >= sizeof(Node)),		/* need the tag, at least */ \
-	newNodeMacroHolder = (Node *) palloc0fast(size), \
+	newNodeMacroHolder = (Node *) palloc0(size), \
 	newNodeMacroHolder->type = (tag), \
 	newNodeMacroHolder \
 )
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index a5f435ee06..784dba271c 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -68,34 +68,38 @@ extern PGDLLIMPORT MemoryContext CurrentMemoryContext;
 /*
  * Fundamental memory-allocation operations (more are in utils/memutils.h)
  */
-extern void *MemoryContextAlloc(MemoryContext context, Size size);
-extern void *MemoryContextAllocZero(MemoryContext context, Size size);
-extern void *MemoryContextAllocZeroAligned(MemoryContext context, Size size);
+extern void *MemoryContextAlloc(MemoryContext context, Size size) pg_attribute_assume_aligned(sizeof(long));
 extern void *MemoryContextAllocExtended(MemoryContext context,
-										Size size, int flags);
+										Size size, int flags) pg_attribute_assume_aligned(sizeof(long));
 
-extern void *palloc(Size size);
-extern void *palloc0(Size size);
-extern void *palloc_extended(Size size, int flags);
-extern void *repalloc(void *pointer, Size size);
+extern void *palloc(Size size) pg_attribute_assume_aligned(sizeof(long));
+extern void *palloc_extended(Size size, int flags) pg_attribute_assume_aligned(sizeof(long));
+extern void *repalloc(void *pointer, Size size) pg_attribute_assume_aligned(sizeof(long));
 extern void pfree(void *pointer);
 
 /*
- * The result of palloc() is always word-aligned, so we can skip testing
- * alignment of the pointer when deciding which MemSet variant to use.
- * Note that this variant does not offer any advantage, and should not be
- * used, unless its "sz" argument is a compile-time constant; therefore, the
- * issue that it evaluates the argument multiple times isn't a problem in
- * practice.
+ * Inline zeroed-memory functions to give the compiler a chance to inline the
+ * memset().
  */
-#define palloc0fast(sz) \
-	( MemSetTest(0, sz) ? \
-		MemoryContextAllocZeroAligned(CurrentMemoryContext, sz) : \
-		MemoryContextAllocZero(CurrentMemoryContext, sz) )
+static inline void *
+palloc0(Size size)
+{
+	void *memory = palloc(size);
+	memset(memory, 0, size);
+	return memory;
+}
+
+static inline void *
+MemoryContextAllocZero(MemoryContext context, Size size)
+{
+	void *memory = MemoryContextAlloc(context, size);
+	memset(memory, 0, size);
+	return memory;
+}
 
 /* Higher-limit allocators. */
-extern void *MemoryContextAllocHuge(MemoryContext context, Size size);
-extern void *repalloc_huge(void *pointer, Size size);
+extern void *MemoryContextAllocHuge(MemoryContext context, Size size) pg_attribute_assume_aligned(sizeof(long));
+extern void *repalloc_huge(void *pointer, Size size) pg_attribute_assume_aligned(sizeof(long));
 
 /*
  * Although this header file is nominally backend-only, certain frontend
-- 
2.22.0

