From 9e731e20f396ec2f1b9fd2d12e69093170b0816e Mon Sep 17 00:00:00 2001
From: David Rowley <dgrowley@gmail.com>
Date: Mon, 5 Jul 2021 21:52:09 +1200
Subject: [PATCH v3 1/4] Reduce the number of pallocs in create_list_bounds

create_list_bounds is rather inefficient in regards to the number of
pallocs it performs.  The code would allocate a single element Datum
pointer array for each Datum in the LIST partitioned table.  It would be
much more efficient in terms of number of pallocs used if we just
allocated a ndatum sized array of Datums and instead of pallocing a single
element Datum pointer array, just point to an element in that Datum array.
Here we do just that.

Also, to further reduce the number of pallocs, change the transient
"all_values" array by making this a single array of PartitionListValue
rather than an array of PartitionListValue pointers pointing into
individually allocated chunks.  This requires some prior knowledge of the
number of Datums that will go in the array, which the new
get_non_null_list_datum_count function takes care of for us.

While we're here, don't forget to pfree the all_values array after we're
done.  create_hash_bounds() already does this with its equivalent array.
Let's be consistent.

There's now no longer any need for the transient List data structure.
Getting rid of that should further speed up this function.

Author: Nitin Jadhav, Justin Pryzby
Reviewed-by: Justin Pryzby, Zhihong Yu
Discussion: https://postgr.es/m/flat/CAMm1aWYFTqEio3bURzZh47jveiHRwgQTiSDvBORczNEz2duZ1Q@mail.gmail.com
---
 src/backend/partitioning/partbounds.c | 89 +++++++++++++++------------
 1 file changed, 50 insertions(+), 39 deletions(-)

diff --git a/src/backend/partitioning/partbounds.c b/src/backend/partitioning/partbounds.c
index 7096d3bf45..12ad45fedf 100644
--- a/src/backend/partitioning/partbounds.c
+++ b/src/backend/partitioning/partbounds.c
@@ -432,6 +432,31 @@ create_hash_bounds(PartitionBoundSpec **boundspecs, int nparts,
 	return boundinfo;
 }
 
+/*
+ * get_non_null_list_datum_count
+ * 		Counts the number of non-null Datums in each partition.
+ */
+static int
+get_non_null_list_datum_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
@@ -441,14 +466,14 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
 				   PartitionKey key, int **mapping)
 {
 	PartitionBoundInfo boundinfo;
-	PartitionListValue **all_values = NULL;
-	ListCell   *cell;
-	int			i = 0;
-	int			ndatums = 0;
+	PartitionListValue *all_values;
+	int			i;
+	int			j;
+	int			ndatums;
 	int			next_index = 0;
 	int			default_index = -1;
 	int			null_index = -1;
-	List	   *non_null_values = NIL;
+	Datum	   *boundDatums;
 
 	boundinfo = (PartitionBoundInfoData *)
 		palloc0(sizeof(PartitionBoundInfoData));
@@ -457,8 +482,12 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
 	boundinfo->null_index = -1;
 	boundinfo->default_index = -1;
 
+	ndatums = get_non_null_list_datum_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++)
+	for (j = 0, i = 0; i < nparts; i++)
 	{
 		PartitionBoundSpec *spec = boundspecs[i];
 		ListCell   *c;
@@ -480,14 +509,12 @@ 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 *)
-					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,33 +526,13 @@ 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++;
-	}
+	/* ensure we found a Datum for every slot in the all_values array */
+	Assert(j == ndatums);
 
-	qsort_arg(all_values, ndatums, sizeof(PartitionListValue *),
+	qsort_arg(all_values, ndatums, sizeof(PartitionListValue),
 			  qsort_partition_list_value_cmp, (void *) key);
 
 	boundinfo->ndatums = ndatums;
@@ -533,6 +540,8 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
 	boundinfo->nindexes = ndatums;
 	boundinfo->indexes = (int *) palloc(ndatums * sizeof(int));
 
+	boundDatums = (Datum *) palloc(ndatums * sizeof(Datum));
+
 	/*
 	 * Copy values.  Canonical indexes are values ranging from 0 to (nparts -
 	 * 1) assigned to each partition such that all datums of a given partition
@@ -541,10 +550,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] = &boundDatums[i];
+		boundinfo->datums[i][0] = datumCopy(all_values[i].value,
 											key->parttypbyval[0],
 											key->parttyplen[0]);
 
@@ -555,6 +564,8 @@ create_list_bounds(PartitionBoundSpec **boundspecs, int nparts,
 		boundinfo->indexes[i] = (*mapping)[orig_index];
 	}
 
+	pfree(all_values);
+
 	/*
 	 * Set the canonical value for null_index, if any.
 	 *
@@ -3697,8 +3708,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],
-- 
2.30.2

