From afb28afbf45efbdd9fa4992a3dc1f1fe7f07afd3 Mon Sep 17 00:00:00 2001 From: Melanie Plageman Date: Mon, 25 Mar 2024 11:05:51 -0400 Subject: [PATCH v16 10/18] Unify parallel and serial BitmapHeapScan iterator interfaces Introduce a new type, BitmapHeapIterator, which allows unified access to both TBMIterator and TBMSharedIterators. This encapsulates the parallel and serial iterators and their access and makes the bitmap heap scan code a bit cleaner. This naturally lends itself to a bit of reorganization of the !node->initialized path in BitmapHeapNext(). Author: Melanie Plageman Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com --- src/backend/access/heap/heapam_handler.c | 5 +- src/backend/executor/nodeBitmapHeapscan.c | 172 ++++++++++++---------- src/include/access/relscan.h | 7 +- src/include/access/tableam.h | 40 +---- src/include/executor/nodeBitmapHeapscan.h | 7 + src/include/nodes/execnodes.h | 13 +- src/tools/pgindent/typedefs.list | 1 + 7 files changed, 124 insertions(+), 121 deletions(-) diff --git a/src/backend/access/heap/heapam_handler.c b/src/backend/access/heap/heapam_handler.c index 9d3e7c7fda..5e57d19d91 100644 --- a/src/backend/access/heap/heapam_handler.c +++ b/src/backend/access/heap/heapam_handler.c @@ -2208,10 +2208,7 @@ heapam_scan_bitmap_next_block(TableScanDesc scan, { CHECK_FOR_INTERRUPTS(); - if (scan->shared_tbmiterator) - tbmres = tbm_shared_iterate(scan->shared_tbmiterator); - else - tbmres = tbm_iterate(scan->tbmiterator); + tbmres = bhs_iterate(&scan->rs_bhs_iterator); if (tbmres == NULL) { diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c index e1b13ddaa6..70b560b8ee 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -57,6 +57,61 @@ static inline void BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan); static bool BitmapShouldInitializeSharedState(ParallelBitmapHeapState *pstate); +/* + * Start iteration on a shared or non-shared bitmap iterator. Note that tbm + * will only be provided by serial BitmapHeapScan callers. dsa and dsp will + * only be provided by parallel BitmapHeapScan callers. + */ +void +bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm, + dsa_area *dsa, dsa_pointer dsp) +{ + Assert(iterator); + + iterator->serial = NULL; + iterator->parallel = NULL; + iterator->exhausted = false; + + /* Allocate a private iterator and attach the shared state to it */ + if (DsaPointerIsValid(dsp)) + iterator->parallel = tbm_attach_shared_iterate(dsa, dsp); + else + iterator->serial = tbm_begin_iterate(tbm); +} + +/* + * Get the next TBMIterateResult from the bitmap iterator. + */ +TBMIterateResult * +bhs_iterate(BitmapHeapIterator *iterator) +{ + Assert(iterator); + Assert(!iterator->exhausted); + + if (iterator->serial) + return tbm_iterate(iterator->serial); + else + return tbm_shared_iterate(iterator->parallel); +} + +/* + * Clean up the bitmap iterator. + */ +void +bhs_end_iterate(BitmapHeapIterator *iterator) +{ + Assert(iterator); + + if (iterator->serial) + tbm_end_iterate(iterator->serial); + else if (iterator->parallel) + tbm_end_shared_iterate(iterator->parallel); + + iterator->serial = NULL; + iterator->parallel = NULL; + iterator->exhausted = true; +} + /* ---------------------------------------------------------------- * BitmapHeapNext @@ -96,43 +151,23 @@ BitmapHeapNext(BitmapHeapScanState *node) */ if (!node->initialized) { - TBMIterator *tbmiterator = NULL; - TBMSharedIterator *shared_tbmiterator = NULL; + /* + * The leader will immediately come out of the function, but others + * will be blocked until leader populates the TBM and wakes them up. + */ + bool init_shared_state = node->pstate ? + BitmapShouldInitializeSharedState(node->pstate) : false; - if (!pstate) + if (!pstate || init_shared_state) { tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node)); if (!tbm || !IsA(tbm, TIDBitmap)) elog(ERROR, "unrecognized result from subplan"); - node->tbm = tbm; - tbmiterator = tbm_begin_iterate(tbm); -#ifdef USE_PREFETCH - if (node->prefetch_maximum > 0) + if (init_shared_state) { - node->prefetch_iterator = tbm_begin_iterate(tbm); - node->prefetch_pages = 0; - node->prefetch_target = -1; - } -#endif /* USE_PREFETCH */ - } - else - { - /* - * The leader will immediately come out of the function, but - * others will be blocked until leader populates the TBM and wakes - * them up. - */ - if (BitmapShouldInitializeSharedState(pstate)) - { - tbm = (TIDBitmap *) MultiExecProcNode(outerPlanState(node)); - if (!tbm || !IsA(tbm, TIDBitmap)) - elog(ERROR, "unrecognized result from subplan"); - - node->tbm = tbm; - /* * Prepare to iterate over the TBM. This will return the * dsa_pointer of the iterator state which will be used by @@ -153,21 +188,9 @@ BitmapHeapNext(BitmapHeapScanState *node) pstate->prefetch_target = -1; } #endif - /* We have initialized the shared state so wake up others. */ BitmapDoneInitializingSharedState(pstate); } - - /* Allocate a private iterator and attach the shared state to it */ - shared_tbmiterator = tbm_attach_shared_iterate(dsa, pstate->tbmiterator); - -#ifdef USE_PREFETCH - if (node->prefetch_maximum > 0) - { - node->shared_prefetch_iterator = - tbm_attach_shared_iterate(dsa, pstate->prefetch_iterator); - } -#endif /* USE_PREFETCH */ } /* @@ -198,8 +221,23 @@ BitmapHeapNext(BitmapHeapScanState *node) node->ss.ss_currentScanDesc = scan; } - scan->tbmiterator = tbmiterator; - scan->shared_tbmiterator = shared_tbmiterator; + bhs_begin_iterate(&scan->rs_bhs_iterator, tbm, dsa, + pstate ? + pstate->tbmiterator : + InvalidDsaPointer); + +#ifdef USE_PREFETCH + if (node->prefetch_maximum > 0) + { + bhs_begin_iterate(&node->pf_iterator, tbm, dsa, + pstate ? + pstate->prefetch_iterator : + InvalidDsaPointer); + /* Only used for serial BHS */ + node->prefetch_pages = 0; + node->prefetch_target = -1; + } +#endif /* USE_PREFETCH */ node->initialized = true; @@ -276,7 +314,7 @@ new_page: * ahead of the current block. */ if (node->pstate == NULL && - node->prefetch_iterator && + !node->pf_iterator.exhausted && node->pfblockno < node->blockno) elog(ERROR, "prefetch and main iterators are out of sync"); @@ -317,21 +355,20 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) { #ifdef USE_PREFETCH ParallelBitmapHeapState *pstate = node->pstate; + BitmapHeapIterator *prefetch_iterator = &node->pf_iterator; TBMIterateResult *tbmpre; if (pstate == NULL) { - TBMIterator *prefetch_iterator = node->prefetch_iterator; - if (node->prefetch_pages > 0) { /* The main iterator has closed the distance by one page */ node->prefetch_pages--; } - else if (prefetch_iterator) + else if (!prefetch_iterator->exhausted) { /* Do not let the prefetch iterator get behind the main one */ - tbmpre = tbm_iterate(prefetch_iterator); + tbmpre = bhs_iterate(prefetch_iterator); node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; } return; @@ -344,8 +381,6 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) */ if (node->prefetch_maximum > 0) { - TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator; - SpinLockAcquire(&pstate->mutex); if (pstate->prefetch_pages > 0) { @@ -365,9 +400,9 @@ BitmapAdjustPrefetchIterator(BitmapHeapScanState *node) * we don't validate the blockno here as we do in non-parallel * case. */ - if (prefetch_iterator) + if (!prefetch_iterator->exhausted) { - tbmpre = tbm_shared_iterate(prefetch_iterator); + tbmpre = bhs_iterate(prefetch_iterator); node->pfblockno = tbmpre ? tbmpre->blockno : InvalidBlockNumber; } } @@ -427,23 +462,21 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) { #ifdef USE_PREFETCH ParallelBitmapHeapState *pstate = node->pstate; + BitmapHeapIterator *prefetch_iterator = &node->pf_iterator; if (pstate == NULL) { - TBMIterator *prefetch_iterator = node->prefetch_iterator; - - if (prefetch_iterator) + if (!prefetch_iterator->exhausted) { while (node->prefetch_pages < node->prefetch_target) { - TBMIterateResult *tbmpre = tbm_iterate(prefetch_iterator); + TBMIterateResult *tbmpre = bhs_iterate(prefetch_iterator); bool skip_fetch; if (tbmpre == NULL) { /* No more pages to prefetch */ - tbm_end_iterate(prefetch_iterator); - node->prefetch_iterator = NULL; + bhs_end_iterate(prefetch_iterator); break; } node->prefetch_pages++; @@ -471,9 +504,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) if (pstate->prefetch_pages < pstate->prefetch_target) { - TBMSharedIterator *prefetch_iterator = node->shared_prefetch_iterator; - - if (prefetch_iterator) + if (!prefetch_iterator->exhausted) { while (1) { @@ -496,12 +527,11 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) if (!do_prefetch) return; - tbmpre = tbm_shared_iterate(prefetch_iterator); + tbmpre = bhs_iterate(prefetch_iterator); if (tbmpre == NULL) { /* No more pages to prefetch */ - tbm_end_shared_iterate(prefetch_iterator); - node->shared_prefetch_iterator = NULL; + bhs_end_iterate(prefetch_iterator); break; } @@ -568,18 +598,14 @@ ExecReScanBitmapHeapScan(BitmapHeapScanState *node) table_rescan(node->ss.ss_currentScanDesc, NULL); /* release bitmaps and buffers if any */ - if (node->prefetch_iterator) - tbm_end_iterate(node->prefetch_iterator); - if (node->shared_prefetch_iterator) - tbm_end_shared_iterate(node->shared_prefetch_iterator); + if (node->pf_iterator.exhausted) + bhs_end_iterate(&node->pf_iterator); if (node->tbm) tbm_free(node->tbm); if (node->pvmbuffer != InvalidBuffer) ReleaseBuffer(node->pvmbuffer); node->tbm = NULL; - node->prefetch_iterator = NULL; node->initialized = false; - node->shared_prefetch_iterator = NULL; node->pvmbuffer = InvalidBuffer; node->recheck = true; node->blockno = InvalidBlockNumber; @@ -624,12 +650,10 @@ ExecEndBitmapHeapScan(BitmapHeapScanState *node) /* * release bitmaps and buffers if any */ - if (node->prefetch_iterator) - tbm_end_iterate(node->prefetch_iterator); + if (!node->pf_iterator.exhausted) + bhs_end_iterate(&node->pf_iterator); if (node->tbm) tbm_free(node->tbm); - if (node->shared_prefetch_iterator) - tbm_end_shared_iterate(node->shared_prefetch_iterator); if (node->pvmbuffer != InvalidBuffer) ReleaseBuffer(node->pvmbuffer); } @@ -667,11 +691,9 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags) scanstate->pvmbuffer = InvalidBuffer; scanstate->exact_pages = 0; scanstate->lossy_pages = 0; - scanstate->prefetch_iterator = NULL; scanstate->prefetch_pages = 0; scanstate->prefetch_target = 0; scanstate->initialized = false; - scanstate->shared_prefetch_iterator = NULL; scanstate->pstate = NULL; scanstate->recheck = true; scanstate->blockno = InvalidBlockNumber; diff --git a/src/include/access/relscan.h b/src/include/access/relscan.h index 92b829cebc..e520186b41 100644 --- a/src/include/access/relscan.h +++ b/src/include/access/relscan.h @@ -20,13 +20,11 @@ #include "storage/buf.h" #include "storage/spin.h" #include "utils/relcache.h" +#include "executor/nodeBitmapHeapscan.h" struct ParallelTableScanDescData; -struct TBMIterator; -struct TBMSharedIterator; - /* * Generic descriptor for table scans. This is the base-class for table scans, * which needs to be embedded in the scans of individual AMs. @@ -44,8 +42,7 @@ typedef struct TableScanDescData ItemPointerData rs_maxtid; /* Only used for Bitmap table scans */ - struct TBMIterator *tbmiterator; - struct TBMSharedIterator *shared_tbmiterator; + BitmapHeapIterator rs_bhs_iterator; /* * Information about type and behaviour of the scan, a bitmask of members diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 42c67a128e..618ab2b449 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -938,13 +938,9 @@ static inline TableScanDesc table_beginscan_bm(Relation rel, Snapshot snapshot, int nkeys, struct ScanKeyData *key, uint32 extra_flags) { - TableScanDesc result; uint32 flags = SO_TYPE_BITMAPSCAN | SO_ALLOW_PAGEMODE | extra_flags; - result = rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); - result->shared_tbmiterator = NULL; - result->tbmiterator = NULL; - return result; + return rel->rd_tableam->scan_begin(rel, snapshot, nkeys, key, NULL, flags); } /* @@ -991,20 +987,9 @@ table_beginscan_tid(Relation rel, Snapshot snapshot) static inline void table_endscan(TableScanDesc scan) { - if (scan->rs_flags & SO_TYPE_BITMAPSCAN) - { - if (scan->shared_tbmiterator) - { - tbm_end_shared_iterate(scan->shared_tbmiterator); - scan->shared_tbmiterator = NULL; - } - - if (scan->tbmiterator) - { - tbm_end_iterate(scan->tbmiterator); - scan->tbmiterator = NULL; - } - } + if (scan->rs_flags & SO_TYPE_BITMAPSCAN && + !scan->rs_bhs_iterator.exhausted) + bhs_end_iterate(&scan->rs_bhs_iterator); scan->rs_rd->rd_tableam->scan_end(scan); } @@ -1016,20 +1001,9 @@ static inline void table_rescan(TableScanDesc scan, struct ScanKeyData *key) { - if (scan->rs_flags & SO_TYPE_BITMAPSCAN) - { - if (scan->shared_tbmiterator) - { - tbm_end_shared_iterate(scan->shared_tbmiterator); - scan->shared_tbmiterator = NULL; - } - - if (scan->tbmiterator) - { - tbm_end_iterate(scan->tbmiterator); - scan->tbmiterator = NULL; - } - } + if (scan->rs_flags & SO_TYPE_BITMAPSCAN && + !scan->rs_bhs_iterator.exhausted) + bhs_end_iterate(&scan->rs_bhs_iterator); scan->rs_rd->rd_tableam->scan_rescan(scan, key, false, false, false, false); } diff --git a/src/include/executor/nodeBitmapHeapscan.h b/src/include/executor/nodeBitmapHeapscan.h index ea003a9caa..7064f54686 100644 --- a/src/include/executor/nodeBitmapHeapscan.h +++ b/src/include/executor/nodeBitmapHeapscan.h @@ -29,4 +29,11 @@ extern void ExecBitmapHeapReInitializeDSM(BitmapHeapScanState *node, extern void ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, ParallelWorkerContext *pwcxt); +extern TBMIterateResult *bhs_iterate(BitmapHeapIterator *iterator); +extern void bhs_begin_iterate(BitmapHeapIterator *iterator, TIDBitmap *tbm, + dsa_area *dsa, dsa_pointer dsp); + +extern void bhs_end_iterate(BitmapHeapIterator *iterator); + + #endif /* NODEBITMAPHEAPSCAN_H */ diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index d96703b04d..714eb3e534 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -1787,6 +1787,13 @@ typedef struct ParallelBitmapHeapState ConditionVariable cv; } ParallelBitmapHeapState; +typedef struct BitmapHeapIterator +{ + struct TBMIterator *serial; + struct TBMSharedIterator *parallel; + bool exhausted; +} BitmapHeapIterator; + /* ---------------- * BitmapHeapScanState information * @@ -1795,12 +1802,11 @@ typedef struct ParallelBitmapHeapState * pvmbuffer buffer for visibility-map lookups of prefetched pages * exact_pages total number of exact pages retrieved * lossy_pages total number of lossy pages retrieved - * prefetch_iterator iterator for prefetching ahead of current page + * pf_iterator for prefetching ahead of current page * prefetch_pages # pages prefetch iterator is ahead of current * prefetch_target current target prefetch distance * prefetch_maximum maximum value for prefetch_target * initialized is node is ready to iterate - * shared_prefetch_iterator shared iterator for prefetching * pstate shared state for parallel bitmap scan * recheck do current page's tuples need recheck * blockno used to validate pf and current block in sync @@ -1815,12 +1821,11 @@ typedef struct BitmapHeapScanState Buffer pvmbuffer; long exact_pages; long lossy_pages; - TBMIterator *prefetch_iterator; int prefetch_pages; int prefetch_target; int prefetch_maximum; bool initialized; - TBMSharedIterator *shared_prefetch_iterator; + BitmapHeapIterator pf_iterator; ParallelBitmapHeapState *pstate; bool recheck; BlockNumber blockno; diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index 01845ee71d..848ce9b30c 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -259,6 +259,7 @@ BitString BitmapAnd BitmapAndPath BitmapAndState +BitmapHeapIterator BitmapHeapPath BitmapHeapScan BitmapHeapScanState -- 2.40.1