public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 4/8] Optimize allocations in bringetbitmap
3+ messages / 3 participants
[nested] [flat]
* [PATCH 4/8] Optimize allocations in bringetbitmap
@ 2021-03-02 18:57 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Tomas Vondra @ 2021-03-02 18:57 UTC (permalink / raw)
The bringetbitmap function allocates memory for various purposes, which
may be quite expensive, depending on the number of scan keys. Instead of
allocating them separately, allocate one bit chunk of memory an carve it
into smaller pieces as needed - all the pieces have the same lifespan,
and it saves quite a bit of CPU and memory overhead.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin.c | 60 ++++++++++++++++++++++++++--------
1 file changed, 47 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index ce0f525c21..758c47f0f4 100644
--- a/src/backend/access/brin/brin.c
+++ b/src/backend/access/brin/brin.c
@@ -373,6 +373,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
int *nkeys,
*nnullkeys;
int keyno;
+ char *ptr;
+ Size len;
+ char *tmp PG_USED_FOR_ASSERTS_ONLY;
opaque = (BrinOpaque *) scan->opaque;
bdesc = opaque->bo_bdesc;
@@ -402,15 +405,52 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* We keep null and regular keys separate, so that we can pass just the
* regular keys to the consistent function easily.
*
+ * To reduce the allocation overhead, we allocate one big chunk and then
+ * carve it into smaller arrays ourselves. All the pieces have exactly the
+ * same lifetime, so that's OK.
+ *
* XXX The widest table can have ~1600 attributes, so this may allocate a
* couple kilobytes of memory). We could invent a more compact approach
* (with just space for used attributes) but that would make the matching
* more complicated, so it may not be a win.
*/
- keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
- nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+ len =
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* regular keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts) +
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) + /* NULL keys */
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ ptr = palloc(len);
+ tmp = ptr;
+
+ keys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nullkeys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ nnullkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ for (int i = 0; i < bdesc->bd_tupdesc->natts; i++)
+ {
+ keys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+
+ nullkeys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+ }
+
+ Assert(tmp + len == ptr);
+
+ /* zero the number of keys */
+ memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
+ memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
/*
* Preprocess the scan keys - split them into per-attribute arrays.
@@ -444,9 +484,9 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
{
FmgrInfo *tmp;
- /* No key/null arrays for this attribute. */
- Assert((keys[keyattno - 1] == NULL) && (nkeys[keyattno - 1] == 0));
- Assert((nullkeys[keyattno - 1] == NULL) && (nnullkeys[keyattno - 1] == 0));
+ /* First time we see this attribute, so no key/null keys. */
+ Assert(nkeys[keyattno - 1] == 0);
+ Assert(nnullkeys[keyattno - 1] == 0);
tmp = index_getprocinfo(idxRel, keyattno,
BRIN_PROCNUM_CONSISTENT);
@@ -457,17 +497,11 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
/* Add key to the proper per-attribute array. */
if (key->sk_flags & SK_ISNULL)
{
- if (!nullkeys[keyattno - 1])
- nullkeys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
nullkeys[keyattno - 1][nnullkeys[keyattno - 1]] = key;
nnullkeys[keyattno - 1]++;
}
else
{
- if (!keys[keyattno - 1])
- keys[keyattno - 1] = palloc0(sizeof(ScanKey) * scan->numberOfKeys);
-
keys[keyattno - 1][nkeys[keyattno - 1]] = key;
nkeys[keyattno - 1]++;
}
--
2.26.2
--------------D524CD37E77B36A2883A7617
Content-Type: text/x-patch; charset=UTF-8;
name="0005-BRIN-bloom-indexes-20210311.patch"
Content-Transfer-Encoding: 8bit
Content-Disposition: attachment;
filename="0005-BRIN-bloom-indexes-20210311.patch"
^ permalink raw reply [nested|flat] 3+ messages in thread
* Buffer Cache Problem
@ 2023-11-07 13:28 jacktby jacktby <[email protected]>
0 siblings, 1 reply; 3+ messages in thread
From: jacktby jacktby @ 2023-11-07 13:28 UTC (permalink / raw)
To: [email protected]
Hi, postgres hackers, I’m studying postgres buffer cache part. So I open this thread to communicate some buffer cache codes design and try to improve some tricky codes.
For Buffer Cache, we know it’s a buffer array, every bucket of this array is consist of a data page and its header which is used to describe the state of the buffer.
This is the origin code of buffer header:
typedef struct BufferDesc
{
BufferTag tag; /* ID of page contained in buffer */
int buf_id; /* buffer's index number (from 0) */
/* state of the tag, containing flags, refcount and usagecount */
pg_atomic_uint32 state;
int wait_backend_pgprocno; /* backend of pin-count waiter */
int freeNext; /* link in freelist chain */
LWLock content_lock; /* to lock access to buffer contents */
} BufferDesc;
For field wait_backend_pgprocno, the comment is "backend of pin-count waiter”, I have problems below:
1. it means which processId is waiting this buffer, right?
2. and if wait_backend_pgprocno is valid, so it says this buffer is in use by one process, right?
3. if one buffer is wait by another process, it means all buffers are out of use, right? So let’s try this: we have 5 buffers with ids (1,2,3,4,5), and they are all in use, now another process with processId 8017 is coming, and it choose buffer id 1, so buffer1’s wait_backend_pgprocno is 8017, but later
buffer4 is released, can process 8017 change to get buffer4? how?
4. wait_backend_pgprocno is a “integer” type, not an array, why can one buffer be wait by only one process?
Hope your reply, thanks!! I’m willing to do contributions after I study buffer cache implementations.
^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: Buffer Cache Problem
@ 2023-11-07 15:45 Matthias van de Meent <[email protected]>
parent: jacktby jacktby <[email protected]>
0 siblings, 0 replies; 3+ messages in thread
From: Matthias van de Meent @ 2023-11-07 15:45 UTC (permalink / raw)
To: jacktby jacktby <[email protected]>; +Cc: [email protected]
On Tue, 7 Nov 2023 at 14:28, jacktby jacktby <[email protected]> wrote:
>
> Hi, postgres hackers, I’m studying postgres buffer cache part. So I open this thread to communicate some buffer cache codes design and try to improve some tricky codes.
>
> For Buffer Cache, we know it’s a buffer array, every bucket of this array is consist of a data page and its header which is used to describe the state of the buffer.
>
> For field wait_backend_pgprocno, the comment is "backend of pin-count waiter”, I have problems below:
Did you read the README at src/backend/storage/buffer/README, as well
as the comments and documentation in and around the buffer-locking
functions?
> 1. it means which processId is waiting this buffer, right?
> 2. and if wait_backend_pgprocno is valid, so it says this buffer is in use by one process, right?
> 3. if one buffer is wait by another process, it means all buffers are out of use, right? So let’s try this: we have 5 buffers with ids (1,2,3,4,5), and they are all in use, now another process with processId 8017 is coming, and it choose buffer id 1, so buffer1’s wait_backend_pgprocno is 8017, but later
> buffer4 is released, can process 8017 change to get buffer4? how?
I believe these questions are generally answered by the README and the
comments in bufmgr.c/buf_internal.h for the functions that try to lock
buffers.
> 4. wait_backend_pgprocno is a “integer” type, not an array, why can one buffer be wait by only one process?
Yes, that is correct. It seems like PostgreSQL has yet to find a
workload requires more than one backend to wait for super exclusive
access to a buffer at the same time.
VACUUM seems to be the only workload that currently can wait and sleep
for this exclusive buffer access, and that is already limited to one
process per relation, so there are no explicit concurrent
super-exclusive waits in the system right now.
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2023-11-07 15:45 UTC | newest]
Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2021-03-02 18:57 [PATCH 4/8] Optimize allocations in bringetbitmap Tomas Vondra <[email protected]>
2023-11-07 13:28 Buffer Cache Problem jacktby jacktby <[email protected]>
2023-11-07 15:45 ` Re: Buffer Cache Problem Matthias van de Meent <[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