From: Melanie Plageman <melanieplageman@gmail.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Thomas Munro <thomas.munro@gmail.com>
Cc: Heikki Linnakangas <hlinnaka@iki.fi>
Cc: Andres Freund <andres@anarazel.de>
Cc: Pg Hackers <pgsql-hackers@postgresql.org>
Cc: Nazir Bilal Yavuz <byavuz81@gmail.com>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Date: Fri, 5 Apr 2024 19:53:30 -0400
Message-ID: <20240405235330.v4didvweb6isodtk@liskov> (raw)
In-Reply-To: <20240405080634.d7c23f46xjhhxf5q@liskov>
References: <CAAKRu_ZS016J_qhs=BnCsteYYC0HgYpAbZB8mDZBvFdy_VD1MQ@mail.gmail.com>
<20240325160709.qjo6txzu6zjxzkqy@liskov>
<20240327193750.3mlcmzqondpj27xe@liskov>
<7e98781c-34e0-4c02-8e5b-0500aa074271@enterprisedb.com>
<CA+hUKGJtm_gkmW_h_02-Q9ZRcG3yOx2uzVqbCTfz7YPnTfs+DA@mail.gmail.com>
<8330278b-9615-4030-8626-51ce714a7f54@enterprisedb.com>
<20240331154551.lty4mundyoyhtixw@liskov>
<20240403225759.sq5hmxdmbalroosy@liskov>
<95d9471f-ead4-4215-b739-f6d6f4c19497@enterprisedb.com>
<20240405080634.d7c23f46xjhhxf5q@liskov>
On Fri, Apr 05, 2024 at 04:06:34AM -0400, Melanie Plageman wrote:
> On Thu, Apr 04, 2024 at 04:35:45PM +0200, Tomas Vondra wrote:
> >
> >
> > On 4/4/24 00:57, Melanie Plageman wrote:
> > > On Sun, Mar 31, 2024 at 11:45:51AM -0400, Melanie Plageman wrote:
> > >> On Fri, Mar 29, 2024 at 12:05:15PM +0100, Tomas Vondra wrote:
> > >>>
> > >>>
> > >>> On 3/29/24 02:12, Thomas Munro wrote:
> > >>>> On Fri, Mar 29, 2024 at 10:43 AM Tomas Vondra
> > >>>> <tomas.vondra@enterprisedb.com> wrote:
> > >>>>> I think there's some sort of bug, triggering this assert in heapam
> > >>>>>
> > >>>>> Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno);
> > >>>>
> > >>>> Thanks for the repro. I can't seem to reproduce it (still trying) but
> > >>>> I assume this is with Melanie's v11 patch set which had
> > >>>> v11-0016-v10-Read-Stream-API.patch.
> > >>>>
> > >>>> Would you mind removing that commit and instead applying the v13
> > >>>> stream_read.c patches[1]? v10 stream_read.c was a little confused
> > >>>> about random I/O combining, which I fixed with a small adjustment to
> > >>>> the conditions for the "if" statement right at the end of
> > >>>> read_stream_look_ahead(). Sorry about that. The fixed version, with
> > >>>> eic=4, with your test query using WHERE a < a, ends its scan with:
> > >>>>
> > >>>
> > >>> I'll give that a try. Unfortunately unfortunately the v11 still has the
> > >>> problem I reported about a week ago:
> > >>>
> > >>> ERROR: prefetch and main iterators are out of sync
> > >>>
> > >>> So I can't run the full benchmarks :-( but master vs. streaming read API
> > >>> should work, I think.
> > >>
> > >> Odd, I didn't notice you reporting this ERROR popping up. Now that I
> > >> take a look, v11 (at least, maybe also v10) had this very sill mistake:
> > >>
> > >> if (scan->bm_parallel == NULL &&
> > >> scan->rs_pf_bhs_iterator &&
> > >> hscan->pfblockno > hscan->rs_base.blockno)
> > >> elog(ERROR, "prefetch and main iterators are out of sync");
> > >>
> > >> It errors out if the prefetch block is ahead of the current block --
> > >> which is the opposite of what we want. I've fixed this in attached v12.
> > >>
> > >> This version also has v13 of the streaming read API. I noticed one
> > >> mistake in my bitmapheap scan streaming read user -- it freed the
> > >> streaming read object at the wrong time. I don't know if this was
> > >> causing any other issues, but it at least is fixed in this version.
> > >
> > > Attached v13 is rebased over master (which includes the streaming read
> > > API now). I also reset the streaming read object on rescan instead of
> > > creating a new one each time.
> > >
> > > I don't know how much chance any of this has of going in to 17 now, but
> > > I thought I would start looking into the regression repro Tomas provided
> > > in [1].
> > >
> >
> > My personal opinion is that we should try to get in as many of the the
> > refactoring patches as possible, but I think it's probably too late for
> > the actual switch to the streaming API.
>
> Cool. In the attached v15, I have dropped all commits that are related
> to the streaming read API and included *only* commits that are
> beneficial to master. A few of the commits are merged or reordered as
> well.
>
> While going through the commits with this new goal in mind (forget about
> the streaming read API for now), I realized that it doesn't make much
> sense to just eliminate the layering violation for the current block and
> leave it there for the prefetch block. I had de-prioritized solving this
> when I thought we would just delete the prefetch code and replace it
> with the streaming read.
>
> Now that we aren't doing that, I've spent the day trying to resolve the
> issues with pushing the prefetch code into heapam.c that I cited in [1].
> 0010 - 0013 are the result of this. They are not very polished yet and
> need more cleanup and review (especially 0011, which is probably too
> large), but I am happy with the solution I came up with.
>
> Basically, there are too many members needed for bitmap heap scan to put
> them all in the HeapScanDescData (don't want to bloat it). So, I've made
> a new BitmapHeapScanDescData and associated begin/rescan/end() functions
>
> In the end, with all patches applied, BitmapHeapNext() loops invoking
> table_scan_bitmap_next_tuple() and table AMs can implement that however
> they choose.
>
> > > I'm also not sure if I should try and group the commits into fewer
> > > commits now or wait until I have some idea of whether or not the
> > > approach in 0013 and 0014 is worth pursuing.
> > >
> >
> > You mean whether to pursue the approach in general, or for v17? I think
> > it looks like the right approach, but for v17 see above :-(
> >
> > As for merging, I wouldn't do that. I looked at the commits and while
> > some of them seem somewhat "trivial", I really like how you organized
> > the commits, and kept those that just "move" code around, and those that
> > actually change stuff. It's much easier to understand, IMO.
> >
> > I went through the first ~10 commits, and added some review - either as
> > a separate commit, when possible, in the code as XXX comment, and also
> > in the commit message. The code tweaks are utterly trivial (whitespace
> > or indentation to make the line shorter). It shouldn't take much time to
> > deal with those, I think.
>
> Attached v15 incorporates your v14-0002-review.
>
> For your v14-0008-review, I actually ended up removing that commit
> because once I removed everything that was for streaming read API, it
> became redundant with another commit.
>
> For your v14-0010-review, we actually can't easily get rid of those
> local variables because we make the iterators before we make the scan
> descriptors and the commit following that commit moves the iterators
> from the BitmapHeapScanState to the scan descriptor.
>
> > I think the main focus should be updating the commit messages. If it was
> > only a single patch, I'd probably try to write the messages myself, but
> > with this many patches it'd be great if you could update those and I'll
> > review that before commit.
>
> I did my best to update the commit messages to be less specific and more
> focused on "why should I care". I found myself wanting to explain why I
> implemented something the way I did and then getting back into the
> implementation details again. I'm not sure if I suceeded in having less
> details and more substance.
>
> > I always struggle with writing commit messages myself, and it takes me
> > ages to write a good one (well, I think the message is good, but who
> > knows ...). But I think a good message should be concise enough to
> > explain what and why it's done. It may reference a thread for all the
> > gory details, but the basic reasoning should be in the commit message.
> > For example the message for "BitmapPrefetch use prefetch block recheck
> > for skip fetch" now says that it "makes more sense to do X" but does not
> > really say why that's the case. The linked message does, but it'd be
> > good to have that in the message (because how would I know how much of
> > the thread to read?).
>
> I fixed that particular one. I tried to take that feedback and apply it
> to other commit messages. I don't know how successful I was...
>
> > Also, it'd be very helpful if you could update the author & reviewed-by
> > fields. I'll review those before commit, ofc, but I admit I lost track
> > of who reviewed which part.
>
> I have updated reviewers. I didn't add reviewers on the ones that
> haven't really been reviewed yet.
>
> > I'd focus on the first ~8-9 commits or so for now, we can commit more if
> > things go reasonably well.
>
> Sounds good. I will spend cleanup time on 0010-0013 tomorrow but would
> love to know if you agree with the direction before I spend more time.
In attached v16, I've split out 0010-0013 into 0011-0017. I think it is
much easier to understand.
While I was doing that, I realized that I should remove the call to
table_rescan() from ExecReScanBitmapHeapScan() and just rely on the new
table_rescan_bm() invoked from BitmapHeapNext(). That is done in the
attached.
0010-0018 still need comments updated but I focused on getting the split
out, reviewable version of them ready. I'll add comments (especially to
0011 table AM functions) tomorrow. I also have to double-check if I
should add any asserts for table AMs about having implemented all of the
new begin/re/endscan() functions.
- Melanie
Attachments:download all (18 files, .tar.gz)v16-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch (text/x-diff, 2.4K) download
From c6ebe66dcf5ff961ef3d2c66a26b21d2246a499b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 12 Feb 2024 18:50:29 -0500
Subject: [PATCH v16 01/18] BitmapHeapScan begin scan after bitmap creation
It makes more sense for BitmapHeapScan to scan the index, build the
bitmap, and then begin the scan on the underlying table.
This is primarily a cosmetic change for now, but later commits will pass
parameters to table_beginscan_bm() that are unavailable in
ExecInitBitmapHeapScan().
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 27 +++++++++++++++++------
1 file changed, 20 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex cee7f45aab..c8c466e3c5 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -178,6 +178,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
#endif /* USE_PREFETCH */
}
++ /*+ * If this is the first scan of the underlying table, create the table+ * scan descriptor and begin the scan.+ */+ if (!scan)+ {+ scan = table_beginscan_bm(node->ss.ss_currentRelation,+ node->ss.ps.state->es_snapshot,+ 0,+ NULL);++ node->ss.ss_currentScanDesc = scan;+ }+
node->initialized = true;
}
@@ -601,7 +616,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
PlanState *outerPlan = outerPlanState(node);
/* rescan to release any page pin */
- table_rescan(node->ss.ss_currentScanDesc, NULL);+ if (node->ss.ss_currentScanDesc)+ table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
if (node->tbmiterator)
@@ -678,7 +694,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* close heap scan
*/
- table_endscan(scanDesc);+ if (scanDesc)+ table_endscan(scanDesc);+
}
/* ----------------------------------------------------------------
@@ -783,11 +801,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ss_currentRelation = currentRelation;
- scanstate->ss.ss_currentScanDesc = table_beginscan_bm(currentRelation,- estate->es_snapshot,- 0,- NULL);-
/*
* all done.
*/
--
2.40.1
v16-0002-BitmapHeapScan-set-can_skip_fetch-later.patch (text/x-diff, 2.3K) download
From 770bf2e20dfd3c29c5fc4937d78ec2e26d3f78d4 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 13 Feb 2024 14:38:41 -0500
Subject: [PATCH v16 02/18] BitmapHeapScan set can_skip_fetch later
Set BitmapHeapScanState->can_skip_fetch in BitmapHeapNext() instead of
in ExecInitBitmapHeapScan(). This is a preliminary step to pushing the
skip fetch optimization into heap AM code.
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 21 +++++++++++----------
1 file changed, 11 insertions(+), 10 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex c8c466e3c5..2148a21531 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -105,6 +105,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ /*+ * We can potentially skip fetching heap pages if we do not need any+ * columns of the table, either for checking non-indexable quals or+ * for returning data. This test is a bit simplistic, as it checks+ * the stronger condition that there's no qual or return tlist at all.+ * But in most cases it's probably not worth working harder than that.+ */+ node->can_skip_fetch = (node->ss.ps.plan->qual == NIL &&+ node->ss.ps.plan->targetlist == NIL);+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -743,16 +753,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
-- /*- * We can potentially skip fetching heap pages if we do not need any- * columns of the table, either for checking non-indexable quals or for- * returning data. This test is a bit simplistic, as it checks the- * stronger condition that there's no qual or return tlist at all. But in- * most cases it's probably not worth working harder than that.- */- scanstate->can_skip_fetch = (node->scan.plan.qual == NIL &&- node->scan.plan.targetlist == NIL);+ scanstate->can_skip_fetch = false;
/*
* Miscellaneous initialization
--
2.40.1
v16-0003-Push-BitmapHeapScan-skip-fetch-optimization-into.patch (text/x-diff, 14.7K) download
From c5d36fc7f201d8e01f68a00b235872165fd0ec94 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Thu, 4 Apr 2024 15:34:25 +0200
Subject: [PATCH v16 03/18] Push BitmapHeapScan skip fetch optimization into
table AM
7c70996ebf0949b142 introduced an optimization to allow bitmap table
scans to skip fetching a block from the heap if none of the underlying
data was needed and the block is marked all visible in the visibility
map. With the addition of table AMs, a FIXME was added to this code
indicating that it should be pushed into table AM-specific code, as not
all table AMs may use a visibility map in the same way.
Resolve this FIXME for the current block. The layering violation is
still present in BitmapHeapScans's prefetching code, which uses the
visibility map to decide whether or not to prefetch a block. However,
this will be fixed in an upcoming commit.
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 14 +++
src/backend/access/heap/heapam_handler.c | 29 +++++
src/backend/executor/nodeBitmapHeapscan.c | 124 +++++++---------------
src/include/access/heapam.h | 10 ++
src/include/access/tableam.h | 11 +-
src/include/nodes/execnodes.h | 8 +-
6 files changed, 102 insertions(+), 94 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex dada2ecd1e..10f2faaa60 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -967,6 +967,8 @@ heap_beginscan(Relation relation, Snapshot snapshot,
scan->rs_base.rs_flags = flags;
scan->rs_base.rs_parallel = parallel_scan;
scan->rs_strategy = NULL; /* set in initscan */
+ scan->rs_vmbuffer = InvalidBuffer;+ scan->rs_empty_tuples_pending = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1055,6 +1057,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ if (BufferIsValid(scan->rs_vmbuffer))+ {+ ReleaseBuffer(scan->rs_vmbuffer);+ scan->rs_vmbuffer = InvalidBuffer;+ }+
/*
* reinitialize scan descriptor
*/
@@ -1074,6 +1082,12 @@ heap_endscan(TableScanDesc sscan)
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
+ if (BufferIsValid(scan->rs_vmbuffer))+ {+ ReleaseBuffer(scan->rs_vmbuffer);+ scan->rs_vmbuffer = InvalidBuffer;+ }+
/*
* decrement relation reference count and free scan descriptor storage
*/
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex 3e7a6b5548..929b2cf8e7 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -27,6 +27,7 @@
#include "access/syncscan.h"
#include "access/tableam.h"
#include "access/tsmapi.h"
+#include "access/visibilitymap.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/index.h"
@@ -2198,6 +2199,24 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
+ /*+ * We can skip fetching the heap page if we don't need any fields from the+ * heap, the bitmap entries don't need rechecking, and all tuples on the+ * page are visible to our transaction.+ */+ if (!(scan->rs_flags & SO_NEED_TUPLE) &&+ !tbmres->recheck &&+ VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &hscan->rs_vmbuffer))+ {+ /* can't be lossy in the skip_fetch case */+ Assert(tbmres->ntuples >= 0);+ Assert(hscan->rs_empty_tuples_pending >= 0);++ hscan->rs_empty_tuples_pending += tbmres->ntuples;++ return true;+ }+
/*
* Ignore any claimed entries past what we think is the end of the
* relation. It may have been extended after the start of our scan (we
@@ -2310,6 +2329,16 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
Page page;
ItemId lp;
+ if (hscan->rs_empty_tuples_pending > 0)+ {+ /*+ * If we don't have to fetch the tuple, just return nulls.+ */+ ExecStoreAllNullTuple(slot);+ hscan->rs_empty_tuples_pending--;+ return true;+ }+
/*
* Out of range? If so, nothing more to look at on this page
*/
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 2148a21531..a8bc5dec53 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -105,16 +105,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
- /*- * We can potentially skip fetching heap pages if we do not need any- * columns of the table, either for checking non-indexable quals or- * for returning data. This test is a bit simplistic, as it checks- * the stronger condition that there's no qual or return tlist at all.- * But in most cases it's probably not worth working harder than that.- */- node->can_skip_fetch = (node->ss.ps.plan->qual == NIL &&- node->ss.ps.plan->targetlist == NIL);-
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -195,10 +185,24 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!scan)
{
+ uint32 extra_flags = 0;++ /*+ * We can potentially skip fetching heap pages if we do not need+ * any columns of the table, either for checking non-indexable+ * quals or for returning data. This test is a bit simplistic, as+ * it checks the stronger condition that there's no qual or return+ * tlist at all. But in most cases it's probably not worth working+ * harder than that.+ */+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)+ extra_flags |= SO_NEED_TUPLE;+
scan = table_beginscan_bm(node->ss.ss_currentRelation,
node->ss.ps.state->es_snapshot,
0,
- NULL);+ NULL,+ extra_flags);
node->ss.ss_currentScanDesc = scan;
}
@@ -208,7 +212,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool skip_fetch;+ bool valid;
CHECK_FOR_INTERRUPTS();
@@ -229,37 +233,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres);+
if (tbmres->ntuples >= 0)
node->exact_pages++;
else
node->lossy_pages++;
- /*- * We can skip fetching the heap page if we don't need any fields- * from the heap, and the bitmap entries don't need rechecking,- * and all tuples on the page are visible to our transaction.- *- * XXX: It's a layering violation that we do these checks above- * tableam, they should probably moved below it at some point.- */- skip_fetch = (node->can_skip_fetch &&- !tbmres->recheck &&- VM_ALL_VISIBLE(node->ss.ss_currentRelation,- tbmres->blockno,- &node->vmbuffer));-- if (skip_fetch)- {- /* can't be lossy in the skip_fetch case */- Assert(tbmres->ntuples >= 0);-- /*- * The number of tuples on this page is put into- * node->return_empty_tuples.- */- node->return_empty_tuples = tbmres->ntuples;- }- else if (!table_scan_bitmap_next_block(scan, tbmres))+ if (!valid)
{
/* AM doesn't think this block is valid, skip */
continue;
@@ -302,52 +283,33 @@ BitmapHeapNext(BitmapHeapScanState *node)
* should happen only when we have determined there is still something
* to do on the current page, else we may uselessly prefetch the same
* page we are just about to request for real.
- *- * XXX: It's a layering violation that we do these checks above- * tableam, they should probably moved below it at some point.
*/
BitmapPrefetch(node, scan);
- if (node->return_empty_tuples > 0)+ /*+ * Attempt to fetch tuple from AM.+ */+ if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))
{
- /*- * If we don't have to fetch the tuple, just return nulls.- */- ExecStoreAllNullTuple(slot);-- if (--node->return_empty_tuples == 0)- {- /* no more tuples to return in the next round */- node->tbmres = tbmres = NULL;- }+ /* nothing more to look at on this page */+ node->tbmres = tbmres = NULL;+ continue;
}
- else++ /*+ * If we are using lossy info, we have to recheck the qual conditions+ * at every tuple.+ */+ if (tbmres->recheck)
{
- /*- * Attempt to fetch tuple from AM.- */- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))+ econtext->ecxt_scantuple = slot;+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))
{
- /* nothing more to look at on this page */- node->tbmres = tbmres = NULL;+ /* Fails recheck, so drop it and loop back for another */+ InstrCountFiltered2(node, 1);+ ExecClearTuple(slot);
continue;
}
-- /*- * If we are using lossy info, we have to recheck the qual- * conditions at every tuple.- */- if (tbmres->recheck)- {- econtext->ecxt_scantuple = slot;- if (!ExecQualAndReset(node->bitmapqualorig, econtext))- {- /* Fails recheck, so drop it and loop back for another */- InstrCountFiltered2(node, 1);- ExecClearTuple(slot);- continue;- }- }
}
/* OK to return this tuple */
@@ -519,7 +481,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* it did for the current heap page; which is not a certainty
* but is true in many cases.
*/
- skip_fetch = (node->can_skip_fetch &&+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
(node->tbmres ? !node->tbmres->recheck : false) &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
@@ -570,7 +532,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
}
/* As above, skip prefetch if we expect not to need page */
- skip_fetch = (node->can_skip_fetch &&+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
(node->tbmres ? !node->tbmres->recheck : false) &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
@@ -640,8 +602,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->vmbuffer != InvalidBuffer)- ReleaseBuffer(node->vmbuffer);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
@@ -651,7 +611,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
node->initialized = false;
node->shared_tbmiterator = NULL;
node->shared_prefetch_iterator = NULL;
- node->vmbuffer = InvalidBuffer;
node->pvmbuffer = InvalidBuffer;
ExecScanReScan(&node->ss);
@@ -696,8 +655,6 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
- if (node->vmbuffer != InvalidBuffer)- ReleaseBuffer(node->vmbuffer);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
@@ -741,8 +698,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->tbm = NULL;
scanstate->tbmiterator = NULL;
scanstate->tbmres = NULL;
- scanstate->return_empty_tuples = 0;- scanstate->vmbuffer = InvalidBuffer;
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
@@ -753,7 +708,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
- scanstate->can_skip_fetch = false;
/*
* Miscellaneous initialization
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex 2765efc4e5..750ea30852 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -76,6 +76,16 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /*+ * These fields are only used for bitmap scans for the "skip fetch"+ * optimization. Bitmap scans needing no fields from the heap may skip+ * fetching an all visible block, instead using the number of tuples per+ * block reported by the bitmap to determine how many NULL-filled tuples+ * to return.+ */+ Buffer rs_vmbuffer;+ int rs_empty_tuples_pending;+
/* these fields only used in page-at-a-time mode and for bitmap scans */
int rs_cindex; /* current tuple's index in vistuples */
int rs_ntuples; /* number of visible tuples on page */
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex e7eeb75409..55f1139757 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -63,6 +63,13 @@ typedef enum ScanOptions
/* unregister snapshot at scan end? */
SO_TEMP_SNAPSHOT = 1 << 9,
++ /*+ * At the discretion of the table AM, bitmap table scans may be able to+ * skip fetching a block from the table if none of the table data is+ * needed. If table data may be needed, set SO_NEED_TUPLE.+ */+ SO_NEED_TUPLE = 1 << 10,
} ScanOptions;
/*
@@ -937,9 +944,9 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
- int nkeys, struct ScanKeyData *key)+ int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
- uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE;+ uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
}
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.hindex e57ebd7288..fa2f70b7a4 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -1794,10 +1794,7 @@ typedef struct ParallelBitmapHeapState
* tbm bitmap obtained from child index scan(s)
* tbmiterator iterator for scanning current pages
* tbmres current-page data
- * can_skip_fetch can we potentially skip tuple fetches in this scan?- * return_empty_tuples number of empty tuples to return- * vmbuffer buffer for visibility-map lookups- * pvmbuffer ditto, for prefetched pages+ * pvmbuffer buffer for visibility-map lookups of prefetched pages
* exact_pages total number of exact pages retrieved
* lossy_pages total number of lossy pages retrieved
* prefetch_iterator iterator for prefetching ahead of current page
@@ -1817,9 +1814,6 @@ typedef struct BitmapHeapScanState
TIDBitmap *tbm;
TBMIterator *tbmiterator;
TBMIterateResult *tbmres;
- bool can_skip_fetch;- int return_empty_tuples;- Buffer vmbuffer;
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
--
2.40.1
v16-0004-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch (text/x-diff, 2.4K) download
From 796e139454a8863c12235d248d76f313da6ef002 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 12 Feb 2024 19:03:24 -0500
Subject: [PATCH v16 04/18] BitmapPrefetch use prefetch block recheck for skip
fetch
As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for
the current block to determine whether or not it could skip prefetching
the proposed prefetch block. This doesn't seem right. We should use the
prefetch block's recheck flag to determine whether or not to prefetch
it. The current block's recheck flag has no relationship to the prefetch
block's recheck flag. See this [1] thread on hackers reporting the
issue.
[1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.gmail.com
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 9 ++-------
1 file changed, 2 insertions(+), 7 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex a8bc5dec53..2e2cec8b3b 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* skip this prefetch call, but continue to run the prefetch
* logic normally. (Would it be better not to increment
* prefetch_pages?)
- *- * This depends on the assumption that the index AM will- * report the same recheck flag for this future heap page as- * it did for the current heap page; which is not a certainty- * but is true in many cases.
*/
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- (node->tbmres ? !node->tbmres->recheck : false) &&+ !tbmpre->recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
&node->pvmbuffer));
@@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
/* As above, skip prefetch if we expect not to need page */
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- (node->tbmres ? !node->tbmres->recheck : false) &&+ !tbmpre->recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
tbmpre->blockno,
&node->pvmbuffer));
--
2.40.1
v16-0005-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch (text/x-diff, 2.5K) download
From 2f9abb4c09528925060b7613fe7d3e52d04d8bea Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 12 Feb 2024 19:04:48 -0500
Subject: [PATCH v16 05/18] Update BitmapAdjustPrefetchIterator parameter type
to BlockNumber
BitmapAdjustPrefetchIterator() only used the blockno member of the
passed in TBMIterateResult to ensure that the prefetch iterator and
regular iterator stay in sync. Pass it the BlockNumber only. This will
allow us to move away from using the TBMIterateResult outside of table
AM specific code.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 2e2cec8b3b..795a893035 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -52,7 +52,7 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- TBMIterateResult *tbmres);+ BlockNumber blockno);
static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
@@ -231,7 +231,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
break;
}
- BitmapAdjustPrefetchIterator(node, tbmres);+ BitmapAdjustPrefetchIterator(node, tbmres->blockno);
valid = table_scan_bitmap_next_block(scan, tbmres);
@@ -342,7 +342,7 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
*/
static inline void
BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
- TBMIterateResult *tbmres)+ BlockNumber blockno)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
@@ -361,7 +361,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
/* Do not let the prefetch iterator get behind the main one */
TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);
- if (tbmpre == NULL || tbmpre->blockno != tbmres->blockno)+ if (tbmpre == NULL || tbmpre->blockno != blockno)
elog(ERROR, "prefetch and main iterators are out of sync");
}
return;
--
2.40.1
v16-0006-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch (text/x-diff, 3.1K) download
From d5b31a8d8e73fc332748856e3bcb52654d3b134e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Tue, 13 Feb 2024 10:17:47 -0500
Subject: [PATCH v16 06/18] Reduce scope of BitmapHeapScan tbmiterator local
variables
A future patch will change where tbmiterators are initialized and saved.
To simplify that diff, move them into a tighter scope.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 20 +++++++++-----------
1 file changed, 9 insertions(+), 11 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 795a893035..8403be84f3 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -71,8 +71,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterator *tbmiterator = NULL;- TBMSharedIterator *shared_tbmiterator = NULL;
TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -85,10 +83,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- if (pstate == NULL)- tbmiterator = node->tbmiterator;- else- shared_tbmiterator = node->shared_tbmiterator;
tbmres = node->tbmres;
/*
@@ -105,6 +99,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ TBMIterator *tbmiterator = NULL;+ TBMSharedIterator *shared_tbmiterator = NULL;+
if (!pstate)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -113,7 +110,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
elog(ERROR, "unrecognized result from subplan");
node->tbm = tbm;
- node->tbmiterator = tbmiterator = tbm_begin_iterate(tbm);+ tbmiterator = tbm_begin_iterate(tbm);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -166,8 +163,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
/* Allocate a private iterator and attach the shared state to it */
- node->shared_tbmiterator = shared_tbmiterator =- tbm_attach_shared_iterate(dsa, pstate->tbmiterator);+ shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
@@ -207,6 +203,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
+ node->tbmiterator = tbmiterator;+ node->shared_tbmiterator = shared_tbmiterator;
node->initialized = true;
}
@@ -222,9 +220,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (tbmres == NULL)
{
if (!pstate)
- node->tbmres = tbmres = tbm_iterate(tbmiterator);+ node->tbmres = tbmres = tbm_iterate(node->tbmiterator);
else
- node->tbmres = tbmres = tbm_shared_iterate(shared_tbmiterator);+ node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
--
2.40.1
v16-0007-table_scan_bitmap_next_block-counts-lossy-and-ex.patch (text/x-diff, 5.1K) download
From b0043ddcc0a4d18746f80aa90c3b6dc32304b92f Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 26 Feb 2024 20:34:07 -0500
Subject: [PATCH v16 07/18] table_scan_bitmap_next_block counts lossy and exact
pages
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So the table AM must keep
track of whether or not individual TBMIterateResult's blocks were
represented lossily in the bitmap for the purposes of EXPLAIN counters.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 8 +++++++-
src/backend/executor/nodeBitmapHeapscan.c | 13 +++----------
src/include/access/tableam.h | 22 ++++++++++++++++------
3 files changed, 26 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex 929b2cf8e7..b5ab104ec2 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2188,7 +2188,8 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres)+ TBMIterateResult *tbmres,+ long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2316,6 +2317,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ if (tbmres->ntuples < 0)+ (*lossy_pages)++;+ else+ (*exact_pages)++;+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 8403be84f3..68f9ded168 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -210,8 +210,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;-
CHECK_FOR_INTERRUPTS();
/*
@@ -231,19 +229,14 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);-- if (tbmres->ntuples >= 0)- node->exact_pages++;- else- node->lossy_pages++;-- if (!valid)+ if (!table_scan_bitmap_next_block(scan, tbmres,+ &node->lossy_pages, &node->exact_pages))
{
/* AM doesn't think this block is valid, skip */
continue;
}
+
/* Adjust the prefetch target */
BitmapAdjustPrefetchTarget(node);
}
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 55f1139757..e2ad8d0728 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -789,6 +789,9 @@ typedef struct TableAmRoutine
* on the page have to be returned, otherwise the tuples at offsets in
* `tbmres->offsets` need to be returned.
*
+ * lossy_pages is incremented if the bitmap is lossy for the selected+ * block; otherwise, exact_pages is incremented.+ *
* XXX: Currently this may only be implemented if the AM uses md.c as its
* storage manager, and uses ItemPointer->ip_blkid in a manner that maps
* blockids directly to the underlying storage. nodeBitmapHeapscan.c
@@ -804,7 +807,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres);+ struct TBMIterateResult *tbmres,+ long *lossy_pages, long *exact_pages);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1968,17 +1972,21 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of- * a bitmap table scan. `scan` needs to have been started via+ * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of a+ * bitmap table scan. `scan` needs to have been started via
* table_beginscan_bm(). Returns false if there are no tuples to be found on
- * the page, true otherwise.+ * the page, true otherwise. lossy_pages is incremented is the block's+ * representation in the bitmap is lossy; otherwise, exact_pages is+ * incremented.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres)+ struct TBMIterateResult *tbmres,+ long *lossy_pages,+ long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1989,7 +1997,9 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres);+ tbmres,+ lossy_pages,+ exact_pages);
}
/*
--
2.40.1
v16-0008-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch (text/x-diff, 4.1K) download
From faa5a08e645c7635a38e3e31a58c0aed10418c6b Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 12 Feb 2024 18:13:41 -0500
Subject: [PATCH v16 08/18] Remove table_scan_bitmap_next_tuple parameter
tbmres
Future commits will push all of the logic for choosing the next block
into the table AM specific code. The table AM will be responsible for
iterating and reading in the right blocks.
Thus, it no longer makes sense to use the TBMIterateResult (which
contains a block number) as a means of communication between
table_scan_bitmap_next_tuple() and table_scan_bitmap_next_block().
Note that this parameter was unused by heap AM's implementation of
table_scan_bitmap_next_tuple().
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 2 +-
src/include/access/tableam.h | 12 +-----------
3 files changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex b5ab104ec2..efd1a66a09 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2327,7 +2327,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 68f9ded168..951a98c101 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -280,7 +280,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* Attempt to fetch tuple from AM.
*/
- if (!table_scan_bitmap_next_tuple(scan, tbmres, slot))+ if (!table_scan_bitmap_next_tuple(scan, slot))
{
/* nothing more to look at on this page */
node->tbmres = tbmres = NULL;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex e2ad8d0728..2efa97f602 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -780,10 +780,7 @@ typedef struct TableAmRoutine
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
- * make sense to perform tuple visibility checks at this time). For some- * AMs it will make more sense to do all the work referencing `tbmres`- * contents here, for others it might be better to defer more work to- * scan_bitmap_next_tuple.+ * make sense to perform tuple visibility checks at this time).
*
* If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples
* on the page have to be returned, otherwise the tuples at offsets in
@@ -814,15 +811,10 @@ typedef struct TableAmRoutine
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * For some AMs it will make more sense to do all the work referencing- * `tbmres` contents in scan_bitmap_next_block, for others it might be- * better to defer more work to this callback.- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot);
/*
@@ -2012,7 +2004,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2024,7 +2015,6 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- tbmres,
slot);
}
--
2.40.1
v16-0009-Make-table_scan_bitmap_next_block-async-friendly.patch (text/x-diff, 22.4K) download
From f22e1320d1746afad905f169dcf4002a9f164f7e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Thu, 14 Mar 2024 12:39:28 -0400
Subject: [PATCH v16 09/18] Make table_scan_bitmap_next_block() async friendly
table_scan_bitmap_next_block() previously returned false if we did not
wish to call table_scan_bitmap_next_tuple() on the tuples on the page.
This could happen when there were no visible tuples on the page or, due
to concurrent activity on the table, the block returned by the iterator
is past the end of the table recorded when the scan started.
This forced the caller to be responsible for determining if additional
blocks should be fetched and then for invoking
table_scan_bitmap_next_block() for these blocks.
It makes more sense for table_scan_bitmap_next_block() to be responsible
for finding a block that is not past the end of the table (as of the
time that the scan began) and for table_scan_bitmap_next_tuple() to
return false if there are no visible tuples on the page.
This also allows us to move responsibility for the iterator to table AM
specific code. This means handling invalid blocks is entirely up to the
table AM.
Author: Melanie Plageman
Reviewed-by: Tomas Vondra, Andres Freund, Heikki Linnakangas
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 58 +++++--
src/backend/executor/nodeBitmapHeapscan.c | 186 ++++++++++------------
src/include/access/relscan.h | 7 +
src/include/access/tableam.h | 68 +++++---
src/include/nodes/execnodes.h | 12 +-
5 files changed, 190 insertions(+), 141 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex efd1a66a09..9d3e7c7fda 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2188,18 +2188,52 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres,+ BlockNumber *blockno, bool *recheck,
long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
- BlockNumber block = tbmres->blockno;+ BlockNumber block;
Buffer buffer;
Snapshot snapshot;
int ntup;
+ TBMIterateResult *tbmres;
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
+ *blockno = InvalidBlockNumber;+ *recheck = true;++ do+ {+ CHECK_FOR_INTERRUPTS();++ if (scan->shared_tbmiterator)+ tbmres = tbm_shared_iterate(scan->shared_tbmiterator);+ else+ tbmres = tbm_iterate(scan->tbmiterator);++ if (tbmres == NULL)+ {+ /* no more entries in the bitmap */+ Assert(hscan->rs_empty_tuples_pending == 0);+ return false;+ }++ /*+ * Ignore any claimed entries past what we think is the end of the+ * relation. It may have been extended after the start of our scan (we+ * only hold an AccessShareLock, and it could be inserts from this+ * backend). We don't take this optimization in SERIALIZABLE+ * isolation though, as we need to examine all invisible tuples+ * reachable by the index.+ */+ } while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks);++ /* Got a valid block */+ *blockno = tbmres->blockno;+ *recheck = tbmres->recheck;+
/*
* We can skip fetching the heap page if we don't need any fields from the
* heap, the bitmap entries don't need rechecking, and all tuples on the
@@ -2218,16 +2252,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
return true;
}
- /*- * Ignore any claimed entries past what we think is the end of the- * relation. It may have been extended after the start of our scan (we- * only hold an AccessShareLock, and it could be inserts from this- * backend). We don't take this optimization in SERIALIZABLE isolation- * though, as we need to examine all invisible tuples reachable by the- * index.- */- if (!IsolationIsSerializable() && block >= hscan->rs_nblocks)- return false;+ block = tbmres->blockno;
/*
* Acquire pin on the target heap page, trading in any pin we held before.
@@ -2322,7 +2347,14 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
else
(*exact_pages)++;
- return ntup > 0;+ /*+ * Return true to indicate that a valid block was found and the bitmap is+ * not exhausted. If there are no visible tuples on this page,+ * hscan->rs_ntuples will be 0 and heapam_scan_bitmap_next_tuple() will+ * return false returning control to this function to advance to the next+ * block in the bitmap.+ */+ return true;
}
static bool
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 951a98c101..e1b13ddaa6 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -51,8 +51,7 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
-static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,- BlockNumber blockno);+static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);
static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
@@ -71,7 +70,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
ExprContext *econtext;
TableScanDesc scan;
TIDBitmap *tbm;
- TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
@@ -83,7 +81,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
tbm = node->tbm;
- tbmres = node->tbmres;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
@@ -111,7 +108,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->tbm = tbm;
tbmiterator = tbm_begin_iterate(tbm);
- node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
@@ -164,7 +160,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
/* Allocate a private iterator and attach the shared state to it */
shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);
- node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
@@ -203,48 +198,19 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- node->tbmiterator = tbmiterator;- node->shared_tbmiterator = shared_tbmiterator;+ scan->tbmiterator = tbmiterator;+ scan->shared_tbmiterator = shared_tbmiterator;+
node->initialized = true;
++ goto new_page;
}
for (;;)
{
- CHECK_FOR_INTERRUPTS();-- /*- * Get next page of results if needed- */- if (tbmres == NULL)- {- if (!pstate)- node->tbmres = tbmres = tbm_iterate(node->tbmiterator);- else- node->tbmres = tbmres = tbm_shared_iterate(node->shared_tbmiterator);- if (tbmres == NULL)- {- /* no more entries in the bitmap */- break;- }-- BitmapAdjustPrefetchIterator(node, tbmres->blockno);-- if (!table_scan_bitmap_next_block(scan, tbmres,- &node->lossy_pages, &node->exact_pages))- {- /* AM doesn't think this block is valid, skip */- continue;- }--- /* Adjust the prefetch target */- BitmapAdjustPrefetchTarget(node);- }- else+ while (table_scan_bitmap_next_tuple(scan, slot))
{
- /*- * Continuing in previously obtained page.- */+ CHECK_FOR_INTERRUPTS();
#ifdef USE_PREFETCH
@@ -266,45 +232,56 @@ BitmapHeapNext(BitmapHeapScanState *node)
SpinLockRelease(&pstate->mutex);
}
#endif /* USE_PREFETCH */
- }- /*- * We issue prefetch requests *after* fetching the current page to try- * to avoid having prefetching interfere with the main I/O. Also, this- * should happen only when we have determined there is still something- * to do on the current page, else we may uselessly prefetch the same- * page we are just about to request for real.- */- BitmapPrefetch(node, scan);+ /*+ * We issue prefetch requests *after* fetching the current page to+ * try to avoid having prefetching interfere with the main I/O.+ * Also, this should happen only when we have determined there is+ * still something to do on the current page, else we may+ * uselessly prefetch the same page we are just about to request+ * for real.+ */+ BitmapPrefetch(node, scan);- /*- * Attempt to fetch tuple from AM.- */- if (!table_scan_bitmap_next_tuple(scan, slot))- {- /* nothing more to look at on this page */- node->tbmres = tbmres = NULL;- continue;+ /*+ * If we are using lossy info, we have to recheck the qual+ * conditions at every tuple.+ */+ if (node->recheck)+ {+ econtext->ecxt_scantuple = slot;+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))+ {+ /* Fails recheck, so drop it and loop back for another */+ InstrCountFiltered2(node, 1);+ ExecClearTuple(slot);+ continue;+ }+ }++ /* OK to return this tuple */+ return slot;
}
+new_page:++ BitmapAdjustPrefetchIterator(node);++ if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck,+ &node->lossy_pages, &node->exact_pages))+ break;+
/*
- * If we are using lossy info, we have to recheck the qual conditions- * at every tuple.+ * If serial, we can error out if the the prefetch block doesn't stay+ * ahead of the current block.
*/
- if (tbmres->recheck)- {- econtext->ecxt_scantuple = slot;- if (!ExecQualAndReset(node->bitmapqualorig, econtext))- {- /* Fails recheck, so drop it and loop back for another */- InstrCountFiltered2(node, 1);- ExecClearTuple(slot);- continue;- }- }+ if (node->pstate == NULL &&+ node->prefetch_iterator &&+ node->pfblockno < node->blockno)+ elog(ERROR, "prefetch and main iterators are out of sync");- /* OK to return this tuple */- return slot;+ /* Adjust the prefetch target */+ BitmapAdjustPrefetchTarget(node);
}
/*
@@ -330,13 +307,17 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
/*
* BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
+ *+ * We keep track of how far the prefetch iterator is ahead of the main+ * iterator in prefetch_pages. For each block the main iterator returns, we+ * decrement prefetch_pages.
*/
static inline void
-BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,- BlockNumber blockno)+BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ TBMIterateResult *tbmpre;
if (pstate == NULL)
{
@@ -350,14 +331,17 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
else if (prefetch_iterator)
{
/* Do not let the prefetch iterator get behind the main one */
- TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);-- if (tbmpre == NULL || tbmpre->blockno != blockno)- elog(ERROR, "prefetch and main iterators are out of sync");+ tbmpre = tbm_iterate(prefetch_iterator);+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
return;
}
+ /*+ * Adjusting the prefetch iterator before invoking+ * table_scan_bitmap_next_block() keeps prefetch distance higher across+ * the parallel workers.+ */
if (node->prefetch_maximum > 0)
{
TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
@@ -382,7 +366,10 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node,
* case.
*/
if (prefetch_iterator)
- tbm_shared_iterate(prefetch_iterator);+ {+ tbmpre = tbm_shared_iterate(prefetch_iterator);+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;+ }
}
}
#endif /* USE_PREFETCH */
@@ -460,6 +447,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
node->prefetch_pages++;
+ node->pfblockno = tbmpre->blockno;
/*
* If we expect not to have to actually read this heap page,
@@ -517,6 +505,8 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
+ node->pfblockno = tbmpre->blockno;+
/* As above, skip prefetch if we expect not to need page */
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
!tbmpre->recheck &&
@@ -578,12 +568,8 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
- if (node->tbmiterator)- tbm_end_iterate(node->tbmiterator);
if (node->prefetch_iterator)
tbm_end_iterate(node->prefetch_iterator);
- if (node->shared_tbmiterator)- tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->tbm)
@@ -591,13 +577,13 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
- node->tbmiterator = NULL;- node->tbmres = NULL;
node->prefetch_iterator = NULL;
node->initialized = false;
- node->shared_tbmiterator = NULL;
node->shared_prefetch_iterator = NULL;
node->pvmbuffer = InvalidBuffer;
+ node->recheck = true;+ node->blockno = InvalidBlockNumber;+ node->pfblockno = InvalidBlockNumber;
ExecScanReScan(&node->ss);
@@ -628,28 +614,24 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
*/
ExecEndNode(outerPlanState(node));
++ /*+ * close heap scan+ */+ if (scanDesc)+ table_endscan(scanDesc);+
/*
* release bitmaps and buffers if any
*/
- if (node->tbmiterator)- tbm_end_iterate(node->tbmiterator);
if (node->prefetch_iterator)
tbm_end_iterate(node->prefetch_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->shared_tbmiterator)- tbm_end_shared_iterate(node->shared_tbmiterator);
if (node->shared_prefetch_iterator)
tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
-- /*- * close heap scan- */- if (scanDesc)- table_endscan(scanDesc);-
}
/* ----------------------------------------------------------------
@@ -682,8 +664,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ps.ExecProcNode = ExecBitmapHeapScan;
scanstate->tbm = NULL;
- scanstate->tbmiterator = NULL;- scanstate->tbmres = NULL;
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
@@ -691,9 +671,11 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->prefetch_pages = 0;
scanstate->prefetch_target = 0;
scanstate->initialized = false;
- scanstate->shared_tbmiterator = NULL;
scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
+ scanstate->recheck = true;+ scanstate->blockno = InvalidBlockNumber;+ scanstate->pfblockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.hindex 521043304a..92b829cebc 100644--- a/src/include/access/relscan.h+++ b/src/include/access/relscan.h@@ -24,6 +24,9 @@
struct ParallelTableScanDescData;
+struct TBMIterator;+struct TBMSharedIterator;+
/*
* Generic descriptor for table scans. This is the base-class for table scans,
* which needs to be embedded in the scans of individual AMs.
@@ -40,6 +43,10 @@ typedef struct TableScanDescData
ItemPointerData rs_mintid;
ItemPointerData rs_maxtid;
+ /* Only used for Bitmap table scans */+ struct TBMIterator *tbmiterator;+ struct TBMSharedIterator *shared_tbmiterator;+
/*
* Information about type and behaviour of the scan, a bitmask of members
* of the ScanOptions enum (see tableam.h).
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 2efa97f602..42c67a128e 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -22,6 +22,7 @@
#include "access/xact.h"
#include "commands/vacuum.h"
#include "executor/tuptable.h"
+#include "nodes/tidbitmap.h"
#include "utils/rel.h"
#include "utils/snapshot.h"
@@ -773,19 +774,14 @@ typedef struct TableAmRoutine
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part- * of a bitmap table scan. `scan` was started via table_beginscan_bm().- * Return false if there are no tuples to be found on the page, true- * otherwise.+ * Prepare to fetch / check / return tuples from `blockno` as part of a+ * bitmap table scan. `scan` was started via table_beginscan_bm(). Return+ * false if the bitmap is exhausted and true otherwise.
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
* make sense to perform tuple visibility checks at this time).
*
- * If `tbmres->blockno` is -1, this is a lossy scan and all visible tuples- * on the page have to be returned, otherwise the tuples at offsets in- * `tbmres->offsets` need to be returned.- *
* lossy_pages is incremented if the bitmap is lossy for the selected
* block; otherwise, exact_pages is incremented.
*
@@ -804,7 +800,7 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,+ BlockNumber *blockno, bool *recheck,
long *lossy_pages, long *exact_pages);
/*
@@ -942,9 +938,13 @@ static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
+ TableScanDesc result;
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);+ result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);+ result->shared_tbmiterator = NULL;+ result->tbmiterator = NULL;+ return result;
}
/*
@@ -991,6 +991,21 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN)+ {+ if (scan->shared_tbmiterator)+ {+ tbm_end_shared_iterate(scan->shared_tbmiterator);+ scan->shared_tbmiterator = NULL;+ }++ if (scan->tbmiterator)+ {+ tbm_end_iterate(scan->tbmiterator);+ scan->tbmiterator = NULL;+ }+ }+
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1001,6 +1016,21 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN)+ {+ if (scan->shared_tbmiterator)+ {+ tbm_end_shared_iterate(scan->shared_tbmiterator);+ scan->shared_tbmiterator = NULL;+ }++ if (scan->tbmiterator)+ {+ tbm_end_iterate(scan->tbmiterator);+ scan->tbmiterator = NULL;+ }+ }+
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
@@ -1964,19 +1994,18 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples from `tbmres->blockno` as part of a- * bitmap table scan. `scan` needs to have been started via- * table_beginscan_bm(). Returns false if there are no tuples to be found on- * the page, true otherwise. lossy_pages is incremented is the block's- * representation in the bitmap is lossy; otherwise, exact_pages is- * incremented.+ * Prepare to fetch / check / return tuples as part of a bitmap table scan.+ * `scan` needs to have been started via table_beginscan_bm(). Returns false if+ * there are no more blocks in the bitmap, true otherwise. lossy_pages is+ * incremented is the block's representation in the bitmap is lossy; otherwise,+ * exact_pages is incremented.
*
* Note, this is an optionally implemented function, therefore should only be
* used after verifying the presence (at plan time or such).
*/
static inline bool
table_scan_bitmap_next_block(TableScanDesc scan,
- struct TBMIterateResult *tbmres,+ BlockNumber *blockno, bool *recheck,
long *lossy_pages,
long *exact_pages)
{
@@ -1989,9 +2018,8 @@ table_scan_bitmap_next_block(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,
- tbmres,- lossy_pages,- exact_pages);+ blockno, recheck,+ lossy_pages, exact_pages);
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.hindex fa2f70b7a4..d96703b04d 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -1792,8 +1792,6 @@ typedef struct ParallelBitmapHeapState
*
* bitmapqualorig execution state for bitmapqualorig expressions
* tbm bitmap obtained from child index scan(s)
- * tbmiterator iterator for scanning current pages- * tbmres current-page data
* pvmbuffer buffer for visibility-map lookups of prefetched pages
* exact_pages total number of exact pages retrieved
* lossy_pages total number of lossy pages retrieved
@@ -1802,9 +1800,11 @@ typedef struct ParallelBitmapHeapState
* prefetch_target current target prefetch distance
* prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
- * shared_tbmiterator shared iterator
* shared_prefetch_iterator shared iterator for prefetching
* pstate shared state for parallel bitmap scan
+ * recheck do current page's tuples need recheck+ * blockno used to validate pf and current block in sync+ * pfblockno used to validate pf stays ahead of current block
* ----------------
*/
typedef struct BitmapHeapScanState
@@ -1812,8 +1812,6 @@ typedef struct BitmapHeapScanState
ScanState ss; /* its first field is NodeTag */
ExprState *bitmapqualorig;
TIDBitmap *tbm;
- TBMIterator *tbmiterator;- TBMIterateResult *tbmres;
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
@@ -1822,9 +1820,11 @@ typedef struct BitmapHeapScanState
int prefetch_target;
int prefetch_maximum;
bool initialized;
- TBMSharedIterator *shared_tbmiterator;
TBMSharedIterator *shared_prefetch_iterator;
ParallelBitmapHeapState *pstate;
+ bool recheck;+ BlockNumber blockno;+ BlockNumber pfblockno;
} BitmapHeapScanState;
/* ----------------
--
2.40.1
v16-0010-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch (text/x-diff, 16.7K) download
From afb28afbf45efbdd9fa4992a3dc1f1fe7f07afd3 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Mon, 25 Mar 2024 11:05:51 -0400
Subject: [PATCH v16 10/18] Unify parallel and serial BitmapHeapScan iterator
interfaces
Introduce a new type, BitmapHeapIterator, which allows unified access to both
TBMIterator and TBMSharedIterators. This encapsulates the parallel and serial
iterators and their access and makes the bitmap heap scan code a bit cleaner.
This naturally lends itself to a bit of reorganization of the
!node->initialized path in BitmapHeapNext().
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 5 +-
src/backend/executor/nodeBitmapHeapscan.c | 172 ++++++++++++----------
src/include/access/relscan.h | 7 +-
src/include/access/tableam.h | 40 +----
src/include/executor/nodeBitmapHeapscan.h | 7 +
src/include/nodes/execnodes.h | 13 +-
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 124 insertions(+), 121 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex 9d3e7c7fda..5e57d19d91 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2208,10 +2208,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
{
CHECK_FOR_INTERRUPTS();
- if (scan->shared_tbmiterator)- tbmres = tbm_shared_iterate(scan->shared_tbmiterator);- else- tbmres = tbm_iterate(scan->tbmiterator);+ tbmres = bhs_iterate(&scan->rs_bhs_iterator);
if (tbmres == NULL)
{
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex e1b13ddaa6..70b560b8ee 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -57,6 +57,61 @@ static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
+/*+ * Start iteration on a shared or non-shared bitmap iterator. Note that tbm+ * will only be provided by serial BitmapHeapScan callers. dsa and dsp will+ * only be provided by parallel BitmapHeapScan callers.+ */+void+bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,+ dsa_area *dsa, dsa_pointer dsp)+{+ Assert(iterator);++ iterator->serial = NULL;+ iterator->parallel = NULL;+ iterator->exhausted = false;++ /* Allocate a private iterator and attach the shared state to it */+ if (DsaPointerIsValid(dsp))+ iterator->parallel = tbm_attach_shared_iterate(dsa, dsp);+ else+ iterator->serial = tbm_begin_iterate(tbm);+}++/*+ * Get the next TBMIterateResult from the bitmap iterator.+ */+TBMIterateResult *+bhs_iterate(BitmapHeapIterator *iterator)+{+ Assert(iterator);+ Assert(!iterator->exhausted);++ if (iterator->serial)+ return tbm_iterate(iterator->serial);+ else+ return tbm_shared_iterate(iterator->parallel);+}++/*+ * Clean up the bitmap iterator.+ */+void+bhs_end_iterate(BitmapHeapIterator *iterator)+{+ Assert(iterator);++ if (iterator->serial)+ tbm_end_iterate(iterator->serial);+ else if (iterator->parallel)+ tbm_end_shared_iterate(iterator->parallel);++ iterator->serial = NULL;+ iterator->parallel = NULL;+ iterator->exhausted = true;+}+
/* ----------------------------------------------------------------
* BitmapHeapNext
@@ -96,43 +151,23 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
- TBMIterator *tbmiterator = NULL;- TBMSharedIterator *shared_tbmiterator = NULL;+ /*+ * The leader will immediately come out of the function, but others+ * will be blocked until leader populates the TBM and wakes them up.+ */+ bool init_shared_state = node->pstate ?+ BitmapShouldInitializeSharedState(node->pstate) : false;- if (!pstate)+ if (!pstate || init_shared_state)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
if (!tbm || !IsA(tbm, TIDBitmap))
elog(ERROR, "unrecognized result from subplan");
-
node->tbm = tbm;
- tbmiterator = tbm_begin_iterate(tbm);-#ifdef USE_PREFETCH- if (node->prefetch_maximum > 0)+ if (init_shared_state)
{
- node->prefetch_iterator = tbm_begin_iterate(tbm);- node->prefetch_pages = 0;- node->prefetch_target = -1;- }-#endif /* USE_PREFETCH */- }- else- {- /*- * The leader will immediately come out of the function, but- * others will be blocked until leader populates the TBM and wakes- * them up.- */- if (BitmapShouldInitializeSharedState(pstate))- {- tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));- if (!tbm || !IsA(tbm, TIDBitmap))- elog(ERROR, "unrecognized result from subplan");-- node->tbm = tbm;-
/*
* Prepare to iterate over the TBM. This will return the
* dsa_pointer of the iterator state which will be used by
@@ -153,21 +188,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
pstate->prefetch_target = -1;
}
#endif
-
/* We have initialized the shared state so wake up others. */
BitmapDoneInitializingSharedState(pstate);
}
-- /* Allocate a private iterator and attach the shared state to it */- shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator);--#ifdef USE_PREFETCH- if (node->prefetch_maximum > 0)- {- node->shared_prefetch_iterator =- tbm_attach_shared_iterate(dsa, pstate->prefetch_iterator);- }-#endif /* USE_PREFETCH */
}
/*
@@ -198,8 +221,23 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- scan->tbmiterator = tbmiterator;- scan->shared_tbmiterator = shared_tbmiterator;+ bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa,+ pstate ?+ pstate->tbmiterator :+ InvalidDsaPointer);++#ifdef USE_PREFETCH+ if (node->prefetch_maximum > 0)+ {+ bhs_begin_iterate(&node->pf_iterator, tbm, dsa,+ pstate ?+ pstate->prefetch_iterator :+ InvalidDsaPointer);+ /* Only used for serial BHS */+ node->prefetch_pages = 0;+ node->prefetch_target = -1;+ }+#endif /* USE_PREFETCH */
node->initialized = true;
@@ -276,7 +314,7 @@ new_page:
* ahead of the current block.
*/
if (node->pstate == NULL &&
- node->prefetch_iterator &&+ !node->pf_iterator.exhausted &&
node->pfblockno < node->blockno)
elog(ERROR, "prefetch and main iterators are out of sync");
@@ -317,21 +355,20 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
TBMIterateResult *tbmpre;
if (pstate == NULL)
{
- TBMIterator *prefetch_iterator = node->prefetch_iterator;-
if (node->prefetch_pages > 0)
{
/* The main iterator has closed the distance by one page */
node->prefetch_pages--;
}
- else if (prefetch_iterator)+ else if (!prefetch_iterator->exhausted)
{
/* Do not let the prefetch iterator get behind the main one */
- tbmpre = tbm_iterate(prefetch_iterator);+ tbmpre = bhs_iterate(prefetch_iterator);
node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
return;
@@ -344,8 +381,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
*/
if (node->prefetch_maximum > 0)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;-
SpinLockAcquire(&pstate->mutex);
if (pstate->prefetch_pages > 0)
{
@@ -365,9 +400,9 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
* we don't validate the blockno here as we do in non-parallel
* case.
*/
- if (prefetch_iterator)+ if (!prefetch_iterator->exhausted)
{
- tbmpre = tbm_shared_iterate(prefetch_iterator);+ tbmpre = bhs_iterate(prefetch_iterator);
node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
}
@@ -427,23 +462,21 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
{
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;
if (pstate == NULL)
{
- TBMIterator *prefetch_iterator = node->prefetch_iterator;-- if (prefetch_iterator)+ if (!prefetch_iterator->exhausted)
{
while (node->prefetch_pages < node->prefetch_target)
{
- TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator);+ TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
bool skip_fetch;
if (tbmpre == NULL)
{
/* No more pages to prefetch */
- tbm_end_iterate(prefetch_iterator);- node->prefetch_iterator = NULL;+ bhs_end_iterate(prefetch_iterator);
break;
}
node->prefetch_pages++;
@@ -471,9 +504,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (pstate->prefetch_pages < pstate->prefetch_target)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;-- if (prefetch_iterator)+ if (!prefetch_iterator->exhausted)
{
while (1)
{
@@ -496,12 +527,11 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (!do_prefetch)
return;
- tbmpre = tbm_shared_iterate(prefetch_iterator);+ tbmpre = bhs_iterate(prefetch_iterator);
if (tbmpre == NULL)
{
/* No more pages to prefetch */
- tbm_end_shared_iterate(prefetch_iterator);- node->shared_prefetch_iterator = NULL;+ bhs_end_iterate(prefetch_iterator);
break;
}
@@ -568,18 +598,14 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
table_rescan(node->ss.ss_currentScanDesc, NULL);
/* release bitmaps and buffers if any */
- if (node->prefetch_iterator)- tbm_end_iterate(node->prefetch_iterator);- if (node->shared_prefetch_iterator)- tbm_end_shared_iterate(node->shared_prefetch_iterator);+ if (node->pf_iterator.exhausted)+ bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
- node->prefetch_iterator = NULL;
node->initialized = false;
- node->shared_prefetch_iterator = NULL;
node->pvmbuffer = InvalidBuffer;
node->recheck = true;
node->blockno = InvalidBlockNumber;
@@ -624,12 +650,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* release bitmaps and buffers if any
*/
- if (node->prefetch_iterator)- tbm_end_iterate(node->prefetch_iterator);+ if (!node->pf_iterator.exhausted)+ bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->shared_prefetch_iterator)- tbm_end_shared_iterate(node->shared_prefetch_iterator);
if (node->pvmbuffer != InvalidBuffer)
ReleaseBuffer(node->pvmbuffer);
}
@@ -667,11 +691,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
- scanstate->prefetch_iterator = NULL;
scanstate->prefetch_pages = 0;
scanstate->prefetch_target = 0;
scanstate->initialized = false;
- scanstate->shared_prefetch_iterator = NULL;
scanstate->pstate = NULL;
scanstate->recheck = true;
scanstate->blockno = InvalidBlockNumber;
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.hindex 92b829cebc..e520186b41 100644--- a/src/include/access/relscan.h+++ b/src/include/access/relscan.h@@ -20,13 +20,11 @@
#include "storage/buf.h"
#include "storage/spin.h"
#include "utils/relcache.h"
+#include "executor/nodeBitmapHeapscan.h"
struct ParallelTableScanDescData;
-struct TBMIterator;-struct TBMSharedIterator;-
/*
* Generic descriptor for table scans. This is the base-class for table scans,
* which needs to be embedded in the scans of individual AMs.
@@ -44,8 +42,7 @@ typedef struct TableScanDescData
ItemPointerData rs_maxtid;
/* Only used for Bitmap table scans */
- struct TBMIterator *tbmiterator;- struct TBMSharedIterator *shared_tbmiterator;+ BitmapHeapIterator rs_bhs_iterator;
/*
* Information about type and behaviour of the scan, a bitmask of members
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 42c67a128e..618ab2b449 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -938,13 +938,9 @@ static inline TableScanDesc
table_beginscan_bm(Relation rel, Snapshot snapshot,
int nkeys, struct ScanKeyData *key, uint32 extra_flags)
{
- TableScanDesc result;
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);- result->shared_tbmiterator = NULL;- result->tbmiterator = NULL;- return result;+ return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
}
/*
@@ -991,20 +987,9 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN)- {- if (scan->shared_tbmiterator)- {- tbm_end_shared_iterate(scan->shared_tbmiterator);- scan->shared_tbmiterator = NULL;- }-- if (scan->tbmiterator)- {- tbm_end_iterate(scan->tbmiterator);- scan->tbmiterator = NULL;- }- }+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&+ !scan->rs_bhs_iterator.exhausted)+ bhs_end_iterate(&scan->rs_bhs_iterator);
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1016,20 +1001,9 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN)- {- if (scan->shared_tbmiterator)- {- tbm_end_shared_iterate(scan->shared_tbmiterator);- scan->shared_tbmiterator = NULL;- }-- if (scan->tbmiterator)- {- tbm_end_iterate(scan->tbmiterator);- scan->tbmiterator = NULL;- }- }+ if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&+ !scan->rs_bhs_iterator.exhausted)+ bhs_end_iterate(&scan->rs_bhs_iterator);
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.hindex ea003a9caa..7064f54686 100644--- a/src/include/executor/nodeBitmapHeapscan.h+++ b/src/include/executor/nodeBitmapHeapscan.h@@ -29,4 +29,11 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
ParallelWorkerContext *pwcxt);
+extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);+extern void bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,+ dsa_area *dsa, dsa_pointer dsp);++extern void bhs_end_iterate(BitmapHeapIterator *iterator);++
#endif /* NODEBITMAPHEAPSCAN_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.hindex d96703b04d..714eb3e534 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -1787,6 +1787,13 @@ typedef struct ParallelBitmapHeapState
ConditionVariable cv;
} ParallelBitmapHeapState;
+typedef struct BitmapHeapIterator+{+ struct TBMIterator *serial;+ struct TBMSharedIterator *parallel;+ bool exhausted;+} BitmapHeapIterator;+
/* ----------------
* BitmapHeapScanState information
*
@@ -1795,12 +1802,11 @@ typedef struct ParallelBitmapHeapState
* pvmbuffer buffer for visibility-map lookups of prefetched pages
* exact_pages total number of exact pages retrieved
* lossy_pages total number of lossy pages retrieved
- * prefetch_iterator iterator for prefetching ahead of current page+ * pf_iterator for prefetching ahead of current page
* prefetch_pages # pages prefetch iterator is ahead of current
* prefetch_target current target prefetch distance
* prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
- * shared_prefetch_iterator shared iterator for prefetching
* pstate shared state for parallel bitmap scan
* recheck do current page's tuples need recheck
* blockno used to validate pf and current block in sync
@@ -1815,12 +1821,11 @@ typedef struct BitmapHeapScanState
Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
- TBMIterator *prefetch_iterator;
int prefetch_pages;
int prefetch_target;
int prefetch_maximum;
bool initialized;
- TBMSharedIterator *shared_prefetch_iterator;+ BitmapHeapIterator pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
BlockNumber blockno;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.listindex 01845ee71d..848ce9b30c 100644--- a/src/tools/pgindent/typedefs.list+++ b/src/tools/pgindent/typedefs.list@@ -259,6 +259,7 @@ BitString
BitmapAnd
BitmapAndPath
BitmapAndState
+BitmapHeapIterator
BitmapHeapPath
BitmapHeapScan
BitmapHeapScanState
--
2.40.1
v16-0011-Add-BitmapHeapScanDesc-and-scan-functions.patch (text/x-diff, 18.0K) download
From eac4e11efc7c19091b8076f6193b90aa3989b19e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 19:30:43 -0400
Subject: [PATCH v16 11/18] Add BitmapHeapScanDesc and scan functions
Future commits will push all prefetching related code into the heap AM
to eliminate a layering violation. Move all of the members used to
manage prefetching for bitmap heap scan from the BitmapHeapScanState
into a new scan descriptor BitmapHeapScanDesc.
With this new scan descriptor, add all of the relevant functions for
beginning, rescanning, and ending a bitmap heap scan. There is a lot of
bitmap-specific logic to do there now, so it makes sense for bitmap heap
scan to have its own.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 98 +++++++++++++++++++----
src/backend/access/heap/heapam_handler.c | 42 +++++-----
src/backend/executor/nodeBitmapHeapscan.c | 47 +++++------
src/include/access/heapam.h | 36 ++++++---
src/include/access/tableam.h | 45 +++++++++--
src/tools/pgindent/typedefs.list | 1 +
6 files changed, 193 insertions(+), 76 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex 10f2faaa60..82faf447e9 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -13,8 +13,11 @@
*
* INTERFACE ROUTINES
* heap_beginscan - begin relation scan
+ * heap_beginscan_bm - begin bitmap heap scan
* heap_rescan - restart a relation scan
+ * heap_rescan_bm - restart a bitmap heap scan
* heap_endscan - end relation scan
+ * heap_endscan_bm - end a bitmap heap scan
* heap_getnext - retrieve next tuple in scan
* heap_fetch - retrieve tuple with given tid
* heap_insert - insert tuple into a relation
@@ -938,6 +941,43 @@ continue_page:
* ----------------------------------------------------------------
*/
+TableScanDesc+heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags)+{+ BitmapHeapScanDesc scan;++ /*+ * increment relation ref count while scanning relation+ *+ * This is just to make really sure the relcache entry won't go away while+ * the scan has a pointer to it. Caller should be holding the rel open+ * anyway, so this is redundant in all normal scenarios...+ */+ RelationIncrementReferenceCount(relation);+ scan = (BitmapHeapScanDesc) palloc(sizeof(BitmapHeapScanDescData));++ scan->heap_common.rs_base.rs_rd = relation;+ scan->heap_common.rs_base.rs_snapshot = snapshot;+ scan->heap_common.rs_base.rs_nkeys = 0;+ scan->heap_common.rs_base.rs_flags = flags;+ scan->heap_common.rs_base.rs_parallel = NULL;+ scan->heap_common.rs_strategy = NULL;++ Assert(snapshot && IsMVCCSnapshot(snapshot));++ /* we only need to set this up once */+ scan->heap_common.rs_ctup.t_tableOid = RelationGetRelid(relation);++ scan->heap_common.rs_parallelworkerdata = NULL;+ scan->heap_common.rs_base.rs_key = NULL;++ initscan(&scan->heap_common, NULL, false);++ scan->vmbuffer = InvalidBuffer;++ return (TableScanDesc) scan;+}+
TableScanDesc
heap_beginscan(Relation relation, Snapshot snapshot,
@@ -967,8 +1007,6 @@ heap_beginscan(Relation relation, Snapshot snapshot,
scan->rs_base.rs_flags = flags;
scan->rs_base.rs_parallel = parallel_scan;
scan->rs_strategy = NULL; /* set in initscan */
- scan->rs_vmbuffer = InvalidBuffer;- scan->rs_empty_tuples_pending = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1026,6 +1064,29 @@ heap_beginscan(Relation relation, Snapshot snapshot,
return (TableScanDesc) scan;
}
+/*+ * Cleanup BitmapHeapScan table state+ */+void+heap_endscan_bm(TableScanDesc sscan)+{+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;++ if (BufferIsValid(scan->heap_common.rs_cbuf))+ ReleaseBuffer(scan->heap_common.rs_cbuf);++ if (BufferIsValid(scan->vmbuffer))+ ReleaseBuffer(scan->vmbuffer);++ /*+ * decrement relation reference count and free scan descriptor storage+ */+ RelationDecrementReferenceCount(scan->heap_common.rs_base.rs_rd);++ pfree(scan);+}++
void
heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
bool allow_strat, bool allow_sync, bool allow_pagemode)
@@ -1057,12 +1118,6 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
- if (BufferIsValid(scan->rs_vmbuffer))- {- ReleaseBuffer(scan->rs_vmbuffer);- scan->rs_vmbuffer = InvalidBuffer;- }-
/*
* reinitialize scan descriptor
*/
@@ -1082,12 +1137,6 @@ heap_endscan(TableScanDesc sscan)
if (BufferIsValid(scan->rs_cbuf))
ReleaseBuffer(scan->rs_cbuf);
- if (BufferIsValid(scan->rs_vmbuffer))- {- ReleaseBuffer(scan->rs_vmbuffer);- scan->rs_vmbuffer = InvalidBuffer;- }-
/*
* decrement relation reference count and free scan descriptor storage
*/
@@ -1334,6 +1383,27 @@ heap_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction,
return true;
}
+void+heap_rescan_bm(TableScanDesc sscan)+{+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;++ if (BufferIsValid(scan->heap_common.rs_cbuf))+ ReleaseBuffer(scan->heap_common.rs_cbuf);++ if (BufferIsValid(scan->vmbuffer))+ ReleaseBuffer(scan->vmbuffer);+ scan->vmbuffer = InvalidBuffer;++ scan->empty_tuples_pending = 0;++ /*+ * reinitialize heap scan descriptor+ */+ initscan(&scan->heap_common, NULL, true);+}++
/*
* heap_fetch - retrieve tuple with given tid
*
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex 5e57d19d91..43721cbd42 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2187,11 +2187,12 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
*/
static bool
-heapam_scan_bitmap_next_block(TableScanDesc scan,+heapam_scan_bitmap_next_block(TableScanDesc sscan,
BlockNumber *blockno, bool *recheck,
long *lossy_pages, long *exact_pages)
{
- HeapScanDesc hscan = (HeapScanDesc) scan;+ BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;+ HeapScanDesc hscan = &scan->heap_common;
BlockNumber block;
Buffer buffer;
Snapshot snapshot;
@@ -2208,12 +2209,12 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
{
CHECK_FOR_INTERRUPTS();
- tbmres = bhs_iterate(&scan->rs_bhs_iterator);+ tbmres = bhs_iterate(&sscan->rs_bhs_iterator);
if (tbmres == NULL)
{
/* no more entries in the bitmap */
- Assert(hscan->rs_empty_tuples_pending == 0);+ Assert(scan->empty_tuples_pending == 0);
return false;
}
@@ -2236,15 +2237,15 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* heap, the bitmap entries don't need rechecking, and all tuples on the
* page are visible to our transaction.
*/
- if (!(scan->rs_flags & SO_NEED_TUPLE) &&+ if (!(hscan->rs_base.rs_flags & SO_NEED_TUPLE) &&
!tbmres->recheck &&
- VM_ALL_VISIBLE(scan->rs_rd, tbmres->blockno, &hscan->rs_vmbuffer))+ VM_ALL_VISIBLE(hscan->rs_base.rs_rd, tbmres->blockno, &scan->vmbuffer))
{
/* can't be lossy in the skip_fetch case */
Assert(tbmres->ntuples >= 0);
- Assert(hscan->rs_empty_tuples_pending >= 0);+ Assert(scan->empty_tuples_pending >= 0);- hscan->rs_empty_tuples_pending += tbmres->ntuples;+ scan->empty_tuples_pending += tbmres->ntuples;
return true;
}
@@ -2255,18 +2256,18 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* Acquire pin on the target heap page, trading in any pin we held before.
*/
hscan->rs_cbuf = ReleaseAndReadBuffer(hscan->rs_cbuf,
- scan->rs_rd,+ hscan->rs_base.rs_rd,
block);
hscan->rs_cblock = block;
buffer = hscan->rs_cbuf;
- snapshot = scan->rs_snapshot;+ snapshot = hscan->rs_base.rs_snapshot;
ntup = 0;
/*
* Prune and repair fragmentation for the whole page, if possible.
*/
- heap_page_prune_opt(scan->rs_rd, buffer);+ heap_page_prune_opt(hscan->rs_base.rs_rd, buffer);
/*
* We must hold share lock on the buffer content while examining tuple
@@ -2294,7 +2295,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
HeapTupleData heapTuple;
ItemPointerSet(&tid, block, offnum);
- if (heap_hot_search_buffer(&tid, scan->rs_rd, buffer, snapshot,+ if (heap_hot_search_buffer(&tid, hscan->rs_base.rs_rd, buffer, snapshot,
&heapTuple, NULL, true))
hscan->rs_vistuples[ntup++] = ItemPointerGetOffsetNumber(&tid);
}
@@ -2320,16 +2321,16 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
continue;
loctup.t_data = (HeapTupleHeader) PageGetItem(page, lp);
loctup.t_len = ItemIdGetLength(lp);
- loctup.t_tableOid = scan->rs_rd->rd_id;+ loctup.t_tableOid = hscan->rs_base.rs_rd->rd_id;
ItemPointerSet(&loctup.t_self, block, offnum);
valid = HeapTupleSatisfiesVisibility(&loctup, snapshot, buffer);
if (valid)
{
hscan->rs_vistuples[ntup++] = offnum;
- PredicateLockTID(scan->rs_rd, &loctup.t_self, snapshot,+ PredicateLockTID(hscan->rs_base.rs_rd, &loctup.t_self, snapshot,
HeapTupleHeaderGetXmin(loctup.t_data));
}
- HeapCheckForSerializableConflictOut(valid, scan->rs_rd, &loctup,+ HeapCheckForSerializableConflictOut(valid, hscan->rs_base.rs_rd, &loctup,
buffer, snapshot);
}
}
@@ -2358,18 +2359,19 @@ static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
TupleTableSlot *slot)
{
- HeapScanDesc hscan = (HeapScanDesc) scan;+ BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;+ HeapScanDesc hscan = &bscan->heap_common;
OffsetNumber targoffset;
Page page;
ItemId lp;
- if (hscan->rs_empty_tuples_pending > 0)+ if (bscan->empty_tuples_pending > 0)
{
/*
* If we don't have to fetch the tuple, just return nulls.
*/
ExecStoreAllNullTuple(slot);
- hscan->rs_empty_tuples_pending--;+ bscan->empty_tuples_pending--;
return true;
}
@@ -2704,6 +2706,10 @@ static const TableAmRoutine heapam_methods = {
.scan_set_tidrange = heap_set_tidrange,
.scan_getnextslot_tidrange = heap_getnextslot_tidrange,
+ .scan_rescan_bm = heap_rescan_bm,+ .scan_begin_bm = heap_beginscan_bm,+ .scan_end_bm = heap_endscan_bm,+
.parallelscan_estimate = table_block_parallelscan_estimate,
.parallelscan_initialize = table_block_parallelscan_initialize,
.parallelscan_reinitialize = table_block_parallelscan_reinitialize,
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 70b560b8ee..0b28c46d08 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -151,6 +151,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ uint32 extra_flags = 0;+
/*
* The leader will immediately come out of the function, but others
* will be blocked until leader populates the TBM and wakes them up.
@@ -193,33 +195,26 @@ BitmapHeapNext(BitmapHeapScanState *node)
}
}
+ /*+ * We can potentially skip fetching heap pages if we do not need any+ * columns of the table, either for checking non-indexable quals or+ * for returning data. This test is a bit simplistic, as it checks+ * the stronger condition that there's no qual or return tlist at all.+ * But in most cases it's probably not worth working harder than that.+ */+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)+ extra_flags |= SO_NEED_TUPLE;+
/*
* If this is the first scan of the underlying table, create the table
* scan descriptor and begin the scan.
*/
- if (!scan)- {- uint32 extra_flags = 0;+ scan = table_beginscan_bm(node->ss.ss_currentScanDesc,+ node->ss.ss_currentRelation,+ node->ss.ps.state->es_snapshot,+ extra_flags);- /*- * We can potentially skip fetching heap pages if we do not need- * any columns of the table, either for checking non-indexable- * quals or for returning data. This test is a bit simplistic, as- * it checks the stronger condition that there's no qual or return- * tlist at all. But in most cases it's probably not worth working- * harder than that.- */- if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)- extra_flags |= SO_NEED_TUPLE;-- scan = table_beginscan_bm(node->ss.ss_currentRelation,- node->ss.ps.state->es_snapshot,- 0,- NULL,- extra_flags);-- node->ss.ss_currentScanDesc = scan;- }+ node->ss.ss_currentScanDesc = scan;
bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa,
pstate ?
@@ -593,10 +588,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
{
PlanState *outerPlan = outerPlanState(node);
- /* rescan to release any page pin */- if (node->ss.ss_currentScanDesc)- table_rescan(node->ss.ss_currentScanDesc, NULL);-
/* release bitmaps and buffers if any */
if (node->pf_iterator.exhausted)
bhs_end_iterate(&node->pf_iterator);
@@ -642,10 +633,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
- * close heap scan+ * close bitmapheap scan
*/
if (scanDesc)
- table_endscan(scanDesc);+ table_endscan_bm(scanDesc);
/*
* release bitmaps and buffers if any
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex 750ea30852..545760aab5 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -76,22 +76,35 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /* these fields only used in page-at-a-time mode and for bitmap scans */+ int rs_cindex; /* current tuple's index in vistuples */+ int rs_ntuples; /* number of visible tuples on page */+ OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */+} HeapScanDescData;+typedef struct HeapScanDescData *HeapScanDesc;+++typedef struct BitmapHeapScanDescData+{+ /* All the non-BitmapHeapScan specific members */+ HeapScanDescData heap_common;++ /*+ * Members common to Parallel and Serial BitmapHeapScan+ */+
/*
* These fields are only used for bitmap scans for the "skip fetch"
* optimization. Bitmap scans needing no fields from the heap may skip
* fetching an all visible block, instead using the number of tuples per
* block reported by the bitmap to determine how many NULL-filled tuples
- * to return.+ * to return. They are common to parallel and serial BitmapHeapScans
*/
- Buffer rs_vmbuffer;- int rs_empty_tuples_pending;+ Buffer vmbuffer;+ int empty_tuples_pending;+} BitmapHeapScanDescData;- /* these fields only used in page-at-a-time mode and for bitmap scans */- int rs_cindex; /* current tuple's index in vistuples */- int rs_ntuples; /* number of visible tuples on page */- OffsetNumber rs_vistuples[MaxHeapTuplesPerPage]; /* their offsets */-} HeapScanDescData;-typedef struct HeapScanDescData *HeapScanDesc;+typedef struct BitmapHeapScanDescData *BitmapHeapScanDesc;
/*
* Descriptor for fetches from heap via an index.
@@ -289,6 +302,11 @@ extern void heap_set_tidrange(TableScanDesc sscan, ItemPointer mintid,
extern bool heap_getnextslot_tidrange(TableScanDesc sscan,
ScanDirection direction,
TupleTableSlot *slot);
+extern TableScanDesc heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags);++extern void heap_endscan_bm(TableScanDesc scan);+extern void heap_rescan_bm(TableScanDesc sscan);+
extern bool heap_fetch(Relation relation, Snapshot snapshot,
HeapTuple tuple, Buffer *userbuf, bool keep_buf);
extern bool heap_hot_search_buffer(ItemPointer tid, Relation relation,
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 618ab2b449..3b0fa0610b 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -391,6 +391,21 @@ typedef struct TableAmRoutine
ScanDirection direction,
TupleTableSlot *slot);
+ /*+ * TODO: add comment+ */+ TableScanDesc (*scan_begin_bm) (Relation rel,+ Snapshot snapshot,+ uint32 flags);++ void (*scan_rescan_bm) (TableScanDesc scan);++ /*+ * Release resources and deallocate scan. If TableScanDesc.temp_snap,+ * TableScanDesc.rs_snapshot needs to be unregistered.+ */+ void (*scan_end_bm) (TableScanDesc scan);+
/* ------------------------------------------------------------------------
* Parallel table scan related functions.
* ------------------------------------------------------------------------
@@ -929,18 +944,34 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
}
/*
- * table_beginscan_bm is an alternative entry point for setting up a- * TableScanDesc for a bitmap heap scan. Although that scan technology is- * really quite unlike a standard seqscan, there is just enough commonality to- * make it worth using the same data structure.+ * table_beginscan_bm is the entry point for setting up a TableScanDesc for a+ * bitmap heap scan.
*/
static inline TableScanDesc
-table_beginscan_bm(Relation rel, Snapshot snapshot,- int nkeys, struct ScanKeyData *key, uint32 extra_flags)+table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,+ uint32 extra_flags)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
- return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);+ /*+ * If this is the first scan of the underlying table, create the table+ * scan descriptor and begin the scan.+ */+ if (!scan)+ scan = rel->rd_tableam->scan_begin_bm(rel, snapshot, flags);++ scan->rs_rd->rd_tableam->scan_rescan_bm(scan);++ return scan;+}++/*+ * End Bitmap Table Scan+ */+static inline void+table_endscan_bm(TableScanDesc scan)+{+ scan->rs_rd->rd_tableam->scan_end_bm(scan);
}
/*
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.listindex 848ce9b30c..8369c15a82 100644--- a/src/tools/pgindent/typedefs.list+++ b/src/tools/pgindent/typedefs.list@@ -263,6 +263,7 @@ BitmapHeapIterator
BitmapHeapPath
BitmapHeapScan
BitmapHeapScanState
+BitmapHeapScanDesc
BitmapIndexScan
BitmapIndexScanState
BitmapOr
--
2.40.1
v16-0012-BitmapHeapScan-initialize-some-prefetch-state-el.patch (text/x-diff, 2.5K) download
From ad2395adba28db8d1bbf0f87c64bf619367d48b9 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 17:38:36 -0400
Subject: [PATCH v16 12/18] BitmapHeapScan initialize some prefetch state
elsewhere
These members can be initialized elsewhere. This makes the diff more
straightforward for moving those members from BitmapHeapScanState to
BitmapHeapScanDesc.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 17 +++++------------
1 file changed, 5 insertions(+), 12 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 0b28c46d08..4f7138bbd9 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -181,13 +181,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
pstate->prefetch_iterator =
tbm_prepare_shared_iterate(tbm);
-- /*- * We don't need the mutex here as we haven't yet woke up- * others.- */- pstate->prefetch_pages = 0;- pstate->prefetch_target = -1;
}
#endif
/* We have initialized the shared state so wake up others. */
@@ -228,9 +221,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
pstate ?
pstate->prefetch_iterator :
InvalidDsaPointer);
- /* Only used for serial BHS */- node->prefetch_pages = 0;- node->prefetch_target = -1;
}
#endif /* USE_PREFETCH */
@@ -601,6 +591,9 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
node->recheck = true;
node->blockno = InvalidBlockNumber;
node->pfblockno = InvalidBlockNumber;
+ /* Only used for serial BHS */+ node->prefetch_pages = 0;+ node->prefetch_target = -1;
ExecScanReScan(&node->ss);
@@ -683,7 +676,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
scanstate->prefetch_pages = 0;
- scanstate->prefetch_target = 0;+ scanstate->prefetch_target = -1;
scanstate->initialized = false;
scanstate->pstate = NULL;
scanstate->recheck = true;
@@ -818,7 +811,7 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
/* Initialize the mutex */
SpinLockInit(&pstate->mutex);
pstate->prefetch_pages = 0;
- pstate->prefetch_target = 0;+ pstate->prefetch_target = -1;
pstate->state = BM_INITIAL;
ConditionVariableInit(&pstate->cv);
--
2.40.1
v16-0013-BitmapHeapScan-begin-iteration-in-rescan.patch (text/x-diff, 4.3K) download
From d16551c7330822ccd0132565773ceab7569aa6b5 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 17:48:08 -0400
Subject: [PATCH v16 13/18] BitmapHeapScan begin iteration in rescan
Now that BitmapHeapScan has its own dedicated rescan function, it is
easy to begin iteration there instead. Doing this in heap AM code will
allow us to move the current block iterator from the TableScanDesc to
the BitmapHeapScanDesc.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 8 +++++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 ++++------
src/include/access/heapam.h | 3 ++-
src/include/access/tableam.h | 8 +++++---
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex 82faf447e9..a876a34b74 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -1384,7 +1384,8 @@ heap_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction,
}
void
-heap_rescan_bm(TableScanDesc sscan)+heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,+ ParallelBitmapHeapState *pstate, dsa_area *dsa)
{
BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
@@ -1401,6 +1402,11 @@ heap_rescan_bm(TableScanDesc sscan)
* reinitialize heap scan descriptor
*/
initscan(&scan->heap_common, NULL, true);
++ bhs_begin_iterate(&scan->heap_common.rs_base.rs_bhs_iterator, tbm, dsa,+ pstate ?+ pstate->tbmiterator :+ InvalidDsaPointer);
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 4f7138bbd9..f8627991d0 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -205,15 +205,13 @@ BitmapHeapNext(BitmapHeapScanState *node)
scan = table_beginscan_bm(node->ss.ss_currentScanDesc,
node->ss.ss_currentRelation,
node->ss.ps.state->es_snapshot,
- extra_flags);+ extra_flags,+ node->tbm,+ node->pstate,+ node->ss.ps.state->es_query_dsa);
node->ss.ss_currentScanDesc = scan;
- bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa,- pstate ?- pstate->tbmiterator :- InvalidDsaPointer);-
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex 545760aab5..7b44c94d29 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -305,7 +305,8 @@ extern bool heap_getnextslot_tidrange(TableScanDesc sscan,
extern TableScanDesc heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags);
extern void heap_endscan_bm(TableScanDesc scan);
-extern void heap_rescan_bm(TableScanDesc sscan);+extern void heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,+ ParallelBitmapHeapState *pstate, dsa_area *dsa);
extern bool heap_fetch(Relation relation, Snapshot snapshot,
HeapTuple tuple, Buffer *userbuf, bool keep_buf);
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 3b0fa0610b..d02f5055b0 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -398,7 +398,8 @@ typedef struct TableAmRoutine
Snapshot snapshot,
uint32 flags);
- void (*scan_rescan_bm) (TableScanDesc scan);+ void (*scan_rescan_bm) (TableScanDesc scan, TIDBitmap *tbm,+ ParallelBitmapHeapState *pstate, dsa_area *dsa);
/*
* Release resources and deallocate scan. If TableScanDesc.temp_snap,
@@ -949,7 +950,8 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,
- uint32 extra_flags)+ uint32 extra_flags, TIDBitmap *tbm,+ ParallelBitmapHeapState *pstate, dsa_area *dsa)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
@@ -960,7 +962,7 @@ table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,
if (!scan)
scan = rel->rd_tableam->scan_begin_bm(rel, snapshot, flags);
- scan->rs_rd->rd_tableam->scan_rescan_bm(scan);+ scan->rs_rd->rd_tableam->scan_rescan_bm(scan, tbm, pstate, dsa);
return scan;
}
--
2.40.1
v16-0014-Move-BitmapHeapScan-current-block-iterator-to-Bi.patch (text/x-diff, 4.2K) download
From 91b04fc00567fe4395c9820eb95e7ee3e10af2ae Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 18:05:02 -0400
Subject: [PATCH v16 14/18] Move BitmapHeapScan current block iterator to
BitmapHeapScanDesc
After beginning iteration in heap AM code, we can move the current block
iterator into the BitmapHeapScanDesc and take it out of the generic
TableScanDesc.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 9 ++++++++-
src/backend/access/heap/heapam_handler.c | 2 +-
src/include/access/heapam.h | 1 +
src/include/access/relscan.h | 3 ---
src/include/access/tableam.h | 8 --------
5 files changed, 10 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex a876a34b74..c35dc3c4e7 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -973,6 +973,9 @@ heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags)
initscan(&scan->heap_common, NULL, false);
+ scan->iterator.serial = NULL;+ scan->iterator.parallel = NULL;+
scan->vmbuffer = InvalidBuffer;
return (TableScanDesc) scan;
@@ -1078,6 +1081,8 @@ heap_endscan_bm(TableScanDesc sscan)
if (BufferIsValid(scan->vmbuffer))
ReleaseBuffer(scan->vmbuffer);
+ bhs_end_iterate(&scan->iterator);+
/*
* decrement relation reference count and free scan descriptor storage
*/
@@ -1396,6 +1401,8 @@ heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
ReleaseBuffer(scan->vmbuffer);
scan->vmbuffer = InvalidBuffer;
+ bhs_end_iterate(&scan->iterator);+
scan->empty_tuples_pending = 0;
/*
@@ -1403,7 +1410,7 @@ heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
*/
initscan(&scan->heap_common, NULL, true);
- bhs_begin_iterate(&scan->heap_common.rs_base.rs_bhs_iterator, tbm, dsa,+ bhs_begin_iterate(&scan->iterator, tbm, dsa,
pstate ?
pstate->tbmiterator :
InvalidDsaPointer);
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex 43721cbd42..cc32d77100 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2209,7 +2209,7 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
{
CHECK_FOR_INTERRUPTS();
- tbmres = bhs_iterate(&sscan->rs_bhs_iterator);+ tbmres = bhs_iterate(&scan->iterator);
if (tbmres == NULL)
{
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex 7b44c94d29..e33e4433a7 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -92,6 +92,7 @@ typedef struct BitmapHeapScanDescData
/*
* Members common to Parallel and Serial BitmapHeapScan
*/
+ BitmapHeapIterator iterator;
/*
* These fields are only used for bitmap scans for the "skip fetch"
diff --git a/src/include/access/relscan.h b/src/include/access/relscan.hindex e520186b41..855b9558bf 100644--- a/src/include/access/relscan.h+++ b/src/include/access/relscan.h@@ -41,9 +41,6 @@ typedef struct TableScanDescData
ItemPointerData rs_mintid;
ItemPointerData rs_maxtid;
- /* Only used for Bitmap table scans */- BitmapHeapIterator rs_bhs_iterator;-
/*
* Information about type and behaviour of the scan, a bitmask of members
* of the ScanOptions enum (see tableam.h).
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex d02f5055b0..f344d97f1f 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -1020,10 +1020,6 @@ table_beginscan_tid(Relation rel, Snapshot snapshot)
static inline void
table_endscan(TableScanDesc scan)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&- !scan->rs_bhs_iterator.exhausted)- bhs_end_iterate(&scan->rs_bhs_iterator);-
scan->rs_rd->rd_tableam->scan_end(scan);
}
@@ -1034,10 +1030,6 @@ static inline void
table_rescan(TableScanDesc scan,
struct ScanKeyData *key)
{
- if (scan->rs_flags & SO_TYPE_BITMAPSCAN &&- !scan->rs_bhs_iterator.exhausted)- bhs_end_iterate(&scan->rs_bhs_iterator);-
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
}
--
2.40.1
v16-0015-Lift-and-shift-BitmapHeapScan-prefetch-funcs-to-.patch (text/x-diff, 14.6K) download
From 86012b83948098f0c0f512676a275bf5595a1294 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 16:59:31 -0400
Subject: [PATCH v16 15/18] Lift and shift BitmapHeapScan prefetch funcs to
heap AM
We will soon move all the members for managing prefetching from the
BitmapHeapScanState to the BitmapHeapScanDesc. First move the prefetch
functions as-is to heapam_handler.c
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 211 +++++++++++++++++++++
src/backend/executor/nodeBitmapHeapscan.c | 214 +---------------------
src/include/access/heapam.h | 4 +
3 files changed, 216 insertions(+), 213 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex cc32d77100..ebaba33406 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2180,6 +2180,217 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
HEAP_USABLE_BYTES_PER_PAGE);
}
+/*+ * BitmapAdjustPrefetchIterator - Adjust the prefetch iterator+ *+ * We keep track of how far the prefetch iterator is ahead of the main+ * iterator in prefetch_pages. For each block the main iterator returns, we+ * decrement prefetch_pages.+ */+void+BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)+{+#ifdef USE_PREFETCH+ ParallelBitmapHeapState *pstate = node->pstate;+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;+ TBMIterateResult *tbmpre;++ if (pstate == NULL)+ {+ if (node->prefetch_pages > 0)+ {+ /* The main iterator has closed the distance by one page */+ node->prefetch_pages--;+ }+ else if (!prefetch_iterator->exhausted)+ {+ /* Do not let the prefetch iterator get behind the main one */+ tbmpre = bhs_iterate(prefetch_iterator);+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;+ }+ return;+ }++ /*+ * Adjusting the prefetch iterator before invoking+ * table_scan_bitmap_next_block() keeps prefetch distance higher across+ * the parallel workers.+ */+ if (node->prefetch_maximum > 0)+ {+ SpinLockAcquire(&pstate->mutex);+ if (pstate->prefetch_pages > 0)+ {+ pstate->prefetch_pages--;+ SpinLockRelease(&pstate->mutex);+ }+ else+ {+ /* Release the mutex before iterating */+ SpinLockRelease(&pstate->mutex);++ /*+ * In case of shared mode, we can not ensure that the current+ * blockno of the main iterator and that of the prefetch iterator+ * are same. It's possible that whatever blockno we are+ * prefetching will be processed by another process. Therefore,+ * we don't validate the blockno here as we do in non-parallel+ * case.+ */+ if (!prefetch_iterator->exhausted)+ {+ tbmpre = bhs_iterate(prefetch_iterator);+ node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;+ }+ }+ }+#endif /* USE_PREFETCH */+}+++/*+ * BitmapAdjustPrefetchTarget - Adjust the prefetch target+ *+ * Increase prefetch target if it's not yet at the max. Note that+ * we will increase it to zero after fetching the very first+ * page/tuple, then to one after the second tuple is fetched, then+ * it doubles as later pages are fetched.+ */+void+BitmapAdjustPrefetchTarget(BitmapHeapScanState *node)+{+#ifdef USE_PREFETCH+ ParallelBitmapHeapState *pstate = node->pstate;++ if (pstate == NULL)+ {+ if (node->prefetch_target >= node->prefetch_maximum)+ /* don't increase any further */ ;+ else if (node->prefetch_target >= node->prefetch_maximum / 2)+ node->prefetch_target = node->prefetch_maximum;+ else if (node->prefetch_target > 0)+ node->prefetch_target *= 2;+ else+ node->prefetch_target++;+ return;+ }++ /* Do an unlocked check first to save spinlock acquisitions. */+ if (pstate->prefetch_target < node->prefetch_maximum)+ {+ SpinLockAcquire(&pstate->mutex);+ if (pstate->prefetch_target >= node->prefetch_maximum)+ /* don't increase any further */ ;+ else if (pstate->prefetch_target >= node->prefetch_maximum / 2)+ pstate->prefetch_target = node->prefetch_maximum;+ else if (pstate->prefetch_target > 0)+ pstate->prefetch_target *= 2;+ else+ pstate->prefetch_target++;+ SpinLockRelease(&pstate->mutex);+ }+#endif /* USE_PREFETCH */+}++/*+ * BitmapPrefetch - Prefetch, if prefetch_pages are behind prefetch_target+ */+void+BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)+{+#ifdef USE_PREFETCH+ ParallelBitmapHeapState *pstate = node->pstate;+ BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;++ if (pstate == NULL)+ {+ if (!prefetch_iterator->exhausted)+ {+ while (node->prefetch_pages < node->prefetch_target)+ {+ TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);+ bool skip_fetch;++ if (tbmpre == NULL)+ {+ /* No more pages to prefetch */+ bhs_end_iterate(prefetch_iterator);+ break;+ }+ node->prefetch_pages++;+ node->pfblockno = tbmpre->blockno;++ /*+ * If we expect not to have to actually read this heap page,+ * skip this prefetch call, but continue to run the prefetch+ * logic normally. (Would it be better not to increment+ * prefetch_pages?)+ */+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&+ !tbmpre->recheck &&+ VM_ALL_VISIBLE(node->ss.ss_currentRelation,+ tbmpre->blockno,+ &node->pvmbuffer));++ if (!skip_fetch)+ PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);+ }+ }++ return;+ }++ if (pstate->prefetch_pages < pstate->prefetch_target)+ {+ if (!prefetch_iterator->exhausted)+ {+ while (1)+ {+ TBMIterateResult *tbmpre;+ bool do_prefetch = false;+ bool skip_fetch;++ /*+ * Recheck under the mutex. If some other process has already+ * done enough prefetching then we need not to do anything.+ */+ SpinLockAcquire(&pstate->mutex);+ if (pstate->prefetch_pages < pstate->prefetch_target)+ {+ pstate->prefetch_pages++;+ do_prefetch = true;+ }+ SpinLockRelease(&pstate->mutex);++ if (!do_prefetch)+ return;++ tbmpre = bhs_iterate(prefetch_iterator);+ if (tbmpre == NULL)+ {+ /* No more pages to prefetch */+ bhs_end_iterate(prefetch_iterator);+ break;+ }++ node->pfblockno = tbmpre->blockno;++ /* As above, skip prefetch if we expect not to need page */+ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&+ !tbmpre->recheck &&+ VM_ALL_VISIBLE(node->ss.ss_currentRelation,+ tbmpre->blockno,+ &node->pvmbuffer));++ if (!skip_fetch)+ PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);+ }+ }+ }+#endif /* USE_PREFETCH */+}++
/* ------------------------------------------------------------------------
* Executor related callbacks for the heap AM
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex f8627991d0..6f1afd427d 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -40,6 +40,7 @@
#include "access/relscan.h"
#include "access/tableam.h"
#include "access/visibilitymap.h"
+#include "access/heapam.h"
#include "executor/executor.h"
#include "executor/nodeBitmapHeapscan.h"
#include "miscadmin.h"
@@ -51,10 +52,6 @@
static TupleTableSlot *BitmapHeapNext(BitmapHeapScanState *node);
static inline void BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate);
-static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);-static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);-static inline void BitmapPrefetch(BitmapHeapScanState *node,- TableScanDesc scan);
static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
/*
@@ -326,215 +323,6 @@ BitmapDoneInitializingSharedState(ParallelBitmapHeapState *pstate)
ConditionVariableBroadcast(&pstate->cv);
}
-/*- * BitmapAdjustPrefetchIterator - Adjust the prefetch iterator- *- * We keep track of how far the prefetch iterator is ahead of the main- * iterator in prefetch_pages. For each block the main iterator returns, we- * decrement prefetch_pages.- */-static inline void-BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)-{-#ifdef USE_PREFETCH- ParallelBitmapHeapState *pstate = node->pstate;- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;- TBMIterateResult *tbmpre;-- if (pstate == NULL)- {- if (node->prefetch_pages > 0)- {- /* The main iterator has closed the distance by one page */- node->prefetch_pages--;- }- else if (!prefetch_iterator->exhausted)- {- /* Do not let the prefetch iterator get behind the main one */- tbmpre = bhs_iterate(prefetch_iterator);- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;- }- return;- }-- /*- * Adjusting the prefetch iterator before invoking- * table_scan_bitmap_next_block() keeps prefetch distance higher across- * the parallel workers.- */- if (node->prefetch_maximum > 0)- {- SpinLockAcquire(&pstate->mutex);- if (pstate->prefetch_pages > 0)- {- pstate->prefetch_pages--;- SpinLockRelease(&pstate->mutex);- }- else- {- /* Release the mutex before iterating */- SpinLockRelease(&pstate->mutex);-- /*- * In case of shared mode, we can not ensure that the current- * blockno of the main iterator and that of the prefetch iterator- * are same. It's possible that whatever blockno we are- * prefetching will be processed by another process. Therefore,- * we don't validate the blockno here as we do in non-parallel- * case.- */- if (!prefetch_iterator->exhausted)- {- tbmpre = bhs_iterate(prefetch_iterator);- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;- }- }- }-#endif /* USE_PREFETCH */-}--/*- * BitmapAdjustPrefetchTarget - Adjust the prefetch target- *- * Increase prefetch target if it's not yet at the max. Note that- * we will increase it to zero after fetching the very first- * page/tuple, then to one after the second tuple is fetched, then- * it doubles as later pages are fetched.- */-static inline void-BitmapAdjustPrefetchTarget(BitmapHeapScanState *node)-{-#ifdef USE_PREFETCH- ParallelBitmapHeapState *pstate = node->pstate;-- if (pstate == NULL)- {- if (node->prefetch_target >= node->prefetch_maximum)- /* don't increase any further */ ;- else if (node->prefetch_target >= node->prefetch_maximum / 2)- node->prefetch_target = node->prefetch_maximum;- else if (node->prefetch_target > 0)- node->prefetch_target *= 2;- else- node->prefetch_target++;- return;- }-- /* Do an unlocked check first to save spinlock acquisitions. */- if (pstate->prefetch_target < node->prefetch_maximum)- {- SpinLockAcquire(&pstate->mutex);- if (pstate->prefetch_target >= node->prefetch_maximum)- /* don't increase any further */ ;- else if (pstate->prefetch_target >= node->prefetch_maximum / 2)- pstate->prefetch_target = node->prefetch_maximum;- else if (pstate->prefetch_target > 0)- pstate->prefetch_target *= 2;- else- pstate->prefetch_target++;- SpinLockRelease(&pstate->mutex);- }-#endif /* USE_PREFETCH */-}--/*- * BitmapPrefetch - Prefetch, if prefetch_pages are behind prefetch_target- */-static inline void-BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)-{-#ifdef USE_PREFETCH- ParallelBitmapHeapState *pstate = node->pstate;- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;-- if (pstate == NULL)- {- if (!prefetch_iterator->exhausted)- {- while (node->prefetch_pages < node->prefetch_target)- {- TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);- bool skip_fetch;-- if (tbmpre == NULL)- {- /* No more pages to prefetch */- bhs_end_iterate(prefetch_iterator);- break;- }- node->prefetch_pages++;- node->pfblockno = tbmpre->blockno;-- /*- * If we expect not to have to actually read this heap page,- * skip this prefetch call, but continue to run the prefetch- * logic normally. (Would it be better not to increment- * prefetch_pages?)- */- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&- !tbmpre->recheck &&- VM_ALL_VISIBLE(node->ss.ss_currentRelation,- tbmpre->blockno,- &node->pvmbuffer));-- if (!skip_fetch)- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);- }- }-- return;- }-- if (pstate->prefetch_pages < pstate->prefetch_target)- {- if (!prefetch_iterator->exhausted)- {- while (1)- {- TBMIterateResult *tbmpre;- bool do_prefetch = false;- bool skip_fetch;-- /*- * Recheck under the mutex. If some other process has already- * done enough prefetching then we need not to do anything.- */- SpinLockAcquire(&pstate->mutex);- if (pstate->prefetch_pages < pstate->prefetch_target)- {- pstate->prefetch_pages++;- do_prefetch = true;- }- SpinLockRelease(&pstate->mutex);-- if (!do_prefetch)- return;-- tbmpre = bhs_iterate(prefetch_iterator);- if (tbmpre == NULL)- {- /* No more pages to prefetch */- bhs_end_iterate(prefetch_iterator);- break;- }-- node->pfblockno = tbmpre->blockno;-- /* As above, skip prefetch if we expect not to need page */- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&- !tbmpre->recheck &&- VM_ALL_VISIBLE(node->ss.ss_currentRelation,- tbmpre->blockno,- &node->pvmbuffer));-- if (!skip_fetch)- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);- }- }- }-#endif /* USE_PREFETCH */-}-
/*
* BitmapHeapRecheck -- access method routine to recheck a tuple in EvalPlanQual
*/
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex e33e4433a7..f81a295685 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -426,6 +426,10 @@ extern bool heapam_scan_analyze_next_tuple(TableScanDesc scan,
double *liverows, double *deadrows,
TupleTableSlot *slot);
+extern void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);+extern void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);+extern void BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan);+
/*
* To avoid leaking too much knowledge about reorderbuffer implementation
* details this is implemented in reorderbuffer.c not heapam_visibility.c
--
2.40.1
v16-0016-Push-BitmapHeapScan-prefetch-code-into-heap-AM.patch (text/x-diff, 25.3K) download
From 7da12cdb96ae5255c815d657048f7a812094fcfa Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 18:48:27 -0400
Subject: [PATCH v16 16/18] Push BitmapHeapScan prefetch code into heap AM
An earlier commit eliminated a table AM layering violation for the
current block in a BitmapHeapScan but left the violation in
BitmapHeapScan prefetching code.
To resolve this, move and manage all state used for prefetching entirely
into the heap AM.
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 33 +++++-
src/backend/access/heap/heapam_handler.c | 125 +++++++++++++++-------
src/backend/executor/nodeBitmapHeapscan.c | 98 +++--------------
src/include/access/heapam.h | 29 ++++-
src/include/access/tableam.h | 18 +---
src/include/executor/nodeBitmapHeapscan.h | 7 ++
src/include/nodes/execnodes.h | 19 ----
7 files changed, 168 insertions(+), 161 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex c35dc3c4e7..5728b2ea8a 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -975,8 +975,10 @@ heap_beginscan_bm(Relation relation, Snapshot snapshot, uint32 flags)
scan->iterator.serial = NULL;
scan->iterator.parallel = NULL;
-
scan->vmbuffer = InvalidBuffer;
+ scan->pf_iterator.serial = NULL;+ scan->pf_iterator.parallel = NULL;+ scan->pvmbuffer = InvalidBuffer;
return (TableScanDesc) scan;
}
@@ -1081,6 +1083,10 @@ heap_endscan_bm(TableScanDesc sscan)
if (BufferIsValid(scan->vmbuffer))
ReleaseBuffer(scan->vmbuffer);
+ if (BufferIsValid(scan->pvmbuffer))+ ReleaseBuffer(scan->pvmbuffer);++ bhs_end_iterate(&scan->pf_iterator);
bhs_end_iterate(&scan->iterator);
/*
@@ -1390,7 +1396,7 @@ heap_getnextslot_tidrange(TableScanDesc sscan, ScanDirection direction,
void
heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
- ParallelBitmapHeapState *pstate, dsa_area *dsa)+ ParallelBitmapHeapState *pstate, dsa_area *dsa, int pf_maximum)
{
BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
@@ -1401,9 +1407,19 @@ heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
ReleaseBuffer(scan->vmbuffer);
scan->vmbuffer = InvalidBuffer;
+ if (BufferIsValid(scan->pvmbuffer))+ ReleaseBuffer(scan->pvmbuffer);+ scan->pvmbuffer = InvalidBuffer;++ bhs_end_iterate(&scan->pf_iterator);
bhs_end_iterate(&scan->iterator);
+ scan->prefetch_maximum = 0;
scan->empty_tuples_pending = 0;
+ scan->pstate = NULL;+ scan->prefetch_target = -1;+ scan->prefetch_pages = 0;+ scan->pfblockno = InvalidBlockNumber;
/*
* reinitialize heap scan descriptor
@@ -1414,6 +1430,19 @@ heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
pstate ?
pstate->tbmiterator :
InvalidDsaPointer);
++#ifdef USE_PREFETCH+ if (pf_maximum > 0)+ {+ bhs_begin_iterate(&scan->pf_iterator, tbm, dsa,+ pstate ?+ pstate->prefetch_iterator :+ InvalidDsaPointer);+ }+#endif /* USE_PREFETCH */++ scan->pstate = pstate;+ scan->prefetch_maximum = pf_maximum;
}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex ebaba33406..fca9c7233e 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -60,6 +60,9 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
OffsetNumber tupoffset);
static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan);
+static inline void BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan);+static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanDesc scan);+static inline void BitmapPrefetch(BitmapHeapScanDesc scan);
static const TableAmRoutine heapam_methods;
@@ -2187,26 +2190,26 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
* iterator in prefetch_pages. For each block the main iterator returns, we
* decrement prefetch_pages.
*/
-void-BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)+static inline void+BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan)
{
#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;+ BitmapHeapIterator *prefetch_iterator = &scan->pf_iterator;+ ParallelBitmapHeapState *pstate = scan->pstate;
TBMIterateResult *tbmpre;
if (pstate == NULL)
{
- if (node->prefetch_pages > 0)+ if (scan->prefetch_pages > 0)
{
/* The main iterator has closed the distance by one page */
- node->prefetch_pages--;+ scan->prefetch_pages--;
}
else if (!prefetch_iterator->exhausted)
{
/* Do not let the prefetch iterator get behind the main one */
tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;+ scan->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
return;
}
@@ -2216,7 +2219,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
* table_scan_bitmap_next_block() keeps prefetch distance higher across
* the parallel workers.
*/
- if (node->prefetch_maximum > 0)+ if (scan->prefetch_maximum > 0)
{
SpinLockAcquire(&pstate->mutex);
if (pstate->prefetch_pages > 0)
@@ -2240,7 +2243,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
if (!prefetch_iterator->exhausted)
{
tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;+ scan->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
}
}
@@ -2256,33 +2259,34 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
* page/tuple, then to one after the second tuple is fetched, then
* it doubles as later pages are fetched.
*/
-void-BitmapAdjustPrefetchTarget(BitmapHeapScanState *node)+static inline void+BitmapAdjustPrefetchTarget(BitmapHeapScanDesc scan)
{
#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;+ ParallelBitmapHeapState *pstate = scan->pstate;+ int prefetch_maximum = scan->prefetch_maximum;
if (pstate == NULL)
{
- if (node->prefetch_target >= node->prefetch_maximum)+ if (scan->prefetch_target >= prefetch_maximum)
/* don't increase any further */ ;
- else if (node->prefetch_target >= node->prefetch_maximum / 2)- node->prefetch_target = node->prefetch_maximum;- else if (node->prefetch_target > 0)- node->prefetch_target *= 2;+ else if (scan->prefetch_target >= prefetch_maximum / 2)+ scan->prefetch_target = prefetch_maximum;+ else if (scan->prefetch_target > 0)+ scan->prefetch_target *= 2;
else
- node->prefetch_target++;+ scan->prefetch_target++;
return;
}
/* Do an unlocked check first to save spinlock acquisitions. */
- if (pstate->prefetch_target < node->prefetch_maximum)+ if (pstate->prefetch_target < prefetch_maximum)
{
SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_target >= node->prefetch_maximum)+ if (pstate->prefetch_target >= prefetch_maximum)
/* don't increase any further */ ;
- else if (pstate->prefetch_target >= node->prefetch_maximum / 2)- pstate->prefetch_target = node->prefetch_maximum;+ else if (pstate->prefetch_target >= prefetch_maximum / 2)+ pstate->prefetch_target = prefetch_maximum;
else if (pstate->prefetch_target > 0)
pstate->prefetch_target *= 2;
else
@@ -2295,18 +2299,19 @@ BitmapAdjustPrefetchTarget(BitmapHeapScanState *node)
/*
* BitmapPrefetch - Prefetch, if prefetch_pages are behind prefetch_target
*/
-void-BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)+static inline void+BitmapPrefetch(BitmapHeapScanDesc scan)
{
#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = node->pstate;- BitmapHeapIterator *prefetch_iterator = &node->pf_iterator;+ ParallelBitmapHeapState *pstate = scan->pstate;+ BitmapHeapIterator *prefetch_iterator = &scan->pf_iterator;+ Relation rel = scan->heap_common.rs_base.rs_rd;
if (pstate == NULL)
{
if (!prefetch_iterator->exhausted)
{
- while (node->prefetch_pages < node->prefetch_target)+ while (scan->prefetch_pages < scan->prefetch_target)
{
TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
bool skip_fetch;
@@ -2317,8 +2322,8 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
bhs_end_iterate(prefetch_iterator);
break;
}
- node->prefetch_pages++;- node->pfblockno = tbmpre->blockno;+ scan->prefetch_pages++;+ scan->pfblockno = tbmpre->blockno;
/*
* If we expect not to have to actually read this heap page,
@@ -2326,14 +2331,14 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* logic normally. (Would it be better not to increment
* prefetch_pages?)
*/
- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&+ skip_fetch = (!(scan->heap_common.rs_base.rs_flags & SO_NEED_TUPLE) &&
!tbmpre->recheck &&
- VM_ALL_VISIBLE(node->ss.ss_currentRelation,+ VM_ALL_VISIBLE(rel,
tbmpre->blockno,
- &node->pvmbuffer));+ &scan->pvmbuffer));
if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);+ PrefetchBuffer(rel, MAIN_FORKNUM, tbmpre->blockno);
}
}
@@ -2373,17 +2378,17 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
- node->pfblockno = tbmpre->blockno;+ scan->pfblockno = tbmpre->blockno;
/* As above, skip prefetch if we expect not to need page */
- skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&+ skip_fetch = (!(scan->heap_common.rs_base.rs_flags & SO_NEED_TUPLE) &&
!tbmpre->recheck &&
- VM_ALL_VISIBLE(node->ss.ss_currentRelation,+ VM_ALL_VISIBLE(rel,
tbmpre->blockno,
- &node->pvmbuffer));+ &scan->pvmbuffer));
if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);+ PrefetchBuffer(rel, MAIN_FORKNUM, tbmpre->blockno);
}
}
}
@@ -2416,6 +2421,8 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
*blockno = InvalidBlockNumber;
*recheck = true;
+ BitmapAdjustPrefetchIterator(scan);+
do
{
CHECK_FOR_INTERRUPTS();
@@ -2556,6 +2563,18 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
else
(*exact_pages)++;
+ /*+ * If serial, we can error out if the the prefetch block doesn't stay+ * ahead of the current block.+ */+ if (scan->pstate == NULL &&+ !scan->pf_iterator.exhausted &&+ scan->pfblockno < block)+ elog(ERROR, "prefetch and main iterators are out of sync");++ /* Adjust the prefetch target */+ BitmapAdjustPrefetchTarget(scan);+
/*
* Return true to indicate that a valid block was found and the bitmap is
* not exhausted. If there are no visible tuples on this page,
@@ -2592,6 +2611,36 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
return false;
+#ifdef USE_PREFETCH++ /*+ * Try to prefetch at least a few pages even before we get to the second+ * page if we don't stop reading after the first tuple.+ */+ if (!bscan->pstate)+ {+ if (bscan->prefetch_target < bscan->prefetch_maximum)+ bscan->prefetch_target++;+ }+ else if (bscan->pstate->prefetch_target < bscan->prefetch_maximum)+ {+ /* take spinlock while updating shared state */+ SpinLockAcquire(&bscan->pstate->mutex);+ if (bscan->pstate->prefetch_target < bscan->prefetch_maximum)+ bscan->pstate->prefetch_target++;+ SpinLockRelease(&bscan->pstate->mutex);+ }++ /*+ * We issue prefetch requests *after* fetching the current page to try to+ * avoid having prefetching interfere with the main I/O. Also, this should+ * happen only when we have determined there is still something to do on+ * the current page, else we may uselessly prefetch the same page we are+ * just about to request for real.+ */+ BitmapPrefetch(bscan);+#endif /* USE_PREFETCH */+
targoffset = hscan->rs_vistuples[hscan->rs_cindex];
page = BufferGetPage(hscan->rs_cbuf);
lp = PageGetItemId(page, targoffset);
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex 6f1afd427d..ae36dfd773 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -124,7 +124,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
TIDBitmap *tbm;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
- dsa_area *dsa = node->ss.ps.state->es_query_dsa;
/*
* extract necessary information from index scan node
@@ -148,7 +147,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ Relation rel = node->ss.ss_currentRelation;
uint32 extra_flags = 0;
+ bool pf_maximum = 0;
/*
* The leader will immediately come out of the function, but others
@@ -157,6 +158,15 @@ BitmapHeapNext(BitmapHeapScanState *node)
bool init_shared_state = node->pstate ?
BitmapShouldInitializeSharedState(node->pstate) : false;
+ /*+ * Maximum number of prefetches for the tablespace if configured,+ * otherwise the current value of the effective_io_concurrency GUC.+ */+#ifdef USE_PREFETCH+ pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);+#endif++
if (!pstate || init_shared_state)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -174,7 +184,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)+ if (pf_maximum > 0)
{
pstate->prefetch_iterator =
tbm_prepare_shared_iterate(tbm);
@@ -200,25 +210,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
* scan descriptor and begin the scan.
*/
scan = table_beginscan_bm(node->ss.ss_currentScanDesc,
- node->ss.ss_currentRelation,+ rel,
node->ss.ps.state->es_snapshot,
extra_flags,
+ pf_maximum,
node->tbm,
node->pstate,
node->ss.ps.state->es_query_dsa);
node->ss.ss_currentScanDesc = scan;
-#ifdef USE_PREFETCH- if (node->prefetch_maximum > 0)- {- bhs_begin_iterate(&node->pf_iterator, tbm, dsa,- pstate ?- pstate->prefetch_iterator :- InvalidDsaPointer);- }-#endif /* USE_PREFETCH */-
node->initialized = true;
goto new_page;
@@ -230,37 +231,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
CHECK_FOR_INTERRUPTS();
-#ifdef USE_PREFETCH-- /*- * Try to prefetch at least a few pages even before we get to the- * second page if we don't stop reading after the first tuple.- */- if (!pstate)- {- if (node->prefetch_target < node->prefetch_maximum)- node->prefetch_target++;- }- else if (pstate->prefetch_target < node->prefetch_maximum)- {- /* take spinlock while updating shared state */- SpinLockAcquire(&pstate->mutex);- if (pstate->prefetch_target < node->prefetch_maximum)- pstate->prefetch_target++;- SpinLockRelease(&pstate->mutex);- }-#endif /* USE_PREFETCH */-- /*- * We issue prefetch requests *after* fetching the current page to- * try to avoid having prefetching interfere with the main I/O.- * Also, this should happen only when we have determined there is- * still something to do on the current page, else we may- * uselessly prefetch the same page we are just about to request- * for real.- */- BitmapPrefetch(node, scan);-
/*
* If we are using lossy info, we have to recheck the qual
* conditions at every tuple.
@@ -283,23 +253,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
new_page:
- BitmapAdjustPrefetchIterator(node);-
if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck,
&node->lossy_pages, &node->exact_pages))
break;
-- /*- * If serial, we can error out if the the prefetch block doesn't stay- * ahead of the current block.- */- if (node->pstate == NULL &&- !node->pf_iterator.exhausted &&- node->pfblockno < node->blockno)- elog(ERROR, "prefetch and main iterators are out of sync");-- /* Adjust the prefetch target */- BitmapAdjustPrefetchTarget(node);
}
/*
@@ -365,21 +321,12 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
PlanState *outerPlan = outerPlanState(node);
/* release bitmaps and buffers if any */
- if (node->pf_iterator.exhausted)- bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->pvmbuffer != InvalidBuffer)- ReleaseBuffer(node->pvmbuffer);
node->tbm = NULL;
node->initialized = false;
- node->pvmbuffer = InvalidBuffer;
node->recheck = true;
node->blockno = InvalidBlockNumber;
- node->pfblockno = InvalidBlockNumber;- /* Only used for serial BHS */- node->prefetch_pages = 0;- node->prefetch_target = -1;
ExecScanReScan(&node->ss);
@@ -418,14 +365,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
table_endscan_bm(scanDesc);
/*
- * release bitmaps and buffers if any+ * release bitmaps if any
*/
- if (!node->pf_iterator.exhausted)- bhs_end_iterate(&node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->pvmbuffer != InvalidBuffer)- ReleaseBuffer(node->pvmbuffer);
}
/* ----------------------------------------------------------------
@@ -458,16 +401,12 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->ss.ps.ExecProcNode = ExecBitmapHeapScan;
scanstate->tbm = NULL;
- scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
- scanstate->prefetch_pages = 0;- scanstate->prefetch_target = -1;
scanstate->initialized = false;
scanstate->pstate = NULL;
scanstate->recheck = true;
scanstate->blockno = InvalidBlockNumber;
- scanstate->pfblockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
@@ -507,13 +446,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->bitmapqualorig =
ExecInitQual(node->bitmapqualorig, (PlanState *) scanstate);
- /*- * Maximum number of prefetches for the tablespace if configured,- * otherwise the current value of the effective_io_concurrency GUC.- */- scanstate->prefetch_maximum =- get_tablespace_io_concurrency(currentRelation->rd_rel->reltablespace);-
scanstate->ss.ss_currentRelation = currentRelation;
/*
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.hindex f81a295685..499e739da7 100644--- a/src/include/access/heapam.h+++ b/src/include/access/heapam.h@@ -93,6 +93,10 @@ typedef struct BitmapHeapScanDescData
* Members common to Parallel and Serial BitmapHeapScan
*/
BitmapHeapIterator iterator;
+ BitmapHeapIterator pf_iterator;++ /* maximum value for prefetch_target */+ int prefetch_maximum;
/*
* These fields are only used for bitmap scans for the "skip fetch"
@@ -102,7 +106,26 @@ typedef struct BitmapHeapScanDescData
* to return. They are common to parallel and serial BitmapHeapScans
*/
Buffer vmbuffer;
+ /* buffer for visibility-map lookups of prefetched pages */+ Buffer pvmbuffer;
int empty_tuples_pending;
++ /*+ * Parallel-only members+ */++ struct ParallelBitmapHeapState *pstate;++ /*+ * Serial-only members+ */++ /* Current target for prefetch distance */+ int prefetch_target;+ /* # pages prefetch iterator is ahead of current */+ int prefetch_pages;+ /* used to validate prefetch block stays ahead of current block */+ BlockNumber pfblockno;
} BitmapHeapScanDescData;
typedef struct BitmapHeapScanDescData *BitmapHeapScanDesc;
@@ -307,7 +330,7 @@ extern TableScanDesc heap_beginscan_bm(Relation relation, Snapshot snapshot, uin
extern void heap_endscan_bm(TableScanDesc scan);
extern void heap_rescan_bm(TableScanDesc sscan, TIDBitmap *tbm,
- ParallelBitmapHeapState *pstate, dsa_area *dsa);+ ParallelBitmapHeapState *pstate, dsa_area *dsa, int pf_maximum);
extern bool heap_fetch(Relation relation, Snapshot snapshot,
HeapTuple tuple, Buffer *userbuf, bool keep_buf);
@@ -426,10 +449,6 @@ extern bool heapam_scan_analyze_next_tuple(TableScanDesc scan,
double *liverows, double *deadrows,
TupleTableSlot *slot);
-extern void BitmapAdjustPrefetchIterator(BitmapHeapScanState *node);-extern void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);-extern void BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan);-
/*
* To avoid leaking too much knowledge about reorderbuffer implementation
* details this is implemented in reorderbuffer.c not heapam_visibility.c
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex f344d97f1f..9ec0a1da2b 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -399,7 +399,8 @@ typedef struct TableAmRoutine
uint32 flags);
void (*scan_rescan_bm) (TableScanDesc scan, TIDBitmap *tbm,
- ParallelBitmapHeapState *pstate, dsa_area *dsa);+ ParallelBitmapHeapState *pstate, dsa_area *dsa,+ int pf_maximum);
/*
* Release resources and deallocate scan. If TableScanDesc.temp_snap,
@@ -801,17 +802,6 @@ typedef struct TableAmRoutine
* lossy_pages is incremented if the bitmap is lossy for the selected
* block; otherwise, exact_pages is incremented.
*
- * XXX: Currently this may only be implemented if the AM uses md.c as its- * storage manager, and uses ItemPointer->ip_blkid in a manner that maps- * blockids directly to the underlying storage. nodeBitmapHeapscan.c- * performs prefetching directly using that interface. This probably- * needs to be rectified at a later point.- *- * XXX: Currently this may only be implemented if the AM uses the- * visibilitymap, as nodeBitmapHeapscan.c unconditionally accesses it to- * perform prefetching. This probably needs to be rectified at a later- * point.- *
* Optional callback, but either both scan_bitmap_next_block and
* scan_bitmap_next_tuple need to exist, or neither.
*/
@@ -950,7 +940,7 @@ table_beginscan_strat(Relation rel, Snapshot snapshot,
*/
static inline TableScanDesc
table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,
- uint32 extra_flags, TIDBitmap *tbm,+ uint32 extra_flags, int pf_maximum, TIDBitmap *tbm,
ParallelBitmapHeapState *pstate, dsa_area *dsa)
{
uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags;
@@ -962,7 +952,7 @@ table_beginscan_bm(TableScanDesc scan, Relation rel, Snapshot snapshot,
if (!scan)
scan = rel->rd_tableam->scan_begin_bm(rel, snapshot, flags);
- scan->rs_rd->rd_tableam->scan_rescan_bm(scan, tbm, pstate, dsa);+ scan->rs_rd->rd_tableam->scan_rescan_bm(scan, tbm, pstate, dsa, pf_maximum);
return scan;
}
diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.hindex 7064f54686..028081db32 100644--- a/src/include/executor/nodeBitmapHeapscan.h+++ b/src/include/executor/nodeBitmapHeapscan.h@@ -29,6 +29,13 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
ParallelWorkerContext *pwcxt);
+typedef struct BitmapHeapIterator+{+ struct TBMIterator *serial;+ struct TBMSharedIterator *parallel;+ bool exhausted;+} BitmapHeapIterator;+
extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);
extern void bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm,
dsa_area *dsa, dsa_pointer dsp);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.hindex 714eb3e534..713de7d50f 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -1787,30 +1787,17 @@ typedef struct ParallelBitmapHeapState
ConditionVariable cv;
} ParallelBitmapHeapState;
-typedef struct BitmapHeapIterator-{- struct TBMIterator *serial;- struct TBMSharedIterator *parallel;- bool exhausted;-} BitmapHeapIterator;-
/* ----------------
* BitmapHeapScanState information
*
* bitmapqualorig execution state for bitmapqualorig expressions
* tbm bitmap obtained from child index scan(s)
- * pvmbuffer buffer for visibility-map lookups of prefetched pages
* exact_pages total number of exact pages retrieved
* lossy_pages total number of lossy pages retrieved
- * pf_iterator for prefetching ahead of current page- * prefetch_pages # pages prefetch iterator is ahead of current- * prefetch_target current target prefetch distance- * prefetch_maximum maximum value for prefetch_target
* initialized is node is ready to iterate
* pstate shared state for parallel bitmap scan
* recheck do current page's tuples need recheck
* blockno used to validate pf and current block in sync
- * pfblockno used to validate pf stays ahead of current block
* ----------------
*/
typedef struct BitmapHeapScanState
@@ -1818,18 +1805,12 @@ typedef struct BitmapHeapScanState
ScanState ss; /* its first field is NodeTag */
ExprState *bitmapqualorig;
TIDBitmap *tbm;
- Buffer pvmbuffer;
long exact_pages;
long lossy_pages;
- int prefetch_pages;- int prefetch_target;- int prefetch_maximum;
bool initialized;
- BitmapHeapIterator pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
BlockNumber blockno;
- BlockNumber pfblockno;
} BitmapHeapScanState;
/* ----------------
--
2.40.1
v16-0017-Move-BitmapHeapScan-initialization-to-helper.patch (text/x-diff, 8.3K) download
From d66c526696f6bcb9d6aa0a8b234d51c58866a6ff Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 19:02:38 -0400
Subject: [PATCH v16 17/18] Move BitmapHeapScan initialization to helper
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/executor/nodeBitmapHeapscan.c | 182 +++++++++++-----------
src/include/access/tableam.h | 6 +-
2 files changed, 98 insertions(+), 90 deletions(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex ae36dfd773..ce47cd51bd 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -109,6 +109,97 @@ bhs_end_iterate(BitmapHeapIterator *iterator)
iterator->exhausted = true;
}
+ /*+ * If we haven't yet performed the underlying index scan, do it, and begin+ * the iteration over the bitmap.+ *+ * For prefetching, we use *two* iterators, one for the pages we are actually+ * scanning and another that runs ahead of the first for prefetching.+ * node->prefetch_pages tracks exactly how many pages ahead the prefetch+ * iterator is. Also, node->prefetch_target tracks the desired prefetch+ * distance, which starts small and increases up to the+ * scan->prefetch_maximum. This is to avoid doing a lot of prefetching in a+ * scan that stops after a few tuples because of a LIMIT.+ */+static void+BitmapHeapInitialize(BitmapHeapScanState *node)+{+ TIDBitmap *tbm;+ ParallelBitmapHeapState *pstate = node->pstate;+ Relation rel = node->ss.ss_currentRelation;+ uint32 extra_flags = 0;+ bool pf_maximum = 0;++ /*+ * The leader will immediately come out of the function, but others will+ * be blocked until leader populates the TBM and wakes them up.+ */+ bool init_shared_state = node->pstate ?+ BitmapShouldInitializeSharedState(node->pstate) : false;++ /*+ * Maximum number of prefetches for the tablespace if configured,+ * otherwise the current value of the effective_io_concurrency GUC.+ */+#ifdef USE_PREFETCH+ pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);+#endif+++ if (!pstate || init_shared_state)+ {+ tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));++ if (!tbm || !IsA(tbm, TIDBitmap))+ elog(ERROR, "unrecognized result from subplan");+ node->tbm = tbm;++ if (init_shared_state)+ {+ /*+ * Prepare to iterate over the TBM. This will return the+ * dsa_pointer of the iterator state which will be used by+ * multiple processes to iterate jointly.+ */+ pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);+#ifdef USE_PREFETCH+ if (pf_maximum > 0)+ {+ pstate->prefetch_iterator =+ tbm_prepare_shared_iterate(tbm);+ }+#endif+ /* We have initialized the shared state so wake up others. */+ BitmapDoneInitializingSharedState(pstate);+ }+ }++ /*+ * We can potentially skip fetching heap pages if we do not need any+ * columns of the table, either for checking non-indexable quals or for+ * returning data. This test is a bit simplistic, as it checks the+ * stronger condition that there's no qual or return tlist at all. But in+ * most cases it's probably not worth working harder than that.+ */+ if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)+ extra_flags |= SO_NEED_TUPLE;++ /*+ * If this is the first scan of the underlying table, create the table+ * scan descriptor and begin the scan.+ */+ node->ss.ss_currentScanDesc = table_beginscan_bm(node->ss.ss_currentScanDesc,+ rel,+ node->ss.ps.state->es_snapshot,+ extra_flags,+ pf_maximum,+ node->tbm,+ node->pstate,+ node->ss.ps.state->es_query_dsa);+++ node->initialized = true;+}
/* ----------------------------------------------------------------
* BitmapHeapNext
@@ -121,9 +212,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
ExprContext *econtext;
TableScanDesc scan;
- TIDBitmap *tbm;
TupleTableSlot *slot;
- ParallelBitmapHeapState *pstate = node->pstate;
/*
* extract necessary information from index scan node
@@ -131,97 +220,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
econtext = node->ss.ps.ps_ExprContext;
slot = node->ss.ss_ScanTupleSlot;
scan = node->ss.ss_currentScanDesc;
- tbm = node->tbm;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
- * the iteration over the bitmap.- *- * For prefetching, we use *two* iterators, one for the pages we are- * actually scanning and another that runs ahead of the first for- * prefetching. node->prefetch_pages tracks exactly how many pages ahead- * the prefetch iterator is. Also, node->prefetch_target tracks the- * desired prefetch distance, which starts small and increases up to the- * node->prefetch_maximum. This is to avoid doing a lot of prefetching in- * a scan that stops after a few tuples because of a LIMIT.+ * the iteration over the bitmap. This happens on rescan as well.
*/
if (!node->initialized)
{
- Relation rel = node->ss.ss_currentRelation;- uint32 extra_flags = 0;- bool pf_maximum = 0;-- /*- * The leader will immediately come out of the function, but others- * will be blocked until leader populates the TBM and wakes them up.- */- bool init_shared_state = node->pstate ?- BitmapShouldInitializeSharedState(node->pstate) : false;-- /*- * Maximum number of prefetches for the tablespace if configured,- * otherwise the current value of the effective_io_concurrency GUC.- */-#ifdef USE_PREFETCH- pf_maximum = get_tablespace_io_concurrency(rel->rd_rel->reltablespace);-#endif--- if (!pstate || init_shared_state)- {- tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));-- if (!tbm || !IsA(tbm, TIDBitmap))- elog(ERROR, "unrecognized result from subplan");- node->tbm = tbm;-- if (init_shared_state)- {- /*- * Prepare to iterate over the TBM. This will return the- * dsa_pointer of the iterator state which will be used by- * multiple processes to iterate jointly.- */- pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);-#ifdef USE_PREFETCH- if (pf_maximum > 0)- {- pstate->prefetch_iterator =- tbm_prepare_shared_iterate(tbm);- }-#endif- /* We have initialized the shared state so wake up others. */- BitmapDoneInitializingSharedState(pstate);- }- }-- /*- * We can potentially skip fetching heap pages if we do not need any- * columns of the table, either for checking non-indexable quals or- * for returning data. This test is a bit simplistic, as it checks- * the stronger condition that there's no qual or return tlist at all.- * But in most cases it's probably not worth working harder than that.- */- if (node->ss.ps.plan->qual != NIL || node->ss.ps.plan->targetlist != NIL)- extra_flags |= SO_NEED_TUPLE;-- /*- * If this is the first scan of the underlying table, create the table- * scan descriptor and begin the scan.- */- scan = table_beginscan_bm(node->ss.ss_currentScanDesc,- rel,- node->ss.ps.state->es_snapshot,- extra_flags,- pf_maximum,- node->tbm,- node->pstate,- node->ss.ps.state->es_query_dsa);-- node->ss.ss_currentScanDesc = scan;-- node->initialized = true;-+ BitmapHeapInitialize(node);+ /* We may have a new scan descriptor */+ scan = node->ss.ss_currentScanDesc;
goto new_page;
}
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex 9ec0a1da2b..eea7b7ff20 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -791,9 +791,9 @@ typedef struct TableAmRoutine
*/
/*
- * Prepare to fetch / check / return tuples from `blockno` as part of a- * bitmap table scan. `scan` was started via table_beginscan_bm(). Return- * false if the bitmap is exhausted and true otherwise.+ * Prepare to fetch / check / return tuples as part of a bitmap table+ * scan. `scan` was started via table_beginscan_bm(). Return false if the+ * bitmap is exhausted and true otherwise.
*
* This will typically read and pin the target block, and do the necessary
* work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might
--
2.40.1
v16-0018-Remove-table_scan_bitmap_next_block.patch (text/x-diff, 14.3K) download
From 6a2a8b7d147e3c5343512bcf4d1d8389ff71bbe8 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <melanieplageman@gmail.com>
Date: Fri, 5 Apr 2024 18:56:45 -0400
Subject: [PATCH v16 18/18] Remove table_scan_bitmap_next_block()
With several of the changes to the control flow of BitmapHeapNext() in
recent commits, table_scan_bitmap_next_tuple() can be responsible for
getting the next block. Do this and remove the table AM API function
table_scan_bitmap_next_block().
Author: Melanie Plageman
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 2 +
src/backend/access/heap/heapam_handler.c | 53 +++++++++-------
src/backend/access/table/tableamapi.c | 2 -
src/backend/executor/nodeBitmapHeapscan.c | 69 +++++++++------------
src/backend/optimizer/util/plancat.c | 2 +-
src/include/access/tableam.h | 73 +++++------------------
src/include/nodes/execnodes.h | 2 -
7 files changed, 78 insertions(+), 125 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.cindex 5728b2ea8a..3ee4d75f17 100644--- a/src/backend/access/heap/heapam.c+++ b/src/backend/access/heap/heapam.c@@ -327,6 +327,8 @@ initscan(HeapScanDesc scan, ScanKey key, bool keep_startblock)
ItemPointerSetInvalid(&scan->rs_ctup.t_self);
scan->rs_cbuf = InvalidBuffer;
scan->rs_cblock = InvalidBlockNumber;
+ scan->rs_cindex = 0;+ scan->rs_ntuples = 0;
/* page-at-a-time fields are always invalid when not rs_inited */
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.cindex fca9c7233e..a6d75cfaa8 100644--- a/src/backend/access/heap/heapam_handler.c+++ b/src/backend/access/heap/heapam_handler.c@@ -2216,8 +2216,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanDesc scan)
/*
* Adjusting the prefetch iterator before invoking
- * table_scan_bitmap_next_block() keeps prefetch distance higher across- * the parallel workers.+ * heapam_bitmap_next_block() keeps prefetch distance higher across the+ * parallel workers.
*/
if (scan->prefetch_maximum > 0)
{
@@ -2397,14 +2397,12 @@ BitmapPrefetch(BitmapHeapScanDesc scan)-/* ------------------------------------------------------------------------- * Executor related callbacks for the heap AM- * ------------------------------------------------------------------------+/*+ * Helper for heapam_scan_bitmap_next_tuple()
*/
-
static bool
heapam_scan_bitmap_next_block(TableScanDesc sscan,
- BlockNumber *blockno, bool *recheck,+ bool *recheck,
long *lossy_pages, long *exact_pages)
{
BitmapHeapScanDesc scan = (BitmapHeapScanDesc) sscan;
@@ -2418,7 +2416,6 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
- *blockno = InvalidBlockNumber;
*recheck = true;
BitmapAdjustPrefetchIterator(scan);
@@ -2447,7 +2444,6 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
} while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks);
/* Got a valid block */
- *blockno = tbmres->blockno;
*recheck = tbmres->recheck;
/*
@@ -2585,9 +2581,16 @@ heapam_scan_bitmap_next_block(TableScanDesc sscan,
return true;
}
++/* ------------------------------------------------------------------------+ * Executor related callbacks for the heap AM+ * ------------------------------------------------------------------------+ */+
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
- TupleTableSlot *slot)+ TupleTableSlot *slot, bool *recheck,+ long *lossy_pages, long *exact_pages)
{
BitmapHeapScanDesc bscan = (BitmapHeapScanDesc) scan;
HeapScanDesc hscan = &bscan->heap_common;
@@ -2595,21 +2598,28 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
Page page;
ItemId lp;
- if (bscan->empty_tuples_pending > 0)+ /*+ * Out of range? If so, nothing more to look at on this page+ */+ while (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
{
/*
- * If we don't have to fetch the tuple, just return nulls.+ * Emit empty tuples before advancing to the next block
*/
- ExecStoreAllNullTuple(slot);- bscan->empty_tuples_pending--;- return true;- }+ if (bscan->empty_tuples_pending > 0)+ {+ /*+ * If we don't have to fetch the tuple, just return nulls.+ */+ ExecStoreAllNullTuple(slot);+ bscan->empty_tuples_pending--;+ return true;+ }- /*- * Out of range? If so, nothing more to look at on this page- */- if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)- return false;+ if (!heapam_scan_bitmap_next_block(scan, recheck,+ lossy_pages, exact_pages))+ return false;+ }
#ifdef USE_PREFETCH
@@ -3010,7 +3020,6 @@ static const TableAmRoutine heapam_methods = {
.relation_estimate_size = heapam_estimate_rel_size,
- .scan_bitmap_next_block = heapam_scan_bitmap_next_block,
.scan_bitmap_next_tuple = heapam_scan_bitmap_next_tuple,
.scan_sample_next_block = heapam_scan_sample_next_block,
.scan_sample_next_tuple = heapam_scan_sample_next_tuple
diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.cindex 55b8caeadf..0b87530a9c 100644--- a/src/backend/access/table/tableamapi.c+++ b/src/backend/access/table/tableamapi.c@@ -90,8 +90,6 @@ GetTableAmRoutine(Oid amhandler)
Assert(routine->relation_estimate_size != NULL);
/* optional, but one callback implies presence of the other */
- Assert((routine->scan_bitmap_next_block == NULL) ==- (routine->scan_bitmap_next_tuple == NULL));
Assert(routine->scan_sample_next_block != NULL);
Assert(routine->scan_sample_next_tuple != NULL);
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.cindex ce47cd51bd..3ce24156dd 100644--- a/src/backend/executor/nodeBitmapHeapscan.c+++ b/src/backend/executor/nodeBitmapHeapscan.c@@ -211,61 +211,52 @@ static TupleTableSlot *
BitmapHeapNext(BitmapHeapScanState *node)
{
ExprContext *econtext;
- TableScanDesc scan;
TupleTableSlot *slot;
-- /*- * extract necessary information from index scan node- */- econtext = node->ss.ps.ps_ExprContext;- slot = node->ss.ss_ScanTupleSlot;- scan = node->ss.ss_currentScanDesc;+ TableScanDesc scan;
/*
* If we haven't yet performed the underlying index scan, do it, and begin
* the iteration over the bitmap. This happens on rescan as well.
*/
if (!node->initialized)
- {
BitmapHeapInitialize(node);
- /* We may have a new scan descriptor */- scan = node->ss.ss_currentScanDesc;- goto new_page;- }- for (;;)+ /*+ * BitmapHeapInitialize() may make the scan descriptor so don't get it+ * from the node until after calling it.+ */+ econtext = node->ss.ps.ps_ExprContext;+ slot = node->ss.ss_ScanTupleSlot;+ scan = node->ss.ss_currentScanDesc;+++ while (table_scan_bitmap_next_tuple(scan, slot, &node->recheck,+ &node->lossy_pages, &node->exact_pages))
{
- while (table_scan_bitmap_next_tuple(scan, slot))- {- CHECK_FOR_INTERRUPTS();+ CHECK_FOR_INTERRUPTS();- /*- * If we are using lossy info, we have to recheck the qual- * conditions at every tuple.- */- if (node->recheck)++ /*+ * If we are using lossy info, we have to recheck the qual conditions+ * at every tuple.+ */+ if (node->recheck)+ {+ econtext->ecxt_scantuple = slot;+ if (!ExecQualAndReset(node->bitmapqualorig, econtext))
{
- econtext->ecxt_scantuple = slot;- if (!ExecQualAndReset(node->bitmapqualorig, econtext))- {- /* Fails recheck, so drop it and loop back for another */- InstrCountFiltered2(node, 1);- ExecClearTuple(slot);- continue;- }+ /* Fails recheck, so drop it and loop back for another */+ InstrCountFiltered2(node, 1);+ ExecClearTuple(slot);+ continue;
}
-- /* OK to return this tuple */- return slot;
}
-new_page:-- if (!table_scan_bitmap_next_block(scan, &node->blockno, &node->recheck,- &node->lossy_pages, &node->exact_pages))- break;+ /* OK to return this tuple */+ return slot;
}
+
/*
* if we get here it means we are at the end of the scan..
*/
@@ -334,7 +325,6 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
node->tbm = NULL;
node->initialized = false;
node->recheck = true;
- node->blockno = InvalidBlockNumber;
ExecScanReScan(&node->ss);
@@ -414,7 +404,6 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->initialized = false;
scanstate->pstate = NULL;
scanstate->recheck = true;
- scanstate->blockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.cindex 6bb53e4346..cf56cc572f 100644--- a/src/backend/optimizer/util/plancat.c+++ b/src/backend/optimizer/util/plancat.c@@ -313,7 +313,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
info->amcanparallel = amroutine->amcanparallel;
info->amhasgettuple = (amroutine->amgettuple != NULL);
info->amhasgetbitmap = amroutine->amgetbitmap != NULL &&
- relation->rd_tableam->scan_bitmap_next_block != NULL;+ relation->rd_tableam->scan_bitmap_next_tuple != NULL;
info->amcanmarkpos = (amroutine->ammarkpos != NULL &&
amroutine->amrestrpos != NULL);
info->amcostestimate = amroutine->amcostestimate;
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.hindex eea7b7ff20..b2f0a61751 100644--- a/src/include/access/tableam.h+++ b/src/include/access/tableam.h@@ -790,34 +790,21 @@ typedef struct TableAmRoutine
* ------------------------------------------------------------------------
*/
- /*- * Prepare to fetch / check / return tuples as part of a bitmap table- * scan. `scan` was started via table_beginscan_bm(). Return false if the- * bitmap is exhausted and true otherwise.- *- * This will typically read and pin the target block, and do the necessary- * work to allow scan_bitmap_next_tuple() to return tuples (e.g. it might- * make sense to perform tuple visibility checks at this time).- *- * lossy_pages is incremented if the bitmap is lossy for the selected- * block; otherwise, exact_pages is incremented.- *- * Optional callback, but either both scan_bitmap_next_block and- * scan_bitmap_next_tuple need to exist, or neither.- */- bool (*scan_bitmap_next_block) (TableScanDesc scan,- BlockNumber *blockno, bool *recheck,- long *lossy_pages, long *exact_pages);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
* if a visible tuple was found, false otherwise.
*
- * Optional callback, but either both scan_bitmap_next_block and- * scan_bitmap_next_tuple need to exist, or neither.+ * recheck is set if recheck is required.+ *+ * The table AM is responsible for reading in blocks and counting (for+ * EXPLAIN) which of those blocks were represented lossily in the bitmap+ * using the lossy_pages and exact_pages counters.
*/
bool (*scan_bitmap_next_tuple) (TableScanDesc scan,
- TupleTableSlot *slot);+ TupleTableSlot *slot,+ bool *recheck,+ long *lossy_pages, long *exact_pages);
/*
* Prepare to fetch tuples from the next block in a sample scan. Return
@@ -1983,45 +1970,13 @@ table_relation_estimate_size(Relation rel, int32 *attr_widths,
*/
/*
- * Prepare to fetch / check / return tuples as part of a bitmap table scan.- * `scan` needs to have been started via table_beginscan_bm(). Returns false if- * there are no more blocks in the bitmap, true otherwise. lossy_pages is- * incremented is the block's representation in the bitmap is lossy; otherwise,- * exact_pages is incremented.- *- * Note, this is an optionally implemented function, therefore should only be- * used after verifying the presence (at plan time or such).- */-static inline bool-table_scan_bitmap_next_block(TableScanDesc scan,- BlockNumber *blockno, bool *recheck,- long *lossy_pages,- long *exact_pages)-{- /*- * We don't expect direct calls to table_scan_bitmap_next_block with valid- * CheckXidAlive for catalog or regular tables. See detailed comments in- * xact.c where these variables are declared.- */- if (unlikely(TransactionIdIsValid(CheckXidAlive) && !bsysscan))- elog(ERROR, "unexpected table_scan_bitmap_next_block call during logical decoding");-- return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan,- blockno, recheck,- lossy_pages, exact_pages);-}--/*- * Fetch the next tuple of a bitmap table scan into `slot` and return true if- * a visible tuple was found, false otherwise.- * table_scan_bitmap_next_block() needs to previously have selected a- * block (i.e. returned true), and no previous- * table_scan_bitmap_next_tuple() for the same block may have- * returned false.+ * Fetch the next tuple of a bitmap table scan into `slot` and return true if a+ * visible tuple was found, false otherwise.
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- TupleTableSlot *slot)+ TupleTableSlot *slot, bool *recheck,+ long *lossy_pages, long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_tuple with valid
@@ -2032,7 +1987,9 @@ table_scan_bitmap_next_tuple(TableScanDesc scan,
elog(ERROR, "unexpected table_scan_bitmap_next_tuple call during logical decoding");
return scan->rs_rd->rd_tableam->scan_bitmap_next_tuple(scan,
- slot);+ slot, recheck,+ lossy_pages,+ exact_pages);
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.hindex 713de7d50f..49ce4ff9a7 100644--- a/src/include/nodes/execnodes.h+++ b/src/include/nodes/execnodes.h@@ -1797,7 +1797,6 @@ typedef struct ParallelBitmapHeapState
* initialized is node is ready to iterate
* pstate shared state for parallel bitmap scan
* recheck do current page's tuples need recheck
- * blockno used to validate pf and current block in sync
* ----------------
*/
typedef struct BitmapHeapScanState
@@ -1810,7 +1809,6 @@ typedef struct BitmapHeapScanState
bool initialized;
ParallelBitmapHeapState *pstate;
bool recheck;
- BlockNumber blockno;
} BitmapHeapScanState;
/* ----------------
--
2.40.1
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: melanieplageman@gmail.com, tomas.vondra@enterprisedb.com, thomas.munro@gmail.com, hlinnaka@iki.fi, andres@anarazel.de, byavuz81@gmail.com
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
In-Reply-To: <20240405235330.v4didvweb6isodtk@liskov>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox