From 78de819e4c19984d9ad124810da4682890ba796a Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Sun, 9 Feb 2020 15:08:14 -0600 Subject: [PATCH v7 5/9] Refactor for consistency/symmetry This moves hash instrumentation out of execGrouping.c / TupleHashTable and into higher level nodes, for consistency with bitmapHeapScan. This might be unimportant and maybe clearer left in execGrouping.c. --- src/backend/commands/explain.c | 18 +++++++-------- src/backend/executor/execGrouping.c | 28 ----------------------- src/backend/executor/nodeAgg.c | 14 ++++++++---- src/backend/executor/nodeRecursiveunion.c | 3 ++- src/backend/executor/nodeSetOp.c | 6 ++++- src/backend/executor/nodeSubplan.c | 10 ++++++-- src/include/executor/executor.h | 1 - src/include/executor/nodeAgg.h | 1 + src/include/nodes/execnodes.h | 24 +++++++++++++++++-- 9 files changed, 57 insertions(+), 48 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index e0dc5a092a..cef19c684b 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -1347,13 +1347,13 @@ ExplainNode(PlanState *planstate, List *ancestors, { ExplainIndentText(es); appendStringInfoString(es->str, "Hashtable: "); - show_tuplehash_info(&subplanstate->hashtable->instrument, es); + show_tuplehash_info(&subplanstate->instrument, es); } if (subplanstate->hashnulls) { ExplainIndentText(es); appendStringInfoString(es->str, "Null Hashtable: "); - show_tuplehash_info(&subplanstate->hashnulls->instrument, es); + show_tuplehash_info(&subplanstate->instrument_nulls, es); } } if (es->indent) @@ -1387,14 +1387,14 @@ ExplainNode(PlanState *planstate, List *ancestors, if (subplanstate && subplanstate->hashtable) { ExplainOpenGroup("Hashtable", "Hashtable", true, es); - show_tuplehash_info(&subplanstate->hashtable->instrument, es); + show_tuplehash_info(&subplanstate->instrument, es); ExplainCloseGroup("Hashtable", "Hashtable", true, es); } if (subplanstate && subplanstate->hashnulls) { ExplainOpenGroup("Null Hashtable", "Null Hashtable", true, es); - show_tuplehash_info(&subplanstate->hashnulls->instrument, es); + show_tuplehash_info(&subplanstate->instrument_nulls, es); ExplainCloseGroup("Null Hashtable", "Null Hashtable", true, es); } } @@ -1925,14 +1925,14 @@ ExplainNode(PlanState *planstate, List *ancestors, { SetOpState *sos = castNode(SetOpState, planstate); if (sos->hashtable) - show_tuplehash_info(&sos->hashtable->instrument, es); + show_tuplehash_info(&sos->instrument, es); } break; case T_RecursiveUnion: { RecursiveUnionState *rus = (RecursiveUnionState *)planstate; if (rus->hashtable) - show_tuplehash_info(&rus->hashtable->instrument, es); + show_tuplehash_info(&rus->instrument, es); break; } case T_Group: @@ -2320,7 +2320,7 @@ show_agg_keys(AggState *astate, List *ancestors, ancestors, es); Assert(astate->num_hashes <= 1); if (astate->num_hashes) - show_tuplehash_info(&astate->perhash[0].hashtable->instrument, es); + show_tuplehash_info(&astate->perhash[0].instrument, es); } ancestors = list_delete_first(ancestors); @@ -2347,7 +2347,7 @@ show_grouping_sets(AggState *aggstate, Agg *agg, show_grouping_set_info(aggstate, agg, NULL, context, useprefix, ancestors, aggstate->num_hashes ? - &aggstate->perhash[setno++].hashtable->instrument : NULL, + &aggstate->perhash[setno++].instrument : NULL, es); foreach(lc, agg->chain) @@ -2360,7 +2360,7 @@ show_grouping_sets(AggState *aggstate, Agg *agg, aggnode->aggstrategy == AGG_MIXED) { Assert(setno < aggstate->num_hashes); - inst = &aggstate->perhash[setno++].hashtable->instrument; + inst = &aggstate->perhash[setno++].instrument; } show_grouping_set_info(aggstate, aggnode, sortnode, diff --git a/src/backend/executor/execGrouping.c b/src/backend/executor/execGrouping.c index 844dd3ba86..009d27b9a8 100644 --- a/src/backend/executor/execGrouping.c +++ b/src/backend/executor/execGrouping.c @@ -188,7 +188,6 @@ BuildTupleHashTableExt(PlanState *parent, hashtable->inputslot = NULL; hashtable->in_hash_funcs = NULL; hashtable->cur_eq_func = NULL; - memset(&hashtable->instrument, 0, sizeof(hashtable->instrument)); /* * If parallelism is in use, even if the master backend is performing the @@ -204,7 +203,6 @@ BuildTupleHashTableExt(PlanState *parent, hashtable->hash_iv = 0; hashtable->hashtab = tuplehash_create(metacxt, nbuckets, hashtable); - UpdateTupleHashTableStats(hashtable, true); /* * We copy the input tuple descriptor just for safety --- we assume all @@ -283,35 +281,9 @@ BuildTupleHashTable(PlanState *parent, void ResetTupleHashTable(TupleHashTable hashtable) { - UpdateTupleHashTableStats(hashtable, false); tuplehash_reset(hashtable->hashtab); } -/* Update instrumentation stats */ -void -UpdateTupleHashTableStats(TupleHashTable hashtable, bool initial) -{ - hashtable->instrument.nbuckets = hashtable->hashtab->size; - if (initial) - { - hashtable->instrument.nbuckets_original = hashtable->hashtab->size; - hashtable->instrument.space_peak_hash = hashtable->hashtab->size * - sizeof(TupleHashEntryData); - hashtable->instrument.space_peak_tuples = 0; - } - else - { - /* hashtable->entrysize includes additionalsize */ - hashtable->instrument.space_peak_hash = Max( - hashtable->instrument.space_peak_hash, - hashtable->hashtab->size * sizeof(TupleHashEntryData)); - - hashtable->instrument.space_peak_tuples = Max( - hashtable->instrument.space_peak_tuples, - hashtable->hashtab->members * hashtable->entrysize); - } -} - /* * Find or create a hashtable entry for the tuple group containing the * given tuple. The tuple must be the same type as the hashtable entries. diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c index e5aa0629a7..867da6eebf 100644 --- a/src/backend/executor/nodeAgg.c +++ b/src/backend/executor/nodeAgg.c @@ -1334,6 +1334,9 @@ build_hash_table(AggState *aggstate, int setno, long nbuckets) hashcxt, tmpcxt, DO_AGGSPLIT_SKIPFINAL(aggstate->aggsplit)); + + InitTupleHashTableStats(perhash->instrument, + perhash->hashtable->hashtab, additionalsize); } /* @@ -1710,9 +1713,10 @@ agg_retrieve_direct(AggState *aggstate) */ initialize_phase(aggstate, 0); aggstate->table_filled = true; - UpdateTupleHashTableStats(aggstate->perhash[0].hashtable, false); + UpdateTupleHashTableStats(aggstate->perhash[0].instrument, + aggstate->perhash[0].hashtable->hashtab); ResetTupleHashIterator(aggstate->perhash[0].hashtable, - &aggstate->perhash[0].hashiter); + &aggstate->perhash[0].hashiter); select_current_set(aggstate, 0, true); return agg_retrieve_hash_table(aggstate); } @@ -1913,7 +1917,8 @@ agg_retrieve_direct(AggState *aggstate) aggstate->current_phase == 1) { for (int i = 0; i < aggstate->num_hashes; i++) - UpdateTupleHashTableStats(aggstate->perhash[i].hashtable, false); + UpdateTupleHashTableStats(aggstate->perhash[i].instrument, + aggstate->perhash[i].hashtable->hashtab); } } @@ -1990,7 +1995,8 @@ agg_fill_hash_table(AggState *aggstate) aggstate->table_filled = true; for (int i = 0; i < aggstate->num_hashes; i++) - UpdateTupleHashTableStats(aggstate->perhash[i].hashtable, false); + UpdateTupleHashTableStats(aggstate->perhash[i].instrument, + aggstate->perhash[i].hashtable->hashtab); /* Initialize to walk the first hash table */ select_current_set(aggstate, 0, true); diff --git a/src/backend/executor/nodeRecursiveunion.c b/src/backend/executor/nodeRecursiveunion.c index 93272c28b1..594abdbdf0 100644 --- a/src/backend/executor/nodeRecursiveunion.c +++ b/src/backend/executor/nodeRecursiveunion.c @@ -50,6 +50,7 @@ build_hash_table(RecursiveUnionState *rustate) rustate->tableContext, rustate->tempContext, false); + InitTupleHashTableStats(rustate->instrument, rustate->hashtable->hashtab, 0); } @@ -157,7 +158,7 @@ ExecRecursiveUnion(PlanState *pstate) } if (node->hashtable) - UpdateTupleHashTableStats(node->hashtable, false); + UpdateTupleHashTableStats(node->instrument, node->hashtable->hashtab); return NULL; } diff --git a/src/backend/executor/nodeSetOp.c b/src/backend/executor/nodeSetOp.c index 9c0e0ab96e..a3860758eb 100644 --- a/src/backend/executor/nodeSetOp.c +++ b/src/backend/executor/nodeSetOp.c @@ -139,6 +139,9 @@ build_hash_table(SetOpState *setopstate) setopstate->tableContext, econtext->ecxt_per_tuple_memory, false); + + InitTupleHashTableStats(setopstate->instrument, + setopstate->hashtable->hashtab, 0); } /* @@ -415,7 +418,8 @@ setop_fill_hash_table(SetOpState *setopstate) setopstate->table_filled = true; /* Initialize to walk the hash table */ - UpdateTupleHashTableStats(setopstate->hashtable, false); + UpdateTupleHashTableStats(setopstate->instrument, + setopstate->hashtable->hashtab); ResetTupleHashIterator(setopstate->hashtable, &setopstate->hashiter); } diff --git a/src/backend/executor/nodeSubplan.c b/src/backend/executor/nodeSubplan.c index 22c32612ba..03533538f6 100644 --- a/src/backend/executor/nodeSubplan.c +++ b/src/backend/executor/nodeSubplan.c @@ -505,6 +505,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) if (node->hashtable) ResetTupleHashTable(node->hashtable); else + { node->hashtable = BuildTupleHashTableExt(node->parent, node->descRight, ncols, @@ -518,6 +519,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) node->hashtablecxt, node->hashtempcxt, false); + InitTupleHashTableStats(node->instrument, node->hashtable->hashtab, 0); + } if (!subplan->unknownEqFalse) { @@ -533,6 +536,7 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) if (node->hashnulls) ResetTupleHashTable(node->hashnulls); else + { node->hashnulls = BuildTupleHashTableExt(node->parent, node->descRight, ncols, @@ -546,6 +550,8 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) node->hashtablecxt, node->hashtempcxt, false); + InitTupleHashTableStats(node->instrument_nulls, node->hashnulls->hashtab, 0); + } } else node->hashnulls = NULL; @@ -621,9 +627,9 @@ buildSubPlanHash(SubPlanState *node, ExprContext *econtext) ExecClearTuple(node->projRight->pi_state.resultslot); MemoryContextSwitchTo(oldcontext); - UpdateTupleHashTableStats(node->hashtable, false); + UpdateTupleHashTableStats(node->instrument, node->hashtable->hashtab); if (node->hashnulls) - UpdateTupleHashTableStats(node->hashnulls, false); + UpdateTupleHashTableStats(node->instrument_nulls, node->hashnulls->hashtab); } /* diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h index f4f2ede207..94890512dc 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -150,7 +150,6 @@ extern TupleHashEntry FindTupleHashEntry(TupleHashTable hashtable, ExprState *eqcomp, FmgrInfo *hashfunctions); extern void ResetTupleHashTable(TupleHashTable hashtable); -extern void UpdateTupleHashTableStats(TupleHashTable hashtable, bool initial); /* * prototypes from functions in execJunk.c diff --git a/src/include/executor/nodeAgg.h b/src/include/executor/nodeAgg.h index 264916f9a9..2072c18b54 100644 --- a/src/include/executor/nodeAgg.h +++ b/src/include/executor/nodeAgg.h @@ -302,6 +302,7 @@ typedef struct AggStatePerHashData AttrNumber *hashGrpColIdxInput; /* hash col indices in input slot */ AttrNumber *hashGrpColIdxHash; /* indices in hash table tuples */ Agg *aggnode; /* original Agg node, for numGroups etc. */ + HashTableInstrumentation instrument; } AggStatePerHashData; diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index b1e2d1f1ae..11fe866c1c 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -691,8 +691,25 @@ typedef struct TupleHashEntryData #define SH_DECLARE #include "lib/simplehash.h" +#define InitTupleHashTableStats(instr, htable, addsize) \ + do{\ + instr.entrysize = sizeof(MinimalTuple) + addsize; \ + instr.nbuckets = htable->size; \ + instr.nbuckets_original = htable->size; \ + instr.space_peak_hash = htable->size * sizeof(TupleHashEntryData); \ + instr.space_peak_tuples = 0; \ + }while(0) + +#define UpdateTupleHashTableStats(instr, htable) \ + do{\ + instr.nbuckets = htable->size; \ + instr.space_peak_hash = Max(instr.space_peak_hash, htable->size*sizeof(TupleHashEntryData)); \ + instr.space_peak_tuples = Max(instr.space_peak_tuples, htable->members*instr.entrysize );\ + }while(0) + typedef struct HashTableInstrumentation { + size_t entrysize; /* Includes additionalsize */ size_t nbuckets; /* number of buckets at end of execution */ size_t nbuckets_original; /* planned number of buckets */ size_t space_peak_hash; /* peak memory usage in bytes */ @@ -717,7 +734,6 @@ typedef struct TupleHashTableData ExprState *cur_eq_func; /* comparator for input vs. table */ uint32 hash_iv; /* hash-function IV */ ExprContext *exprcontext; /* expression context */ - HashTableInstrumentation instrument; } TupleHashTableData; typedef tuplehash_iterator TupleHashIterator; @@ -883,6 +899,8 @@ typedef struct SubPlanState FmgrInfo *lhs_hash_funcs; /* hash functions for lefthand datatype(s) */ FmgrInfo *cur_eq_funcs; /* equality functions for LHS vs. table */ ExprState *cur_eq_comp; /* equality comparator for LHS vs. table */ + HashTableInstrumentation instrument; + HashTableInstrumentation instrument_nulls; /* instrumentation for nulls hashtable */ } SubPlanState; /* ---------------- @@ -1291,6 +1309,7 @@ typedef struct RecursiveUnionState MemoryContext tempContext; /* short-term context for comparisons */ TupleHashTable hashtable; /* hash table for tuples already seen */ MemoryContext tableContext; /* memory context containing hash table */ + HashTableInstrumentation instrument; } RecursiveUnionState; /* ---------------- @@ -1609,7 +1628,7 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; - HashTableInstrumentation instrument; + HashTableInstrumentation instrument; } BitmapHeapScanState; /* ---------------- @@ -2324,6 +2343,7 @@ typedef struct SetOpState MemoryContext tableContext; /* memory context containing hash table */ bool table_filled; /* hash table filled yet? */ TupleHashIterator hashiter; /* for iterating through hash table */ + HashTableInstrumentation instrument; } SetOpState; /* ---------------- -- 2.17.0