public inbox for [email protected]help / color / mirror / Atom feed
Re: Fix vcregress plpython3 warning 18+ messages / 2 participants [nested] [flat]
* Re: Fix vcregress plpython3 warning @ 2022-01-07 14:41 Juan José Santamaría Flecha <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Juan José Santamaría Flecha @ 2022-01-07 14:41 UTC (permalink / raw) To: Andrew Dunstan <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]> On Fri, Jan 7, 2022 at 3:24 PM Andrew Dunstan <[email protected]> wrote: > > In that case, just this should work: > > s/EXTENSION (\S*?)plpython2?u/EXTENSION $1plpython3u/g ; > > LGTM. Regards, Juan José Santamaría Flecha ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v12 12/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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.40.1 --6jpz2j246qmht4bt Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v12-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --3o7pc6dfau5a5hry Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v10-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --owzzsiozz6hgpp7e Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v11-0013-Separate-TBM-Shared-Iterator-and-TBMIterateResul.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --kqqpqghcwbcc3dt5 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v7-0011-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --xqq4defy3uncu6k6 Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v8-0011-table_scan_bitmap_next_block-counts-lossy-and-exa.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 | 29 +++++++---------------------- src/include/nodes/tidbitmap.h | 12 ++++++++++-- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c index e8ab5d78fcc..d2bf8f44d50 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; /* @@ -1467,8 +1453,7 @@ tbm_attach_shared_iterate(dsa_area *dsa, dsa_pointer dp) * 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.40.1 --w4wcjcocxsm37usi Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v6-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
* [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size @ 2024-02-16 01:13 Melanie Plageman <[email protected]> 0 siblings, 0 replies; 18+ messages in thread From: Melanie Plageman @ 2024-02-16 01:13 UTC (permalink / raw) 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 e8ab5d78fc..1dc4c99bf9 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 1945f0639b..432fae5296 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.40.1 --7mdtsjmrzitrgzgx Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="v9-0012-Separate-TBM-Shared-Iterator-and-TBMIterateResult.patch" ^ permalink raw reply [nested|flat] 18+ messages in thread
end of thread, other threads:[~2024-02-16 01:13 UTC | newest] Thread overview: 18+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2022-01-07 14:41 Re: Fix vcregress plpython3 warning Juan José Santamaría Flecha <[email protected]> 2024-02-16 01:13 [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v12 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v10 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v11 12/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v7 10/13] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v8 10/17] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v6 11/14] Hard-code TBMIterateResult offsets array size Melanie Plageman <[email protected]> 2024-02-16 01:13 [PATCH v9 11/17] Hard-code TBMIterateResult offsets array size 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