From 64d69f1249731b740c510cd86163517a8b1320ec Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 25 Mar 2024 11:05:51 -0400 Subject: [PATCH v10 10/17] 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, 116 insertions(+), 107 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 2ad785e511..c76849a98e 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2133,10 +2133,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 7e73583fe5..fe471a8a0c 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); +} /* ---------------------------------------------------------------- @@ -97,43 +147,23 @@ BitmapHeapNext(BitmapHeapScanState *node) */ if (!node->initialized) { - TBMIterator *tbmiterator = NULL; - TBMSharedIterator *shared_tbmiterator = NULL; + /* + * The leader will immediately come out of the function, but others + * will be blocked until leader populates the TBM and wakes them up. + */ + bool init_shared_state = node->pstate ? + BitmapShouldInitializeSharedState(node->pstate) : false; - if (!pstate) + if (!pstate || init_shared_state) { tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node)); if (!tbm || !IsA(tbm, TIDBitmap)) elog(ERROR, "unrecognized result from subplan"); - node->tbm = tbm; - tbmiterator = tbm_begin_iterate(tbm); -#ifdef USE_PREFETCH - if (node->prefetch_maximum > 0) - { - 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 @@ -154,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 */ } /* @@ -198,8 +216,21 @@ BitmapHeapNext(BitmapHeapScanState *node) extra_flags); } - 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; @@ -280,7 +311,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"); @@ -321,12 +352,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 */ @@ -335,7 +365,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; @@ -348,8 +378,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (node->prefetch_maximum > 0) { - TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator; - SpinLockAcquire(&pstate->mutex); if (pstate->prefetch_pages > 0) { @@ -371,7 +399,7 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (prefetch_iterator) { - tbmpre = tbm_shared_iterate(prefetch_iterator); + tbmpre = bhs_iterate(prefetch_iterator); node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; } } @@ -431,23 +459,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++; @@ -475,8 +502,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) @@ -500,12 +525,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; } @@ -572,18 +597,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; @@ -628,12 +652,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); } @@ -671,11 +693,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 92b829cebc..fb22f305bf 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 a820cc8c99..29387166c1 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -957,8 +957,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; } @@ -1021,17 +1020,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); @@ -1046,17 +1036,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 ea003a9caa..cb56d20dc6 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 8688bc5ab0..52cedd1b35 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1778,6 +1778,8 @@ typedef struct ParallelBitmapHeapState ConditionVariable cv; } ParallelBitmapHeapState; +struct BitmapHeapIterator; + /* ---------------- * BitmapHeapScanState information * @@ -1786,12 +1788,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 @@ -1806,12 +1807,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 4679660837..6d5cb0bdaa 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -259,6 +259,7 @@ BitString BitmapAnd BitmapAndPath BitmapAndState +BitmapHeapIterator BitmapHeapPath BitmapHeapScan BitmapHeapScanState -- 2.40.1