From 0c882a298881056176a27ccc44c5c3bb7c8f308c Mon Sep 17 00:00:00 2001 From: David Rowley Date: Fri, 27 Oct 2023 10:41:55 +1300 Subject: [PATCH] Optimize various aggregate deserialization functions, take 2 f0efa5aec added initReadOnlyStringInfo to allow a StringInfo to be initialized from an existing buffer and also relaxed the requirement that a StringInfo's buffer must be NUL terminated at data[len]. Now that we have that, there's no need for these aggregate deserial functions to use appendBinaryStringInfo() as that rather wastefully palloc'd a new buffer and memcpy'd in the bytea's buffer. Instead, we can just use the bytea's buffer and point the StringInfo directly to that using the new initializer function. In Amdahl's law, this speeds up the serial portion of parallel aggregates and makes sum(numeric), avg(numeric), var_pop(numeric), var_samp(numeric), variance(numeric), stddev_pop(numeric), stddev_samp(numeric), stddev(numeric), array_agg(anyarray), string_agg(text) and string_agg(bytea) scale better in parallel queries. Author: David Rowley Discussion: https://postgr.es/m/CAApHDvr%3De-YOigriSHHm324a40HPqcUhSp6pWWgjz5WwegR%3DcQ%40mail.gmail.com --- src/backend/utils/adt/array_userfuncs.c | 20 +++++-------- src/backend/utils/adt/numeric.c | 40 ++++++++++--------------- src/backend/utils/adt/varlena.c | 10 +++---- 3 files changed, 28 insertions(+), 42 deletions(-) diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c index c831a9395c6..57bd7e82bc4 100644 --- a/src/backend/utils/adt/array_userfuncs.c +++ b/src/backend/utils/adt/array_userfuncs.c @@ -723,12 +723,11 @@ array_agg_deserialize(PG_FUNCTION_ARGS) sstate = PG_GETARG_BYTEA_PP(0); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); /* element_type */ element_type = pq_getmsgint(&buf, 4); @@ -815,7 +814,6 @@ array_agg_deserialize(PG_FUNCTION_ARGS) } pq_getmsgend(&buf); - pfree(buf.data); PG_RETURN_POINTER(result); } @@ -1124,12 +1122,11 @@ array_agg_array_deserialize(PG_FUNCTION_ARGS) sstate = PG_GETARG_BYTEA_PP(0); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); /* element_type */ element_type = pq_getmsgint(&buf, 4); @@ -1187,7 +1184,6 @@ array_agg_array_deserialize(PG_FUNCTION_ARGS) memcpy(result->lbs, temp, sizeof(result->lbs)); pq_getmsgend(&buf); - pfree(buf.data); PG_RETURN_POINTER(result); } diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c index 3c3184f15b5..bf61fd7dbc0 100644 --- a/src/backend/utils/adt/numeric.c +++ b/src/backend/utils/adt/numeric.c @@ -5190,12 +5190,11 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS) init_var(&tmp_var); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); result = makeNumericAggStateCurrentContext(false); @@ -5222,7 +5221,6 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS) result->nInfcount = pq_getmsgint64(&buf); pq_getmsgend(&buf); - pfree(buf.data); free_var(&tmp_var); @@ -5306,12 +5304,11 @@ numeric_deserialize(PG_FUNCTION_ARGS) init_var(&tmp_var); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); result = makeNumericAggStateCurrentContext(false); @@ -5342,7 +5339,6 @@ numeric_deserialize(PG_FUNCTION_ARGS) result->nInfcount = pq_getmsgint64(&buf); pq_getmsgend(&buf); - pfree(buf.data); free_var(&tmp_var); @@ -5677,12 +5673,11 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS) init_var(&tmp_var); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); result = makePolyNumAggStateCurrentContext(false); @@ -5706,7 +5701,6 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS) #endif pq_getmsgend(&buf); - pfree(buf.data); free_var(&tmp_var); @@ -5868,12 +5862,11 @@ int8_avg_deserialize(PG_FUNCTION_ARGS) init_var(&tmp_var); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); result = makePolyNumAggStateCurrentContext(false); @@ -5889,7 +5882,6 @@ int8_avg_deserialize(PG_FUNCTION_ARGS) #endif pq_getmsgend(&buf); - pfree(buf.data); free_var(&tmp_var); diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 72e1e24fe02..ec4e580d7fe 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5289,12 +5289,11 @@ string_agg_deserialize(PG_FUNCTION_ARGS) sstate = PG_GETARG_BYTEA_PP(0); /* - * Copy the bytea into a StringInfo so that we can "receive" it using the - * standard recv-function infrastructure. + * Initialize a StringInfo so that we can "receive" it using the standard + * recv-function infrastructure. */ - initStringInfo(&buf); - appendBinaryStringInfo(&buf, - VARDATA_ANY(sstate), VARSIZE_ANY_EXHDR(sstate)); + initReadOnlyStringInfo(&buf, VARDATA_ANY(sstate), + VARSIZE_ANY_EXHDR(sstate)); result = makeStringAggState(fcinfo); @@ -5307,7 +5306,6 @@ string_agg_deserialize(PG_FUNCTION_ARGS) appendBinaryStringInfo(result, data, datalen); pq_getmsgend(&buf); - pfree(buf.data); PG_RETURN_POINTER(result); }