From 83ea86a095bdf6f5c829299906e9f5cfdee9e2b4 Mon Sep 17 00:00:00 2001 From: "yizhi.fzh" Date: Sun, 25 Feb 2024 17:25:36 +0800 Subject: [PATCH v8 4/5] Adding tts_value_mctx to TupleTableSlot This context is used to hold the detoast datum for now. With this change, we don't need to iterate pre_detoast_attrs and pfree each of them any more during ExecClearTuple, instead we just MemoryContextReset, which will be more effective. However the cost is we adds 1KB MemoryContext to each TupleTableSlot unconditionally. As of now, slot->pre_detoast_attrs is still needed in the ExecCopySlot case. --- src/backend/executor/execExprInterp.c | 2 +- src/backend/executor/execTuples.c | 50 ++++++++++++++++----------- src/backend/nodes/bitmapset.c | 9 ----- src/include/executor/tuptable.h | 17 +++++---- src/include/nodes/bitmapset.h | 13 ++++++- 5 files changed, 53 insertions(+), 38 deletions(-) diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 878235ae76..13abe54c0b 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -195,7 +195,7 @@ ExecSlotDetoastDatum(TupleTableSlot *slot, int attnum) VARATT_IS_EXTENDED(slot->tts_values[attnum])) { Datum oldDatum; - MemoryContext old = MemoryContextSwitchTo(slot->tts_mcxt); + MemoryContext old = MemoryContextSwitchTo(slot->tts_values_mctx); oldDatum = slot->tts_values[attnum]; slot->tts_values[attnum] = PointerGetDatum(detoast_attr( diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c index 830e1d4ea6..3a56d1c55d 100644 --- a/src/backend/executor/execTuples.c +++ b/src/backend/executor/execTuples.c @@ -375,6 +375,15 @@ tts_heap_materialize(TupleTableSlot *slot) oldContext = MemoryContextSwitchTo(slot->tts_mcxt); + /* + * tts_values is treated invalidated since tts_nvalid will is set to 0, + * so let's free the pre-detoast datum. + * + * call ExecFreePreDetoastDatum before tts_nvalid is set to 0 is for the + * fast path of ExecFreePreDetoastDatum. + */ + ExecFreePreDetoastDatum(slot); + /* * Have to deform from scratch, otherwise tts_values[] entries could point * into the non-materialized tuple (which might be gone when accessed). @@ -399,13 +408,6 @@ tts_heap_materialize(TupleTableSlot *slot) slot->tts_flags |= TTS_FLAG_SHOULDFREE; MemoryContextSwitchTo(oldContext); - - /* - * tts_values is treated invalidated since tts_nvalid is set to 0, so - * let's free the pre-detoast datum. - */ - ExecFreePreDetoastDatum(slot); - } static void @@ -463,6 +465,9 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree) tts_heap_clear(slot); + /* slot_nvalid = 0 */ + ExecFreePreDetoastDatum(slot); + slot->tts_nvalid = 0; hslot->tuple = tuple; hslot->off = 0; @@ -472,8 +477,6 @@ tts_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, bool shouldFree) if (shouldFree) slot->tts_flags |= TTS_FLAG_SHOULDFREE; - /* slot_nvalid = 0 */ - ExecFreePreDetoastDatum(slot); } @@ -552,6 +555,10 @@ tts_minimal_materialize(TupleTableSlot *slot) oldContext = MemoryContextSwitchTo(slot->tts_mcxt); + + /* slot_nvalid = 0 */ + ExecFreePreDetoastDatum(slot); + /* * Have to deform from scratch, otherwise tts_values[] entries could point * into the non-materialized tuple (which might be gone when accessed). @@ -584,9 +591,6 @@ tts_minimal_materialize(TupleTableSlot *slot) mslot->minhdr.t_data = (HeapTupleHeader) ((char *) mslot->mintuple - MINIMAL_TUPLE_OFFSET); MemoryContextSwitchTo(oldContext); - - /* slot_nvalid = 0 */ - ExecFreePreDetoastDatum(slot); } static void @@ -646,6 +650,9 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree Assert(TTS_EMPTY(slot)); slot->tts_flags &= ~TTS_FLAG_EMPTY; + + /* tts_nvalid = 0 */ + ExecFreePreDetoastDatum(slot); slot->tts_nvalid = 0; mslot->off = 0; @@ -658,8 +665,6 @@ tts_minimal_store_tuple(TupleTableSlot *slot, MinimalTuple mtup, bool shouldFree if (shouldFree) slot->tts_flags |= TTS_FLAG_SHOULDFREE; - /* tts_nvalid = 0 */ - ExecFreePreDetoastDatum(slot); } @@ -756,6 +761,10 @@ tts_buffer_heap_materialize(TupleTableSlot *slot) * into the non-materialized tuple (which might be gone when accessed). */ bslot->base.off = 0; + + /* slot_nvalid = 0 */ + ExecFreePreDetoastDatum(slot); + slot->tts_nvalid = 0; if (!bslot->base.tuple) @@ -794,9 +803,6 @@ tts_buffer_heap_materialize(TupleTableSlot *slot) slot->tts_flags |= TTS_FLAG_SHOULDFREE; MemoryContextSwitchTo(oldContext); - - /* slot_nvalid = 0 */ - ExecFreePreDetoastDatum(slot); } static void @@ -896,6 +902,10 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, } slot->tts_flags &= ~TTS_FLAG_EMPTY; + + /* tts_nvalid = 0 */ + ExecFreePreDetoastDatum(slot); + slot->tts_nvalid = 0; bslot->base.tuple = tuple; bslot->base.off = 0; @@ -930,9 +940,6 @@ tts_buffer_heap_store_tuple(TupleTableSlot *slot, HeapTuple tuple, */ ReleaseBuffer(buffer); } - - /* tts_nvalid = 0 */ - ExecFreePreDetoastDatum(slot); } /* @@ -1166,6 +1173,9 @@ MakeTupleTableSlot(TupleDesc tupleDesc, slot->tts_flags |= TTS_FLAG_FIXED; slot->tts_tupleDescriptor = tupleDesc; slot->tts_mcxt = CurrentMemoryContext; + slot->tts_values_mctx = GenerationContextCreate(slot->tts_mcxt, + "tts_value_ctx", + ALLOCSET_START_SMALL_SIZES); slot->tts_nvalid = 0; if (tupleDesc != NULL) diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c index 40cfea2308..3e1ccfc8dd 100644 --- a/src/backend/nodes/bitmapset.c +++ b/src/backend/nodes/bitmapset.c @@ -1485,15 +1485,6 @@ bitset_init(size_t size) return result; } -/* - * bitset_clear - clear the bits only, but the memory is still there. - */ -void -bitset_clear(Bitset *a) -{ - if (a != NULL) - memset(a->words, 0, sizeof(bitmapword) * a->nwords); -} void bitset_free(Bitset *a) diff --git a/src/include/executor/tuptable.h b/src/include/executor/tuptable.h index 8f3eba7fbb..53782a993f 100644 --- a/src/include/executor/tuptable.h +++ b/src/include/executor/tuptable.h @@ -20,6 +20,7 @@ #include "access/tupdesc.h" #include "nodes/bitmapset.h" #include "storage/buf.h" +#include "utils/memutils.h" /*---------- * The executor stores tuples in a "tuple table" which is a List of @@ -127,6 +128,7 @@ typedef struct TupleTableSlot #define FIELDNO_TUPLETABLESLOT_ISNULL 6 bool *tts_isnull; /* current per-attribute isnull flags */ MemoryContext tts_mcxt; /* slot itself is in this context */ + MemoryContext tts_values_mctx; /* reset when tts_values[*] is invalidated. */ ItemPointerData tts_tid; /* stored tuple's tid */ Oid tts_tableOid; /* table oid of tuple */ @@ -452,14 +454,11 @@ slot_getsysattr(TupleTableSlot *slot, int attnum, bool *isnull) static inline void ExecFreePreDetoastDatum(TupleTableSlot *slot) { - int attnum; - - attnum = -1; - while ((attnum = bitset_next_member(slot->pre_detoasted_attrs, attnum)) >= 0) - { - pfree((void *) slot->tts_values[attnum]); - } + /* We have called this when tts_nvalid is set to 0. */ + if (slot->tts_nvalid == 0) + return; + MemoryContextResetOnly(slot->tts_values_mctx); bitset_clear(slot->pre_detoasted_attrs); } @@ -545,6 +544,10 @@ ExecCopySlot(TupleTableSlot *dstslot, TupleTableSlot *srcslot) dstslot->pre_detoasted_attrs = bitset_copy(srcslot->pre_detoasted_attrs); + MemoryContextSwitchTo(old); + + old = MemoryContextSwitchTo(dstslot->tts_values_mctx); + while ((attnum = bitset_next_member(dstslot->pre_detoasted_attrs, attnum)) >= 0) { struct varlena *datum = (struct varlena *) srcslot->tts_values[attnum]; diff --git a/src/include/nodes/bitmapset.h b/src/include/nodes/bitmapset.h index 95ff37c6e9..e433842217 100644 --- a/src/include/nodes/bitmapset.h +++ b/src/include/nodes/bitmapset.h @@ -143,7 +143,6 @@ extern uint32 bitmap_hash(const void *key, Size keysize); extern int bitmap_match(const void *key1, const void *key2, Size keysize); extern Bitset *bitset_init(size_t size); -extern void bitset_clear(Bitset *a); extern void bitset_free(Bitset *a); extern bool bitset_is_empty(Bitset *a); extern Bitset *bitset_copy(Bitset *a); @@ -152,4 +151,16 @@ extern void bitset_del_member(Bitset *a, int x); extern int bitset_is_member(int bit, Bitset *a); extern int bitset_next_member(const Bitset *a, int prevbit); extern Bitmapset *bitset_to_bitmap(Bitset *a); + +/* + * bitset_clear - clear the bits only, but the memory is still there. + * this is used in performance critical path, so inline this one specially. + */ +inline static void +bitset_clear(Bitset *a) +{ + if (a != NULL) + memset(a->words, 0, sizeof(bitmapword) * a->nwords); +} + #endif /* BITMAPSET_H */ -- 2.34.1