From 86939c5c0c8c835a0cd50d4a32dda0f08413c363 Mon Sep 17 00:00:00 2001
From: Justin Pryzby <pryzbyj@telsasoft.com>
Date: Wed, 12 Feb 2020 23:40:45 -0600
Subject: [PATCH v2 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.
---
 src/backend/commands/explain.c            |  2 ++
 src/backend/executor/nodeBitmapHeapscan.c |  3 +++
 src/backend/nodes/tidbitmap.c             | 20 ++++++++++++++++++++
 src/include/nodes/execnodes.h             |  1 +
 src/include/nodes/tidbitmap.h             |  4 ++++
 5 files changed, 30 insertions(+)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 67a9840..d71f5f1 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2908,6 +2908,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 ae8a11d..9ae99a3 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 e102589..d1ef07c 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -43,6 +43,7 @@
 #include "access/htup_details.h"
 #include "nodes/bitmapset.h"
 #include "nodes/tidbitmap.h"
+#include "nodes/execnodes.h"
 #include "storage/lwlock.h"
 #include "utils/dsa.h"
 #include "utils/hashutils.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 */
+	hash_instrumentation 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)
@@ -1148,6 +1151,23 @@ tbm_end_iterate(TBMIterator *iterator)
 }
 
 /*
+ * tbm_instrumentation - return pointer instrumentation data
+ *
+ * Returned data is within the iterator's tbm, and destroyed with it.
+ */
+hash_instrumentation *
+tbm_instrumentation(TIDBitmap *tbm)
+{
+	if (tbm->pagetable) {
+		tbm->instrument.nbuckets = tbm->pagetable->size;
+		tbm->instrument.space_peak_hash = sizeof(PagetableEntry) * tbm->pagetable->size;
+		tbm->instrument.space_peak_tuples = sizeof(BlockNumber) * (tbm->nchunks ? tbm->maxentries : tbm->pagetable->members);
+	}
+
+	return &tbm->instrument;
+}
+
+/*
  * tbm_end_shared_iterate - finish a shared iteration over a TIDBitmap
  *
  * This doesn't free any of the shared state associated with the iterator,
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index f929585..f5740c7 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1607,6 +1607,7 @@ typedef struct BitmapHeapScanState
 	TBMSharedIterator *shared_tbmiterator;
 	TBMSharedIterator *shared_prefetch_iterator;
 	ParallelBitmapHeapState *pstate;
+	hash_instrumentation	instrument;
 } BitmapHeapScanState;
 
 /* ----------------
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index d562fca..811a497 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 hash_instrumentation hash_instrumentation;
+
 /*
  * 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 hash_instrumentation *tbm_instrumentation(TIDBitmap *tbm);
 
 #endif							/* TIDBITMAP_H */
-- 
2.7.4

