pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
To: Justin Pryzby <pryzby@telsasoft.com>
Cc: Zhihong Yu <zyu@yugabyte.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Removed extra memory allocations from create_list_bounds
Date: Tue, 18 May 2021 22:58:41 +0530
Message-ID: <CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com> (raw )
In-Reply-To: <20210517150310.GG373@telsasoft.com >
References: <CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com >
<20210516170029.GB373@telsasoft.com >
<CALNJ-vS4g3QNmPe8WcgBmJQOzS0K_OAujW_1T0J2JXqpBQoBFw@mail.gmail.com >
<CAMm1aWYr9=qPCTeNzVCEbF=Nr=RLSnt_vdCDTm9VjLS5L5JuOw@mail.gmail.com >
<20210517150310.GG373@telsasoft.com >
> The CFBOT had no issues with the patches, so I suspect an issue on your side.
> http://cfbot.cputube.org/nitin-jadhav.html
I am getting the following error when I try to apply in my machine.
$ git apply ../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:18:
trailing whitespace.
/*
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:19:
trailing whitespace.
* get_non_null_count_list_bounds
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:20:
trailing whitespace.
* Calculates the total number of non null bound values of
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:21:
trailing whitespace.
* all the partitions.
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:22:
trailing whitespace.
*/
error: patch failed: src/backend/partitioning/partbounds.c:432
error: src/backend/partitioning/partbounds.c: patch does not apply
However I was able to apply it by adding '--reject --whitespace=fix'.
$ git apply --reject --whitespace=fix
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:18:
trailing whitespace.
/*
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:19:
trailing whitespace.
* get_non_null_count_list_bounds
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:20:
trailing whitespace.
* Calculates the total number of non null bound values of
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:21:
trailing whitespace.
* all the partitions.
../patches/0001-Removed-extra-memory-allocations-from-create_list_bo.patch:22:
trailing whitespace.
*/
Checking patch src/backend/partitioning/partbounds.c...
Applied patch src/backend/partitioning/partbounds.c cleanly.
warning: squelched 30 whitespace errors
warning: 35 lines add whitespace errors.
I have rebased all the patches on top of
'v2_0001-removed_extra_mem_alloc_from_create_list_bounds.patch'.
Attaching all the patches here.
--
Thanks & Regards,
Nitin Jadhav
On Mon, May 17, 2021 at 8:33 PM Justin Pryzby <pryzby@telsasoft.com> wrote:
>
> On Mon, May 17, 2021 at 08:22:25PM +0530, Nitin Jadhav wrote:
> > I agree and thanks for creating those patches. I am not able to apply
> > the patch on the latest HEAD. Kindly check and upload the modified
> > patches.
>
> The CFBOT had no issues with the patches, so I suspect an issue on your side.
> http://cfbot.cputube.org/nitin-jadhav.html
>
> --
> Justin
Attachments: download all (5 files, .tar.gz)
v2_0001-removed_extra_mem_alloc_from_create_list_bounds.patch (application/octet-stream, 3.5K) download
From ee6a19144c6aa9f623353916d7a560df31ae42e6 Mon Sep 17 00:00:00 2001
From: Nitin <nitin.jadhav@enterprisedb.com>
Date: Tue, 18 May 2021 22:31:55 +0530
Subject: [PATCH 1/5] removed_extra_mem_alloc_from_create_list_bounds
---
src/backend/partitioning/partbounds.c | 64 +++++++++++++++++++----------------
1 file changed, 35 insertions(+), 29 deletions(-)
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 7925fcc..e0a04fe 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -433,6 +433,32 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
}
/*
+ * get_non_null_list_bounds_count
+ * Calculates the total number of non null bound values of
+ * all the partitions.
+ */
+static int
+get_non_null_list_bounds_count(PartitionBoundSpec **boundspecs, int nparts)
+{
+ int i = 0;
+ int count = 0;
+
+ for (i = 0; i < nparts; i++)
+ {
+ ListCell *c;
+
+ foreach(c, boundspecs[i]->listdatums)
+ {
+ Const *val = castNode(Const, lfirst(c));
+
+ if (!val->constisnull)
+ count++;
+ }
+ }
+
+ return count;
+}
+/*
* create_list_bounds
* Create a PartitionBoundInfo for a list partitioned table
*/
@@ -442,13 +468,12 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
{
PartitionBoundInfo boundinfo;
PartitionListValue **all_values = NULL;
- ListCell *cell;
int i = 0;
+ int j = 0;
int ndatums = 0;
int next_index = 0;
int default_index = -1;
int null_index = -1;
- List *non_null_values = NIL;
boundinfo = (PartitionBoundInfoData *)
palloc0(sizeof(PartitionBoundInfoData));
@@ -457,6 +482,10 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
boundinfo->null_index = -1;
boundinfo->default_index = -1;
+ ndatums = get_non_null_list_bounds_count(boundspecs, nparts);
+ all_values = (PartitionListValue **)
+ palloc(ndatums * sizeof(PartitionListValue *));
+
/* Create a unified list of non-null values across all partitions. */
for (i = 0; i < nparts; i++)
{
@@ -480,14 +509,14 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
foreach(c, spec->listdatums)
{
Const *val = castNode(Const, lfirst(c));
- PartitionListValue *list_value = NULL;
if (!val->constisnull)
{
- list_value = (PartitionListValue *)
+ all_values[j] = (PartitionListValue *)
palloc0(sizeof(PartitionListValue));
- list_value->index = i;
- list_value->value = val->constvalue;
+ all_values[j]->index = i;
+ all_values[j]->value = val->constvalue;
+ j++;
}
else
{
@@ -499,32 +528,9 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
elog(ERROR, "found null more than once");
null_index = i;
}
-
- if (list_value)
- non_null_values = lappend(non_null_values, list_value);
}
}
- ndatums = list_length(non_null_values);
-
- /*
- * Collect all list values in one array. Alongside the value, we also save
- * the index of partition the value comes from.
- */
- all_values = (PartitionListValue **)
- palloc(ndatums * sizeof(PartitionListValue *));
- i = 0;
- foreach(cell, non_null_values)
- {
- PartitionListValue *src = lfirst(cell);
-
- all_values[i] = (PartitionListValue *)
- palloc(sizeof(PartitionListValue));
- all_values[i]->value = src->value;
- all_values[i]->index = src->index;
- i++;
- }
-
qsort_arg(all_values, ndatums, sizeof(PartitionListValue *),
qsort_partition_list_value_cmp, (void *) key);
--
1.8.3.1
v2_0003-Same-for-create_hash_bounds-PartitionHashBound.patch (application/octet-stream, 3.3K) download
From 9d8782e0213fea5d677a00abe78bd406bfc231fa Mon Sep 17 00:00:00 2001
From: Nitin <nitin.jadhav@enterprisedb.com>
Date: Tue, 18 May 2021 22:40:04 +0530
Subject: [PATCH 3/5] Same-for-create_hash_bounds-PartitionHashBound
---
src/backend/partitioning/partbounds.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index f91efba..5cf7672 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -358,7 +358,7 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
PartitionKey key, int **mapping)
{
PartitionBoundInfo boundinfo;
- PartitionHashBound **hbounds = NULL;
+ PartitionHashBound *hbounds;
int i;
int ndatums = 0;
int greatest_modulus;
@@ -371,8 +371,8 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
boundinfo->default_index = -1;
ndatums = nparts;
- hbounds = (PartitionHashBound **)
- palloc(nparts * sizeof(PartitionHashBound *));
+ hbounds = (PartitionHashBound *)
+ palloc(nparts * sizeof(PartitionHashBound));
/* Convert from node to the internal representation */
for (i = 0; i < nparts; i++)
@@ -382,18 +382,17 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
if (spec->strategy != PARTITION_STRATEGY_HASH)
elog(ERROR, "invalid strategy in partition bound spec");
- hbounds[i] = (PartitionHashBound *) palloc(sizeof(PartitionHashBound));
- hbounds[i]->modulus = spec->modulus;
- hbounds[i]->remainder = spec->remainder;
- hbounds[i]->index = i;
+ hbounds[i].modulus = spec->modulus;
+ hbounds[i].remainder = spec->remainder;
+ hbounds[i].index = i;
}
/* Sort all the bounds in ascending order */
- qsort(hbounds, nparts, sizeof(PartitionHashBound *),
+ qsort(hbounds, nparts, sizeof(PartitionHashBound),
qsort_partition_hbound_cmp);
/* After sorting, moduli are now stored in ascending order. */
- greatest_modulus = hbounds[ndatums - 1]->modulus;
+ greatest_modulus = hbounds[ndatums - 1].modulus;
boundinfo->ndatums = ndatums;
boundinfo->datums = (Datum **) palloc0(ndatums * sizeof(Datum *));
@@ -409,8 +408,8 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
*/
for (i = 0; i < nparts; i++)
{
- int modulus = hbounds[i]->modulus;
- int remainder = hbounds[i]->remainder;
+ int modulus = hbounds[i].modulus;
+ int remainder = hbounds[i].remainder;
boundinfo->datums[i] = (Datum *) palloc(2 * sizeof(Datum));
boundinfo->datums[i][0] = Int32GetDatum(modulus);
@@ -424,8 +423,7 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
remainder += modulus;
}
- (*mapping)[hbounds[i]->index] = i;
- pfree(hbounds[i]);
+ (*mapping)[hbounds[i].index] = i;
}
pfree(hbounds);
@@ -3676,8 +3674,8 @@ partition_hash_bsearch(PartitionBoundInfo boundinfo,
static int32
qsort_partition_hbound_cmp(const void *a, const void *b)
{
- PartitionHashBound *h1 = (*(PartitionHashBound *const *) a);
- PartitionHashBound *h2 = (*(PartitionHashBound *const *) b);
+ PartitionHashBound *const h1 = (PartitionHashBound *const) a;
+ PartitionHashBound *const h2 = (PartitionHashBound *const) b;
return partition_hbound_cmp(h1->modulus, h1->remainder,
h2->modulus, h2->remainder);
--
1.8.3.1
v2_0004-Allocate-datum-arrays-in-bulk-to-avoid-palloc-overhead.patch (application/octet-stream, 3.1K) download
From 838f97cd1c36dbbfa05f6a813c56042acd012d5b Mon Sep 17 00:00:00 2001
From: Nitin <nitin.jadhav@enterprisedb.com>
Date: Tue, 18 May 2021 22:42:12 +0530
Subject: [PATCH 4/5] Allocate-datum-arrays-in-bulk-to-avoid-palloc-overhe
---
src/backend/partitioning/partbounds.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 5cf7672..f1a692c 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -362,6 +362,7 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
int i;
int ndatums = 0;
int greatest_modulus;
+ Datum *datumptr;
boundinfo = (PartitionBoundInfoData *)
palloc0(sizeof(PartitionBoundInfoData));
@@ -406,12 +407,14 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
* pairs) as there are partitions. Indexes are simply values ranging from
* 0 to (nparts - 1).
*/
+ datumptr = (Datum *) palloc0(ndatums * 2 * sizeof(Datum));
for (i = 0; i < nparts; i++)
{
int modulus = hbounds[i].modulus;
int remainder = hbounds[i].remainder;
- boundinfo->datums[i] = (Datum *) palloc(2 * sizeof(Datum));
+ boundinfo->datums[i] = datumptr;
+ datumptr += 2;
boundinfo->datums[i][0] = Int32GetDatum(modulus);
boundinfo->datums[i][1] = Int32GetDatum(remainder);
@@ -472,6 +475,7 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
int next_index = 0;
int default_index = -1;
int null_index = -1;
+ Datum *datumptr;
boundinfo = (PartitionBoundInfoData *)
palloc0(sizeof(PartitionBoundInfoData));
@@ -541,11 +545,12 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
* receive the same value. The value for a given partition is the index of
* that partition's smallest datum in the all_values[] array.
*/
+ datumptr = (Datum *) palloc0(ndatums * sizeof(Datum));
for (i = 0; i < ndatums; i++)
{
int orig_index = all_values[i].index;
- boundinfo->datums[i] = (Datum *) palloc(sizeof(Datum));
+ boundinfo->datums[i] = datumptr++;
boundinfo->datums[i][0] = datumCopy(all_values[i].value,
key->parttypbyval[0],
key->parttyplen[0]);
@@ -609,6 +614,7 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts,
int ndatums = 0;
int default_index = -1;
int next_index = 0;
+ Datum *datumptr;
boundinfo = (PartitionBoundInfoData *)
palloc0(sizeof(PartitionBoundInfoData));
@@ -733,12 +739,13 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts,
boundinfo->nindexes = ndatums + 1;
boundinfo->indexes = (int *) palloc((ndatums + 1) * sizeof(int));
+ datumptr = (Datum *) palloc0(ndatums * key->partnatts * sizeof(Datum));
for (i = 0; i < ndatums; i++)
{
int j;
- boundinfo->datums[i] = (Datum *) palloc(key->partnatts *
- sizeof(Datum));
+ boundinfo->datums[i] = datumptr;
+ datumptr += key->partnatts;
boundinfo->kind[i] = (PartitionRangeDatumKind *)
palloc(key->partnatts *
sizeof(PartitionRangeDatumKind));
--
1.8.3.1
v2_0005-create_range_bounds-pfree-intermediate-results.patch (application/octet-stream, 943B) download
From 382237eae1e143f60da9a4424992327c89a89b10 Mon Sep 17 00:00:00 2001
From: Nitin <nitin.jadhav@enterprisedb.com>
Date: Tue, 18 May 2021 22:43:58 +0530
Subject: [PATCH 5/5] create_range_bounds-pfree-intermediate-results
---
src/backend/partitioning/partbounds.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index f1a692c..67a0fce 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -715,6 +715,8 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts,
prev = cur;
}
+ pfree(all_bounds);
+
/* Update ndatums to hold the count of distinct datums. */
ndatums = k;
@@ -781,6 +783,8 @@ create_range_bounds(PartitionBoundSpec **boundspecs, int nparts,
}
}
+ pfree(rbounds);
+
/* Set the canonical value for default_index, if any. */
if (default_index != -1)
{
--
1.8.3.1
v2_0002-allocate-the-PartitionListValue-as-a-single-chunk.patch (application/octet-stream, 2.9K) download
From 433eb17cbcca7e2200a1416109097d91158f4b31 Mon Sep 17 00:00:00 2001
From: Nitin <nitin.jadhav@enterprisedb.com>
Date: Tue, 18 May 2021 22:37:08 +0530
Subject: [PATCH 2/5] allocate-the-PartitionListValue-as-a-single-chunk
---
src/backend/partitioning/partbounds.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index e0a04fe..f91efba 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -467,7 +467,7 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
PartitionKey key, int **mapping)
{
PartitionBoundInfo boundinfo;
- PartitionListValue **all_values = NULL;
+ PartitionListValue *all_values;
int i = 0;
int j = 0;
int ndatums = 0;
@@ -483,8 +483,8 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
boundinfo->default_index = -1;
ndatums = get_non_null_list_bounds_count(boundspecs, nparts);
- all_values = (PartitionListValue **)
- palloc(ndatums * sizeof(PartitionListValue *));
+ all_values = (PartitionListValue *)
+ palloc(ndatums * sizeof(PartitionListValue));
/* Create a unified list of non-null values across all partitions. */
for (i = 0; i < nparts; i++)
@@ -512,10 +512,8 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
if (!val->constisnull)
{
- all_values[j] = (PartitionListValue *)
- palloc0(sizeof(PartitionListValue));
- all_values[j]->index = i;
- all_values[j]->value = val->constvalue;
+ all_values[j].index = i;
+ all_values[j].value = val->constvalue;
j++;
}
else
@@ -531,7 +529,7 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
}
}
- qsort_arg(all_values, ndatums, sizeof(PartitionListValue *),
+ qsort_arg(all_values, ndatums, sizeof(PartitionListValue),
qsort_partition_list_value_cmp, (void *) key);
boundinfo->ndatums = ndatums;
@@ -547,10 +545,10 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
*/
for (i = 0; i < ndatums; i++)
{
- int orig_index = all_values[i]->index;
+ int orig_index = all_values[i].index;
boundinfo->datums[i] = (Datum *) palloc(sizeof(Datum));
- boundinfo->datums[i][0] = datumCopy(all_values[i]->value,
+ boundinfo->datums[i][0] = datumCopy(all_values[i].value,
key->parttypbyval[0],
key->parttyplen[0]);
@@ -3693,8 +3691,8 @@ qsort_partition_hbound_cmp(const void *a, const void *b)
static int32
qsort_partition_list_value_cmp(const void *a, const void *b, void *arg)
{
- Datum val1 = (*(PartitionListValue *const *) a)->value,
- val2 = (*(PartitionListValue *const *) b)->value;
+ Datum val1 = ((PartitionListValue *const) a)->value,
+ val2 = ((PartitionListValue *const) b)->value;
PartitionKey key = (PartitionKey) arg;
return DatumGetInt32(FunctionCall2Coll(&key->partsupfunc[0],
--
1.8.3.1
view thread (20+ messages) latest in thread
Message-ID: <CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com>
Permalink: ../CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com/
Also on: postgresql.org/message-id/CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: nitinjadhavpostgres@gmail.com, pryzby@telsasoft.com, zyu@yugabyte.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Removed extra memory allocations from create_list_bounds
In-Reply-To: <CAMm1aWb0KSOCHexbaeit7gy_p4Uhs18EWyBdPyis4bfw4ids7A@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox