From 4496610ce2f06849be75a7582cf767635c8346f2 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Thu, 26 Dec 2024 18:47:32 +1300 Subject: [PATCH v9 2/3] Add special case tuple deform code for no-varwidth tuples If the tuple flags state that a tuple has no variable width columns then there's no point in adding a check for attlen == -1. We can omit that and save a branch when deforming tuples from such tables. --- src/backend/executor/execTuples.c | 96 ++++++++++++++++++++++--------- src/include/access/tupmacs.h | 11 ++++ 2 files changed, 81 insertions(+), 26 deletions(-) diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index a57d82b049..2bb7b6fc15 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -994,11 +994,12 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, * slot_deform_heap_tuple_internal * An always inline helper function for use in slot_deform_heap_tuple to * allow the compiler to emit specialized versions of this function for - * various combinations of "slow" and "hasnulls". For example, if a - * given tuple has no nulls, then we needn't check "hasnulls" for every - * attribute that we're deforming. The caller can just call this - * function with hasnulls set to constant-false and have the compiler - * remove the constant-false branches and emit more optimal code. + * various combinations of "slow", "hasnulls" and "hasvarwidth". For + * example, if a given tuple has no nulls, then we needn't check + * "hasnulls" for every attribute that we're deforming. The caller can + * just call this function with hasnulls set to constant-false and have + * the compiler remove the constant-false branches and emit more optimal + * code. * * Returns the next attnum to deform, which can be equal to natts when the * function manages to deform all requested attributes. *offp is an input and @@ -1008,6 +1009,11 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, * of this tuple must use a version of this function with "slow" passed as * true. * + * Callers can pass "hasvarwidth" as true even if the tuple has no variable + * width columns, however, callers *must not* pass it as false if there are + * variable width columns. The compiler can omit the attlen == -1 checks when + * it's known there are no variable-width columns. + * * Callers cannot assume when we return "attnum" (i.e. all requested * attributes have been deformed) that slow mode isn't required for any * additional deforming as the final attribute may have caused a switch to @@ -1016,7 +1022,8 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, static pg_attribute_always_inline int slot_deform_heap_tuple_internal(TupleTableSlot *slot, HeapTuple tuple, int attnum, int natts, bool slow, - bool hasnulls, uint32 *offp, bool *slowp) + bool hasnulls, bool hasvarwidth, uint32 *offp, + bool *slowp) { TupleDesc tupleDesc = slot->tts_tupleDescriptor; Datum *values = slot->tts_values; @@ -1050,7 +1057,7 @@ slot_deform_heap_tuple_internal(TupleTableSlot *slot, HeapTuple tuple, /* calculate the offset of this attribute */ if (!slow && thisatt->attcacheoff >= 0) *offp = thisatt->attcacheoff; - else if (thisatt->attlen == -1) + else if (hasvarwidth && thisatt->attlen == -1) { /* * We can only cache the offset for a varlena attribute if the @@ -1082,10 +1089,13 @@ slot_deform_heap_tuple_internal(TupleTableSlot *slot, HeapTuple tuple, values[attnum] = fetchatt(thisatt, tp + *offp); - *offp = att_addlength_pointer(*offp, thisatt->attlen, tp + *offp); + if (hasvarwidth) + *offp = att_addlength_pointer(*offp, thisatt->attlen, tp + *offp); + else + *offp = att_addlength_pointer_byval(*offp, thisatt->attlen); /* check if we need to switch to slow mode */ - if (!slow) + if (!slow && hasvarwidth) { /* * We're unable to deform any further if the above code set @@ -1121,6 +1131,7 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, int natts) { bool hasnulls = HeapTupleHasNulls(tuple); + bool hasvarwidth = HeapTupleHasVarWidth(tuple); int attnum; uint32 off; /* offset in tuple data */ bool slow; /* can we use/set attcacheoff? */ @@ -1161,35 +1172,68 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp, { /* Tuple without any NULLs? We can skip doing any NULL checking */ if (!hasnulls) - attnum = slot_deform_heap_tuple_internal(slot, - tuple, - attnum, - natts, - false, /* slow */ - false, /* hasnulls */ - &off, - &slow); + { + if (!hasvarwidth) + attnum = slot_deform_heap_tuple_internal(slot, + tuple, + attnum, + natts, + false, /* slow */ + false, /* hasnulls */ + false, /* hasvarwidth */ + &off, + &slow); + + else + attnum = slot_deform_heap_tuple_internal(slot, + tuple, + attnum, + natts, + false, /* slow */ + false, /* hasnulls */ + true, /* hasvarwidth */ + &off, + &slow); + } else - attnum = slot_deform_heap_tuple_internal(slot, - tuple, - attnum, - natts, - false, /* slow */ - true, /* hasnulls */ - &off, - &slow); + { + if (!hasvarwidth) + attnum = slot_deform_heap_tuple_internal(slot, + tuple, + attnum, + natts, + false, /* slow */ + true, /* hasnulls */ + false, /* hasvarwidth */ + &off, + &slow); + else + attnum = slot_deform_heap_tuple_internal(slot, + tuple, + attnum, + natts, + false, /* slow */ + true, /* hasnulls */ + true, /* hasvarwidth */ + &off, + &slow); + } } /* If there's still work to do then we must be in slow mode */ if (attnum < natts) { - /* XXX is it worth adding a separate call when hasnulls is false? */ + /* + * Pass hasvarwidth as true so we don't have to inline another version + * of the function. + */ attnum = slot_deform_heap_tuple_internal(slot, tuple, attnum, natts, true, /* slow */ hasnulls, + true, /* hasvarwidth */ &off, &slow); } diff --git a/src/include/access/tupmacs.h b/src/include/access/tupmacs.h index 0de67e3602..c4ab2cd3ee 100644 --- a/src/include/access/tupmacs.h +++ b/src/include/access/tupmacs.h @@ -199,6 +199,17 @@ fetch_att(const void *T, bool attbyval, int attlen) )) \ ) +/* + * att_addlength_pointer_byval, as att_addlength_pointer but for byval + * attribute types *only*. It's not valid to use this macro for + * variable-width types. + */ +#define att_addlength_pointer_byval(cur_offset, attlen) \ +( \ + AssertMacro((attlen) > 0), \ + (cur_offset) + (attlen) \ +) + #ifndef FRONTEND /* * store_att_byval is a partial inverse of fetch_att: store a given Datum -- 2.34.1