public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 3/9] Optimize allocations in bringetbitmap
2+ messages / 2 participants
[nested] [flat]
* [PATCH 3/9] Optimize allocations in bringetbitmap
@ 2020-09-13 10:12 Tomas Vondra <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Tomas Vondra @ 2020-09-13 10:12 UTC (permalink / raw)
The bringetbitmap function allocates memory for various purposes, which
may be quite expensive, depending on the number of scan keys. Instead of
allocating them separately, allocate one bit chunk of memory an carve it
into smaller pieces as needed - all the pieces have the same lifespan,
and it saves quite a bit of CPU and memory overhead.
Author: Tomas Vondra <[email protected]>
Discussion: https://postgr.es/m/[email protected]
---
src/backend/access/brin/brin.c | 62 +++++++++++++++++++++++++++-------
1 file changed, 49 insertions(+), 13 deletions(-)
diff --git a/src/backend/access/brin/brin.c b/src/backend/access/brin/brin.c
index 14da9ed17f..c7f7175a2a 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;
@@ -398,11 +401,50 @@ bringetbitmap(IndexScanDesc scan, TIDBitmap *tbm)
* Make room for per-attribute lists of scan keys that we'll pass to the
* consistent support procedure. We keep null and regular keys separate,
* so that we can easily pass regular keys to the consistent function.
+ *
+ * To reduce the allocation overhead, we allocate one big chunk and then
+ * carve it into smaller arrays ourselves. All the pieces have exactly
+ * the same lifetime, so that's OK.
*/
- keys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nullkeys = palloc0(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
- nkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
- nnullkeys = palloc0(sizeof(int) * bdesc->bd_tupdesc->natts);
+ len =
+ /* regular keys */
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) +
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts) +
+ /* NULL keys */
+ MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts) +
+ MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys) * bdesc->bd_tupdesc->natts +
+ MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ ptr = palloc(len);
+ tmp = ptr;
+
+ keys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nullkeys = (ScanKey **) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey *) * bdesc->bd_tupdesc->natts);
+
+ nkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ nnullkeys = (int *) ptr;
+ ptr += MAXALIGN(sizeof(int) * bdesc->bd_tupdesc->natts);
+
+ for (int i = 0; i < bdesc->bd_tupdesc->natts; i++)
+ {
+ keys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+
+ nullkeys[i] = (ScanKey *) ptr;
+ ptr += MAXALIGN(sizeof(ScanKey) * scan->numberOfKeys);
+ }
+
+ Assert(tmp + len == ptr);
+
+ /* zero the number of keys */
+ memset(nkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
+ memset(nnullkeys, 0, sizeof(int) * bdesc->bd_tupdesc->natts);
/*
* Preprocess the scan keys - split them into per-attribute arrays.
@@ -438,9 +480,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);
@@ -451,17 +493,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
--------------0E72B707603BED22B4040825
Content-Type: text/x-patch; charset=UTF-8;
name="0002-Move-IS-NOT-NULL-handling-from-BRIN-support-20210211.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0002-Move-IS-NOT-NULL-handling-from-BRIN-support-20210211.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: Patch: Global Unique Index
@ 2023-01-13 22:22 Cary Huang <[email protected]>
0 siblings, 0 replies; 2+ messages in thread
From: Cary Huang @ 2023-01-13 22:22 UTC (permalink / raw)
To: Greg Stark <[email protected]>; +Cc: Pgsql Hackers <[email protected]>
On 2022-11-30 2:30 p.m., Greg Stark wrote:
> On Tue, 29 Nov 2022 at 21:16, Tom Lane <[email protected]> wrote:
>> I actually think that that problem should be soluble with a
>> slightly different approach. The thing that feels insoluble
>> is that you can't do this without acquiring sufficient locks
>> to prevent addition of new partitions while the insertion is
>> in progress. That will be expensive in itself, and it will
>> turn ATTACH PARTITION into a performance disaster.
> I think there`s a lot of room to manoeuvre here. This is a new feature
> that doesn't need to be 100% complete or satisfy any existing
> standard. There are lots of options for compromises that leave room
> for future improvements.
>
> 1) We could just say sure ATTACH is slow if you're attaching an
> non-empty partition
> 2) We could invent a concept like convalidated and let people attach a
> partition without validating the uniqueness and then validate it later
> concurrently
> 3) We could say ATTACH doesn't work now and come up with a better
> strategy in the future
>
> Also, don't I vaguely recall something in exclusion constraints about
> having some kind of in-memory "intent" list where you declared that
> you're about to insert a value, you validate it doesn't violate the
> constraint and then you're free to insert it because anyone else will
> see your intent in memory? There might be a need for some kind of
> global object that only holds inserted keys long enough that other
> sessions are guaranteed to see the key in the correct index. And that
> could maybe even be in memory rather than on disk.
>
> This isn't a simple project but I don't think it's impossible as long
> as we keep an open mind about the requirements.
In the current global unique index implementation, ATTACH can be slow if
there are concurrent inserts happening. ATTACH tries to acquire
shareLock on all existing partitions and partition-to-be before it scans
and sorts them for uniqueness check. It will release them only after all
partitions have been checked. If there are concurrent inserts, ATTACH
has to wait for all inserts complete. Likewise, if ATTACH is in
progress, inserts have to wait as well. This is an issue now.
If we were to make ATTACH acquire a lower level lock (AccessShareLock),
scans a partition, and then release it. there is nothing stopping any
concurrent inserts from inserting a conflict right after it finishes
checking. This is another issue. There is no transaction level lock
being triggered here like in multiple concurent inserts case
Another email thread called "create index concurrently on partitioned
index" discuss some approaches that may be used to solve the attach
issue here, basically to allow ATTACH PARTITION CONCURRENTLY...
regards
Cary Huang
---------------------------------
HighGo Software Canada
^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2023-01-13 22:22 UTC | newest]
Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-09-13 10:12 [PATCH 3/9] Optimize allocations in bringetbitmap Tomas Vondra <[email protected]>
2023-01-13 22:22 Re: Patch: Global Unique Index Cary Huang <[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