Re: [PATCHES] vacuum.c refactoring
Patch applied. Thanks.
---
Manfred Koizar wrote:
>. rename variables
> . cur_buffer -> dst_buffer
> . ToPage -> dst_page
> . cur_page -> dst_vacpage
>. move variable declarations into block where variable is used
>. various Asserts instead of elog(ERROR, ...)
>. extract functionality from repair_frag() into new routines
> . move_chain_tuple()
> . move_plain_tuple()
> . update_hint_bits()
>. create type ExecContext
>. add comments
>
> This patch does not intend to change any behaviour. It passes make
> check, make installcheck and some manual tests. It might be hard to
> review, because some lines are affected by more than one change. If
> it's too much to swallow at once, I can provide it in smaller chunks ...
>
> Servus
> Manfred
> diff -Ncr ../base/src/backend/commands/vacuum.c src/backend/commands/vacuum.c
> *** ../base/src/backend/commands/vacuum.c Mon May 31 21:24:05 2004
> --- src/backend/commands/vacuum.c Wed Jun 2 21:46:59 2004
> ***
> *** 99,104
> --- 99,162
> VTupleLink vtlinks;
> } VRelStats;
>
> + /*--
> + * ExecContext:
> + *
> + * As these variables always appear together, we put them into one struct
> + * and pull initialization and cleanup into separate routines.
> + * ExecContext is used by repair_frag() and move_xxx_tuple(). More
> + * accurately: It is *used* only in move_xxx_tuple(), but because this
> + * routine is called many times, we initialize the struct just once in
> + * repair_frag() and pass it on to move_xxx_tuple().
> + */
> + typedef struct ExecContextData
> + {
> + ResultRelInfo *resultRelInfo;
> + EState *estate;
> + TupleTable tupleTable;
> + TupleTableSlot *slot;
> + } ExecContextData;
> + typedef ExecContextData *ExecContext;
> +
> + static void
> + ExecContext_Init(ExecContext ec, Relation rel)
> + {
> + TupleDesc tupdesc = RelationGetDescr(rel);
> +
> + /*
> + * We need a ResultRelInfo and an EState so we can use the regular
> + * executor's index-entry-making machinery.
> + */
> + ec->estate = CreateExecutorState();
> +
> + ec->resultRelInfo = makeNode(ResultRelInfo);
> + ec->resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
> + ec->resultRelInfo->ri_RelationDesc = rel;
> + ec->resultRelInfo->ri_TrigDesc = NULL; /* we don't fire triggers */
> +
> + ExecOpenIndices(ec->resultRelInfo);
> +
> + ec->estate->es_result_relations = ec->resultRelInfo;
> + ec->estate->es_num_result_relations = 1;
> + ec->estate->es_result_relation_info = ec->resultRelInfo;
> +
> + /* Set up a dummy tuple table too */
> + ec->tupleTable = ExecCreateTupleTable(1);
> + ec->slot = ExecAllocTableSlot(ec->tupleTable);
> + ExecSetSlotDescriptor(ec->slot, tupdesc, false);
> + }
> +
> + static void
> + ExecContext_Finish(ExecContext ec)
> + {
> + ExecDropTupleTable(ec->tupleTable, true);
> + ExecCloseIndices(ec->resultRelInfo);
> + FreeExecutorState(ec->estate);
> + }
> + /*
> + * End of ExecContext Implementation
> + *--
> + */
>
> static MemoryContext vac_context = NULL;
>
> ***
> *** 122,127
> --- 180,196
> static void repair_frag(VRelStats *vacrelstats, Relation onerel,
> VacPageList vacuum_pages, VacPageList fraged_pages,
> int nindexes, Relation *Irel);
> + static void move_chain_tuple(Relation rel,
> + Buffer old_buf, Page old_page, HeapTuple
> old_tup,
> + Buffer dst_buf, Page dst_page, VacPage
> dst_vacpage,
> + ExecContext ec, ItemPointer ctid, bool
> cleanVpd);
> + static void move_plain_tuple(Relation rel,
> + Buffer old_buf, Page old_page, HeapTuple
> old_tup,
> + Buffer dst_buf, Page dst_page, VacPage
> dst_vacpage,
> + ExecContext ec);
> + static void update_hint_bits(Relation rel, VacPageList fraged_pages,
> + int num_fraged_pages, BlockNumber
> last_move_dest_block,
> + int num_moved);
> static void vacuum_heap(VRelStats *vacrelstats, Relation onerel,
> VacPageList vacpagelist);
> static void vacuum_page(Relation onerel, Buffer buffer, VacPage vacpage);
> ***
> *** 675,681
> static void
> vac_truncate_clog(TransactionId vacuumXID, TransactionId frozenXID)
> {
> ! TransactionId myXID;
> Relationrelation;
> HeapScanDesc scan;
> HeapTuple tuple;
> --- 744,750
>
[PATCHES] vacuum.c refactoring
. rename variables
. cur_buffer -> dst_buffer
. ToPage -> dst_page
. cur_page -> dst_vacpage
. move variable declarations into block where variable is used
. various Asserts instead of elog(ERROR, ...)
. extract functionality from repair_frag() into new routines
. move_chain_tuple()
. move_plain_tuple()
. update_hint_bits()
. create type ExecContext
. add comments
This patch does not intend to change any behaviour. It passes make
check, make installcheck and some manual tests. It might be hard to
review, because some lines are affected by more than one change. If
it's too much to swallow at once, I can provide it in smaller chunks ...
Servus
Manfred
diff -Ncr ../base/src/backend/commands/vacuum.c src/backend/commands/vacuum.c
*** ../base/src/backend/commands/vacuum.c Mon May 31 21:24:05 2004
--- src/backend/commands/vacuum.c Wed Jun 2 21:46:59 2004
***
*** 99,104
--- 99,162
VTupleLink vtlinks;
} VRelStats;
+ /*--
+ * ExecContext:
+ *
+ * As these variables always appear together, we put them into one struct
+ * and pull initialization and cleanup into separate routines.
+ * ExecContext is used by repair_frag() and move_xxx_tuple(). More
+ * accurately: It is *used* only in move_xxx_tuple(), but because this
+ * routine is called many times, we initialize the struct just once in
+ * repair_frag() and pass it on to move_xxx_tuple().
+ */
+ typedef struct ExecContextData
+ {
+ ResultRelInfo *resultRelInfo;
+ EState *estate;
+ TupleTable tupleTable;
+ TupleTableSlot *slot;
+ } ExecContextData;
+ typedef ExecContextData *ExecContext;
+
+ static void
+ ExecContext_Init(ExecContext ec, Relation rel)
+ {
+ TupleDesc tupdesc = RelationGetDescr(rel);
+
+ /*
+* We need a ResultRelInfo and an EState so we can use the regular
+* executor's index-entry-making machinery.
+*/
+ ec->estate = CreateExecutorState();
+
+ ec->resultRelInfo = makeNode(ResultRelInfo);
+ ec->resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
+ ec->resultRelInfo->ri_RelationDesc = rel;
+ ec->resultRelInfo->ri_TrigDesc = NULL; /* we don't fire triggers */
+
+ ExecOpenIndices(ec->resultRelInfo);
+
+ ec->estate->es_result_relations = ec->resultRelInfo;
+ ec->estate->es_num_result_relations = 1;
+ ec->estate->es_result_relation_info = ec->resultRelInfo;
+
+ /* Set up a dummy tuple table too */
+ ec->tupleTable = ExecCreateTupleTable(1);
+ ec->slot = ExecAllocTableSlot(ec->tupleTable);
+ ExecSetSlotDescriptor(ec->slot, tupdesc, false);
+ }
+
+ static void
+ ExecContext_Finish(ExecContext ec)
+ {
+ ExecDropTupleTable(ec->tupleTable, true);
+ ExecCloseIndices(ec->resultRelInfo);
+ FreeExecutorState(ec->estate);
+ }
+ /*
+ * End of ExecContext Implementation
+ *--
+ */
static MemoryContext vac_context = NULL;
***
*** 122,127
--- 180,196
static void repair_frag(VRelStats *vacrelstats, Relation onerel,
VacPageList vacuum_pages, VacPageList fraged_pages,
int nindexes, Relation *Irel);
+ static void move_chain_tuple(Relation rel,
+Buffer old_buf, Page old_page, HeapTuple
old_tup,
+Buffer dst_buf, Page dst_page, VacPage
dst_vacpage,
+ExecContext ec, ItemPointer ctid, bool
cleanVpd);
+ static void move_plain_tuple(Relation rel,
+Buffer old_buf, Page old_page, HeapTuple
old_tup,
+Buffer dst_buf, Page dst_page, VacPage
dst_vacpage,
+ExecContext ec);
+ static void update_hint_bits(Relation rel, VacPageList fraged_pages,
+ int num_fraged_pages, BlockNumber
last_move_dest_block,
+ int num_moved);
static void vacuum_heap(VRelStats *vacrelstats, Relation onerel,
VacPageList vacpagelist);
static void vacuum_page(Relation onerel, Buffer buffer, VacPage vacpage);
***
*** 675,681
static void
vac_truncate_clog(TransactionId vacuumXID, TransactionId frozenXID)
{
! TransactionId myXID;
Relationrelation;
HeapScanDesc scan;
HeapTuple tuple;
--- 744,750
static void
vac_truncate_clog(TransactionId vacuumXID, TransactionId frozenXID)
{
! TransactionId myXID = GetCurrentTransactionId();
Relationrelation;
HeapScanDesc scan;
HeapTuple tuple;
***
*** 683,689
boolvac
