From 3b8b55f3b2790d709751cc6736f39cd6c17fd4af Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Wed, 29 May 2024 12:19:03 +1200
Subject: [PATCH v1 4/4] Optimize alignment calculations in tuple form/deform

This converts TupleDescDeformAttr.attalign from a char which is directly
derived from pg_attribute.attalign into a uint8 which specifies the
number of bytes to align the column by.  This removes the complexity of
checking each char value and transforming that into the appropriate
alignment call.  This can just be a simple TYPEALIGN passing in the
number of bytes.
---
 contrib/amcheck/verify_heapam.c        |  6 +--
 contrib/pageinspect/heapfuncs.c        |  6 +--
 src/backend/access/brin/brin_tuple.c   |  8 ++--
 src/backend/access/common/heaptuple.c  | 52 +++++++++++++-------------
 src/backend/access/common/indextuple.c | 22 +++++------
 src/backend/access/common/tupdesc.c    | 20 +++++++++-
 src/backend/executor/execTuples.c      | 19 +++++-----
 src/include/access/tupdesc.h           |  8 ++--
 src/include/access/tupmacs.h           | 15 ++++++++
 9 files changed, 93 insertions(+), 63 deletions(-)

diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c
index 125d594826..ac8e38d2a6 100644
--- a/contrib/amcheck/verify_heapam.c
+++ b/contrib/amcheck/verify_heapam.c
@@ -1592,7 +1592,7 @@ check_tuple_attribute(HeapCheckContext *ctx)
 	/* Skip non-varlena values, but update offset first */
 	if (thisatt->attlen != -1)
 	{
-		ctx->offset = att_align_nominal(ctx->offset, thisatt->attalign);
+		ctx->offset = att_align_nominal_fast(ctx->offset, thisatt->attalign);
 		ctx->offset = att_addlength_pointer(ctx->offset, thisatt->attlen,
 											tp + ctx->offset);
 		if (ctx->tuphdr->t_hoff + ctx->offset > ctx->lp_len)
@@ -1608,8 +1608,8 @@ check_tuple_attribute(HeapCheckContext *ctx)
 	}
 
 	/* Ok, we're looking at a varlena attribute. */
-	ctx->offset = att_align_pointer(ctx->offset, thisatt->attalign, -1,
-									tp + ctx->offset);
+	ctx->offset = att_align_pointer_fast(ctx->offset, thisatt->attalign, -1,
+										 tp + ctx->offset);
 
 	/* Get the (possibly corrupt) varlena datum */
 	attdatum = fetchatt_fast(thisatt, tp + ctx->offset);
diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c
index e610aa69c6..a0d5e511be 100644
--- a/contrib/pageinspect/heapfuncs.c
+++ b/contrib/pageinspect/heapfuncs.c
@@ -354,8 +354,8 @@ tuple_data_split_internal(Oid relid, char *tupdata,
 
 			if (attr->attlen == -1)
 			{
-				off = att_align_pointer(off, attr->attalign, -1,
-										tupdata + off);
+				off = att_align_pointer_fast(off, attr->attalign, -1,
+											 tupdata + off);
 
 				/*
 				 * As VARSIZE_ANY throws an exception if it can't properly
@@ -373,7 +373,7 @@ tuple_data_split_internal(Oid relid, char *tupdata,
 			}
 			else
 			{
-				off = att_align_nominal(off, attr->attalign);
+				off = att_align_nominal_fast(off, attr->attalign);
 				len = attr->attlen;
 			}
 
diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c
index 213b873867..f3ea5b3097 100644
--- a/src/backend/access/brin/brin_tuple.c
+++ b/src/backend/access/brin/brin_tuple.c
@@ -703,13 +703,13 @@ brin_deconstruct_tuple(BrinDesc *brdesc,
 
 			if (thisatt->attlen == -1)
 			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
+				off = att_align_pointer_fast(off, thisatt->attalign, -1,
+											 tp + off);
 			}
 			else
 			{
-				/* not varlena, so safe to use att_align_nominal */
-				off = att_align_nominal(off, thisatt->attalign);
+				/* not varlena, so safe to use att_align_nominal_fast */
+				off = att_align_nominal_fast(off, thisatt->attalign);
 			}
 
 			values[stored++] = fetchatt_fast(thisatt, tp + off);
diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c
index 963b88c27b..0a6eb663be 100644
--- a/src/backend/access/common/heaptuple.c
+++ b/src/backend/access/common/heaptuple.c
@@ -250,13 +250,13 @@ heap_compute_data_size(TupleDesc tupleDesc,
 			 * we want to flatten the expanded value so that the constructed
 			 * tuple doesn't depend on it
 			 */
-			data_length = att_align_nominal(data_length, atti->attalign);
+			data_length = att_align_nominal_fast(data_length, atti->attalign);
 			data_length += EOH_get_flat_size(DatumGetEOHP(val));
 		}
 		else
 		{
-			data_length = att_align_datum(data_length, atti->attalign,
-										  atti->attlen, val);
+			data_length = att_align_datum_fast(data_length, atti->attalign,
+											   atti->attlen, val);
 			data_length = att_addlength_datum(data_length, atti->attlen,
 											  val);
 		}
@@ -307,13 +307,13 @@ fill_val(TupleDescDeformAttr *att,
 	}
 
 	/*
-	 * XXX we use the att_align macros on the pointer value itself, not on an
-	 * offset.  This is a bit of a hack.
+	 * XXX we use the att_align_nominal_fast macro on the pointer value
+	 * itself, not on an offset.  This is a bit of a hack.
 	 */
 	if (DeformAttrByVal(att))
 	{
 		/* pass-by-value */
-		data = (char *) att_align_nominal(data, att->attalign);
+		data = (char *) att_align_nominal_fast(data, att->attalign);
 		store_att_byval(data, datum, att->attlen);
 		data_length = att->attlen;
 	}
@@ -333,8 +333,7 @@ fill_val(TupleDescDeformAttr *att,
 				 */
 				ExpandedObjectHeader *eoh = DatumGetEOHP(datum);
 
-				data = (char *) att_align_nominal(data,
-												  att->attalign);
+				data = (char *) att_align_nominal_fast(data, att->attalign);
 				data_length = EOH_get_flat_size(eoh);
 				EOH_flatten_into(eoh, data, data_length);
 			}
@@ -363,8 +362,7 @@ fill_val(TupleDescDeformAttr *att,
 		else
 		{
 			/* full 4-byte header varlena */
-			data = (char *) att_align_nominal(data,
-											  att->attalign);
+			data = (char *) att_align_nominal_fast(data, att->attalign);
 			data_length = VARSIZE(val);
 			memcpy(data, val, data_length);
 		}
@@ -373,14 +371,14 @@ fill_val(TupleDescDeformAttr *att,
 	{
 		/* cstring ... never needs alignment */
 		*infomask |= HEAP_HASVARWIDTH;
-		Assert(att->attalign == TYPALIGN_CHAR);
+		Assert(att->attalign == sizeof(char));
 		data_length = strlen(DatumGetCString(datum)) + 1;
 		memcpy(data, DatumGetPointer(datum), data_length);
 	}
 	else
 	{
 		/* fixed-length pass-by-reference */
-		data = (char *) att_align_nominal(data, att->attalign);
+		data = (char *) att_align_nominal_fast(data, att->attalign);
 		Assert(att->attlen > 0);
 		data_length = att->attlen;
 		memcpy(data, DatumGetPointer(datum), data_length);
@@ -634,7 +632,7 @@ nocachegetattr(HeapTuple tup,
 			if (att->attlen <= 0)
 				break;
 
-			off = att_align_nominal(off, att->attalign);
+			off = att_align_nominal_fast(off, att->attalign);
 
 			att->attcacheoff = off;
 
@@ -683,19 +681,19 @@ nocachegetattr(HeapTuple tup,
 				 * either an aligned or unaligned value.
 				 */
 				if (usecache &&
-					off == att_align_nominal(off, att->attalign))
+					off == att_align_nominal_fast(off, att->attalign))
 					att->attcacheoff = off;
 				else
 				{
-					off = att_align_pointer(off, att->attalign, -1,
-											tp + off);
+					off = att_align_pointer_fast(off, att->attalign, -1,
+												 tp + off);
 					usecache = false;
 				}
 			}
 			else
 			{
-				/* not varlena, so safe to use att_align_nominal */
-				off = att_align_nominal(off, att->attalign);
+				/* not varlena, so safe to use att_align_nominal_fast */
+				off = att_align_nominal_fast(off, att->attalign);
 
 				if (usecache)
 					att->attcacheoff = off;
@@ -898,10 +896,10 @@ expand_tuple(HeapTuple *targetHeapTuple,
 			{
 				TupleDescDeformAttr *att = TupleDescDeformAttr(tupleDesc, attnum);
 
-				targetDataLen = att_align_datum(targetDataLen,
-												att->attalign,
-												att->attlen,
-												attrmiss[attnum].am_value);
+				targetDataLen = att_align_datum_fast(targetDataLen,
+													 att->attalign,
+													 att->attlen,
+													 attrmiss[attnum].am_value);
 
 				targetDataLen = att_addlength_pointer(targetDataLen,
 													  att->attlen,
@@ -1397,19 +1395,19 @@ heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc,
 			 * an aligned or unaligned value.
 			 */
 			if (!slow &&
-				off == att_align_nominal(off, thisatt->attalign))
+				off == att_align_nominal_fast(off, thisatt->attalign))
 				thisatt->attcacheoff = off;
 			else
 			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
+				off = att_align_pointer_fast(off, thisatt->attalign, -1,
+											 tp + off);
 				slow = true;
 			}
 		}
 		else
 		{
-			/* not varlena, so safe to use att_align_nominal */
-			off = att_align_nominal(off, thisatt->attalign);
+			/* not varlena, so safe to use att_align_nominal_fast */
+			off = att_align_nominal_fast(off, thisatt->attalign);
 
 			if (!slow)
 				thisatt->attcacheoff = off;
diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c
index aac4214cc7..5445300ceb 100644
--- a/src/backend/access/common/indextuple.c
+++ b/src/backend/access/common/indextuple.c
@@ -363,7 +363,7 @@ nocache_index_getattr(IndexTuple tup,
 			if (att->attlen <= 0)
 				break;
 
-			off = att_align_nominal(off, att->attalign);
+			off = att_align_nominal_fast(off, att->attalign);
 
 			att->attcacheoff = off;
 
@@ -412,19 +412,19 @@ nocache_index_getattr(IndexTuple tup,
 				 * either an aligned or unaligned value.
 				 */
 				if (usecache &&
-					off == att_align_nominal(off, att->attalign))
+					off == att_align_nominal_fast(off, att->attalign))
 					att->attcacheoff = off;
 				else
 				{
-					off = att_align_pointer(off, att->attalign, -1,
-											tp + off);
+					off = att_align_pointer_fast(off, att->attalign, -1,
+												 tp + off);
 					usecache = false;
 				}
 			}
 			else
 			{
-				/* not varlena, so safe to use att_align_nominal */
-				off = att_align_nominal(off, att->attalign);
+				/* not varlena, so safe to use att_align_nominal_fast */
+				off = att_align_nominal_fast(off, att->attalign);
 
 				if (usecache)
 					att->attcacheoff = off;
@@ -513,19 +513,19 @@ index_deform_tuple_internal(TupleDesc tupleDescriptor,
 			 * an aligned or unaligned value.
 			 */
 			if (!slow &&
-				off == att_align_nominal(off, thisatt->attalign))
+				off == att_align_nominal_fast(off, thisatt->attalign))
 				thisatt->attcacheoff = off;
 			else
 			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
+				off = att_align_pointer_fast(off, thisatt->attalign, -1,
+											 tp + off);
 				slow = true;
 			}
 		}
 		else
 		{
-			/* not varlena, so safe to use att_align_nominal */
-			off = att_align_nominal(off, thisatt->attalign);
+			/* not varlena, so safe to use att_align_nominal_fast */
+			off = att_align_nominal_fast(off, thisatt->attalign);
 
 			if (!slow)
 				thisatt->attcacheoff = off;
diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index ee996f465a..cd6e3be4fd 100644
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -77,7 +77,25 @@ populate_TupleDescAttr(TupleDescDeformAttr *dst, Form_pg_attribute src)
 	if (src->attnotnull)
 		dst->attflags |= DEFORM_ATTR_FLAG_IS_NOTNULL;
 
-	dst->attalign = src->attalign;
+	switch (src->attalign)
+	{
+		case TYPALIGN_INT:
+			dst->attalign = ALIGNOF_INT;
+			break;
+		case TYPALIGN_CHAR:
+			dst->attalign = sizeof(char);
+			break;
+		case TYPALIGN_DOUBLE:
+			dst->attalign = ALIGNOF_DOUBLE;
+			break;
+		case TYPALIGN_SHORT:
+			dst->attalign = ALIGNOF_SHORT;
+			break;
+		default:
+			dst->attalign = 0;
+			elog(ERROR, "invalid attalign value: %c", src->attalign);
+			break;
+	}
 }
 
 /*
diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index e651934918..ffb0666e91 100644
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -202,12 +202,12 @@ tts_virtual_materialize(TupleTableSlot *slot)
 			 * We want to flatten the expanded value so that the materialized
 			 * slot doesn't depend on it.
 			 */
-			sz = att_align_nominal(sz, att->attalign);
+			sz = att_align_nominal_fast(sz, att->attalign);
 			sz += EOH_get_flat_size(DatumGetEOHP(val));
 		}
 		else
 		{
-			sz = att_align_nominal(sz, att->attalign);
+			sz = att_align_nominal_fast(sz, att->attalign);
 			sz = att_addlength_datum(sz, att->attlen, val);
 		}
 	}
@@ -242,8 +242,7 @@ tts_virtual_materialize(TupleTableSlot *slot)
 			 */
 			ExpandedObjectHeader *eoh = DatumGetEOHP(val);
 
-			data = (char *) att_align_nominal(data,
-											  att->attalign);
+			data = (char *) att_align_nominal_fast(data, att->attalign);
 			data_length = EOH_get_flat_size(eoh);
 			EOH_flatten_into(eoh, data, data_length);
 
@@ -254,7 +253,7 @@ tts_virtual_materialize(TupleTableSlot *slot)
 		{
 			Size		data_length = 0;
 
-			data = (char *) att_align_nominal(data, att->attalign);
+			data = (char *) att_align_nominal_fast(data, att->attalign);
 			data_length = att_addlength_datum(data_length, att->attlen, val);
 
 			memcpy(data, DatumGetPointer(val), data_length);
@@ -1067,19 +1066,19 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp,
 			 * an aligned or unaligned value.
 			 */
 			if (!slow &&
-				off == att_align_nominal(off, thisatt->attalign))
+				off == att_align_nominal_fast(off, thisatt->attalign))
 				thisatt->attcacheoff = off;
 			else
 			{
-				off = att_align_pointer(off, thisatt->attalign, -1,
-										tp + off);
+				off = att_align_pointer_fast(off, thisatt->attalign, -1,
+											 tp + off);
 				slow = true;
 			}
 		}
 		else
 		{
-			/* not varlena, so safe to use att_align_nominal */
-			off = att_align_nominal(off, thisatt->attalign);
+			/* not varlena, so safe to use att_align_nominal_fast */
+			off = att_align_nominal_fast(off, thisatt->attalign);
 
 			if (!slow)
 				thisatt->attcacheoff = off;
diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h
index 908c32254e..5150b155b3 100644
--- a/src/include/access/tupdesc.h
+++ b/src/include/access/tupdesc.h
@@ -51,10 +51,10 @@ typedef struct TupleConstr
  */
 typedef struct TupleDescDeformAttr
 {
-	int32	attcacheoff;
-	int16	attlen;
-	uint8	attflags;
-	char	attalign;
+	int32	attcacheoff; /* fixed offset into tuple, if known, or -1 */
+	int16	attlen;		/* attr len in bytes or -1 = varlen, -2 = cstring */
+	uint8	attflags;	/* bit flags for compact storage of bool fields */
+	uint8	attalign;	/* alignment requirement in bytes */
 } TupleDescDeformAttr;
 
 #define DEFORM_ATTR_FLAG_BYVAL			(1 << 0)
diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h
index f96820f64a..7cce4e48a8 100644
--- a/src/include/access/tupmacs.h
+++ b/src/include/access/tupmacs.h
@@ -92,6 +92,12 @@ fetch_att(const void *T, bool attbyval, int attlen)
 	att_align_nominal(cur_offset, attalign) \
 )
 
+#define att_align_datum_fast(cur_offset, attalign, attlen, attdatum) \
+( \
+	((attlen) == -1 && VARATT_IS_SHORT(DatumGetPointer(attdatum))) ? \
+	(uintptr_t) (cur_offset) : \
+	TYPEALIGN(attalign, cur_offset))
+
 /*
  * att_align_pointer performs the same calculation as att_align_datum,
  * but is used when walking a tuple.  attptr is the current actual data
@@ -113,6 +119,12 @@ fetch_att(const void *T, bool attbyval, int attlen)
 	att_align_nominal(cur_offset, attalign) \
 )
 
+#define att_align_pointer_fast(cur_offset, attalign, attlen, attptr) \
+( \
+	((attlen) == -1 && VARATT_NOT_PAD_BYTE(attptr)) ? \
+	(uintptr_t) (cur_offset) : \
+	TYPEALIGN(attalign, cur_offset))
+
 /*
  * att_align_nominal aligns the given offset as needed for a datum of alignment
  * requirement attalign, ignoring any consideration of packed varlena datums.
@@ -139,6 +151,9 @@ fetch_att(const void *T, bool attbyval, int attlen)
 	   ))) \
 )
 
+#define att_align_nominal_fast(cur_offset, attalign) \
+	TYPEALIGN(attalign, (uintptr_t) cur_offset)
+
 /*
  * att_addlength_datum increments the given offset by the space needed for
  * the given Datum variable.  attdatum is only accessed if we are dealing
-- 
2.34.1

