public inbox for [email protected]  

help / color / mirror / Atom feed

From: Chao Li <[email protected]>
To: PostgreSQL-development <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Fujii Masao <[email protected]>
Subject: Avoid unnecessary StringInfo allocation in tablesync COPY buffer
Date: Sat, 9 May 2026 14:10:50 +0800
Message-ID: <[email protected]> (raw)

Hi,

I found this issue while reviewing the patch [1] and was suggested use a separate thread for the issue.

In tablesync.c, copy_table() currently does:
```
   copybuf = makeStringInfo();
```

But copybuf is only used by copy_read_data(), and there it's really just acting as a small state holder for data, len, and cursor, rather than as a normal growable StringInfo. That means we do not need to allocate a StringInfo object or its backing buffer at all.

It would be cleaner to use a plain StringInfoData and simply reinitialize or zero it in copy_table(). See the attached patch for the proposed change.

David Rowley has made several cleanup changes in this area to prefer stack-allocated StringInfoData, for example a63bbc811d41b3567eb37fe2636e660a852dbbf2. This change seems consistent with that direction.

[1] https://postgr.es/m/CAOzEurQKuy3RiPkd=25PEwEzaqHuGvEOf=X7vaVzhgNjaukYzA@mail.gmail.com

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/






Attachments:

  [application/octet-stream] v1-0001-Avoid-unnecessary-StringInfo-allocation-in-tables.patch (2.6K, 2-v1-0001-Avoid-unnecessary-StringInfo-allocation-in-tables.patch)
  download | inline diff:
From cffb27e65afaf3b7b0202f4e14518a424cf52ae3 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Sat, 9 May 2026 14:00:02 +0800
Subject: [PATCH v1] Avoid unnecessary StringInfo allocation in tablesync COPY
 buffer

copybuf is only used to track the data pointer, length, and cursor for
leftover COPY data. The buffer allocated by makeStringInfo() is not used,
because copybuf->data is later replaced by the buffer returned from
walrcv_receive().

Use a static StringInfoData instead and reset it before starting COPY.

Author: Chao Li <[email protected]>
---
 src/backend/replication/logical/tablesync.c | 22 ++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index eb718114297..1dee2480e2f 100644
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -126,7 +126,7 @@
 
 List	   *table_states_not_ready = NIL;
 
-static StringInfo copybuf = NULL;
+static StringInfoData copybuf = {0};
 
 /*
  * Wait until the relation sync state is set in the catalog to the expected
@@ -649,13 +649,13 @@ copy_read_data(void *outbuf, int minread, int maxread)
 	int			avail;
 
 	/* If there are some leftover data from previous read, use it. */
-	avail = copybuf->len - copybuf->cursor;
+	avail = copybuf.len - copybuf.cursor;
 	if (avail)
 	{
 		if (avail > maxread)
 			avail = maxread;
-		memcpy(outbuf, &copybuf->data[copybuf->cursor], avail);
-		copybuf->cursor += avail;
+		memcpy(outbuf, &copybuf.data[copybuf.cursor], avail);
+		copybuf.cursor += avail;
 		maxread -= avail;
 		bytesread += avail;
 	}
@@ -680,16 +680,16 @@ copy_read_data(void *outbuf, int minread, int maxread)
 			else
 			{
 				/* Process the data */
-				copybuf->data = buf;
-				copybuf->len = len;
-				copybuf->cursor = 0;
+				copybuf.data = buf;
+				copybuf.len = len;
+				copybuf.cursor = 0;
 
-				avail = copybuf->len - copybuf->cursor;
+				avail = copybuf.len - copybuf.cursor;
 				if (avail > maxread)
 					avail = maxread;
-				memcpy(outbuf, &copybuf->data[copybuf->cursor], avail);
+				memcpy(outbuf, &copybuf.data[copybuf.cursor], avail);
 				outbuf = (char *) outbuf + avail;
-				copybuf->cursor += avail;
+				copybuf.cursor += avail;
 				maxread -= avail;
 				bytesread += avail;
 			}
@@ -1199,7 +1199,7 @@ copy_table(Relation rel)
 						lrel.nspname, lrel.relname, res->err)));
 	walrcv_clear_result(res);
 
-	copybuf = makeStringInfo();
+	memset(&copybuf, 0, sizeof(copybuf));
 
 	pstate = make_parsestate(NULL);
 	(void) addRangeTableEntryForRelation(pstate, rel, AccessShareLock,
-- 
2.50.1 (Apple Git-155)

view thread (3+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: Avoid unnecessary StringInfo allocation in tablesync COPY buffer
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox