initstringinfowithlen.patch

text/plain

Filename: initstringinfowithlen.patch
Type: text/plain
Part: 0
Message: Re: Making aggregate deserialization (and WAL receive) functions slightly faster

Patch

Format: unified
File+
src/backend/replication/logical/applyparallelworker.c 1 4
src/backend/replication/logical/proto.c 6 7
src/backend/replication/logical/worker.c 1 4
src/backend/tcop/postgres.c 9 14
src/backend/utils/adt/array_userfuncs.c 6 16
src/backend/utils/adt/numeric.c 12 32
src/backend/utils/adt/rowtypes.c 7 9
src/backend/utils/adt/varlena.c 3 8
src/common/stringinfo.c 19 0
src/include/lib/stringinfo.h 24 1
diff --git a/src/backend/replication/logical/applyparallelworker.c b/src/backend/replication/logical/applyparallelworker.c
index 82f48a488e..fdfbd45e2c 100644
--- a/src/backend/replication/logical/applyparallelworker.c
+++ b/src/backend/replication/logical/applyparallelworker.c
@@ -774,10 +774,7 @@ LogicalParallelApplyLoop(shm_mq_handle *mqh)
 			if (len == 0)
 				elog(ERROR, "invalid message length");
 
-			s.cursor = 0;
-			s.maxlen = -1;
-			s.data = (char *) data;
-			s.len = len;
+			initStringInfoFromStringWithLen(&s, data, len);
 
 			/*
 			 * The first byte of messages sent from leader apply worker to
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index d52c8963eb..0d65e2836b 100644
--- a/src/backend/replication/logical/proto.c
+++ b/src/backend/replication/logical/proto.c
@@ -879,6 +879,7 @@ logicalrep_read_tuple(StringInfo in, LogicalRepTupleData *tuple)
 	/* Read the data */
 	for (i = 0; i < natts; i++)
 	{
+		char	   *buff;
 		char		kind;
 		int			len;
 		StringInfo	value = &tuple->colvalues[i];
@@ -899,19 +900,17 @@ logicalrep_read_tuple(StringInfo in, LogicalRepTupleData *tuple)
 				len = pq_getmsgint(in, 4);	/* read length */
 
 				/* and data */
-				value->data = palloc(len + 1);
-				pq_copymsgbytes(in, value->data, len);
+				buff = palloc(len + 1);
+				pq_copymsgbytes(in, buff, len);
 
 				/*
 				 * Not strictly necessary for LOGICALREP_COLUMN_BINARY, but
 				 * per StringInfo practice.
 				 */
-				value->data[len] = '\0';
+				buff[len] = '\0';
 
-				/* make StringInfo fully valid */
-				value->len = len;
-				value->cursor = 0;
-				value->maxlen = len;
+				/* make StringInfo */
+				initStringInfoFromStringWithLen(value, buff, len);
 				break;
 			default:
 				elog(ERROR, "unrecognized data representation type '%c'", kind);
diff --git a/src/backend/replication/logical/worker.c b/src/backend/replication/logical/worker.c
index 597947410f..b4bf3111cf 100644
--- a/src/backend/replication/logical/worker.c
+++ b/src/backend/replication/logical/worker.c
@@ -3582,10 +3582,7 @@ LogicalRepApplyLoop(XLogRecPtr last_received)
 					/* Ensure we are reading the data into our memory context. */
 					MemoryContextSwitchTo(ApplyMessageContext);
 
-					s.data = buf;
-					s.len = len;
-					s.cursor = 0;
-					s.maxlen = -1;
+					initStringInfoFromStringWithLen(&s, buf, len);
 
 					c = pq_getmsgbyte(&s);
 
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 21b9763183..17407d9edb 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1816,23 +1816,18 @@ exec_bind_message(StringInfo input_message)
 
 			if (!isNull)
 			{
-				const char *pvalue = pq_getmsgbytes(input_message, plength);
+				char *pvalue = unconstify(char *, pq_getmsgbytes(input_message, plength));
 
 				/*
-				 * Rather than copying data around, we just set up a phony
-				 * StringInfo pointing to the correct portion of the message
-				 * buffer.  We assume we can scribble on the message buffer so
-				 * as to maintain the convention that StringInfos have a
-				 * trailing null.  This is grotty but is a big win when
-				 * dealing with very large parameter strings.
+				 * Rather than copying data around, we just initialize a StringInfo
+				 * pointing to the correct portion of the message buffer.  We assume we
+				 * can scribble on the message buffer so as to maintain the convention
+				 * that StringInfos have a trailing null.  This is grotty but is a big win
+				 * when dealing with very large parameter strings.
 				 */
-				pbuf.data = unconstify(char *, pvalue);
-				pbuf.maxlen = plength + 1;
-				pbuf.len = plength;
-				pbuf.cursor = 0;
-
-				csave = pbuf.data[plength];
-				pbuf.data[plength] = '\0';
+				csave = pvalue[plength];
+				pvalue[plength] = '\0';
+				initStringInfoFromStringWithLen(&pbuf, pvalue, plength);
 			}
 			else
 			{
diff --git a/src/backend/utils/adt/array_userfuncs.c b/src/backend/utils/adt/array_userfuncs.c
index 7f87df45df..7731a6a76f 100644
--- a/src/backend/utils/adt/array_userfuncs.c
+++ b/src/backend/utils/adt/array_userfuncs.c
@@ -722,14 +722,9 @@ array_agg_deserialize(PG_FUNCTION_ARGS)
 
 	sstate = PG_GETARG_BYTEA_PP(0);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	/* element_type */
 	element_type = pq_getmsgint(&buf, 4);
@@ -1133,14 +1128,9 @@ array_agg_array_deserialize(PG_FUNCTION_ARGS)
 
 	sstate = PG_GETARG_BYTEA_PP(0);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	/* element_type */
 	element_type = pq_getmsgint(&buf, 4);
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index f4b885005f..6e7826631f 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -5189,14 +5189,9 @@ numeric_avg_deserialize(PG_FUNCTION_ARGS)
 
 	init_var(&tmp_var);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	result = makeNumericAggStateCurrentContext(false);
 
@@ -5305,14 +5300,9 @@ numeric_deserialize(PG_FUNCTION_ARGS)
 
 	init_var(&tmp_var);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	result = makeNumericAggStateCurrentContext(false);
 
@@ -5676,14 +5666,9 @@ numeric_poly_deserialize(PG_FUNCTION_ARGS)
 
 	init_var(&tmp_var);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	result = makePolyNumAggStateCurrentContext(false);
 
@@ -5867,14 +5852,9 @@ int8_avg_deserialize(PG_FUNCTION_ARGS)
 
 	init_var(&tmp_var);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	result = makePolyNumAggStateCurrentContext(false);
 
diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index ad176651d8..fafd32c116 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -623,21 +623,19 @@ record_recv(PG_FUNCTION_ARGS)
 		}
 		else
 		{
+			char	   *strbuff;
+
 			/*
-			 * Rather than copying data around, we just set up a phony
-			 * StringInfo pointing to the correct portion of the input buffer.
-			 * We assume we can scribble on the input buffer so as to maintain
+			 * Initalize a new StringInfo using the correct portion of the input
+			 * buffer.  We assume we can scribble on the input buffer so as to maintain
 			 * the convention that StringInfos have a trailing null.
 			 */
-			item_buf.data = &buf->data[buf->cursor];
-			item_buf.maxlen = itemlen + 1;
-			item_buf.len = itemlen;
-			item_buf.cursor = 0;
-
+			strbuff = &buf->data[buf->cursor];
 			buf->cursor += itemlen;
+			buf->data[buf->cursor] = '\0';
+			initStringInfoFromStringWithLen(&item_buf, strbuff, itemlen);
 
 			csave = buf->data[buf->cursor];
-			buf->data[buf->cursor] = '\0';
 
 			bufptr = &item_buf;
 			nulls[i] = false;
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 1aff04fa77..ade96bbdc8 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -5288,14 +5288,9 @@ string_agg_deserialize(PG_FUNCTION_ARGS)
 
 	sstate = PG_GETARG_BYTEA_PP(0);
 
-	/*
-	 * Fake up a StringInfo pointing to the bytea's value so we can "receive"
-	 * the serialized aggregate state value.
-	 */
-	buf.data = VARDATA_ANY(sstate);
-	buf.len = VARSIZE_ANY_EXHDR(sstate);
-	buf.maxlen = 0;
-	buf.cursor = 0;
+	initStringInfoFromStringWithLen(&buf,
+									VARDATA_ANY(sstate),
+									VARSIZE_ANY_EXHDR(sstate));
 
 	result = makeStringAggState(fcinfo);
 
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index 05b22b5c53..b87073c924 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -30,6 +30,7 @@
 #endif
 
 #include "lib/stringinfo.h"
+#include "port/pg_bitutils.h"
 
 
 /*
@@ -320,6 +321,24 @@ enlargeStringInfo(StringInfo str, int needed)
 	if (needed <= str->maxlen)
 		return;					/* got enough space already */
 
+
+	/*
+	 * maxlen may be -1 if the string is externally managed.  Check for that and
+	 * palloc a new buffer instead of reallocating the existing buffer
+	 */
+	if (unlikely(str->maxlen == -1))
+	{
+		char *newdata;
+
+		/* make the string no longer read-only */
+		newlen = pg_nextpower2_32(str->len * 2);
+		newdata = (char *) palloc(newlen);
+		memcpy(newdata, str->data, str->len + 1);
+		str->data = newdata;
+		str->maxlen = newlen;
+		return;
+	}
+
 	/*
 	 * We don't want to allocate just a little more space with each append;
 	 * for efficiency, double the buffer size each time it overflows.
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 36a416f8e0..b9161d6680 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -27,7 +27,10 @@
  *		maxlen	is the allocated size in bytes of 'data', i.e. the maximum
  *				string size (including the terminating '\0' char) that we can
  *				currently store in 'data' without having to reallocate
- *				more space.  We must always have maxlen > len.
+ *				more space.  We must always have maxlen > len except in the
+ *				case when maxlen is set to -1 in which case 'data' is classed
+				"read-only".  Before appending to a read-only StringInfoData we
+				must always copy the 'data' out into a new allocation.
  *		cursor	is initialized to zero by makeStringInfo or initStringInfo,
  *				but is not otherwise touched by the stringinfo.c routines.
  *				Some routines use it to scan through a StringInfo.
@@ -79,6 +82,26 @@ extern StringInfo makeStringInfo(void);
  */
 extern void initStringInfo(StringInfo str);
 
+/*------------------------
+ * initStringInfoFromStringWithLen
+ * Initialize a StringInfoData struct from an existing string without copying
+ * the string.  The caller is responsible for ensuring the given string
+ * remains valid as long as the StringInfo is.  The given string must be
+ * NUL terminated at 'len' bytes.  Calls to this are used in performance
+ * critical locations where allocating a new buffer and copying would be too
+ * costly.
+ */
+static inline void
+initStringInfoFromStringWithLen(StringInfo str, char *data, int len)
+{
+	Assert(data[len] == '\0');
+
+	str->data = data;
+	str->maxlen = -1;
+	str->len = len;
+	str->cursor = 0;
+}
+
 /*------------------------
  * resetStringInfo
  * Clears the current content of the StringInfo, if any. The