From 9e731e20f396ec2f1b9fd2d12e69093170b0816e Mon Sep 17 00:00:00 2001 From: David Rowley 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