From db448ce5ad36a2754e4e75900b180260143aacf8 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Sat, 21 Dec 2024 09:43:26 +1300 Subject: [PATCH] Optimize alignment calculations in tuple form/deform Here we convert CompactAttribute.attalign from a char, which is directly derived from pg_attribute.attalign into a uint8, which stores the number of bytes to align the column's value by in the tuple. This allows tuple deformation and tuple size calculations to move away from using the inefficient att_align_nominal() macro, which manually checks each TYPALIGN_* char to translate that into the alignment bytes for the given type. Effectively, this commit changes those to TYPEALIGN calls, which are branchless and only perform some simple arithmetic with some bit-twiddling. The removed branches were often mispredicted by CPUs, especially so in real-world tables which often contain a mishmash of different types with different alignment requirements. Author: David Rowley Reviewed-by: Andres Freund, Victor Yegorov Discussion: https://postgr.es/m/CAApHDvrBztXP3yx=NKNmo3xwFAFhEdyPnvrDg3=M0RhDs+4vYw@mail.gmail.com --- contrib/amcheck/verify_heapam.c | 6 +-- contrib/pageinspect/heapfuncs.c | 6 +-- src/backend/access/brin/brin_tuple.c | 10 +++-- src/backend/access/common/attmap.c | 6 +-- src/backend/access/common/heaptuple.c | 52 +++++++++++++------------- src/backend/access/common/indextuple.c | 22 +++++------ src/backend/access/common/tupdesc.c | 20 +++++++++- src/backend/executor/execExprInterp.c | 2 +- src/backend/executor/execTuples.c | 20 +++++----- src/backend/jit/llvm/llvmjit_deform.c | 17 +-------- src/include/access/tupdesc.h | 2 +- src/include/access/tupmacs.h | 27 +++++++++++++ 12 files changed, 110 insertions(+), 80 deletions(-) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index c1847d1b8a4..d393e6ca33d 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -1596,7 +1596,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_nominal_alignby(ctx->offset, thisatt->attalignby); ctx->offset = att_addlength_pointer(ctx->offset, thisatt->attlen, tp + ctx->offset); if (ctx->tuphdr->t_hoff + ctx->offset > ctx->lp_len) @@ -1612,8 +1612,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_pointer_alignby(ctx->offset, thisatt->attalignby, -1, + tp + ctx->offset); /* Get the (possibly corrupt) varlena datum */ attdatum = fetchatt(thisatt, tp + ctx->offset); diff --git a/contrib/pageinspect/heapfuncs.c b/contrib/pageinspect/heapfuncs.c index 8c1b7d38aa6..41ff597199c 100644 --- a/contrib/pageinspect/heapfuncs.c +++ b/contrib/pageinspect/heapfuncs.c @@ -357,8 +357,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_pointer_alignby(off, attr->attalignby, -1, + tupdata + off); /* * As VARSIZE_ANY throws an exception if it can't properly @@ -376,7 +376,7 @@ tuple_data_split_internal(Oid relid, char *tupdata, } else { - off = att_align_nominal(off, attr->attalign); + off = att_nominal_alignby(off, attr->attalignby); len = attr->attlen; } diff --git a/src/backend/access/brin/brin_tuple.c b/src/backend/access/brin/brin_tuple.c index aae646be5dd..458784a35fb 100644 --- a/src/backend/access/brin/brin_tuple.c +++ b/src/backend/access/brin/brin_tuple.c @@ -703,13 +703,15 @@ brin_deconstruct_tuple(BrinDesc *brdesc, if (thisatt->attlen == -1) { - off = att_align_pointer(off, thisatt->attalign, -1, - tp + off); + off = att_pointer_alignby(off, + thisatt->attalignby, + -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_nominal_alignby */ + off = att_nominal_alignby(off, thisatt->attalignby); } values[stored++] = fetchatt(thisatt, tp + off); diff --git a/src/backend/access/common/attmap.c b/src/backend/access/common/attmap.c index 0805c4121ee..934397c9e58 100644 --- a/src/backend/access/common/attmap.c +++ b/src/backend/access/common/attmap.c @@ -315,13 +315,13 @@ check_attrmap_match(TupleDesc indesc, /* * If it's a dropped column and the corresponding input column is also - * dropped, we don't need a conversion. However, attlen and attalign - * must agree. + * dropped, we don't need a conversion. However, attlen and + * attalignby must agree. */ if (attrMap->attnums[i] == 0 && inatt->attisdropped && inatt->attlen == outatt->attlen && - inatt->attalign == outatt->attalign) + inatt->attalignby == outatt->attalignby) continue; return false; diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index 982e7222c49..c297a3bb9e7 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -251,13 +251,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_nominal_alignby(data_length, atti->attalignby); data_length += EOH_get_flat_size(DatumGetEOHP(val)); } else { - data_length = att_align_datum(data_length, atti->attalign, - atti->attlen, val); + data_length = att_datum_alignby(data_length, atti->attalignby, + atti->attlen, val); data_length = att_addlength_datum(data_length, atti->attlen, val); } @@ -308,13 +308,13 @@ fill_val(CompactAttribute *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_nominal_alignby macro on the pointer value itself, + * not on an offset. This is a bit of a hack. */ if (att->attbyval) { /* pass-by-value */ - data = (char *) att_align_nominal(data, att->attalign); + data = (char *) att_nominal_alignby(data, att->attalignby); store_att_byval(data, datum, att->attlen); data_length = att->attlen; } @@ -334,8 +334,7 @@ fill_val(CompactAttribute *att, */ ExpandedObjectHeader *eoh = DatumGetEOHP(datum); - data = (char *) att_align_nominal(data, - att->attalign); + data = (char *) att_nominal_alignby(data, att->attalignby); data_length = EOH_get_flat_size(eoh); EOH_flatten_into(eoh, data, data_length); } @@ -363,8 +362,7 @@ fill_val(CompactAttribute *att, else { /* full 4-byte header varlena */ - data = (char *) att_align_nominal(data, - att->attalign); + data = (char *) att_nominal_alignby(data, att->attalignby); data_length = VARSIZE(val); memcpy(data, val, data_length); } @@ -373,14 +371,14 @@ fill_val(CompactAttribute *att, { /* cstring ... never needs alignment */ *infomask |= HEAP_HASVARWIDTH; - Assert(att->attalign == TYPALIGN_CHAR); + Assert(att->attalignby == 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_nominal_alignby(data, att->attalignby); 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_nominal_alignby(off, att->attalignby); 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_nominal_alignby(off, att->attalignby)) att->attcacheoff = off; else { - off = att_align_pointer(off, att->attalign, -1, - tp + off); + off = att_pointer_alignby(off, att->attalignby, -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_nominal_alignby */ + off = att_nominal_alignby(off, att->attalignby); if (usecache) att->attcacheoff = off; @@ -898,10 +896,10 @@ expand_tuple(HeapTuple *targetHeapTuple, { CompactAttribute *att = TupleDescCompactAttr(tupleDesc, attnum); - targetDataLen = att_align_datum(targetDataLen, - att->attalign, - att->attlen, - attrmiss[attnum].am_value); + targetDataLen = att_datum_alignby(targetDataLen, + att->attalignby, + att->attlen, + attrmiss[attnum].am_value); targetDataLen = att_addlength_pointer(targetDataLen, att->attlen, @@ -1396,19 +1394,19 @@ heap_deform_tuple(HeapTuple tuple, TupleDesc tupleDesc, * an aligned or unaligned value. */ if (!slow && - off == att_align_nominal(off, thisatt->attalign)) + off == att_nominal_alignby(off, thisatt->attalignby)) thisatt->attcacheoff = off; else { - off = att_align_pointer(off, thisatt->attalign, -1, - tp + off); + off = att_pointer_alignby(off, thisatt->attalignby, -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_nominal_alignby */ + off = att_nominal_alignby(off, thisatt->attalignby); if (!slow) thisatt->attcacheoff = off; diff --git a/src/backend/access/common/indextuple.c b/src/backend/access/common/indextuple.c index 38aeb230879..a846b3d4a99 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_nominal_alignby(off, att->attalignby); 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_nominal_alignby(off, att->attalignby)) att->attcacheoff = off; else { - off = att_align_pointer(off, att->attalign, -1, - tp + off); + off = att_pointer_alignby(off, att->attalignby, -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_nominal_alignby */ + off = att_nominal_alignby(off, att->attalignby); 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_nominal_alignby(off, thisatt->attalignby)) thisatt->attcacheoff = off; else { - off = att_align_pointer(off, thisatt->attalign, -1, - tp + off); + off = att_pointer_alignby(off, thisatt->attalignby, -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_nominal_alignby */ + off = att_nominal_alignby(off, thisatt->attalignby); if (!slow) thisatt->attcacheoff = off; diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c index 1a8d6481a29..9fec6e33865 100644 --- a/src/backend/access/common/tupdesc.c +++ b/src/backend/access/common/tupdesc.c @@ -80,7 +80,25 @@ populate_compact_attribute(TupleDesc tupdesc, int attnum) dst->attgenerated = (src->attgenerated != '\0'); dst->attnotnull = src->attnotnull; - dst->attalign = src->attalign; + switch (src->attalign) + { + case TYPALIGN_INT: + dst->attalignby = ALIGNOF_INT; + break; + case TYPALIGN_CHAR: + dst->attalignby = sizeof(char); + break; + case TYPALIGN_DOUBLE: + dst->attalignby = ALIGNOF_DOUBLE; + break; + case TYPALIGN_SHORT: + dst->attalignby = ALIGNOF_SHORT; + break; + default: + dst->attalignby = 0; + elog(ERROR, "invalid attalign value: %c", src->attalign); + break; + } } /* diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 56e13d20a87..d2987663e63 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -5304,7 +5304,7 @@ ExecEvalWholeRowVar(ExprState *state, ExprEvalStep *op, ExprContext *econtext) if (slot->tts_isnull[i]) continue; /* null is always okay */ if (vattr->attlen != sattr->attlen || - vattr->attalign != sattr->attalign) + vattr->attalignby != sattr->attalignby) ereport(ERROR, (errcode(ERRCODE_DATATYPE_MISMATCH), errmsg("table row type and query-specified row type do not match"), diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 875515777b6..5d81c812673 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_nominal_alignby(sz, att->attalignby); sz += EOH_get_flat_size(DatumGetEOHP(val)); } else { - sz = att_align_nominal(sz, att->attalign); + sz = att_nominal_alignby(sz, att->attalignby); sz = att_addlength_datum(sz, att->attlen, val); } } @@ -242,8 +242,8 @@ tts_virtual_materialize(TupleTableSlot *slot) */ ExpandedObjectHeader *eoh = DatumGetEOHP(val); - data = (char *) att_align_nominal(data, - att->attalign); + data = (char *) att_nominal_alignby(data, + att->attalignby); data_length = EOH_get_flat_size(eoh); EOH_flatten_into(eoh, data, data_length); @@ -254,7 +254,7 @@ tts_virtual_materialize(TupleTableSlot *slot) { Size data_length = 0; - data = (char *) att_align_nominal(data, att->attalign); + data = (char *) att_nominal_alignby(data, att->attalignby); data_length = att_addlength_datum(data_length, att->attlen, val); memcpy(data, DatumGetPointer(val), data_length); @@ -1067,19 +1067,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_nominal_alignby(off, thisatt->attalignby)) thisatt->attcacheoff = off; else { - off = att_align_pointer(off, thisatt->attalign, -1, - tp + off); + off = att_pointer_alignby(off, thisatt->attalignby, -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_nominal_alignby */ + off = att_nominal_alignby(off, thisatt->attalignby); if (!slow) thisatt->attcacheoff = off; diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c index f49e7bce7d0..88ef2bb06ce 100644 --- a/src/backend/jit/llvm/llvmjit_deform.c +++ b/src/backend/jit/llvm/llvmjit_deform.c @@ -395,7 +395,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, { CompactAttribute *att = TupleDescCompactAttr(desc, attnum); LLVMValueRef v_incby; - int alignto; + int alignto = att->attalignby; LLVMValueRef l_attno = l_int16_const(lc, attnum); LLVMValueRef v_attdatap; LLVMValueRef v_resultp; @@ -494,21 +494,6 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, } LLVMPositionBuilderAtEnd(b, attcheckalignblocks[attnum]); - /* determine required alignment */ - if (att->attalign == TYPALIGN_INT) - alignto = ALIGNOF_INT; - else if (att->attalign == TYPALIGN_CHAR) - alignto = 1; - else if (att->attalign == TYPALIGN_DOUBLE) - alignto = ALIGNOF_DOUBLE; - else if (att->attalign == TYPALIGN_SHORT) - alignto = ALIGNOF_SHORT; - else - { - elog(ERROR, "unknown alignment"); - alignto = 0; - } - /* ------ * Even if alignment is required, we can skip doing it if provably * unnecessary: diff --git a/src/include/access/tupdesc.h b/src/include/access/tupdesc.h index 4406617fea6..e61a4affa46 100644 --- a/src/include/access/tupdesc.h +++ b/src/include/access/tupdesc.h @@ -75,7 +75,7 @@ typedef struct CompactAttribute bool attisdropped; /* as FormData_pg_attribute.attisdropped */ bool attgenerated; /* FormData_pg_attribute.attgenerated != '\0' */ bool attnotnull; /* as FormData_pg_attribute.attnotnull */ - char attalign; /* alignment requirement */ + uint8 attalignby; /* alignment requirement in bytes */ } CompactAttribute; /* diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h index fcf09ed95f4..0de67e3602a 100644 --- a/src/include/access/tupmacs.h +++ b/src/include/access/tupmacs.h @@ -91,6 +91,16 @@ fetch_att(const void *T, bool attbyval, int attlen) att_align_nominal(cur_offset, attalign) \ ) +/* + * Similar to att_align_datum, but accepts a number of bytes, typically from + * CompactAttribute.attalignby to align the Datum by. + */ +#define att_datum_alignby(cur_offset, attalignby, attlen, attdatum) \ + ( \ + ((attlen) == -1 && VARATT_IS_SHORT(DatumGetPointer(attdatum))) ? \ + (uintptr_t) (cur_offset) : \ + TYPEALIGN(attalignby, 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 @@ -112,6 +122,16 @@ fetch_att(const void *T, bool attbyval, int attlen) att_align_nominal(cur_offset, attalign) \ ) +/* + * Similar to att_align_pointer, but accepts a number of bytes, typically from + * CompactAttribute.attalignby to align the pointer by. + */ +#define att_pointer_alignby(cur_offset, attalignby, attlen, attptr) \ + ( \ + ((attlen) == -1 && VARATT_NOT_PAD_BYTE(attptr)) ? \ + (uintptr_t) (cur_offset) : \ + TYPEALIGN(attalignby, 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. @@ -138,6 +158,13 @@ fetch_att(const void *T, bool attbyval, int attlen) ))) \ ) +/* + * Similar to att_align_nominal, but accepts a number of bytes, typically from + * CompactAttribute.attalignby to align the offset by. + */ +#define att_nominal_alignby(cur_offset, attalignby) \ + TYPEALIGN(attalignby, 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