public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Melanie Plageman <[email protected]>
Cc: Thomas Munro <[email protected]>
Cc: Heikki Linnakangas <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Pg Hackers <[email protected]>
Cc: Nazir Bilal Yavuz <[email protected]>
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
Date: Thu, 4 Apr 2024 16:35:45 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <20240403225759.sq5hmxdmbalroosy@liskov>
References: <[email protected]>
<CAAKRu_Yzm6ae-Njj9DHwk0bbFC6QpzbjB=aY39civtpsUYgORQ@mail.gmail.com>
<[email protected]>
<CAAKRu_ZS016J_qhs=BnCsteYYC0HgYpAbZB8mDZBvFdy_VD1MQ@mail.gmail.com>
<20240325160709.qjo6txzu6zjxzkqy@liskov>
<20240327193750.3mlcmzqondpj27xe@liskov>
<[email protected]>
<CA+hUKGJtm_gkmW_h_02-Q9ZRcG3yOx2uzVqbCTfz7YPnTfs+DA@mail.gmail.com>
<[email protected]>
<20240331154551.lty4mundyoyhtixw@liskov>
<20240403225759.sq5hmxdmbalroosy@liskov>
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
>>>> <[email protected]> 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.
If someone else feels like committing that part, I won't stand in the
way, but I'm not quite convinced it won't cause regressions. Maybe it's
OK but I'd need more time to do more tests, collect data, and so on. And
I don't think we have that, especially considering we'd still need to
commit the other parts first.
> 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.
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 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?).
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'd focus on the first ~8-9 commits or so for now, we can commit more if
things go reasonably well.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachments:
[text/x-patch] v14-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch (2.8K, ../[email protected]/2-v14-0001-BitmapHeapScan-begin-scan-after-bitmap-creation.patch)
download | inline diff:
From 8d42c0e61cda5f7a8b574fc6a775bec84370ba26 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 18:50:29 -0500
Subject: [PATCH v14 01/19] BitmapHeapScan begin scan after bitmap creation
There is no reason for a BitmapHeapScan to begin the scan of the
underlying table in ExecInitBitmapHeapScan(). Instead, do so after
completing the index scan and building the bitmap.
---
src/backend/access/table/tableam.c | 1 -
src/backend/executor/nodeBitmapHeapscan.c | 26 +++++++++++++++++------
2 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index 805d222cebc..b0f61c65f36 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,7 +120,6 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
NULL, flags);
}
-
/* ----------------------------------------------------------------------------
* Parallel table scan related functions.
* ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cee7f45aabe..93fdcd226bf 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -178,6 +178,20 @@ 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 = node->ss.ss_currentScanDesc = table_beginscan_bm(
+ node->ss.ss_currentRelation,
+ node->ss.ps.state->es_snapshot,
+ 0,
+ NULL);
+ }
+
node->initialized = true;
}
@@ -601,7 +615,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 +693,9 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* close heap scan
*/
- table_endscan(scanDesc);
+ if (scanDesc)
+ table_endscan(scanDesc);
+
}
/* ----------------------------------------------------------------
@@ -783,11 +800,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.44.0
[text/x-patch] v14-0002-review.patch (1.9K, ../[email protected]/3-v14-0002-review.patch)
download | inline diff:
From d2160a9958f8185ab4803e9e778b370a6a6dde32 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 15:15:18 +0200
Subject: [PATCH v14 02/19] review
so is this an optimization, bugfix, or what? what difference does
it make in practice? or is it just cosmetic change?
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Mark Dilger
Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com
---
src/backend/access/table/tableam.c | 1 +
src/backend/executor/nodeBitmapHeapscan.c | 11 ++++++-----
2 files changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/backend/access/table/tableam.c b/src/backend/access/table/tableam.c
index b0f61c65f36..805d222cebc 100644
--- a/src/backend/access/table/tableam.c
+++ b/src/backend/access/table/tableam.c
@@ -120,6 +120,7 @@ table_beginscan_catalog(Relation relation, int nkeys, struct ScanKeyData *key)
NULL, flags);
}
+
/* ----------------------------------------------------------------------------
* Parallel table scan related functions.
* ----------------------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 93fdcd226bf..c8c466e3c5c 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -185,11 +185,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!scan)
{
- scan = node->ss.ss_currentScanDesc = table_beginscan_bm(
- node->ss.ss_currentRelation,
- node->ss.ps.state->es_snapshot,
- 0,
- NULL);
+ scan = table_beginscan_bm(node->ss.ss_currentRelation,
+ node->ss.ps.state->es_snapshot,
+ 0,
+ NULL);
+
+ node->ss.ss_currentScanDesc = scan;
}
node->initialized = true;
--
2.44.0
[text/x-patch] v14-0003-BitmapHeapScan-set-can_skip_fetch-later.patch (2.4K, ../[email protected]/4-v14-0003-BitmapHeapScan-set-can_skip_fetch-later.patch)
download | inline diff:
From ee72a2b5c846408ba9549d9d4c8b6d419fe3c311 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 13 Feb 2024 14:38:41 -0500
Subject: [PATCH v14 03/19] BitmapHeapScan set can_skip_fetch later
Set BitmapHeapScanState->can_skip_fetch in BitmapHeapNext() when
!BitmapHeapScanState->initialized instead of in
ExecInitBitmapHeapScan(). This is a preliminary step to removing
can_skip_fetch from BitmapHeapScanState and setting it in table AM
specific code.
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Mark Dilger
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.c
index c8c466e3c5c..2148a21531a 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.44.0
[text/x-patch] v14-0004-Push-BitmapHeapScan-skip-fetch-optimization-into.patch (15.1K, ../[email protected]/5-v14-0004-Push-BitmapHeapScan-skip-fetch-optimization-into.patch)
download | inline diff:
From ee62f42859a69cab1039155e2f15d0239a3f3dc1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 15:34:25 +0200
Subject: [PATCH v14 04/19] 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 and implement it for the heap
table AM by moving the vmbuffer and other fields needed for the
optimization from the BitmapHeapScanState into the HeapScanDescData.
heapam_scan_bitmap_next_block() now decides whether or not to skip
fetching the block before reading it in and
heapam_scan_bitmap_next_tuple() returns NULL-filled tuples for skipped
blocks.
The layering violation is still present in BitmapHeapScans's prefetching
code. However, this will be eliminated when prefetching is implemented
using the upcoming streaming read API discussed in [1].
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, 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.c
index dada2ecd1e3..10f2faaa60b 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.c
index 3e7a6b5548b..929b2cf8e71 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.c
index 2148a21531a..a8bc5dec53d 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.h
index 2765efc4e5e..750ea30852e 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.h
index e7eeb754098..55f11397576 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.h
index e7ff8e4992f..4880f346bf1 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.44.0
[text/x-patch] v14-0005-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch (2.5K, ../[email protected]/6-v14-0005-BitmapPrefetch-use-prefetch-block-recheck-for-sk.patch)
download | inline diff:
From e3fa2f25b5631bb9a5e27515be8161ef89418c40 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 19:03:24 -0500
Subject: [PATCH v14 05/19] 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. It makes more sense for it to use the
recheck flag from the TBMIterateResult for the prefetch block instead.
See this [1] thread on hackers reporting the issue.
XXX I think this commit message should "distill" the reasoning from [1]
to justify why this "makes more sense". The details can be left to the
thread, but the basics should be in this message I think.
[1] https://www.postgresql.org/message-id/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%40mail.gmail.com
Author: Melanie Plageman
Reviewed-by: Andres Freund, Heikki Linnakangas, Mark Dilger
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.c
index a8bc5dec53d..2e2cec8b3b5 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.44.0
[text/x-patch] v14-0006-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch (2.4K, ../[email protected]/7-v14-0006-Update-BitmapAdjustPrefetchIterator-parameter-ty.patch)
download | inline diff:
From 9b66215da19d8b6dc1e135204d9b427eb47c99d5 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 19:04:48 -0500
Subject: [PATCH v14 06/19] 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: ???????
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.c
index 2e2cec8b3b5..795a893035d 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.44.0
[text/x-patch] v14-0007-table_scan_bitmap_next_block-returns-lossy-or-ex.patch (4.4K, ../[email protected]/8-v14-0007-table_scan_bitmap_next_block-returns-lossy-or-ex.patch)
download | inline diff:
From 742490b1d4ef3b41eaece6b9fc0475d52b60ca39 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 26 Feb 2024 20:34:07 -0500
Subject: [PATCH v14 07/19] table_scan_bitmap_next_block() returns lossy or
exact
Future commits will remove the TBMIterateResult from BitmapHeapNext() --
pushing it into the table AM-specific code. So, the table AM must inform
BitmapHeapNext() whether or not the current block is lossy or exact for
the purposes of the counters used in EXPLAIN.
---
src/backend/access/heap/heapam_handler.c | 5 ++++-
src/backend/executor/nodeBitmapHeapscan.c | 10 +++++-----
src/include/access/tableam.h | 14 ++++++++++----
3 files changed, 19 insertions(+), 10 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 929b2cf8e71..a511bbfa88d 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,
+ bool *lossy)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block = tbmres->blockno;
@@ -2316,6 +2317,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
+ *lossy = tbmres->ntuples < 0;
+
return ntup > 0;
}
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 795a893035d..56943b211fa 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -212,7 +212,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid;
+ bool valid, lossy;
CHECK_FOR_INTERRUPTS();
@@ -233,12 +233,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
BitmapAdjustPrefetchIterator(node, tbmres->blockno);
- valid = table_scan_bitmap_next_block(scan, tbmres);
+ valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
- if (tbmres->ntuples >= 0)
- node->exact_pages++;
- else
+ if (lossy)
node->lossy_pages++;
+ else
+ node->exact_pages++;
if (!valid)
{
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index 55f11397576..75c5ee956db 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 indicates whether or not the block's representation in the bitmap
+ * is lossy or exact.
+ *
* 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,
+ bool *lossy);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1971,14 +1975,16 @@ 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.
+ * the page, true otherwise. lossy is set to true if bitmap is lossy for the
+ * selected block and false otherwise.
*
* 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,
+ bool *lossy)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1989,7 +1995,7 @@ 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);
}
/*
--
2.44.0
[text/x-patch] v14-0008-review.patch (841B, ../[email protected]/9-v14-0008-review.patch)
download | inline diff:
From ae9308ef69e04e1231bfd718c59482320cc9dde3 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 15:51:03 +0200
Subject: [PATCH v14 08/19] review
I wonder if returning "exact" instead of lossy would be better, for the
silly reason that the if branching in BitmapHeapNext would be the same.
---
src/backend/executor/nodeBitmapHeapscan.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 56943b211fa..9b2034c2f83 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -212,7 +212,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
for (;;)
{
- bool valid, lossy;
+ bool valid,
+ lossy;
CHECK_FOR_INTERRUPTS();
--
2.44.0
[text/x-patch] v14-0009-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch (2.9K, ../[email protected]/10-v14-0009-Reduce-scope-of-BitmapHeapScan-tbmiterator-local.patch)
download | inline diff:
From 597254156f86cb57bb0340b68d52e8a3aa7d981e Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Tue, 13 Feb 2024 10:17:47 -0500
Subject: [PATCH v14 09/19] Reduce scope of BitmapHeapScan tbmiterator local
variables
To simplify the diff of a future commit which will move the TBMIterators
into the scan descriptor, define them in a narrower scope now.
---
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.c
index 9b2034c2f83..cf778f61d52 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;
}
@@ -223,9 +221,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.44.0
[text/x-patch] v14-0010-review.patch (838B, ../[email protected]/11-v14-0010-review.patch)
download | inline diff:
From 7e4e7dfcc334198b83ccc86d15dc6aa6f8189a36 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 15:58:57 +0200
Subject: [PATCH v14 10/19] review
---
src/backend/executor/nodeBitmapHeapscan.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index cf778f61d52..6787547b9ff 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -99,6 +99,10 @@ BitmapHeapNext(BitmapHeapScanState *node)
*/
if (!node->initialized)
{
+ /*
+ * XXX why don't we assign directly to the node-> fields? Then we
+ * would not need these local variables at all.
+ */
TBMIterator *tbmiterator = NULL;
TBMSharedIterator *shared_tbmiterator = NULL;
--
2.44.0
[text/x-patch] v14-0011-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch (4.4K, ../[email protected]/12-v14-0011-Remove-table_scan_bitmap_next_tuple-parameter-tb.patch)
download | inline diff:
From 3fb2c24d79f8cdc8e33007697db362ad5b50b4fc Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Mon, 12 Feb 2024 18:13:41 -0500
Subject: [PATCH v14 11/19] Remove table_scan_bitmap_next_tuple parameter
tbmres
With the addition of the proposed streaming read API [1],
table_scan_bitmap_next_block() will no longer take a TBMIterateResult as
an input. Instead table AMs will be responsible for implementing a
callback for the streaming read API which specifies which blocks should
be prefetched and read.
Thus, it no longer makes sense to use the TBMIterateResult 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().
XXX Does it make sense to merge this without the switch to the new API?
Imagine an AM chose to do the work in table_scan_bitmap_next_tuple. Now
it would have to do that in the other callback. Is that OK, or would it
make it harder/less efficient? Ultimately the AM will have to do that
anyway, so maybe it doesn't mattter.
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%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.c
index a511bbfa88d..02bab2e93f4 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2324,7 +2324,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.c
index 6787547b9ff..14a5af30087 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -292,7 +292,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.h
index 75c5ee956db..1b545f2b678 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);
/*
@@ -2008,7 +2000,6 @@ table_scan_bitmap_next_block(TableScanDesc scan,
*/
static inline bool
table_scan_bitmap_next_tuple(TableScanDesc scan,
- struct TBMIterateResult *tbmres,
TupleTableSlot *slot)
{
/*
@@ -2020,7 +2011,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.44.0
[text/x-patch] v14-0012-Make-table_scan_bitmap_next_block-async-friendly.patch (23.0K, ../[email protected]/13-v14-0012-Make-table_scan_bitmap_next_block-async-friendly.patch)
download | inline diff:
From 7dff0fdde3016536244a78efe0803ddcb9fdb8ee Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 16:05:04 +0200
Subject: [PATCH v14 12/19] 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.
These changes will enable bitmapheapscan to use the future streaming
read API [1]. Table AMs will implement a streaming read API callback
returning the next block to fetch. In heap AM's case, the callback will
use the iterator to identify the next block to fetch. Since choosing the
next block will no longer the responsibility of BitmapHeapNext(), the
streaming read control flow requires these changes to
table_scan_bitmap_next_block().
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com
---
src/backend/access/heap/heapam_handler.c | 59 +++++--
src/backend/executor/nodeBitmapHeapscan.c | 199 ++++++++++------------
src/include/access/relscan.h | 7 +
src/include/access/tableam.h | 68 +++++---
src/include/nodes/execnodes.h | 12 +-
5 files changed, 195 insertions(+), 150 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 02bab2e93f4..eea3b7f149e 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2188,18 +2188,51 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
- TBMIterateResult *tbmres,
- bool *lossy)
+ bool *recheck, bool *lossy, BlockNumber *blockno)
{
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 +2251,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.
@@ -2319,7 +2343,14 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
*lossy = tbmres->ntuples < 0;
- 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.c
index 14a5af30087..5f987b40ede 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);
@@ -70,8 +69,8 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
ExprContext *econtext;
TableScanDesc scan;
+ bool lossy;
TIDBitmap *tbm;
- TBMIterateResult *tbmres;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
@@ -83,7 +82,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
@@ -115,7 +113,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->tbm = tbm;
tbmiterator = tbm_begin_iterate(tbm);
- node->tbmres = tbmres = NULL;
#ifdef USE_PREFETCH
if (node->prefetch_maximum > 0)
@@ -168,7 +165,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)
@@ -207,56 +203,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 (;;)
{
- bool valid,
- lossy;
-
- 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);
-
- valid = table_scan_bitmap_next_block(scan, tbmres, &lossy);
-
- if (lossy)
- node->lossy_pages++;
- else
- node->exact_pages++;
-
- if (!valid)
- {
- /* 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
@@ -278,45 +237,60 @@ 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->recheck, &lossy, &node->blockno))
+ break;
+
+ if (lossy)
+ node->lossy_pages++;
+ else
+ node->exact_pages++;
+
/*
- * 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);
}
/*
@@ -342,13 +316,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)
{
@@ -362,14 +340,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;
@@ -394,7 +375,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 */
@@ -472,6 +456,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,
@@ -529,6 +514,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 &&
@@ -590,12 +577,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)
@@ -603,13 +586,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);
@@ -640,28 +623,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);
-
}
/* ----------------------------------------------------------------
@@ -694,8 +673,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;
@@ -703,9 +680,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.h
index 521043304ab..92b829cebc7 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.h
index 1b545f2b678..74ddd5d66d3 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 indicates whether or not the block's representation in the bitmap
* is lossy or exact.
*
@@ -804,8 +800,8 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- struct TBMIterateResult *tbmres,
- bool *lossy);
+ bool *recheck, bool *lossy,
+ BlockNumber *blockno);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -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,17 @@ 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 is set to true if bitmap is lossy for the
- * selected block and false otherwise.
+ * 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 is set to true
+ * if bitmap is lossy for the selected block and false otherwise.
*
* 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,
- bool *lossy)
+ bool *recheck, bool *lossy, BlockNumber *blockno)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1986,8 +2014,8 @@ table_scan_bitmap_next_block(TableScanDesc scan,
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,
- tbmres, lossy);
+ return scan->rs_rd->rd_tableam->scan_bitmap_next_block(scan, recheck,
+ lossy, blockno);
}
/*
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 4880f346bf1..8e344155679 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.44.0
[text/x-patch] v14-0013-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch (16.1K, ../[email protected]/14-v14-0013-Unify-parallel-and-serial-BitmapHeapScan-iterato.patch)
download | inline diff:
From 53362e02e069747a413aa11cf25df918a94b1cc1 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <[email protected]>
Date: Thu, 4 Apr 2024 16:05:53 +0200
Subject: [PATCH v14 13/19] 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(). Now, on the first scan, the the
iterator is created after the scan descriptor is created.
---
src/backend/access/heap/heapam_handler.c | 5 +-
src/backend/executor/nodeBitmapHeapscan.c | 163 ++++++++++++----------
src/include/access/relscan.h | 7 +-
src/include/access/tableam.h | 29 +---
src/include/executor/nodeBitmapHeapscan.h | 10 ++
src/include/nodes/execnodes.h | 8 +-
src/tools/pgindent/typedefs.list | 1 +
7 files changed, 114 insertions(+), 109 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index eea3b7f149e..e086cbd2cfd 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2207,10 +2207,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.c
index 5f987b40ede..a1f9acc2cfb 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -56,6 +56,56 @@ static inline void BitmapAdjustPrefetchTarget(BitmapHeapScanState *node);
static inline void BitmapPrefetch(BitmapHeapScanState *node,
TableScanDesc scan);
static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate);
+static BitmapHeapIterator *bhs_begin_iterate(TIDBitmap *tbm,
+ dsa_pointer shared_area,
+ dsa_area *personal_area);
+
+BitmapHeapIterator *
+bhs_begin_iterate(TIDBitmap *tbm, dsa_pointer shared_area, dsa_area *personal_area)
+{
+ BitmapHeapIterator *result = palloc(sizeof(BitmapHeapIterator));
+
+ result->serial = NULL;
+ result->parallel = NULL;
+
+ /* Allocate a private iterator and attach the shared state to it */
+ if (DsaPointerIsValid(shared_area))
+ result->parallel = tbm_attach_shared_iterate(personal_area, shared_area);
+ else
+ result->serial = tbm_begin_iterate(tbm);
+
+ return result;
+}
+
+TBMIterateResult *
+bhs_iterate(BitmapHeapIterator *iterator)
+{
+ Assert(iterator);
+
+ if (iterator->serial)
+ return tbm_iterate(iterator->serial);
+ else
+ return tbm_shared_iterate(iterator->parallel);
+}
+
+void
+bhs_end_iterate(BitmapHeapIterator *iterator)
+{
+ Assert(iterator);
+
+ if (iterator->serial)
+ {
+ tbm_end_iterate(iterator->serial);
+ iterator->serial = NULL;
+ }
+ else
+ {
+ tbm_end_shared_iterate(iterator->parallel);
+ iterator->parallel = NULL;
+ }
+
+ pfree(iterator);
+}
/* ----------------------------------------------------------------
@@ -98,46 +148,22 @@ BitmapHeapNext(BitmapHeapScanState *node)
if (!node->initialized)
{
/*
- * XXX why don't we assign directly to the node-> fields? Then we
- * would not need these local variables at all.
+ * The leader will immediately come out of the function, but others
+ * will be blocked until leader populates the TBM and wakes them up.
*/
- TBMIterator *tbmiterator = NULL;
- TBMSharedIterator *shared_tbmiterator = NULL;
+ 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)
- {
- 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))
+ if (init_shared_state)
{
- 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
@@ -158,21 +184,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 */
}
/*
@@ -203,8 +217,21 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- scan->tbmiterator = tbmiterator;
- scan->shared_tbmiterator = shared_tbmiterator;
+ scan->rs_bhs_iterator = bhs_begin_iterate(tbm,
+ pstate ? pstate->tbmiterator : InvalidDsaPointer,
+ dsa);
+
+#ifdef USE_PREFETCH
+ if (node->prefetch_maximum > 0)
+ {
+ node->pf_iterator = bhs_begin_iterate(tbm,
+ pstate ? pstate->prefetch_iterator : InvalidDsaPointer,
+ dsa);
+ /* Only used for serial BHS */
+ node->prefetch_pages = 0;
+ node->prefetch_target = -1;
+ }
+#endif /* USE_PREFETCH */
node->initialized = true;
@@ -285,7 +312,7 @@ new_page:
* ahead of the current block.
*/
if (node->pstate == NULL &&
- node->prefetch_iterator &&
+ node->pf_iterator &&
node->pfblockno < node->blockno)
elog(ERROR, "prefetch and main iterators are out of sync");
@@ -326,12 +353,11 @@ 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 */
@@ -340,7 +366,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
else if (prefetch_iterator)
{
/* 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;
@@ -353,8 +379,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
*/
if (node->prefetch_maximum > 0)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
-
SpinLockAcquire(&pstate->mutex);
if (pstate->prefetch_pages > 0)
{
@@ -376,7 +400,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
*/
if (prefetch_iterator)
{
- tbmpre = tbm_shared_iterate(prefetch_iterator);
+ tbmpre = bhs_iterate(prefetch_iterator);
node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
}
}
@@ -436,23 +460,22 @@ 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)
{
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);
+ node->pf_iterator = NULL;
break;
}
node->prefetch_pages++;
@@ -480,8 +503,6 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (pstate->prefetch_pages < pstate->prefetch_target)
{
- TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator;
-
if (prefetch_iterator)
{
while (1)
@@ -505,12 +526,12 @@ 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);
+ node->pf_iterator = NULL;
break;
}
@@ -577,18 +598,17 @@ 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)
+ {
+ bhs_end_iterate(node->pf_iterator);
+ node->pf_iterator = NULL;
+ }
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;
@@ -633,12 +653,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
/*
* release bitmaps and buffers if any
*/
- if (node->prefetch_iterator)
- tbm_end_iterate(node->prefetch_iterator);
+ if (node->pf_iterator)
+ 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);
}
@@ -676,11 +694,10 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
scanstate->pvmbuffer = InvalidBuffer;
scanstate->exact_pages = 0;
scanstate->lossy_pages = 0;
- scanstate->prefetch_iterator = NULL;
+ scanstate->pf_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.h
index 92b829cebc7..fb22f305bf6 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -20,12 +20,12 @@
#include "storage/buf.h"
#include "storage/spin.h"
#include "utils/relcache.h"
+#include "executor/nodeBitmapHeapscan.h"
struct ParallelTableScanDescData;
-struct TBMIterator;
-struct TBMSharedIterator;
+struct BitmapHeapIterator;
/*
* Generic descriptor for table scans. This is the base-class for table scans,
@@ -44,8 +44,7 @@ typedef struct TableScanDescData
ItemPointerData rs_maxtid;
/* Only used for Bitmap table scans */
- struct TBMIterator *tbmiterator;
- struct TBMSharedIterator *shared_tbmiterator;
+ struct 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.h
index 74ddd5d66d3..d4fbf0d889b 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -942,8 +942,7 @@ table_beginscan_bm(Relation rel, Snapshot snapshot,
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;
+ result->rs_bhs_iterator = NULL;
return result;
}
@@ -993,17 +992,8 @@ 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;
- }
+ bhs_end_iterate(scan->rs_bhs_iterator);
+ scan->rs_bhs_iterator = NULL;
}
scan->rs_rd->rd_tableam->scan_end(scan);
@@ -1018,17 +1008,8 @@ table_rescan(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;
- }
+ bhs_end_iterate(scan->rs_bhs_iterator);
+ scan->rs_bhs_iterator = NULL;
}
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.h
index ea003a9caae..cb56d20dc6f 100644
--- a/src/include/executor/nodeBitmapHeapscan.h
+++ b/src/include/executor/nodeBitmapHeapscan.h
@@ -28,5 +28,15 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
ParallelContext *pcxt);
extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node,
ParallelWorkerContext *pwcxt);
+typedef struct BitmapHeapIterator
+{
+ struct TBMIterator *serial;
+ struct TBMSharedIterator *parallel;
+} BitmapHeapIterator;
+
+extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);
+
+extern void bhs_end_iterate(BitmapHeapIterator *iterator);
+
#endif /* NODEBITMAPHEAPSCAN_H */
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 8e344155679..cf8b4995f0d 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1787,6 +1787,8 @@ typedef struct ParallelBitmapHeapState
ConditionVariable cv;
} ParallelBitmapHeapState;
+struct BitmapHeapIterator;
+
/* ----------------
* BitmapHeapScanState information
*
@@ -1795,12 +1797,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 +1816,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;
+ struct BitmapHeapIterator *pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
BlockNumber blockno;
diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list
index b98e330e3ed..bcee567b65b 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.44.0
[text/x-patch] v14-0014-table_scan_bitmap_next_block-counts-lossy-and-ex.patch (5.2K, ../[email protected]/15-v14-0014-table_scan_bitmap_next_block-counts-lossy-and-ex.patch)
download | inline diff:
From f1290c0c5127abc0755a5ead9d1d77549bd01cc7 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 17:09:12 -0400
Subject: [PATCH v14 14/19] table_scan_bitmap_next_block counts lossy and exact
pages
Now that the table_scan_bitmap_next_block() callback only returns false
when the bitmap is exhausted, it is simpler to move the management of
the lossy and exact page counters into it. We will eventually remove
this callback and table_scan_bitmap_next_tuple() will update those
counters when a new block is read in.
---
src/backend/access/heap/heapam_handler.c | 8 ++++++--
src/backend/executor/nodeBitmapHeapscan.c | 9 ++-------
src/include/access/tableam.h | 21 +++++++++++++--------
3 files changed, 21 insertions(+), 17 deletions(-)
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index e086cbd2cfd..fe31b0efee4 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,
- bool *recheck, bool *lossy, BlockNumber *blockno)
+ bool *recheck, BlockNumber *blockno,
+ long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
BlockNumber block;
@@ -2338,7 +2339,10 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
- *lossy = tbmres->ntuples < 0;
+ if (tbmres->ntuples < 0)
+ (*lossy_pages)++;
+ else
+ (*exact_pages)++;
/*
* Return true to indicate that a valid block was found and the bitmap is
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index a1f9acc2cfb..261e00849a7 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -119,7 +119,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
{
ExprContext *econtext;
TableScanDesc scan;
- bool lossy;
TIDBitmap *tbm;
TupleTableSlot *slot;
ParallelBitmapHeapState *pstate = node->pstate;
@@ -299,14 +298,10 @@ new_page:
BitmapAdjustPrefetchIterator(node);
- if (!table_scan_bitmap_next_block(scan, &node->recheck, &lossy, &node->blockno))
+ if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno,
+ &node->lossy_pages, &node->exact_pages))
break;
- if (lossy)
- node->lossy_pages++;
- else
- node->exact_pages++;
-
/*
* If serial, we can error out if the the prefetch block doesn't stay
* ahead of the current block.
diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h
index d4fbf0d889b..70e538b76ba 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -782,8 +782,8 @@ typedef struct TableAmRoutine
* 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 indicates whether or not the block's representation in the bitmap
- * is lossy or exact.
+ * lossy_pages is incremented if the block's representation in the bitmap
+ * is lossy, 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
@@ -800,8 +800,10 @@ typedef struct TableAmRoutine
* scan_bitmap_next_tuple need to exist, or neither.
*/
bool (*scan_bitmap_next_block) (TableScanDesc scan,
- bool *recheck, bool *lossy,
- BlockNumber *blockno);
+ bool *recheck,
+ BlockNumber *blockno,
+ long *lossy_pages,
+ long *exact_pages);
/*
* Fetch the next tuple of a bitmap table scan into `slot` and return true
@@ -1977,15 +1979,17 @@ 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 is set to true
- * if bitmap is lossy for the selected block and false otherwise.
+ * there are no more blocks in the bitmap, true otherwise. lossy_pages is
+ * incremented if bitmap is lossy for the selected block and exact_pages is
+ * incremented otherwise.
*
* 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,
- bool *recheck, bool *lossy, BlockNumber *blockno)
+ bool *recheck, BlockNumber *blockno,
+ long *lossy_pages, long *exact_pages)
{
/*
* We don't expect direct calls to table_scan_bitmap_next_block with valid
@@ -1996,7 +2000,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, recheck,
- lossy, blockno);
+ blockno, lossy_pages,
+ exact_pages);
}
/*
--
2.44.0
[text/x-patch] v14-0015-Hard-code-TBMIterateResult-offsets-array-size.patch (5.4K, ../[email protected]/16-v14-0015-Hard-code-TBMIterateResult-offsets-array-size.patch)
download | inline diff:
From 0e379ec9ce12ecc1fc1b9b0d93ea99a174a63966 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 15 Feb 2024 20:13:43 -0500
Subject: [PATCH v14 15/19] Hard-code TBMIterateResult offsets array size
TIDBitmap's TBMIterateResult had a flexible sized array of tuple offsets
but the API always allocated MaxHeapTuplesPerPage OffsetNumbers.
Creating a fixed-size aray of size MaxHeapTuplesPerPage is more clear
for the API user.
---
src/backend/nodes/tidbitmap.c | 33 +++++++--------------------------
src/include/nodes/tidbitmap.h | 12 ++++++++++--
2 files changed, 17 insertions(+), 28 deletions(-)
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index e8ab5d78fcc..1dc4c99bf99 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -40,7 +40,6 @@
#include <limits.h>
-#include "access/htup_details.h"
#include "common/hashfn.h"
#include "common/int.h"
#include "nodes/bitmapset.h"
@@ -48,14 +47,6 @@
#include "storage/lwlock.h"
#include "utils/dsa.h"
-/*
- * The maximum number of tuples per page is not large (typically 256 with
- * 8K pages, or 1024 with 32K pages). So there's not much point in making
- * the per-page bitmaps variable size. We just legislate that the size
- * is this:
- */
-#define MAX_TUPLES_PER_PAGE MaxHeapTuplesPerPage
-
/*
* When we have to switch over to lossy storage, we use a data structure
* with one bit per page, where all pages having the same number DIV
@@ -67,7 +58,7 @@
* table, using identical data structures. (This is because the memory
* management for hashtables doesn't easily/efficiently allow space to be
* transferred easily from one hashtable to another.) Therefore it's best
- * if PAGES_PER_CHUNK is the same as MAX_TUPLES_PER_PAGE, or at least not
+ * if PAGES_PER_CHUNK is the same as MaxHeapTuplesPerPage, or at least not
* too different. But we also want PAGES_PER_CHUNK to be a power of 2 to
* avoid expensive integer remainder operations. So, define it like this:
*/
@@ -79,7 +70,7 @@
#define BITNUM(x) ((x) % BITS_PER_BITMAPWORD)
/* number of active words for an exact page: */
-#define WORDS_PER_PAGE ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD + 1)
+#define WORDS_PER_PAGE ((MaxHeapTuplesPerPage - 1) / BITS_PER_BITMAPWORD + 1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)
@@ -181,7 +172,7 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/*
@@ -222,7 +213,7 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output; /* MUST BE LAST (because variable-size) */
+ TBMIterateResult output;
};
/* Local function prototypes */
@@ -390,7 +381,7 @@ tbm_add_tuples(TIDBitmap *tbm, const ItemPointer tids, int ntids,
bitnum;
/* safety check to ensure we don't overrun bit array bounds */
- if (off < 1 || off > MAX_TUPLES_PER_PAGE)
+ if (off < 1 || off > MaxHeapTuplesPerPage)
elog(ERROR, "tuple offset out of range: %u", off);
/*
@@ -692,12 +683,7 @@ tbm_begin_iterate(TIDBitmap *tbm)
Assert(tbm->iterating != TBM_ITERATING_SHARED);
- /*
- * Create the TBMIterator struct, with enough trailing space to serve the
- * needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMIterator *) palloc(sizeof(TBMIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = palloc(sizeof(TBMIterator));
iterator->tbm = tbm;
/*
@@ -1463,12 +1449,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp)
TBMSharedIterator *iterator;
TBMSharedIteratorState *istate;
- /*
- * Create the TBMSharedIterator struct, with enough trailing space to
- * serve the needs of the TBMIterateResult sub-struct.
- */
- iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator) +
- MAX_TUPLES_PER_PAGE * sizeof(OffsetNumber));
+ iterator = (TBMSharedIterator *) palloc0(sizeof(TBMSharedIterator));
istate = (TBMSharedIteratorState *) dsa_get_address(dsa, dp);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 1945f0639bf..432fae52962 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -22,6 +22,7 @@
#ifndef TIDBITMAP_H
#define TIDBITMAP_H
+#include "access/htup_details.h"
#include "storage/itemptr.h"
#include "utils/dsa.h"
@@ -41,9 +42,16 @@ typedef struct TBMIterateResult
{
BlockNumber blockno; /* page number containing tuples */
int ntuples; /* -1 indicates lossy result */
- bool recheck; /* should the tuples be rechecked? */
/* Note: recheck is always true if ntuples < 0 */
- OffsetNumber offsets[FLEXIBLE_ARRAY_MEMBER];
+ bool recheck; /* should the tuples be rechecked? */
+
+ /*
+ * The maximum number of tuples per page is not large (typically 256 with
+ * 8K pages, or 1024 with 32K pages). So there's not much point in making
+ * the per-page bitmaps variable size. We just legislate that the size is
+ * this:
+ */
+ OffsetNumber offsets[MaxHeapTuplesPerPage];
} TBMIterateResult;
/* function prototypes in nodes/tidbitmap.c */
--
2.44.0
[text/x-patch] v14-0016-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch (20.7K, ../[email protected]/17-v14-0016-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch)
download | inline diff:
From 99f4982bc9f861f080b2aa71caf92169b5c21334 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Thu, 15 Feb 2024 21:23:41 -0500
Subject: [PATCH v14 16/19] Separate TBM[Shared]Iterator and TBMIterateResult
Remove the TBMIterateResult from the TBMIterator and TBMSharedIterator
and have tbm_[shared_]iterate() take a TBMIterateResult as a parameter.
This will allow multiple TBMIterateResults to exist concurrently
allowing asynchronous use of the TIDBitmap for prefetching, for example.
tbm_[shared]_iterate() now sets blockno to InvalidBlockNumber when the
bitmap is exhausted instead of returning NULL.
BitmapHeapScan callers of tbm_iterate make a TBMIterateResult locally
and pass it in.
Because GIN only needs a single TBMIterateResult, inline the matchResult
in the GinScanEntry to avoid having to separately manage memory for the
TBMIterateResult.
---
src/backend/access/gin/ginget.c | 48 +++++++++------
src/backend/access/gin/ginscan.c | 2 +-
src/backend/access/heap/heapam_handler.c | 30 +++++-----
src/backend/executor/nodeBitmapHeapscan.c | 47 ++++++++-------
src/backend/nodes/tidbitmap.c | 73 ++++++++++++-----------
src/include/access/gin_private.h | 2 +-
src/include/executor/nodeBitmapHeapscan.h | 2 +-
src/include/nodes/tidbitmap.h | 4 +-
8 files changed, 113 insertions(+), 95 deletions(-)
diff --git a/src/backend/access/gin/ginget.c b/src/backend/access/gin/ginget.c
index 0b4f2ebadb6..3aa457a29e1 100644
--- a/src/backend/access/gin/ginget.c
+++ b/src/backend/access/gin/ginget.c
@@ -332,10 +332,22 @@ restartScanEntry:
entry->list = NULL;
entry->nlist = 0;
entry->matchBitmap = NULL;
- entry->matchResult = NULL;
entry->reduceResult = false;
entry->predictNumberResult = 0;
+ /*
+ * MTODO: is it enough to set blockno to InvalidBlockNumber? In all the
+ * places were we previously set matchResult to NULL, I just set blockno
+ * to InvalidBlockNumber. It seems like this should be okay because that
+ * is usually what we check before using the matchResult members. But it
+ * might be safer to zero out the offsets array. But that is expensive.
+ */
+ entry->matchResult.blockno = InvalidBlockNumber;
+ entry->matchResult.ntuples = 0;
+ entry->matchResult.recheck = true;
+ memset(entry->matchResult.offsets, 0,
+ sizeof(OffsetNumber) * MaxHeapTuplesPerPage);
+
/*
* we should find entry, and begin scan of posting tree or just store
* posting list in memory
@@ -374,6 +386,7 @@ restartScanEntry:
{
if (entry->matchIterator)
tbm_end_iterate(entry->matchIterator);
+ entry->matchResult.blockno = InvalidBlockNumber;
entry->matchIterator = NULL;
tbm_free(entry->matchBitmap);
entry->matchBitmap = NULL;
@@ -823,18 +836,19 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
{
/*
* If we've exhausted all items on this block, move to next block
- * in the bitmap.
+ * in the bitmap. tbm_iterate() sets matchResult->blockno to
+ * InvalidBlockNumber when the bitmap is exhausted.
*/
- while (entry->matchResult == NULL ||
- (entry->matchResult->ntuples >= 0 &&
- entry->offset >= entry->matchResult->ntuples) ||
- entry->matchResult->blockno < advancePastBlk ||
+ while ((!BlockNumberIsValid(entry->matchResult.blockno)) ||
+ (entry->matchResult.ntuples >= 0 &&
+ entry->offset >= entry->matchResult.ntuples) ||
+ entry->matchResult.blockno < advancePastBlk ||
(ItemPointerIsLossyPage(&advancePast) &&
- entry->matchResult->blockno == advancePastBlk))
+ entry->matchResult.blockno == advancePastBlk))
{
- entry->matchResult = tbm_iterate(entry->matchIterator);
+ tbm_iterate(entry->matchIterator, &entry->matchResult);
- if (entry->matchResult == NULL)
+ if (!BlockNumberIsValid(entry->matchResult.blockno))
{
ItemPointerSetInvalid(&entry->curItem);
tbm_end_iterate(entry->matchIterator);
@@ -858,10 +872,10 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
* We're now on the first page after advancePast which has any
* items on it. If it's a lossy result, return that.
*/
- if (entry->matchResult->ntuples < 0)
+ if (entry->matchResult.ntuples < 0)
{
ItemPointerSetLossyPage(&entry->curItem,
- entry->matchResult->blockno);
+ entry->matchResult.blockno);
/*
* We might as well fall out of the loop; we could not
@@ -875,27 +889,27 @@ entryGetItem(GinState *ginstate, GinScanEntry entry,
* Not a lossy page. Skip over any offsets <= advancePast, and
* return that.
*/
- if (entry->matchResult->blockno == advancePastBlk)
+ if (entry->matchResult.blockno == advancePastBlk)
{
/*
* First, do a quick check against the last offset on the
* page. If that's > advancePast, so are all the other
* offsets, so just go back to the top to get the next page.
*/
- if (entry->matchResult->offsets[entry->matchResult->ntuples - 1] <= advancePastOff)
+ if (entry->matchResult.offsets[entry->matchResult.ntuples - 1] <= advancePastOff)
{
- entry->offset = entry->matchResult->ntuples;
+ entry->offset = entry->matchResult.ntuples;
continue;
}
/* Otherwise scan to find the first item > advancePast */
- while (entry->matchResult->offsets[entry->offset] <= advancePastOff)
+ while (entry->matchResult.offsets[entry->offset] <= advancePastOff)
entry->offset++;
}
ItemPointerSet(&entry->curItem,
- entry->matchResult->blockno,
- entry->matchResult->offsets[entry->offset]);
+ entry->matchResult.blockno,
+ entry->matchResult.offsets[entry->offset]);
entry->offset++;
/* Done unless we need to reduce the result */
diff --git a/src/backend/access/gin/ginscan.c b/src/backend/access/gin/ginscan.c
index af24d38544e..033d5253394 100644
--- a/src/backend/access/gin/ginscan.c
+++ b/src/backend/access/gin/ginscan.c
@@ -106,7 +106,7 @@ ginFillScanEntry(GinScanOpaque so, OffsetNumber attnum,
ItemPointerSetMin(&scanEntry->curItem);
scanEntry->matchBitmap = NULL;
scanEntry->matchIterator = NULL;
- scanEntry->matchResult = NULL;
+ scanEntry->matchResult.blockno = InvalidBlockNumber;
scanEntry->list = NULL;
scanEntry->nlist = 0;
scanEntry->offset = InvalidOffsetNumber;
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index fe31b0efee4..ec16d8c5e3c 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2196,7 +2196,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Buffer buffer;
Snapshot snapshot;
int ntup;
- TBMIterateResult *tbmres;
+ TBMIterateResult tbmres;
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
@@ -2208,9 +2208,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
{
CHECK_FOR_INTERRUPTS();
- tbmres = bhs_iterate(scan->rs_bhs_iterator);
+ bhs_iterate(scan->rs_bhs_iterator, &tbmres);
- if (tbmres == NULL)
+ if (!BlockNumberIsValid(tbmres.blockno))
{
/* no more entries in the bitmap */
Assert(hscan->rs_empty_tuples_pending == 0);
@@ -2225,11 +2225,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* isolation though, as we need to examine all invisible tuples
* reachable by the index.
*/
- } while (!IsolationIsSerializable() && tbmres->blockno >= hscan->rs_nblocks);
+ } while (!IsolationIsSerializable() && tbmres.blockno >= hscan->rs_nblocks);
/* Got a valid block */
- *blockno = tbmres->blockno;
- *recheck = tbmres->recheck;
+ *blockno = tbmres.blockno;
+ *recheck = tbmres.recheck;
/*
* We can skip fetching the heap page if we don't need any fields from the
@@ -2237,19 +2237,19 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
* 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))
+ !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(tbmres.ntuples >= 0);
Assert(hscan->rs_empty_tuples_pending >= 0);
- hscan->rs_empty_tuples_pending += tbmres->ntuples;
+ hscan->rs_empty_tuples_pending += tbmres.ntuples;
return true;
}
- block = tbmres->blockno;
+ block = tbmres.blockno;
/*
* Acquire pin on the target heap page, trading in any pin we held before.
@@ -2278,7 +2278,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
/*
* We need two separate strategies for lossy and non-lossy cases.
*/
- if (tbmres->ntuples >= 0)
+ if (tbmres.ntuples >= 0)
{
/*
* Bitmap is non-lossy, so we just look through the offsets listed in
@@ -2287,9 +2287,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
*/
int curslot;
- for (curslot = 0; curslot < tbmres->ntuples; curslot++)
+ for (curslot = 0; curslot < tbmres.ntuples; curslot++)
{
- OffsetNumber offnum = tbmres->offsets[curslot];
+ OffsetNumber offnum = tbmres.offsets[curslot];
ItemPointerData tid;
HeapTupleData heapTuple;
@@ -2339,7 +2339,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
- if (tbmres->ntuples < 0)
+ if (tbmres.ntuples < 0)
(*lossy_pages)++;
else
(*exact_pages)++;
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
index 261e00849a7..1086e20234b 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -77,15 +77,16 @@ bhs_begin_iterate(TIDBitmap *tbm, dsa_pointer shared_area, dsa_area *personal_ar
return result;
}
-TBMIterateResult *
-bhs_iterate(BitmapHeapIterator *iterator)
+void
+bhs_iterate(BitmapHeapIterator *iterator, TBMIterateResult *result)
{
Assert(iterator);
+ Assert(result);
if (iterator->serial)
- return tbm_iterate(iterator->serial);
+ tbm_iterate(iterator->serial, result);
else
- return tbm_shared_iterate(iterator->parallel);
+ tbm_shared_iterate(iterator->parallel, result);
}
void
@@ -349,7 +350,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
#ifdef USE_PREFETCH
ParallelBitmapHeapState *pstate = node->pstate;
BitmapHeapIterator *prefetch_iterator = node->pf_iterator;
- TBMIterateResult *tbmpre;
+ TBMIterateResult tbmpre;
if (pstate == NULL)
{
@@ -361,8 +362,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
else if (prefetch_iterator)
{
/* Do not let the prefetch iterator get behind the main one */
- tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ node->pfblockno = tbmpre.blockno;
}
return;
}
@@ -395,8 +396,8 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node)
*/
if (prefetch_iterator)
{
- tbmpre = bhs_iterate(prefetch_iterator);
- node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber;
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ node->pfblockno = tbmpre.blockno;
}
}
}
@@ -463,10 +464,12 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
{
while (node->prefetch_pages < node->prefetch_target)
{
- TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator);
+ TBMIterateResult tbmpre;
bool skip_fetch;
- if (tbmpre == NULL)
+ bhs_iterate(prefetch_iterator, &tbmpre);
+
+ if (!BlockNumberIsValid(tbmpre.blockno))
{
/* No more pages to prefetch */
bhs_end_iterate(prefetch_iterator);
@@ -474,7 +477,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
node->prefetch_pages++;
- node->pfblockno = tbmpre->blockno;
+ node->pfblockno = tbmpre.blockno;
/*
* If we expect not to have to actually read this heap page,
@@ -483,13 +486,13 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
* prefetch_pages?)
*/
skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) &&
- !tbmpre->recheck &&
+ !tbmpre.recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
- tbmpre->blockno,
+ tbmpre.blockno,
&node->pvmbuffer));
if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);
+ PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre.blockno);
}
}
@@ -502,7 +505,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
{
while (1)
{
- TBMIterateResult *tbmpre;
+ TBMIterateResult tbmpre;
bool do_prefetch = false;
bool skip_fetch;
@@ -521,8 +524,8 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
if (!do_prefetch)
return;
- tbmpre = bhs_iterate(prefetch_iterator);
- if (tbmpre == NULL)
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ if (!BlockNumberIsValid(tbmpre.blockno))
{
/* No more pages to prefetch */
bhs_end_iterate(prefetch_iterator);
@@ -530,17 +533,17 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan)
break;
}
- node->pfblockno = tbmpre->blockno;
+ 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 &&
+ !tbmpre.recheck &&
VM_ALL_VISIBLE(node->ss.ss_currentRelation,
- tbmpre->blockno,
+ tbmpre.blockno,
&node->pvmbuffer));
if (!skip_fetch)
- PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre->blockno);
+ PrefetchBuffer(scan->rs_rd, MAIN_FORKNUM, tbmpre.blockno);
}
}
}
diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index 1dc4c99bf99..309a44bdb84 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -172,7 +172,6 @@ struct TBMIterator
int spageptr; /* next spages index */
int schunkptr; /* next schunks index */
int schunkbit; /* next bit to check in current schunk */
- TBMIterateResult output;
};
/*
@@ -213,7 +212,6 @@ struct TBMSharedIterator
PTEntryArray *ptbase; /* pagetable element array */
PTIterationArray *ptpages; /* sorted exact page index list */
PTIterationArray *ptchunks; /* sorted lossy page index list */
- TBMIterateResult output;
};
/* Local function prototypes */
@@ -944,20 +942,21 @@ tbm_advance_schunkbit(PagetableEntry *chunk, int *schunkbitp)
/*
* tbm_iterate - scan through next page of a TIDBitmap
*
- * Returns a TBMIterateResult representing one page, or NULL if there are
- * no more pages to scan. Pages are guaranteed to be delivered in numerical
- * order. If result->ntuples < 0, then the bitmap is "lossy" and failed to
- * remember the exact tuples to look at on this page --- the caller must
- * examine all tuples on the page and check if they meet the intended
- * condition. If result->recheck is true, only the indicated tuples need
- * be examined, but the condition must be rechecked anyway. (For ease of
- * testing, recheck is always set true when ntuples < 0.)
+ * Caller must pass in a TBMIterateResult to be filled.
+ *
+ * Pages are guaranteed to be delivered in numerical order. tbmres->blockno is
+ * set to InvalidBlockNumber when there are no more pages to scan. If
+ * tbmres->ntuples < 0, then the bitmap is "lossy" and failed to remember the
+ * exact tuples to look at on this page --- the caller must examine all tuples
+ * on the page and check if they meet the intended condition. If
+ * tbmres->recheck is true, only the indicated tuples need be examined, but the
+ * condition must be rechecked anyway. (For ease of testing, recheck is always
+ * set true when ntuples < 0.)
*/
-TBMIterateResult *
-tbm_iterate(TBMIterator *iterator)
+void
+tbm_iterate(TBMIterator *iterator, TBMIterateResult *tbmres)
{
TIDBitmap *tbm = iterator->tbm;
- TBMIterateResult *output = &(iterator->output);
Assert(tbm->iterating == TBM_ITERATING_PRIVATE);
@@ -985,6 +984,7 @@ tbm_iterate(TBMIterator *iterator)
* If both chunk and per-page data remain, must output the numerically
* earlier page.
*/
+ Assert(tbmres);
if (iterator->schunkptr < tbm->nchunks)
{
PagetableEntry *chunk = tbm->schunks[iterator->schunkptr];
@@ -995,11 +995,11 @@ tbm_iterate(TBMIterator *iterator)
chunk_blockno < tbm->spages[iterator->spageptr]->blockno)
{
/* Return a lossy page indicator from the chunk */
- output->blockno = chunk_blockno;
- output->ntuples = -1;
- output->recheck = true;
+ tbmres->blockno = chunk_blockno;
+ tbmres->ntuples = -1;
+ tbmres->recheck = true;
iterator->schunkbit++;
- return output;
+ return;
}
}
@@ -1015,16 +1015,17 @@ tbm_iterate(TBMIterator *iterator)
page = tbm->spages[iterator->spageptr];
/* scan bitmap to extract individual offset numbers */
- ntuples = tbm_extract_page_tuple(page, output);
- output->blockno = page->blockno;
- output->ntuples = ntuples;
- output->recheck = page->recheck;
+ ntuples = tbm_extract_page_tuple(page, tbmres);
+ tbmres->blockno = page->blockno;
+ tbmres->ntuples = ntuples;
+ tbmres->recheck = page->recheck;
iterator->spageptr++;
- return output;
+ return;
}
/* Nothing more in the bitmap */
- return NULL;
+ tbmres->blockno = InvalidBlockNumber;
+ return;
}
/*
@@ -1034,10 +1035,9 @@ tbm_iterate(TBMIterator *iterator)
* across multiple processes. We need to acquire the iterator LWLock,
* before accessing the shared members.
*/
-TBMIterateResult *
-tbm_shared_iterate(TBMSharedIterator *iterator)
+void
+tbm_shared_iterate(TBMSharedIterator *iterator, TBMIterateResult *tbmres)
{
- TBMIterateResult *output = &iterator->output;
TBMSharedIteratorState *istate = iterator->state;
PagetableEntry *ptbase = NULL;
int *idxpages = NULL;
@@ -1088,13 +1088,13 @@ tbm_shared_iterate(TBMSharedIterator *iterator)
chunk_blockno < ptbase[idxpages[istate->spageptr]].blockno)
{
/* Return a lossy page indicator from the chunk */
- output->blockno = chunk_blockno;
- output->ntuples = -1;
- output->recheck = true;
+ tbmres->blockno = chunk_blockno;
+ tbmres->ntuples = -1;
+ tbmres->recheck = true;
istate->schunkbit++;
LWLockRelease(&istate->lock);
- return output;
+ return;
}
}
@@ -1104,21 +1104,22 @@ tbm_shared_iterate(TBMSharedIterator *iterator)
int ntuples;
/* scan bitmap to extract individual offset numbers */
- ntuples = tbm_extract_page_tuple(page, output);
- output->blockno = page->blockno;
- output->ntuples = ntuples;
- output->recheck = page->recheck;
+ ntuples = tbm_extract_page_tuple(page, tbmres);
+ tbmres->blockno = page->blockno;
+ tbmres->ntuples = ntuples;
+ tbmres->recheck = page->recheck;
istate->spageptr++;
LWLockRelease(&istate->lock);
- return output;
+ return;
}
LWLockRelease(&istate->lock);
/* Nothing more in the bitmap */
- return NULL;
+ tbmres->blockno = InvalidBlockNumber;
+ return;
}
/*
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 3013a44bae1..3b432263bb0 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -353,7 +353,7 @@ typedef struct GinScanEntryData
/* for a partial-match or full-scan query, we accumulate all TIDs here */
TIDBitmap *matchBitmap;
TBMIterator *matchIterator;
- TBMIterateResult *matchResult;
+ TBMIterateResult matchResult;
/* used for Posting list and one page in Posting tree */
ItemPointerData *list;
diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h
index cb56d20dc6f..3c330f86e62 100644
--- a/src/include/executor/nodeBitmapHeapscan.h
+++ b/src/include/executor/nodeBitmapHeapscan.h
@@ -34,7 +34,7 @@ typedef struct BitmapHeapIterator
struct TBMSharedIterator *parallel;
} BitmapHeapIterator;
-extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator);
+extern void bhs_iterate(BitmapHeapIterator *iterator, TBMIterateResult *result);
extern void bhs_end_iterate(BitmapHeapIterator *iterator);
diff --git a/src/include/nodes/tidbitmap.h b/src/include/nodes/tidbitmap.h
index 432fae52962..f000c1af28f 100644
--- a/src/include/nodes/tidbitmap.h
+++ b/src/include/nodes/tidbitmap.h
@@ -72,8 +72,8 @@ extern bool tbm_is_empty(const TIDBitmap *tbm);
extern TBMIterator *tbm_begin_iterate(TIDBitmap *tbm);
extern dsa_pointer tbm_prepare_shared_iterate(TIDBitmap *tbm);
-extern TBMIterateResult *tbm_iterate(TBMIterator *iterator);
-extern TBMIterateResult *tbm_shared_iterate(TBMSharedIterator *iterator);
+extern void tbm_iterate(TBMIterator *iterator, TBMIterateResult *tbmres);
+extern void tbm_shared_iterate(TBMSharedIterator *iterator, TBMIterateResult *tbmres);
extern void tbm_end_iterate(TBMIterator *iterator);
extern void tbm_end_shared_iterate(TBMSharedIterator *iterator);
extern TBMSharedIterator *tbm_attach_shared_iterate(dsa_area *dsa,
--
2.44.0
[text/x-patch] v14-0017-Push-BitmapHeapScan-prefetch-code-into-heapam.c.patch (31.5K, ../[email protected]/18-v14-0017-Push-BitmapHeapScan-prefetch-code-into-heapam.c.patch)
download | inline diff:
From 8f271b04d9108b2c36b30141d807ef4976d59526 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 09:42:23 -0400
Subject: [PATCH v14 17/19] Push BitmapHeapScan prefetch code into heapam.c
In preparation for transitioning to using the streaming read API for
prefetching [1], move all of the BitmapHeapScanState members related to
prefetching and the functions for accessing them into the
HeapScanDescData and TableScanDescData. Members that still need to be
accessed in BitmapHeapNext() could not be moved into heap AM-specific
code. Specifically, parallel iterator setup requires several components
which seem odd to pass to the table AM API.
[1] https://www.postgresql.org/message-id/flat/CA%2BhUKGJkOiOCa%2Bmag4BF%2BzHo7qo%3Do9CFheB8%3Dg6uT5TUm2gkvA%40mail.gmail.com
---
src/backend/access/heap/heapam.c | 26 ++
src/backend/access/heap/heapam_handler.c | 262 +++++++++++++++++
src/backend/executor/nodeBitmapHeapscan.c | 341 ++--------------------
src/include/access/heapam.h | 17 ++
src/include/access/relscan.h | 8 +
src/include/access/tableam.h | 26 +-
src/include/nodes/execnodes.h | 14 -
7 files changed, 355 insertions(+), 339 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 10f2faaa60b..bda6abb8d0c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -967,8 +967,16 @@ 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_base.blockno = InvalidBlockNumber;
+
scan->rs_vmbuffer = InvalidBuffer;
scan->rs_empty_tuples_pending = 0;
+ scan->pvmbuffer = InvalidBuffer;
+
+ scan->pfblockno = InvalidBlockNumber;
+ scan->prefetch_target = -1;
+ scan->prefetch_pages = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1051,6 +1059,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
}
+ scan->rs_base.blockno = InvalidBlockNumber;
+
+ scan->pfblockno = InvalidBlockNumber;
+ scan->prefetch_target = -1;
+ scan->prefetch_pages = 0;
+
/*
* unpin scan buffers
*/
@@ -1063,6 +1077,12 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
scan->rs_vmbuffer = InvalidBuffer;
}
+ if (BufferIsValid(scan->pvmbuffer))
+ {
+ ReleaseBuffer(scan->pvmbuffer);
+ scan->pvmbuffer = InvalidBuffer;
+ }
+
/*
* reinitialize scan descriptor
*/
@@ -1088,6 +1108,12 @@ heap_endscan(TableScanDesc sscan)
scan->rs_vmbuffer = InvalidBuffer;
}
+ if (BufferIsValid(scan->pvmbuffer))
+ {
+ ReleaseBuffer(scan->pvmbuffer);
+ scan->pvmbuffer = 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.c
index ec16d8c5e3c..0138a23e9d4 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(HeapScanDesc scan);
+static inline void BitmapAdjustPrefetchTarget(HeapScanDesc scan);
+static inline void BitmapPrefetch(HeapScanDesc scan);
static const TableAmRoutine heapam_methods;
@@ -2186,6 +2189,73 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
* ------------------------------------------------------------------------
*/
+/*
+ * 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(HeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator;
+ ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
+ TBMIterateResult tbmpre;
+
+ if (pstate == NULL)
+ {
+ if (scan->prefetch_pages > 0)
+ {
+ /* The main iterator has closed the distance by one page */
+ scan->prefetch_pages--;
+ }
+ else if (prefetch_iterator)
+ {
+ /* Do not let the prefetch iterator get behind the main one */
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ scan->pfblockno = tbmpre.blockno;
+ }
+ return;
+ }
+
+ /*
+ * Adjusting the prefetch iterator before invoking
+ * table_scan_bitmap_next_block() keeps prefetch distance higher across
+ * the parallel workers.
+ */
+ if (scan->rs_base.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)
+ {
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ scan->pfblockno = tbmpre.blockno;
+ }
+ }
+ }
+#endif /* USE_PREFETCH */
+}
+
static bool
heapam_scan_bitmap_next_block(TableScanDesc scan,
bool *recheck, BlockNumber *blockno,
@@ -2204,6 +2274,8 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
*blockno = InvalidBlockNumber;
*recheck = true;
+ BitmapAdjustPrefetchIterator(hscan);
+
do
{
CHECK_FOR_INTERRUPTS();
@@ -2344,6 +2416,18 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
else
(*exact_pages)++;
+ /*
+ * If serial, we can error out if the the prefetch block doesn't stay
+ * ahead of the current block.
+ */
+ 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");
+
+ /* Adjust the prefetch target */
+ BitmapAdjustPrefetchTarget(hscan);
+
/*
* 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,
@@ -2354,6 +2438,154 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
return true;
}
+/*
+ * 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(HeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
+ int prefetch_maximum = scan->rs_base.prefetch_maximum;
+
+ if (pstate == NULL)
+ {
+ if (scan->prefetch_target >= prefetch_maximum)
+ /* don't increase any further */ ;
+ else if (scan->prefetch_target >= prefetch_maximum / 2)
+ scan->prefetch_target = prefetch_maximum;
+ else if (scan->prefetch_target > 0)
+ scan->prefetch_target *= 2;
+ else
+ scan->prefetch_target++;
+ return;
+ }
+
+ /* Do an unlocked check first to save spinlock acquisitions. */
+ if (pstate->prefetch_target < prefetch_maximum)
+ {
+ SpinLockAcquire(&pstate->mutex);
+ if (pstate->prefetch_target >= prefetch_maximum)
+ /* don't increase any further */ ;
+ else if (pstate->prefetch_target >= prefetch_maximum / 2)
+ pstate->prefetch_target = 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(HeapScanDesc scan)
+{
+#ifdef USE_PREFETCH
+ ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
+ BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator;
+
+ if (pstate == NULL)
+ {
+ if (prefetch_iterator)
+ {
+ while (scan->prefetch_pages < scan->prefetch_target)
+ {
+ TBMIterateResult tbmpre;
+ bool skip_fetch;
+
+ bhs_iterate(prefetch_iterator, &tbmpre);
+
+ if (!BlockNumberIsValid(tbmpre.blockno))
+ {
+ /* No more pages to prefetch */
+ bhs_end_iterate(prefetch_iterator);
+ scan->rs_base.rs_pf_bhs_iterator = NULL;
+ break;
+ }
+ scan->prefetch_pages++;
+ scan->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_base.rs_flags & SO_NEED_TUPLE) &&
+ !tbmpre.recheck &&
+ VM_ALL_VISIBLE(scan->rs_base.rs_rd,
+ tbmpre.blockno,
+ &scan->pvmbuffer));
+
+ if (!skip_fetch)
+ PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno);
+ }
+ }
+
+ return;
+ }
+
+ if (pstate->prefetch_pages < pstate->prefetch_target)
+ {
+ if (prefetch_iterator)
+ {
+ 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;
+
+ bhs_iterate(prefetch_iterator, &tbmpre);
+ if (!BlockNumberIsValid(tbmpre.blockno))
+ {
+ /* No more pages to prefetch */
+ bhs_end_iterate(prefetch_iterator);
+ scan->rs_base.rs_pf_bhs_iterator = NULL;
+ break;
+ }
+
+ scan->pfblockno = tbmpre.blockno;
+
+ /* As above, skip prefetch if we expect not to need page */
+ skip_fetch = (!(scan->rs_base.rs_flags & SO_NEED_TUPLE) &&
+ !tbmpre.recheck &&
+ VM_ALL_VISIBLE(scan->rs_base.rs_rd,
+ tbmpre.blockno,
+ &scan->pvmbuffer));
+
+ if (!skip_fetch)
+ PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno);
+ }
+ }
+ }
+#endif /* USE_PREFETCH */
+}
+
static bool
heapam_scan_bitmap_next_tuple(TableScanDesc scan,
TupleTableSlot *slot)
@@ -2379,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 (!scan->bm_parallel)
+ {
+ if (hscan->prefetch_target < scan->prefetch_maximum)
+ hscan->prefetch_target++;
+ }
+ else if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum)
+ {
+ /* take spinlock while updating shared state */
+ SpinLockAcquire(&scan->bm_parallel->mutex);
+ if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum)
+ scan->bm_parallel->prefetch_target++;
+ SpinLockRelease(&scan->bm_parallel->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(hscan);
+#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.c
index 1086e20234b..ea309cf93e3 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -51,10 +51,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);
static BitmapHeapIterator *bhs_begin_iterate(TIDBitmap *tbm,
dsa_pointer shared_area,
@@ -122,7 +118,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
TableScanDesc scan;
TIDBitmap *tbm;
TupleTableSlot *slot;
- ParallelBitmapHeapState *pstate = node->pstate;
dsa_area *dsa = node->ss.ps.state->es_query_dsa;
/*
@@ -142,7 +137,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
* 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
+ * 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.
*/
if (!node->initialized)
@@ -154,7 +149,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
bool init_shared_state = node->pstate ?
BitmapShouldInitializeSharedState(node->pstate) : false;
- if (!pstate || init_shared_state)
+ /*
+ * Maximum number of prefetches for the tablespace if configured,
+ * otherwise the current value of the effective_io_concurrency GUC.
+ */
+ int pf_maximum = 0;
+#ifdef USE_PREFETCH
+ pf_maximum = get_tablespace_io_concurrency(node->ss.ss_currentRelation->rd_rel->reltablespace);
+#endif
+
+ if (!node->pstate || init_shared_state)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -169,23 +173,16 @@ BitmapHeapNext(BitmapHeapScanState *node)
* dsa_pointer of the iterator state which will be used by
* multiple processes to iterate jointly.
*/
- pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
+ node->pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
+ if (pf_maximum > 0)
{
- pstate->prefetch_iterator =
+ 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. */
- BitmapDoneInitializingSharedState(pstate);
+ BitmapDoneInitializingSharedState(node->pstate);
}
}
@@ -217,19 +214,19 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
+ scan->prefetch_maximum = pf_maximum;
+ scan->bm_parallel = node->pstate;
+
scan->rs_bhs_iterator = bhs_begin_iterate(tbm,
- pstate ? pstate->tbmiterator : InvalidDsaPointer,
+ scan->bm_parallel ? scan->bm_parallel->tbmiterator : InvalidDsaPointer,
dsa);
#ifdef USE_PREFETCH
- if (node->prefetch_maximum > 0)
+ if (scan->prefetch_maximum > 0)
{
- node->pf_iterator = bhs_begin_iterate(tbm,
- pstate ? pstate->prefetch_iterator : InvalidDsaPointer,
- dsa);
- /* Only used for serial BHS */
- node->prefetch_pages = 0;
- node->prefetch_target = -1;
+ scan->rs_pf_bhs_iterator = bhs_begin_iterate(tbm,
+ scan->bm_parallel ? scan->bm_parallel->prefetch_iterator : InvalidDsaPointer,
+ dsa);
}
#endif /* USE_PREFETCH */
@@ -244,36 +241,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
@@ -297,23 +264,9 @@ BitmapHeapNext(BitmapHeapScanState *node)
new_page:
- BitmapAdjustPrefetchIterator(node);
-
- if (!table_scan_bitmap_next_block(scan, &node->recheck, &node->blockno,
+ if (!table_scan_bitmap_next_block(scan, &node->recheck, &scan->blockno,
&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 &&
- node->pfblockno < node->blockno)
- elog(ERROR, "prefetch and main iterators are out of sync");
-
- /* Adjust the prefetch target */
- BitmapAdjustPrefetchTarget(node);
}
/*
@@ -337,219 +290,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)
- {
- /* Do not let the prefetch iterator get behind the main one */
- bhs_iterate(prefetch_iterator, &tbmpre);
- node->pfblockno = tbmpre.blockno;
- }
- 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)
- {
- bhs_iterate(prefetch_iterator, &tbmpre);
- node->pfblockno = tbmpre.blockno;
- }
- }
- }
-#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)
- {
- while (node->prefetch_pages < node->prefetch_target)
- {
- TBMIterateResult tbmpre;
- bool skip_fetch;
-
- bhs_iterate(prefetch_iterator, &tbmpre);
-
- if (!BlockNumberIsValid(tbmpre.blockno))
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- node->pf_iterator = NULL;
- 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)
- {
- 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;
-
- bhs_iterate(prefetch_iterator, &tbmpre);
- if (!BlockNumberIsValid(tbmpre.blockno))
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- node->pf_iterator = NULL;
- 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
*/
@@ -595,22 +335,12 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node)
if (node->ss.ss_currentScanDesc)
table_rescan(node->ss.ss_currentScanDesc, NULL);
- /* release bitmaps and buffers if any */
- if (node->pf_iterator)
- {
- bhs_end_iterate(node->pf_iterator);
- node->pf_iterator = NULL;
- }
+ /* release bitmaps if any */
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;
ExecScanReScan(&node->ss);
@@ -649,14 +379,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node)
table_endscan(scanDesc);
/*
- * release bitmaps and buffers if any
+ * release bitmaps if any
*/
- if (node->pf_iterator)
- bhs_end_iterate(node->pf_iterator);
if (node->tbm)
tbm_free(node->tbm);
- if (node->pvmbuffer != InvalidBuffer)
- ReleaseBuffer(node->pvmbuffer);
}
/* ----------------------------------------------------------------
@@ -689,17 +415,11 @@ 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->pf_iterator = NULL;
- scanstate->prefetch_pages = 0;
- scanstate->prefetch_target = 0;
scanstate->initialized = false;
scanstate->pstate = NULL;
scanstate->recheck = true;
- scanstate->blockno = InvalidBlockNumber;
- scanstate->pfblockno = InvalidBlockNumber;
/*
* Miscellaneous initialization
@@ -739,13 +459,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;
/*
@@ -829,7 +542,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);
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 750ea30852e..2e262bf4354 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -86,6 +86,23 @@ typedef struct HeapScanDescData
Buffer rs_vmbuffer;
int rs_empty_tuples_pending;
+ /*
+ * These fields only used for prefetching in bitmap table scans
+ */
+
+ /* buffer for visibility-map lookups of prefetched pages */
+ Buffer pvmbuffer;
+
+ /*
+ * These fields only used in serial BHS
+ */
+ /* 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;
+
/* 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/relscan.h b/src/include/access/relscan.h
index fb22f305bf6..7938b741d66 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -26,6 +26,7 @@
struct ParallelTableScanDescData;
struct BitmapHeapIterator;
+struct ParallelBitmapHeapState;
/*
* Generic descriptor for table scans. This is the base-class for table scans,
@@ -45,6 +46,13 @@ typedef struct TableScanDescData
/* Only used for Bitmap table scans */
struct BitmapHeapIterator *rs_bhs_iterator;
+ struct BitmapHeapIterator *rs_pf_bhs_iterator;
+
+ /* maximum value for prefetch_target */
+ int prefetch_maximum;
+ struct ParallelBitmapHeapState *bm_parallel;
+ /* used to validate BHS prefetch and current block stay in sync */
+ BlockNumber blockno;
/*
* 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.h
index 70e538b76ba..a2b229fb877 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -785,17 +785,6 @@ typedef struct TableAmRoutine
* lossy_pages is incremented if the block's representation in the bitmap
* is lossy, 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.
*/
@@ -945,6 +934,9 @@ table_beginscan_bm(Relation rel, Snapshot snapshot,
result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
result->rs_bhs_iterator = NULL;
+ result->rs_pf_bhs_iterator = NULL;
+ result->prefetch_maximum = 0;
+ result->bm_parallel = NULL;
return result;
}
@@ -996,6 +988,12 @@ table_endscan(TableScanDesc scan)
{
bhs_end_iterate(scan->rs_bhs_iterator);
scan->rs_bhs_iterator = NULL;
+
+ if (scan->rs_pf_bhs_iterator)
+ {
+ bhs_end_iterate(scan->rs_pf_bhs_iterator);
+ scan->rs_pf_bhs_iterator = NULL;
+ }
}
scan->rs_rd->rd_tableam->scan_end(scan);
@@ -1012,6 +1010,12 @@ table_rescan(TableScanDesc scan,
{
bhs_end_iterate(scan->rs_bhs_iterator);
scan->rs_bhs_iterator = NULL;
+
+ if (scan->rs_pf_bhs_iterator)
+ {
+ bhs_end_iterate(scan->rs_pf_bhs_iterator);
+ scan->rs_pf_bhs_iterator = NULL;
+ }
}
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index cf8b4995f0d..592215d5ee7 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1794,18 +1794,11 @@ struct BitmapHeapIterator;
*
* 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
@@ -1813,18 +1806,11 @@ 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;
- struct BitmapHeapIterator *pf_iterator;
ParallelBitmapHeapState *pstate;
bool recheck;
- BlockNumber blockno;
- BlockNumber pfblockno;
} BitmapHeapScanState;
/* ----------------
--
2.44.0
[text/x-patch] v14-0018-Remove-table_scan_bitmap_next_block.patch (11.8K, ../[email protected]/19-v14-0018-Remove-table_scan_bitmap_next_block.patch)
download | inline diff:
From 8076d43ea02fa6d5bc137ce3721846a3dc492b72 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 15:43:10 -0400
Subject: [PATCH v14 18/19] 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(). Heap AM's implementation of
table_scan_bitmap_next_tuple() now calls the original
heapam_scan_bitmap_next_block() function, but it is no longer an
implementation of a table AM callback but instead a helper for
heapam_scan_bitmap_next_tuple()
---
src/backend/access/heap/heapam.c | 2 +
src/backend/access/heap/heapam_handler.c | 48 ++++++++-------
src/backend/access/table/tableamapi.c | 2 -
src/backend/executor/nodeBitmapHeapscan.c | 45 ++++++--------
src/backend/optimizer/util/plancat.c | 2 +-
src/include/access/tableam.h | 75 +++++------------------
6 files changed, 63 insertions(+), 111 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index bda6abb8d0c..6c575ffc168 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -324,6 +324,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.c
index 0138a23e9d4..3e92725a821 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -2183,12 +2183,6 @@ heapam_estimate_rel_size(Relation rel, int32 *attr_widths,
HEAP_USABLE_BYTES_PER_PAGE);
}
-
-/* ------------------------------------------------------------------------
- * Executor related callbacks for the heap AM
- * ------------------------------------------------------------------------
- */
-
/*
* BitmapAdjustPrefetchIterator - Adjust the prefetch iterator
*
@@ -2222,8 +2216,8 @@ BitmapAdjustPrefetchIterator(HeapScanDesc 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->rs_base.prefetch_maximum > 0)
{
@@ -2586,30 +2580,43 @@ BitmapPrefetch(HeapScanDesc scan)
#endif /* USE_PREFETCH */
}
+/* ------------------------------------------------------------------------
+ * 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)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
OffsetNumber targoffset;
Page page;
ItemId lp;
- if (hscan->rs_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);
- hscan->rs_empty_tuples_pending--;
- return true;
- }
+ 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
- */
- if (hscan->rs_cindex < 0 || hscan->rs_cindex >= hscan->rs_ntuples)
- return false;
+ if (!heapam_scan_bitmap_next_block(scan, recheck, &scan->blockno,
+ lossy_pages, exact_pages))
+ return false;
+ }
#ifdef USE_PREFETCH
@@ -3006,7 +3013,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.c
index 55b8caeadf2..0b87530a9c4 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.c
index ea309cf93e3..7f651004113 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -231,44 +231,35 @@ BitmapHeapNext(BitmapHeapScanState *node)
#endif /* USE_PREFETCH */
node->initialized = true;
-
- goto new_page;
}
- for (;;)
+ 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->recheck, &scan->blockno,
- &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..
*/
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 6bb53e4346f..cf56cc572fe 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.h
index a2b229fb877..79689ec3773 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -773,36 +773,20 @@ 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.
- *
- * 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 block's representation in the bitmap
- * is lossy, 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,
- bool *recheck,
- BlockNumber *blockno,
- 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
@@ -1981,44 +1965,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 if bitmap is lossy for the selected block and exact_pages is
- * incremented otherwise.
- *
- * 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,
- bool *recheck, BlockNumber *blockno,
- 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, recheck,
- blockno, 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
@@ -2029,7 +1982,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);
}
/*
--
2.44.0
[text/x-patch] v14-0019-BitmapHeapScan-uses-read-stream-API.patch (26.2K, ../[email protected]/20-v14-0019-BitmapHeapScan-uses-read-stream-API.patch)
download | inline diff:
From 9a98aa4c74d301500d5d3ce9b0ddf46360167a41 Mon Sep 17 00:00:00 2001
From: Melanie Plageman <[email protected]>
Date: Fri, 22 Mar 2024 16:51:40 -0400
Subject: [PATCH v14 19/19] BitmapHeapScan uses read stream API
Remove all of the code to do prefetching from BitmapHeapScan code and
rely on the read stream API prefetching. Heap table AM implements a read
stream callback which uses the iterator to get the next valid block that
needs to be fetched for the read stream API.
---
src/backend/access/heap/heapam.c | 95 ++++--
src/backend/access/heap/heapam_handler.c | 347 +++-------------------
src/backend/executor/nodeBitmapHeapscan.c | 43 +--
src/include/access/heapam.h | 21 +-
src/include/access/relscan.h | 6 -
src/include/access/tableam.h | 14 -
src/include/nodes/execnodes.h | 9 +-
7 files changed, 115 insertions(+), 420 deletions(-)
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 6c575ffc168..b0af15eb9c0 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -113,6 +113,8 @@ static XLogRecPtr log_heap_new_cid(Relation relation, HeapTuple tup);
static HeapTuple ExtractReplicaIdentity(Relation relation, HeapTuple tp, bool key_required,
bool *copy);
+static BlockNumber bitmapheap_stream_read_next(ReadStream *pgsr, void *pgsr_private,
+ void *per_buffer_data);
/*
* Each tuple lock mode has a corresponding heavyweight lock, and one or two
@@ -969,16 +971,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_base.blockno = InvalidBlockNumber;
-
scan->rs_vmbuffer = InvalidBuffer;
scan->rs_empty_tuples_pending = 0;
- scan->pvmbuffer = InvalidBuffer;
-
- scan->pfblockno = InvalidBlockNumber;
- scan->prefetch_target = -1;
- scan->prefetch_pages = 0;
/*
* Disable page-at-a-time mode if it's not a MVCC-safe snapshot.
@@ -1033,6 +1027,23 @@ heap_beginscan(Relation relation, Snapshot snapshot,
initscan(scan, key, false);
+ /*
+ * Make the read stream object for BitmapHeapScan. BitmapHeapScan does not
+ * use a BufferAccessStrategy, so we could do this before initscan(), but
+ * wait until after so the scan descriptor is fully set up. On rescan, the
+ * stream will be reset.
+ */
+ if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN)
+ {
+ scan->rs_read_stream = read_stream_begin_relation(READ_STREAM_DEFAULT,
+ scan->rs_strategy,
+ scan->rs_base.rs_rd,
+ MAIN_FORKNUM,
+ bitmapheap_stream_read_next,
+ scan,
+ sizeof(TBMIterateResult));
+ }
+
return (TableScanDesc) scan;
}
@@ -1061,12 +1072,6 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
scan->rs_base.rs_flags &= ~SO_ALLOW_PAGEMODE;
}
- scan->rs_base.blockno = InvalidBlockNumber;
-
- scan->pfblockno = InvalidBlockNumber;
- scan->prefetch_target = -1;
- scan->prefetch_pages = 0;
-
/*
* unpin scan buffers
*/
@@ -1079,11 +1084,8 @@ heap_rescan(TableScanDesc sscan, ScanKey key, bool set_params,
scan->rs_vmbuffer = InvalidBuffer;
}
- if (BufferIsValid(scan->pvmbuffer))
- {
- ReleaseBuffer(scan->pvmbuffer);
- scan->pvmbuffer = InvalidBuffer;
- }
+ if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN && scan->rs_read_stream)
+ read_stream_reset(scan->rs_read_stream);
/*
* reinitialize scan descriptor
@@ -1110,11 +1112,8 @@ heap_endscan(TableScanDesc sscan)
scan->rs_vmbuffer = InvalidBuffer;
}
- if (BufferIsValid(scan->pvmbuffer))
- {
- ReleaseBuffer(scan->pvmbuffer);
- scan->pvmbuffer = InvalidBuffer;
- }
+ if (scan->rs_base.rs_flags & SO_TYPE_BITMAPSCAN && scan->rs_read_stream)
+ read_stream_end(scan->rs_read_stream);
/*
* decrement relation reference count and free scan descriptor storage
@@ -10130,3 +10129,51 @@ HeapCheckForSerializableConflictOut(bool visible, Relation relation,
CheckForSerializableConflictOut(relation, xid, snapshot);
}
+
+static BlockNumber
+bitmapheap_stream_read_next(ReadStream *pgsr, void *private_data,
+ void *per_buffer_data)
+{
+ TBMIterateResult *tbmres = per_buffer_data;
+ HeapScanDesc hdesc = (HeapScanDesc) private_data;
+
+ for (;;)
+ {
+ CHECK_FOR_INTERRUPTS();
+
+ bhs_iterate(hdesc->rs_base.rs_bhs_iterator, tbmres);
+
+ /* no more entries in the bitmap */
+ if (!BlockNumberIsValid(tbmres->blockno))
+ return InvalidBlockNumber;
+
+ /*
+ * 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() && tbmres->blockno >= hdesc->rs_nblocks)
+ continue;
+
+ /*
+ * 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 (!(hdesc->rs_base.rs_flags & SO_NEED_TUPLE) &&
+ !tbmres->recheck &&
+ VM_ALL_VISIBLE(hdesc->rs_base.rs_rd, tbmres->blockno, &hdesc->rs_vmbuffer))
+ {
+ hdesc->rs_empty_tuples_pending += tbmres->ntuples;
+ continue;
+ }
+
+ return tbmres->blockno;
+ }
+
+ /* not reachable */
+ Assert(false);
+}
diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c
index 3e92725a821..cc6289b6343 100644
--- a/src/backend/access/heap/heapam_handler.c
+++ b/src/backend/access/heap/heapam_handler.c
@@ -60,9 +60,6 @@ static bool SampleHeapTupleVisible(TableScanDesc scan, Buffer buffer,
OffsetNumber tupoffset);
static BlockNumber heapam_scan_get_blocks_done(HeapScanDesc hscan);
-static inline void BitmapAdjustPrefetchIterator(HeapScanDesc scan);
-static inline void BitmapAdjustPrefetchTarget(HeapScanDesc scan);
-static inline void BitmapPrefetch(HeapScanDesc scan);
static const TableAmRoutine heapam_methods;
@@ -2183,147 +2180,68 @@ 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.
- */
-static inline void
-BitmapAdjustPrefetchIterator(HeapScanDesc scan)
-{
-#ifdef USE_PREFETCH
- BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator;
- ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
- TBMIterateResult tbmpre;
-
- if (pstate == NULL)
- {
- if (scan->prefetch_pages > 0)
- {
- /* The main iterator has closed the distance by one page */
- scan->prefetch_pages--;
- }
- else if (prefetch_iterator)
- {
- /* Do not let the prefetch iterator get behind the main one */
- bhs_iterate(prefetch_iterator, &tbmpre);
- scan->pfblockno = tbmpre.blockno;
- }
- return;
- }
-
- /*
- * Adjusting the prefetch iterator before invoking
- * heapam_bitmap_next_block() keeps prefetch distance higher across the
- * parallel workers.
- */
- if (scan->rs_base.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)
- {
- bhs_iterate(prefetch_iterator, &tbmpre);
- scan->pfblockno = tbmpre.blockno;
- }
- }
- }
-#endif /* USE_PREFETCH */
-}
static bool
-heapam_scan_bitmap_next_block(TableScanDesc scan,
- bool *recheck, BlockNumber *blockno,
+heapam_scan_bitmap_next_block(TableScanDesc scan, bool *recheck,
long *lossy_pages, long *exact_pages)
{
HeapScanDesc hscan = (HeapScanDesc) scan;
+ void *per_buffer_data;
BlockNumber block;
Buffer buffer;
Snapshot snapshot;
int ntup;
- TBMIterateResult tbmres;
+ TBMIterateResult *tbmres;
+
+ Assert(hscan->rs_read_stream);
hscan->rs_cindex = 0;
hscan->rs_ntuples = 0;
- *blockno = InvalidBlockNumber;
*recheck = true;
- BitmapAdjustPrefetchIterator(hscan);
-
- do
+ /* Release buffer containing previous block. */
+ if (BufferIsValid(hscan->rs_cbuf))
{
- CHECK_FOR_INTERRUPTS();
+ ReleaseBuffer(hscan->rs_cbuf);
+ hscan->rs_cbuf = InvalidBuffer;
+ }
- bhs_iterate(scan->rs_bhs_iterator, &tbmres);
+ hscan->rs_cbuf = read_stream_next_buffer(hscan->rs_read_stream, &per_buffer_data);
- if (!BlockNumberIsValid(tbmres.blockno))
+ if (BufferIsInvalid(hscan->rs_cbuf))
+ {
+ if (BufferIsValid(hscan->rs_vmbuffer))
{
- /* no more entries in the bitmap */
- Assert(hscan->rs_empty_tuples_pending == 0);
- return false;
+ ReleaseBuffer(hscan->rs_vmbuffer);
+ hscan->rs_vmbuffer = InvalidBuffer;
}
/*
- * 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.
+ * Bitmap is exhausted. Time to emit empty tuples if relevant. We emit
+ * all empty tuples at the end instead of emitting them per block we
+ * skip fetching. This is necessary because the streaming read API
+ * will only return TBMIterateResults for blocks actually fetched.
+ * When we skip fetching a block, we keep track of how many empty
+ * tuples to emit at the end of the BitmapHeapScan. We do not recheck
+ * all NULL tuples.
*/
- } while (!IsolationIsSerializable() && tbmres.blockno >= hscan->rs_nblocks);
+ *recheck = false;
+ return hscan->rs_empty_tuples_pending > 0;
+ }
- /* Got a valid block */
- *blockno = tbmres.blockno;
- *recheck = tbmres.recheck;
+ Assert(per_buffer_data);
- /*
- * 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);
+ tbmres = per_buffer_data;
- hscan->rs_empty_tuples_pending += tbmres.ntuples;
+ Assert(BufferGetBlockNumber(hscan->rs_cbuf) == tbmres->blockno);
- return true;
- }
+ *recheck = tbmres->recheck;
- block = tbmres.blockno;
+ hscan->rs_cblock = tbmres->blockno;
+ hscan->rs_ntuples = tbmres->ntuples;
- /*
- * Acquire pin on the target heap page, trading in any pin we held before.
- */
- hscan->rs_cbuf = ReleaseAndReadBuffer(hscan->rs_cbuf,
- scan->rs_rd,
- block);
- hscan->rs_cblock = block;
+ block = tbmres->blockno;
buffer = hscan->rs_cbuf;
snapshot = scan->rs_snapshot;
@@ -2344,7 +2262,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
/*
* We need two separate strategies for lossy and non-lossy cases.
*/
- if (tbmres.ntuples >= 0)
+ if (tbmres->ntuples >= 0)
{
/*
* Bitmap is non-lossy, so we just look through the offsets listed in
@@ -2353,9 +2271,9 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
*/
int curslot;
- for (curslot = 0; curslot < tbmres.ntuples; curslot++)
+ for (curslot = 0; curslot < tbmres->ntuples; curslot++)
{
- OffsetNumber offnum = tbmres.offsets[curslot];
+ OffsetNumber offnum = tbmres->offsets[curslot];
ItemPointerData tid;
HeapTupleData heapTuple;
@@ -2405,23 +2323,11 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
Assert(ntup <= MaxHeapTuplesPerPage);
hscan->rs_ntuples = ntup;
- if (tbmres.ntuples < 0)
+ if (tbmres->ntuples < 0)
(*lossy_pages)++;
else
(*exact_pages)++;
- /*
- * If serial, we can error out if the the prefetch block doesn't stay
- * ahead of the current block.
- */
- 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");
-
- /* Adjust the prefetch target */
- BitmapAdjustPrefetchTarget(hscan);
-
/*
* 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,
@@ -2432,153 +2338,6 @@ heapam_scan_bitmap_next_block(TableScanDesc scan,
return true;
}
-/*
- * 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(HeapScanDesc scan)
-{
-#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
- int prefetch_maximum = scan->rs_base.prefetch_maximum;
-
- if (pstate == NULL)
- {
- if (scan->prefetch_target >= prefetch_maximum)
- /* don't increase any further */ ;
- else if (scan->prefetch_target >= prefetch_maximum / 2)
- scan->prefetch_target = prefetch_maximum;
- else if (scan->prefetch_target > 0)
- scan->prefetch_target *= 2;
- else
- scan->prefetch_target++;
- return;
- }
-
- /* Do an unlocked check first to save spinlock acquisitions. */
- if (pstate->prefetch_target < prefetch_maximum)
- {
- SpinLockAcquire(&pstate->mutex);
- if (pstate->prefetch_target >= prefetch_maximum)
- /* don't increase any further */ ;
- else if (pstate->prefetch_target >= prefetch_maximum / 2)
- pstate->prefetch_target = 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(HeapScanDesc scan)
-{
-#ifdef USE_PREFETCH
- ParallelBitmapHeapState *pstate = scan->rs_base.bm_parallel;
- BitmapHeapIterator *prefetch_iterator = scan->rs_base.rs_pf_bhs_iterator;
-
- if (pstate == NULL)
- {
- if (prefetch_iterator)
- {
- while (scan->prefetch_pages < scan->prefetch_target)
- {
- TBMIterateResult tbmpre;
- bool skip_fetch;
-
- bhs_iterate(prefetch_iterator, &tbmpre);
-
- if (!BlockNumberIsValid(tbmpre.blockno))
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- scan->rs_base.rs_pf_bhs_iterator = NULL;
- break;
- }
- scan->prefetch_pages++;
- scan->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_base.rs_flags & SO_NEED_TUPLE) &&
- !tbmpre.recheck &&
- VM_ALL_VISIBLE(scan->rs_base.rs_rd,
- tbmpre.blockno,
- &scan->pvmbuffer));
-
- if (!skip_fetch)
- PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno);
- }
- }
-
- return;
- }
-
- if (pstate->prefetch_pages < pstate->prefetch_target)
- {
- if (prefetch_iterator)
- {
- 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;
-
- bhs_iterate(prefetch_iterator, &tbmpre);
- if (!BlockNumberIsValid(tbmpre.blockno))
- {
- /* No more pages to prefetch */
- bhs_end_iterate(prefetch_iterator);
- scan->rs_base.rs_pf_bhs_iterator = NULL;
- break;
- }
-
- scan->pfblockno = tbmpre.blockno;
-
- /* As above, skip prefetch if we expect not to need page */
- skip_fetch = (!(scan->rs_base.rs_flags & SO_NEED_TUPLE) &&
- !tbmpre.recheck &&
- VM_ALL_VISIBLE(scan->rs_base.rs_rd,
- tbmpre.blockno,
- &scan->pvmbuffer));
-
- if (!skip_fetch)
- PrefetchBuffer(scan->rs_base.rs_rd, MAIN_FORKNUM, tbmpre.blockno);
- }
- }
- }
-#endif /* USE_PREFETCH */
-}
/* ------------------------------------------------------------------------
* Executor related callbacks for the heap AM
@@ -2613,41 +2372,11 @@ heapam_scan_bitmap_next_tuple(TableScanDesc scan,
return true;
}
- if (!heapam_scan_bitmap_next_block(scan, recheck, &scan->blockno,
+ if (!heapam_scan_bitmap_next_block(scan, recheck,
lossy_pages, exact_pages))
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 (!scan->bm_parallel)
- {
- if (hscan->prefetch_target < scan->prefetch_maximum)
- hscan->prefetch_target++;
- }
- else if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum)
- {
- /* take spinlock while updating shared state */
- SpinLockAcquire(&scan->bm_parallel->mutex);
- if (scan->bm_parallel->prefetch_target < scan->prefetch_maximum)
- scan->bm_parallel->prefetch_target++;
- SpinLockRelease(&scan->bm_parallel->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(hscan);
-#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.c
index 7f651004113..38a6262e3c4 100644
--- a/src/backend/executor/nodeBitmapHeapscan.c
+++ b/src/backend/executor/nodeBitmapHeapscan.c
@@ -131,14 +131,6 @@ BitmapHeapNext(BitmapHeapScanState *node)
/*
* 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.
*/
if (!node->initialized)
{
@@ -149,15 +141,6 @@ 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.
- */
- int pf_maximum = 0;
-#ifdef USE_PREFETCH
- pf_maximum = get_tablespace_io_concurrency(node->ss.ss_currentRelation->rd_rel->reltablespace);
-#endif
-
if (!node->pstate || init_shared_state)
{
tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node));
@@ -174,13 +157,7 @@ BitmapHeapNext(BitmapHeapScanState *node)
* multiple processes to iterate jointly.
*/
node->pstate->tbmiterator = tbm_prepare_shared_iterate(tbm);
-#ifdef USE_PREFETCH
- if (pf_maximum > 0)
- {
- node->pstate->prefetch_iterator =
- tbm_prepare_shared_iterate(tbm);
- }
-#endif
+
/* We have initialized the shared state so wake up others. */
BitmapDoneInitializingSharedState(node->pstate);
}
@@ -214,22 +191,12 @@ BitmapHeapNext(BitmapHeapScanState *node)
node->ss.ss_currentScanDesc = scan;
}
- scan->prefetch_maximum = pf_maximum;
scan->bm_parallel = node->pstate;
scan->rs_bhs_iterator = bhs_begin_iterate(tbm,
scan->bm_parallel ? scan->bm_parallel->tbmiterator : InvalidDsaPointer,
dsa);
-#ifdef USE_PREFETCH
- if (scan->prefetch_maximum > 0)
- {
- scan->rs_pf_bhs_iterator = bhs_begin_iterate(tbm,
- scan->bm_parallel ? scan->bm_parallel->prefetch_iterator : InvalidDsaPointer,
- dsa);
- }
-#endif /* USE_PREFETCH */
-
node->initialized = true;
}
@@ -526,14 +493,10 @@ ExecBitmapHeapInitializeDSM(BitmapHeapScanState *node,
return;
pstate = shm_toc_allocate(pcxt->toc, sizeof(ParallelBitmapHeapState));
-
pstate->tbmiterator = 0;
- pstate->prefetch_iterator = 0;
/* Initialize the mutex */
SpinLockInit(&pstate->mutex);
- pstate->prefetch_pages = 0;
- pstate->prefetch_target = -1;
pstate->state = BM_INITIAL;
ConditionVariableInit(&pstate->cv);
@@ -564,11 +527,7 @@ ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node,
if (DsaPointerIsValid(pstate->tbmiterator))
tbm_free_shared_area(dsa, pstate->tbmiterator);
- if (DsaPointerIsValid(pstate->prefetch_iterator))
- tbm_free_shared_area(dsa, pstate->prefetch_iterator);
-
pstate->tbmiterator = InvalidDsaPointer;
- pstate->prefetch_iterator = InvalidDsaPointer;
}
/* ----------------------------------------------------------------
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 2e262bf4354..8702a20741b 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -26,6 +26,7 @@
#include "storage/dsm.h"
#include "storage/lockdefs.h"
#include "storage/shm_toc.h"
+#include "storage/read_stream.h"
#include "utils/relcache.h"
#include "utils/snapshot.h"
@@ -76,6 +77,9 @@ typedef struct HeapScanDescData
*/
ParallelBlockTableScanWorkerData *rs_parallelworkerdata;
+ /* Streaming read control object for scans supporting it */
+ ReadStream *rs_read_stream;
+
/*
* These fields are only used for bitmap scans for the "skip fetch"
* optimization. Bitmap scans needing no fields from the heap may skip
@@ -86,23 +90,6 @@ typedef struct HeapScanDescData
Buffer rs_vmbuffer;
int rs_empty_tuples_pending;
- /*
- * These fields only used for prefetching in bitmap table scans
- */
-
- /* buffer for visibility-map lookups of prefetched pages */
- Buffer pvmbuffer;
-
- /*
- * These fields only used in serial BHS
- */
- /* 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;
-
/* 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/relscan.h b/src/include/access/relscan.h
index 7938b741d66..02893bf99bc 100644
--- a/src/include/access/relscan.h
+++ b/src/include/access/relscan.h
@@ -46,13 +46,7 @@ typedef struct TableScanDescData
/* Only used for Bitmap table scans */
struct BitmapHeapIterator *rs_bhs_iterator;
- struct BitmapHeapIterator *rs_pf_bhs_iterator;
-
- /* maximum value for prefetch_target */
- int prefetch_maximum;
struct ParallelBitmapHeapState *bm_parallel;
- /* used to validate BHS prefetch and current block stay in sync */
- BlockNumber blockno;
/*
* 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.h
index 79689ec3773..aead772e6fd 100644
--- a/src/include/access/tableam.h
+++ b/src/include/access/tableam.h
@@ -918,8 +918,6 @@ table_beginscan_bm(Relation rel, Snapshot snapshot,
result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags);
result->rs_bhs_iterator = NULL;
- result->rs_pf_bhs_iterator = NULL;
- result->prefetch_maximum = 0;
result->bm_parallel = NULL;
return result;
}
@@ -972,12 +970,6 @@ table_endscan(TableScanDesc scan)
{
bhs_end_iterate(scan->rs_bhs_iterator);
scan->rs_bhs_iterator = NULL;
-
- if (scan->rs_pf_bhs_iterator)
- {
- bhs_end_iterate(scan->rs_pf_bhs_iterator);
- scan->rs_pf_bhs_iterator = NULL;
- }
}
scan->rs_rd->rd_tableam->scan_end(scan);
@@ -994,12 +986,6 @@ table_rescan(TableScanDesc scan,
{
bhs_end_iterate(scan->rs_bhs_iterator);
scan->rs_bhs_iterator = NULL;
-
- if (scan->rs_pf_bhs_iterator)
- {
- bhs_end_iterate(scan->rs_pf_bhs_iterator);
- scan->rs_pf_bhs_iterator = NULL;
- }
}
scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false);
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index 592215d5ee7..8d86b900a0b 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1767,11 +1767,7 @@ typedef enum
/* ----------------
* ParallelBitmapHeapState information
* tbmiterator iterator for scanning current pages
- * prefetch_iterator iterator for prefetching ahead of current page
- * mutex mutual exclusion for the prefetching variable
- * and state
- * prefetch_pages # pages prefetch iterator is ahead of current
- * prefetch_target current target prefetch distance
+ * mutex mutual exclusion for state
* state current state of the TIDBitmap
* cv conditional wait variable
* ----------------
@@ -1779,10 +1775,7 @@ typedef enum
typedef struct ParallelBitmapHeapState
{
dsa_pointer tbmiterator;
- dsa_pointer prefetch_iterator;
slock_t mutex;
- int prefetch_pages;
- int prefetch_target;
SharedBitmapState state;
ConditionVariable cv;
} ParallelBitmapHeapState;
--
2.44.0
view thread (21+ messages) latest in thread
reply
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BitmapHeapScan streaming read user and prelim refactoring
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox