From 80e69b60eacd1b7e3dab2533b81ed1e5502a1d02 Mon Sep 17 00:00:00 2001 From: Justin Pryzby Date: Wed, 12 Feb 2020 23:40:45 -0600 Subject: [PATCH v6 4/7] implement hash stats for bitmapHeapScan.. TIDBitmap is a private structure, so add an accessor function to return its instrumentation, and duplicate instrumentation struct in BitmapHeapState. The instrumentation itself could be implemented in simplehash.h. But I think the higher layer BitmapHeapScan would have to include an instrumentation struct anyway, since explain.c cannot look into tbm->pagetable to get .instrument (and the pagetable structure itself doesn't match tuplehash). Also, if instrumentation were implemented in simplehash.h, I think every insertion or deletion would need to check ->members and ->size (which isn't necessary for Agg, but is necessary in the general case, and specifically for tidbitmap, since it actually DELETEs hashtable entries). Or else simplehash would need a new function like UpdateTupleHashStats, which the higher level nodes would need to call after filling the hashtable or before deleting tuples, which seems to defeat the purpose of implementing stats at a lower layer. Note, this doesn't affect any regression tests, since hashtable isn't allocated during "explain". Note that "explain analyze" would show memory stats, which we'd have to filter. --- src/backend/commands/explain.c | 5 ++-- src/backend/executor/nodeBitmapHeapscan.c | 3 +++ src/backend/nodes/tidbitmap.c | 29 +++++++++++++++++++++++ src/include/nodes/execnodes.h | 1 + src/include/nodes/tidbitmap.h | 4 ++++ 5 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 5791ee0cb9..637480de79 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -1734,8 +1734,7 @@ ExplainNode(PlanState *planstate, List *ancestors, if (plan->qual) show_instrumentation_count("Rows Removed by Filter", 1, planstate, es); - if (es->analyze) - show_tidbitmap_info((BitmapHeapScanState *) planstate, es); + show_tidbitmap_info((BitmapHeapScanState *) planstate, es); break; case T_SampleScan: show_tablesample(((SampleScan *) plan)->tablesample, @@ -2927,6 +2926,8 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es) appendStringInfoChar(es->str, '\n'); } } + + show_tuplehash_info(&planstate->instrument, es); } /* diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index ae8a11da30..9ae99a326e 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -182,6 +182,8 @@ BitmapHeapNext(BitmapHeapScanState *node) #endif /* USE_PREFETCH */ } node->initialized = true; + if (node->tbm) + node->instrument = *tbm_instrumentation(node->tbm); } for (;;) @@ -744,6 +746,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->shared_tbmiterator = NULL; scanstate->shared_prefetch_iterator = NULL; scanstate->pstate = NULL; + memset(&scanstate->instrument, 0, sizeof(scanstate->instrument)); /* * We can potentially skip fetching heap pages if we do not need any diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index ad4e071ca3..ab81cce3b2 100644 --- a/src/backend/nodes/tidbitmap.c +++ b/src/backend/nodes/tidbitmap.c @@ -44,6 +44,7 @@ #include "common/hashfn.h" #include "nodes/bitmapset.h" #include "nodes/tidbitmap.h" +#include "nodes/execnodes.h" #include "storage/lwlock.h" #include "utils/dsa.h" @@ -166,6 +167,7 @@ struct TIDBitmap dsa_pointer ptpages; /* dsa_pointer to the page array */ dsa_pointer ptchunks; /* dsa_pointer to the chunk array */ dsa_area *dsa; /* reference to per-query dsa area */ + HashTableInstrumentation instrument; /* Returned by accessor function */ }; /* @@ -294,6 +296,7 @@ tbm_create_pagetable(TIDBitmap *tbm) Assert(tbm->pagetable == NULL); tbm->pagetable = pagetable_create(tbm->mcxt, 128, tbm); + tbm->instrument.nbuckets_original = tbm->pagetable->size; /* If entry1 is valid, push it into the hashtable */ if (tbm->status == TBM_ONE_PAGE) @@ -1147,6 +1150,32 @@ tbm_end_iterate(TBMIterator *iterator) pfree(iterator); } +/* + * tbm_instrumentation - update stored stats and return pointer to + * instrumentation structure + * + * This updates stats when called. + * Returned data is within the iterator's tbm, and destroyed with it. + */ +HashTableInstrumentation * +tbm_instrumentation(TIDBitmap *tbm) +{ + if (tbm->pagetable) + { + tbm->instrument.nbuckets = tbm->pagetable->size; + tbm->instrument.space_peak_hash = sizeof(PagetableEntry) * tbm->pagetable->size; + + /* + * If there are lossy pages, then at one point, we filled maxentries; + * otherwise, number of pages is "->members". + */ + tbm->instrument.space_peak_tuples = sizeof(BlockNumber) * + (tbm->nchunks>0 ? tbm->maxentries : tbm->pagetable->members); + } + + return &tbm->instrument; +} + /* * tbm_end_shared_iterate - finish a shared iteration over a TIDBitmap * diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index cfeada5f1d..b1e2d1f1ae 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1609,6 +1609,7 @@ typedef struct BitmapHeapScanState TBMSharedIterator *shared_tbmiterator; TBMSharedIterator *shared_prefetch_iterator; ParallelBitmapHeapState *pstate; + HashTableInstrumentation instrument; } BitmapHeapScanState; /* ---------------- diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h index d562fcae34..de0cdfb91f 100644 --- a/src/include/nodes/tidbitmap.h +++ b/src/include/nodes/tidbitmap.h @@ -26,6 +26,9 @@ #include "utils/dsa.h" +/* Forward decl */ +typedef struct HashTableInstrumentation HashTableInstrumentation; + /* * Actual bitmap representation is private to tidbitmap.c. Callers can * do IsA(x, TIDBitmap) on it, but nothing else. @@ -71,5 +74,6 @@ extern void tbm_end_shared_iterate(TBMSharedIterator *iterator); extern TBMSharedIterator *tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp); extern long tbm_calculate_entries(double maxbytes); +extern HashTableInstrumentation *tbm_instrumentation(TIDBitmap *tbm); #endif /* TIDBITMAP_H */ -- 2.17.0