initstringinfo_changes_v3.patch

text/plain

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

Patch

Format: unified
Series: patch v3
File+
src/backend/replication/logical/proto.c 5 7
src/backend/replication/logical/worker.c 1 4
src/backend/tcop/postgres.c 10 14
src/backend/utils/adt/rowtypes.c 6 9
src/common/stringinfo.c 7 0
src/include/lib/stringinfo.h 77 5
diff --git a/src/backend/replication/logical/proto.c b/src/backend/replication/logical/proto.c
index d52c8963eb..ce9d5b4059 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,16 @@ 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;
+				initStringInfoFromString(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..b574188d70 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;
+					initReadOnlyStringInfo(&s, buf, len);
 
 					c = pq_getmsgbyte(&s);
 
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index f3c9f1f9ba..94b963d6e6 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1816,23 +1816,19 @@ exec_bind_message(StringInfo input_message)
 
 			if (!isNull)
 			{
-				const char *pvalue = pq_getmsgbytes(input_message, plength);
+				char *pvalue;
 
 				/*
-				 * 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';
+				pvalue = unconstify(char *, pq_getmsgbytes(input_message, plength));
+				csave = pvalue[plength];
+				pvalue[plength] = '\0';
+				initReadOnlyStringInfo(&pbuf, pvalue, plength);
 			}
 			else
 			{
diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index ad176651d8..c7ee263439 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -623,21 +623,18 @@ 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;
-
 			csave = buf->data[buf->cursor];
 			buf->data[buf->cursor] = '\0';
+			initReadOnlyStringInfo(&item_buf, strbuff, itemlen);
 
 			bufptr = &item_buf;
 			nulls[i] = false;
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index 05b22b5c53..6897c78d9f 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"
 
 
 /*
@@ -74,6 +75,9 @@ initStringInfo(StringInfo str)
 void
 resetStringInfo(StringInfo str)
 {
+	/* Don't allow resets of read-only StringInfos */
+	Assert(str->maxlen != 0);
+
 	str->data[0] = '\0';
 	str->len = 0;
 	str->cursor = 0;
@@ -284,6 +288,9 @@ enlargeStringInfo(StringInfo str, int needed)
 {
 	int			newlen;
 
+	/* Validate this is not a read-only StringInfo */
+	Assert(str->maxlen != 0);
+
 	/*
 	 * Guard against out-of-range "needed" values.  Without this, we can get
 	 * an overflow or infinite loop in the following.
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 36a416f8e0..3bd605d528 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -27,10 +27,18 @@
  *		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.
- *		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.
+ *				more space.  We must always have maxlen > len, except
+ *				in the read-only case described below.
+ *		cursor	is initialized to zero by makeStringInfo, initStringInfo,
+ *				initReadOnlyStringInfo and initStringInfoFromString but is not
+ *				otherwise touched by the stringinfo.c routines.  Some routines
+ *				use it to scan through a StringInfo.
+ *
+ * As a special case, a StringInfoData can be initialized with a read-only
+ * string buffer.  In this case "data" does not necessarily point at a
+ * palloc'd chunk, and management of the buffer storage is the caller's
+ * responsibility.  maxlen is set to zero to indicate that this is the case.
+ * Read-only StringInfoDatas cannot be appended to.
  *-------------------------
  */
 typedef struct StringInfoData
@@ -45,7 +53,7 @@ typedef StringInfoData *StringInfo;
 
 
 /*------------------------
- * There are two ways to create a StringInfo object initially:
+ * There are four ways to create a StringInfo object initially:
  *
  * StringInfo stringptr = makeStringInfo();
  *		Both the StringInfoData and the data buffer are palloc'd.
@@ -56,8 +64,30 @@ typedef StringInfoData *StringInfo;
  *		This is the easiest approach for a StringInfo object that will
  *		only live as long as the current routine.
  *
+ * StringInfoData string;
+ * initReadOnlyStringInfo(&string, existingbuf, len);
+ *		The StringInfoData's data field is set to point directly to the
+ *		existing buffer and the StringInfoData's len is set to the given len.
+ *		The given buffer can point to memory that's not managed by palloc or
+ *		is pointing part way through a palloc'd chunk.  The maxlen field is
+ *		set to 0.  A read-only StringInfo cannot be appended to using any of
+ *		the appendStringInfo functions or reset with resetStringInfo().  The
+ *		given buffer must be NUL-terminated.
+ *
+ * StringInfoData string;
+ * initStringInfoFromString(&string, palloced_buf, len);
+ *		The StringInfoData's data field is set to point directly to the given
+ *		buffer and the StringInfoData's len is set to the given len.  This
+ *		method of initialization is useful when the buffer already exists.
+ *		StringInfos initialized this way can be appended to using the
+ *		appendStringInfo functions and reset with resetStringInfo().  The
+ *		given buffer must be NUL-terminated.
+ *
  * To destroy a StringInfo, pfree() the data buffer, and then pfree() the
  * StringInfoData if it was palloc'd.  There's no special support for this.
+ * However, if the StringInfo was initialized using initReadOnlyStringInfo()
+ * then the caller will need to consider if it is safe to pfree the data
+ * buffer.
  *
  * NOTE: some routines build up a string using StringInfo, and then
  * release the StringInfoData but return the data string itself to their
@@ -79,6 +109,48 @@ extern StringInfo makeStringInfo(void);
  */
 extern void initStringInfo(StringInfo str);
 
+/*------------------------
+ * initReadOnlyStringInfo
+ * 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 StringInfoData does.  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.  Read-only StringInfoData's may not be appended to using any of the
+ * appendStringInfo functions or reset with resetStringInfo().
+ *
+ * 'data' does not need to point directly to a palloc'd chunk of memory.
+ */
+static inline void
+initReadOnlyStringInfo(StringInfo str, char *data, int len)
+{
+	Assert(data[len] == '\0');
+
+	str->data = data;
+	str->maxlen = 0;	/* read-only */
+	str->len = len;
+	str->cursor = 0;
+}
+
+/*------------------------
+ * initStringInfoFromString
+ * Initialize a StringInfoData struct from an existing string without copying
+ * the string.  'data' must be a valid palloc'd chunk of memory that can have
+ * repalloc() called should more space be required during a call to any of the
+ * appendStringInfo functions.
+ *
+ * The given string must be NUL terminated at 'len' bytes.
+ */
+static inline void
+initStringInfoFromString(StringInfo str, char *data, int len)
+{
+	Assert(data[len] == '\0');
+
+	str->data = data;
+	str->maxlen = str->len = len;
+	str->cursor = 0;
+}
+
 /*------------------------
  * resetStringInfo
  * Clears the current content of the StringInfo, if any. The