public inbox for [email protected]  
help / color / mirror / Atom feed
Re: pgbench with libevent?
23+ messages / 4 participants
[nested] [flat]

* Re: pgbench with libevent?
@ 2023-08-14 00:35 Fabien COELHO <[email protected]>
  2023-08-14 02:58 ` Re: pgbench with libevent? Thomas Munro <[email protected]>
  0 siblings, 1 reply; 23+ messages in thread

From: Fabien COELHO @ 2023-08-14 00:35 UTC (permalink / raw)
  To: PostgreSQL Developers <[email protected]>


> Pgbench is managing clients I/Os manually with select or poll. Much of this 
> could be managed by libevent.

Or maybe libuv (used by nodejs?).

From preliminary testing libevent seems not too good at fine grain time 
management which are used for throttling, whereas libuv advertised that it 
is good at it, although what it does is yet to be seen.

Note: libev had no updates in 8 years.

-- 
Fabien.






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: pgbench with libevent?
  2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
@ 2023-08-14 02:58 ` Thomas Munro <[email protected]>
  2023-08-14 06:06   ` Re: pgbench with libevent? Tatsuo Ishii <[email protected]>
  2023-08-14 10:09   ` Re: pgbench with libevent? Fabien COELHO <[email protected]>
  0 siblings, 2 replies; 23+ messages in thread

From: Thomas Munro @ 2023-08-14 02:58 UTC (permalink / raw)
  To: Fabien COELHO <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

On Mon, Aug 14, 2023 at 12:35 PM Fabien COELHO <[email protected]> wrote:
> > Pgbench is managing clients I/Os manually with select or poll. Much of this
> > could be managed by libevent.
>
> Or maybe libuv (used by nodejs?).
>
> From preliminary testing libevent seems not too good at fine grain time
> management which are used for throttling, whereas libuv advertised that it
> is good at it, although what it does is yet to be seen.

Do you think our WaitEventSet stuff could be good here, if made
frontend-friendly?






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: pgbench with libevent?
  2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
  2023-08-14 02:58 ` Re: pgbench with libevent? Thomas Munro <[email protected]>
@ 2023-08-14 06:06   ` Tatsuo Ishii <[email protected]>
  2023-08-14 07:15     ` Re: pgbench with libevent? Thomas Munro <[email protected]>
  1 sibling, 1 reply; 23+ messages in thread

From: Tatsuo Ishii @ 2023-08-14 06:06 UTC (permalink / raw)
  To: [email protected]; +Cc: [email protected]; [email protected]

> On Mon, Aug 14, 2023 at 12:35 PM Fabien COELHO <[email protected]> wrote:
>> > Pgbench is managing clients I/Os manually with select or poll. Much of this
>> > could be managed by libevent.
>>
>> Or maybe libuv (used by nodejs?).
>>
>> From preliminary testing libevent seems not too good at fine grain time
>> management which are used for throttling, whereas libuv advertised that it
>> is good at it, although what it does is yet to be seen.
> 
> Do you think our WaitEventSet stuff could be good here, if made
> frontend-friendly?

Interesting. In my understanding this also needs to make Latch
frontend-friendly?

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp


^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: pgbench with libevent?
  2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
  2023-08-14 02:58 ` Re: pgbench with libevent? Thomas Munro <[email protected]>
  2023-08-14 06:06   ` Re: pgbench with libevent? Tatsuo Ishii <[email protected]>
@ 2023-08-14 07:15     ` Thomas Munro <[email protected]>
  2023-08-14 10:22       ` Re: pgbench with libevent? Fabien COELHO <[email protected]>
  0 siblings, 1 reply; 23+ messages in thread

From: Thomas Munro @ 2023-08-14 07:15 UTC (permalink / raw)
  To: Tatsuo Ishii <[email protected]>; +Cc: [email protected]; [email protected]

On Mon, Aug 14, 2023 at 6:07 PM Tatsuo Ishii <[email protected]> wrote:
> Interesting. In my understanding this also needs to make Latch
> frontend-friendly?

It could be refactored to support a different subset of event types --
maybe just sockets, no latches and obviously no 'postmaster death'.
But figuring out how to make latches work between threads might also
be interesting for future projects...

Maybe Fabien has completion-based I/O in mind (not just "readiness").
That's something that some of those libraries can do, IIUC.  For
example, when your thread wakes up, it tells you "your socket read is
finished, the data is already in your target buffer".  As opposed to
"you can now call recv() without blocking", so you avoid another trip
into the kernel.  But that's also something we'll eventually want to
figure out in the server.






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: pgbench with libevent?
  2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
  2023-08-14 02:58 ` Re: pgbench with libevent? Thomas Munro <[email protected]>
  2023-08-14 06:06   ` Re: pgbench with libevent? Tatsuo Ishii <[email protected]>
  2023-08-14 07:15     ` Re: pgbench with libevent? Thomas Munro <[email protected]>
@ 2023-08-14 10:22       ` Fabien COELHO <[email protected]>
  0 siblings, 0 replies; 23+ messages in thread

From: Fabien COELHO @ 2023-08-14 10:22 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: Tatsuo Ishii <[email protected]>; PostgreSQL Developers <[email protected]>


>> Interesting. In my understanding this also needs to make Latch
>> frontend-friendly?
>
> It could be refactored to support a different subset of event types --
> maybe just sockets, no latches and obviously no 'postmaster death'.
> But figuring out how to make latches work between threads might also
> be interesting for future projects...
>
> Maybe Fabien has completion-based I/O in mind (not just "readiness").

Pgbench is really a primitive client on top of libpq. ISTM that 
completion-based I/O would require to enhance libpq asynchronous-ity, not 
just expose its underlying fd to allow asynchronous implementations.
Currently pgbench only actuall "waits" for results from the server
and testing PQisBusy to check whether they are there.

> That's something that some of those libraries can do, IIUC.  For
> example, when your thread wakes up, it tells you "your socket read is
> finished, the data is already in your target buffer".

Indeed, libevent has a higher level "buffer" oriented API.

> As opposed to "you can now call recv() without blocking", so you avoid 
> another trip into the kernel.  But that's also something we'll 
> eventually want to figure out in the server.

-- 
Fabien.






^ permalink  raw  reply  [nested|flat] 23+ messages in thread

* Re: pgbench with libevent?
  2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
  2023-08-14 02:58 ` Re: pgbench with libevent? Thomas Munro <[email protected]>
@ 2023-08-14 10:09   ` Fabien COELHO <[email protected]>
  1 sibling, 0 replies; 23+ messages in thread

From: Fabien COELHO @ 2023-08-14 10:09 UTC (permalink / raw)
  To: Thomas Munro <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>


Hello Thomas,

>>> Pgbench is managing clients I/Os manually with select or poll. Much of this
>>> could be managed by libevent.
>>
>> Or maybe libuv (used by nodejs?).
>>
>> From preliminary testing libevent seems not too good at fine grain time
>> management which are used for throttling, whereas libuv advertised that it
>> is good at it, although what it does is yet to be seen.
>
> Do you think our WaitEventSet stuff could be good here, if made
> frontend-friendly?

Interesting question.

AFAICS, the answer is that it could indeed probably fit the task, but it 
would require significant work to make it thread-compatible, and to 
untangle it from IsUnderPosmaster/postmaster death, memory context, 
elog/ereport, and other back-end specific stuff.

If you remove all that with a clean abstraction (quite a task), then once 
done the question could be why not use libevent/libuv/… in the backend 
instead of maintaining more or less the same thing inside postgres?

So ISTM that as far as pgbench is concerned it would be much simpler to 
use libevent/libuv/… directly if the pros are enough and the cons not 
redhibitory, and provided that the needed detailed features are really 
there.

-- 
Fabien.

^ permalink  raw  reply  [nested|flat] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ 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; 23+ 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] 23+ messages in thread


end of thread, other threads:[~2024-02-16 01:13 UTC | newest]

Thread overview: 23+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-08-14 00:35 Re: pgbench with libevent? Fabien COELHO <[email protected]>
2023-08-14 02:58 ` Thomas Munro <[email protected]>
2023-08-14 06:06   ` Tatsuo Ishii <[email protected]>
2023-08-14 07:15     ` Thomas Munro <[email protected]>
2023-08-14 10:22       ` Fabien COELHO <[email protected]>
2023-08-14 10:09   ` Fabien COELHO <[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 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 v7 10/13] 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 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 v9 11/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 v10 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 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 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 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