On Thu, Mar 19, 2020 at 9:26 PM Justin Pryzby <pry...@telsasoft.com> wrote:
>
> On Mon, Mar 16, 2020 at 09:08:36AM -0400, James Coleman wrote:
> > Does the original optimization cover parallel bitmap heap scans like this?
>
> It works for parallel bitmap only scans.
>
> template1=# explain analyze select count(*) from exp where a between 25 and 
> 35 and d between 5 and 10;
>  Finalize Aggregate  (cost=78391.68..78391.69 rows=1 width=8) (actual 
> time=525.972..525.972 rows=1 loops=1)
>    ->  Gather  (cost=78391.47..78391.68 rows=2 width=8) (actual 
> time=525.416..533.133 rows=3 loops=1)
>          Workers Planned: 2
>          Workers Launched: 2
>          ->  Partial Aggregate  (cost=77391.47..77391.48 rows=1 width=8) 
> (actual time=518.406..518.406 rows=1 loops=3)
>                ->  Parallel Bitmap Heap Scan on exp  (cost=31825.37..77245.01 
> rows=58582 width=0) (actual time=296.309..508.440 rows=43887 loops=3)
>                      Recheck Cond: ((a >= 25) AND (a <= 35) AND (d >= 5) AND 
> (d <= 10))
>                      Heap Blocks: unfetched=4701 exact=9650
>                      ->  BitmapAnd  (cost=31825.37..31825.37 rows=140597 
> width=0) (actual time=282.590..282.590 rows=0 loops=1)
>                            ->  Bitmap Index Scan on index_exp_a  
> (cost=0.00..15616.99 rows=1166456 width=0) (actual time=147.036..147.036 
> rows=1099872 loops=1)
>                                  Index Cond: ((a >= 25) AND (a <= 35))
>                            ->  Bitmap Index Scan on index_exp_d  
> (cost=0.00..16137.82 rows=1205339 width=0) (actual time=130.366..130.366 
> rows=1200000 loops=1)
>                                  Index Cond: ((d >= 5) AND (d <= 10))
>
>
> > +++ b/src/backend/commands/explain.c
> > @@ -2777,6 +2777,8 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, 
> > ExplainState *es)
> >  {
> >       if (es->format != EXPLAIN_FORMAT_TEXT)
> >       {
> > +             ExplainPropertyInteger("Unfetched Heap Blocks", NULL,
> > +                                                        
> > planstate->unfetched_pages, es);
> >               ExplainPropertyInteger("Exact Heap Blocks", NULL,
> >                                                          
> > planstate->exact_pages, es);
> >               ExplainPropertyInteger("Lossy Heap Blocks", NULL,
> > @@ -2784,10 +2786,14 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, 
> > ExplainState *es)
> >       }
> >       else
> >       {
> > -             if (planstate->exact_pages > 0 || planstate->lossy_pages > 0)
> > +             if (planstate->exact_pages > 0 || planstate->lossy_pages > 0
> > +                     || planstate->unfetched_pages > 0)
> >               {
> >                       ExplainIndentText(es);
> >                       appendStringInfoString(es->str, "Heap Blocks:");
> > +                     if (planstate->unfetched_pages > 0)
> > +                             appendStringInfo(es->str, " unfetched=%ld",
> > +                                                              
> > planstate->unfetched_pages);
> >                       if (planstate->exact_pages > 0)
> >                               appendStringInfo(es->str, " exact=%ld", 
> > planstate->exact_pages);
> >                       if (planstate->lossy_pages > 0)

Awesome, thanks for confirming with an actual plan.

> I don't think it matters in nontext mode, but at least in text mode, I think
> maybe the Unfetched blocks should be output after the exact and lossy blocks,
> in case someone is parsing it, and because bitmap-only is a relatively new
> feature.  Its output is probably less common than exact/lossy.

I tweaked that (and a comment that didn't reference the change); see attached.

James
From 3d52fc04cab36819a93638c104ae1a81208c54df Mon Sep 17 00:00:00 2001
From: James Coleman <jtc...@gmail.com>
Date: Sun, 15 Mar 2020 20:27:19 -0400
Subject: [PATCH v4] Show bitmap only unfetched page count to EXPLAIN

7c70996ebf0949b142a99 added an optimization to bitmap heap scans to
avoid fetching the heap page where the visibility map makes that
possible, much like index only scans.

However that commit didn't add this output to EXPLAIN, so it's not
obvious when the optimization is kicking in and when it's not. So, track
the number of skipped pages and report it in EXPLAIN output.

Author: Jeff Janes
Reviewers: Emre Hasegeli, Alexey Bashtanov, James Coleman
---
 src/backend/commands/explain.c            | 10 ++++++++--
 src/backend/executor/nodeBitmapHeapscan.c |  5 +++--
 src/include/nodes/execnodes.h             |  2 ++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index d901dc4a50..52c35e640d 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -2770,7 +2770,7 @@ show_hash_info(HashState *hashstate, ExplainState *es)
 }
 
 /*
- * If it's EXPLAIN ANALYZE, show exact/lossy pages for a BitmapHeapScan node
+ * If it's EXPLAIN ANALYZE, show exact/lossy/unfetched pages for a BitmapHeapScan node
  */
 static void
 show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
@@ -2781,10 +2781,13 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
 							   planstate->exact_pages, es);
 		ExplainPropertyInteger("Lossy Heap Blocks", NULL,
 							   planstate->lossy_pages, es);
+		ExplainPropertyInteger("Unfetched Heap Blocks", NULL,
+							   planstate->unfetched_pages, es);
 	}
 	else
 	{
-		if (planstate->exact_pages > 0 || planstate->lossy_pages > 0)
+		if (planstate->exact_pages > 0 || planstate->lossy_pages > 0
+			|| planstate->unfetched_pages > 0)
 		{
 			ExplainIndentText(es);
 			appendStringInfoString(es->str, "Heap Blocks:");
@@ -2792,6 +2795,9 @@ show_tidbitmap_info(BitmapHeapScanState *planstate, ExplainState *es)
 				appendStringInfo(es->str, " exact=%ld", planstate->exact_pages);
 			if (planstate->lossy_pages > 0)
 				appendStringInfo(es->str, " lossy=%ld", planstate->lossy_pages);
+			if (planstate->unfetched_pages > 0)
+				appendStringInfo(es->str, " unfetched=%ld",
+								 planstate->unfetched_pages);
 			appendStringInfoChar(es->str, '\n');
 		}
 	}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index ae8a11da30..dce955636f 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -231,14 +231,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
 				 * node->return_empty_tuples.
 				 */
 				node->return_empty_tuples = tbmres->ntuples;
+				node->unfetched_pages++;
 			}
 			else if (!table_scan_bitmap_next_block(scan, tbmres))
 			{
 				/* AM doesn't think this block is valid, skip */
 				continue;
 			}
-
-			if (tbmres->ntuples >= 0)
+			else if (tbmres->ntuples >= 0)
 				node->exact_pages++;
 			else
 				node->lossy_pages++;
@@ -734,6 +734,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	scanstate->pvmbuffer = InvalidBuffer;
 	scanstate->exact_pages = 0;
 	scanstate->lossy_pages = 0;
+	scanstate->unfetched_pages = 0;
 	scanstate->prefetch_iterator = NULL;
 	scanstate->prefetch_pages = 0;
 	scanstate->prefetch_target = 0;
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cd3ddf781f..32400be08c 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1567,6 +1567,7 @@ typedef struct ParallelBitmapHeapState
  *		pvmbuffer		   ditto, for prefetched pages
  *		exact_pages		   total number of exact pages retrieved
  *		lossy_pages		   total number of lossy pages retrieved
+ *		unfetched_pages	   total number of pages not retrieved due to vm
  *		prefetch_iterator  iterator for prefetching ahead of current page
  *		prefetch_pages	   # pages prefetch iterator is ahead of current
  *		prefetch_target    current target prefetch distance
@@ -1591,6 +1592,7 @@ typedef struct BitmapHeapScanState
 	Buffer		pvmbuffer;
 	long		exact_pages;
 	long		lossy_pages;
+	long		unfetched_pages;
 	TBMIterator *prefetch_iterator;
 	int			prefetch_pages;
 	int			prefetch_target;
-- 
2.17.1

Reply via email to