agora inbox for [email protected]help / color / mirror / Atom feed
[PATCH 3/8] Optimize allocations in bringetbitmap 4+ messages / 2 participants [nested] [flat]
* [PATCH 3/8] Optimize allocations in bringetbitmap @ 2020-09-13 10:12 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Tomas Vondra @ 2020-09-13 10:12 UTC (permalink / raw) The bringetbitmap function allocates memory for various purposes, which may be quite expensive, depending on the number of scan keys. Instead of allocating them separately, allocate one bit chunk of memory an carve it into smaller pieces as needed - all the pieces have the same lifespan, and it saves quite a bit of CPU and memory overhead. Author: Tomas Vondra <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin.c | 62 +++++++++++++++++++++++++++------- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c index 55851376d8..8d23f2864b 100644 --- a/src/backend/access/brin/brin.c +++ b/src/backend/access/brin/brin.c @@ -372,6 +372,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) int *nkeys, *nnullkeys; int keyno; + char *ptr; + Size len; + char *tmp PG_USED_FOR_ASSERTS_ONLY; opaque = (BrinOpaque *) scan->opaque; bdesc = opaque->bo_bdesc; @@ -397,11 +400,50 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) * Make room for per-attribute lists of scan keys that we'll pass to the * consistent support procedure. We keep null and regular keys separate, * so that we can easily pass regular keys to the consistent function. + * + * To reduce the allocation overhead, we allocate one big chunk and then + * carve it into smaller arrays ourselves. All the pieces have exactly + * the same lifetime, so that's OK. */ - keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); - nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); - nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); - nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts); + len = + /* regular keys */ + MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + + MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys * bdesc->bd_tupdesc->natts) + + MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts) + + /* NULL keys */ + MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + + MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys * bdesc->bd_tupdesc->natts) + + MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts); + + ptr = palloc(len); + tmp = ptr; + + keys = (ScanKey **) ptr; + ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); + + nullkeys = (ScanKey **) ptr; + ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts); + + nkeys = (int *) ptr; + ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts); + + nnullkeys = (int *) ptr; + ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts); + + for (int i = 0; i < bdesc->bd_tupdesc->natts; i++) + { + keys[i] = (ScanKey *) ptr; + ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys); + + nullkeys[i] = (ScanKey *) ptr; + ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys); + } + + Assert(tmp + len == ptr); + + /* zero the number of keys */ + memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts); + memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts); /* * Preprocess the scan keys - split them into per-attribute arrays. @@ -437,9 +479,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) { FmgrInfo *tmp; - /* No key/null arrays for this attribute. */ - Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0)); - Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0)); + /* First time we see this attribute, so no key/null keys. */ + Assert(nkeys[keyattno - 1] == 0); + Assert(nnullkeys[keyattno - 1] == 0); tmp = index_getprocinfo(idxRel, keyattno, BRIN_PROCNUM_CONSISTENT); @@ -450,17 +492,11 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm) /* Add key to the proper per-attribute array. */ if (key->sk_flags & SK_ISNULL) { - if (!nullkeys[keyattno - 1]) - nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); - nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key; nnullkeys[keyattno - 1]++; } else { - if (!keys[keyattno - 1]) - keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys); - keys[keyattno - 1][nkeys[keyattno - 1]] = key; nkeys[keyattno - 1]++; } -- 2.26.2 --------------CF71AF65F7C337C37C24B045 Content-Type: text/x-patch; charset=UTF-8; name="0004-BRIN-bloom-indexes-20210112.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="0004-BRIN-bloom-indexes-20210112.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v17 4/9] Use prefetch block recheck value to determine skip_fetch @ 2024-02-13 00:03 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it should skip prefetching the proposed prefetch block. As explained in the comment, this assumed the index AM will report the same recheck value for the future page as it did for the current page - but there's no guarantee. This only affects prefetching - we may prefetch blocks unecessarily and not prefetch blocks that will be needed. But we don't need to rely on that assumption - we know the recheck flag for the block we're considering prefetching. Author: Melanie Plageman Reviewed-by: Tomas Vondra, Andres Freund Discussion: https://postgr.es/m/CAAKRu_ZwCwWFeL_H3ia26bP2e7HiKLWt0ZmGXPVwPO6uXq0vaA%40mail.gmail.com Discussion: https://postgr.es/m/CAAKRu_bxrXeZ2rCnY8LyeC2Ls88KpjWrQ%2BopUrXDRXdcfwFZGA%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 920d99c0330..3e3c976ff6f 100644 --- a/src/backend/executor/nodeBitmapHeapscan.c +++ b/src/backend/executor/nodeBitmapHeapscan.c @@ -475,14 +475,9 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) * skip this prefetch call, but continue to run the prefetch * logic normally. (Would it be better not to increment * prefetch_pages?) - * - * This depends on the assumption that the index AM will - * report the same recheck flag for this future heap page as - * it did for the current heap page; which is not a certainty - * but is true in many cases. */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); @@ -533,7 +528,7 @@ BitmapPrefetch(BitmapHeapScanState *node, TableScanDesc scan) /* As above, skip prefetch if we expect not to need page */ skip_fetch = (!(scan->rs_flags & SO_NEED_TUPLE) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --etwsg7gfkf6ny266 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v17-0005-Change-BitmapAdjustPrefetchIterator-to-accept-Bl.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v18 04/10] Use prefetch block recheck value to determine skip_fetch @ 2024-02-13 00:03 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it should skip prefetching the proposed prefetch block. As explained in the comment, this assumed the index AM will report the same recheck value for the future page as it did for the current page - but there's no guarantee. This only affects prefetching - we may prefetch blocks unecessarily and not prefetch blocks that will be needed. But we don't need to rely on that assumption - we know the recheck flag for the block we're considering prefetching. Author: Melanie Plageman Reviewed-by: Tomas Vondra, Andres Freund, Tom Lane Discussion: https://postgr.es/m/1939305.1712415547%40sss.pgh.pa.us --- 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 5f768701a5e..6f843908032 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_TUPLES) && - (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_TUPLES) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --zmcd37za4qx3kv73 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v18-0005-Change-BitmapAdjustPrefetchIterator-to-accept-Bl.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v19 04/21] Use prefetch block recheck value to determine skip_fetch @ 2024-02-13 00:03 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Melanie Plageman @ 2024-02-13 00:03 UTC (permalink / raw) As of 7c70996ebf0949b142a9, BitmapPrefetch() used the recheck flag for the current block to determine whether or not it should skip prefetching the proposed prefetch block. As explained in the comment, this assumed the index AM will report the same recheck value for the future page as it did for the current page - but there's no guarantee. This only affects prefetching - we may prefetch blocks unecessarily and not prefetch blocks that will be needed. But we don't need to rely on that assumption - we know the recheck flag for the block we're considering prefetching. Author: Melanie Plageman Reviewed-by: Tomas Vondra, Andres Freund, Tom Lane Discussion: https://postgr.es/m/1939305.1712415547%40sss.pgh.pa.us --- 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 5f768701a5e..6f843908032 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_TUPLES) && - (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_TUPLES) && - (node->tbmres ? !node->tbmres->recheck : false) && + !tbmpre->recheck && VM_ALL_VISIBLE(node->ss.ss_currentRelation, tbmpre->blockno, &node->pvmbuffer)); -- 2.40.1 --clv5reqgj4vdihqz Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v19-0005-Change-BitmapAdjustPrefetchIterator-to-accept-Bl.patch" ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2024-02-13 00:03 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-13 10:12 [PATCH 3/8] Optimize allocations in bringetbitmap Tomas Vondra <[email protected]> 2024-02-13 00:03 [PATCH v17 4/9] Use prefetch block recheck value to determine skip_fetch Melanie Plageman <[email protected]> 2024-02-13 00:03 [PATCH v18 04/10] Use prefetch block recheck value to determine skip_fetch Melanie Plageman <[email protected]> 2024-02-13 00:03 [PATCH v19 04/21] Use prefetch block recheck value to determine skip_fetch Melanie Plageman <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox