Hi,
On 2020-01-13 15:18:04 -0800, Andres Freund wrote:
> On 2020-01-11 22:32:45 -0500, Tom Lane wrote:
> > I wrote:
> > > I saw at this point that the remaining top spots were
> > > enlargeStringInfo and appendBinaryStringInfo, so I experimented
> > > with inlining them (again, see patch below). That *did* move
> > > the needle: down to 72691 ms, or 20% better than HEAD.
> >
> > Oh ... marking the test in the inline part of enlargeStringInfo()
> > as unlikely() helps quite a bit more: 66100 ms, a further 9% gain.
> > Might be over-optimizing for this particular case, perhaps
> >
> > (But ... I'm not finding these numbers to be super reproducible
> > across different ASLR layouts. So take it with a grain of salt.)
>
> FWIW, I've also observed, in another thread (the node func generation
> thing [1]), that inlining enlargeStringInfo() helps a lot, especially
> when inlining some of its callers. Moving e.g. appendStringInfo() inline
> allows the compiler to sometimes optimize away the strlen. But if
> e.g. an inlined appendBinaryStringInfo() still calls enlargeStringInfo()
> unconditionally, successive appends cannot optimize away memory accesses
> for ->len/->data.
With a set of patches doing so, int4send itself is not a significant
factor for my test benchmark [1] anymore. The assembly looks about as
good as one could hope, I think:
# save rbx on the stack
0x00000000004b7f90 <+0>: push %rbx
0x00000000004b7f91 <+1>: sub $0x20,%rsp
# store integer to be sent into rbx
0x00000000004b7f95 <+5>: mov 0x20(%rdi),%rbx
# palloc length argument
0x00000000004b7f99 <+9>: mov $0x9,%edi
0x00000000004b7f9e <+14>: callq 0x5d9aa0 <palloc>
# store integer in buffer (ebx is 4 byte portion of rbx)
0x00000000004b7fa3 <+19>: movbe %ebx,0x4(%rax)
# store varlena header
0x00000000004b7fa8 <+24>: movl $0x20,(%rax)
# restore stack and rbx registerz
0x00000000004b7fae <+30>: add $0x20,%rsp
0x00000000004b7fb2 <+34>: pop %rbx
0x00000000004b7fb3 <+35>: retq
All the $0x20 constants are a bit confusing, but they just happen to be
the same for int4send. It's the size of the stack frame,
offset for FunctionCallInfoBaseData->args[0], the varlena header (and then the
stack
frame again) respectively.
Note that I had to annotate palloc with __attribute__((malloc)) to make
the compiler understand that palloc's returned value will not alias with
anything problematic (e.g. the potential of aliasing with fcinfo
prevents optimizing to the above without that annotation). I think such
annotations would be a good idea anyway, precisely because they allow
the compiler to optimize code significantly better.
These together yields about a 1.8x speedup for me. The profile shows
that the overhead now is overwhelmingly elsewhere:
+ 26.30% postgres postgres [.] CopyOneRowTo
+ 13.40% postgres postgres [.] tts_buffer_heap_getsomeattrs
+ 10.61% postgres postgres [.] AllocSetAlloc
+ 9.26% postgres libc-2.29.so [.] __memmove_avx_unaligned_erms
+ 7.32% postgres postgres [.] SendFunctionCall
+ 6.02% postgres postgres [.] palloc
+ 4.45% postgres postgres [.] int4send
+ 3.68% postgres libc-2.29.so [.] _IO_fwrite
+ 2.71% postgres postgres [.] heapgettup_pagemode
+ 1.96% postgres postgres [.] AllocSetReset
+ 1.83% postgres postgres [.] CopySendEndOfRow
+ 1.75% postgres libc-2.29.so [.] _IO_file_xsputn@@GLIBC_2.2.5
+ 1.60% postgres postgres [.] ExecStoreBufferHeapTuple
+ 1.57% postgres postgres [.] DoCopyTo
+ 1.16% postgres postgres [.] memcpy@plt
+ 1.07% postgres postgres [.] heapgetpage
Even without using the new pq_begintypesend_ex()/initStringInfoEx(), the
generated code is still considerably better than before, yielding a
1.58x speedup. Tallocator overhead unsurprisingly is higher:
+ 24.93% postgres postgres [.] CopyOneRowTo
+ 17.10% postgres postgres [.] AllocSetAlloc
+ 10.09% postgres postgres [.] tts_buffer_heap_getsomeattrs
+ 6.50% postgres libc-2.29.so [.] __memmove_avx_unaligned_erms
+ 5.99% postgres postgres [.] SendFunctionCall
+ 5.11% postgres postgres [.] palloc
+ 3.95% postgres libc-2.29.so [.] _int_malloc
+ 3.38% postgres postgres [.] int4send
+ 2.54% postgres postgres [.] heapgettup_pagemode
+ 2.11% postgres libc-2.29.so [.] _int_free
+ 2.06% postgres postgres [.] MemoryContextReset
+ 2.02% postgres postgres [.] AllocSetReset
+ 1.97% postgres libc-2.29.so [.] _IO_fwrite
+ 1.47% postgres postgres [.] DoCopyTo
+ 1.14% postgres postgres [.] ExecStoreBufferHeapTuple
+ 1.06% postgres libc-2.29.so [.] _IO_file_xsputn@@GLIBC_2.2.5
+ 1.04% postgres libc-2.29.so [.] malloc
Adding a few pg_restrict*, and using appendBinaryStringInfoNT instead of
appendBinaryStringInfo in CopySend* gains another 1.05x.
This does result in some code growth, but given the size of the
improvements, and that the improvements are significant even without
code changes to callsites, that seems worth it.
before:
text data bss dec hex filename
8482739 172304 204240 8859283 872e93 src/backend/postgres
after:
text data bss dec hex filename
8604300 172304 204240 8980844 89096c src/backend/postgres
Regards,
Andres
[1]
CREATE TABLE lotsaints4(c01 int4 NOT NULL, c02 int4 NOT NULL, c03 int4 NOT
NULL, c04 int4 NOT NULL, c05 int4 NOT NULL, c06 int4 NOT NULL, c07 int4 NOT
NULL, c08 int4 NOT NULL, c09 int4 NOT NULL, c10 int4 NOT NULL);
INSERT INTO lotsaints4 SELECT ((random() * 2^31) - 1)::int, ((random() * 2^31)
- 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int,
((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31)
- 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int,
((random() * 2^31) - 1)::int FROM generate_series(1, 2000000);
VACUUM FREEZE lotsaints4;
COPY lotsaints4 TO '/dev/null' WITH binary;
CREATE TABLE lotsaints8(c01 int8 NOT NULL, c02 int8 NOT NULL, c03 int8 NOT
NULL, c04 int8 NOT NULL, c05 int8 NOT NULL, c06 int8 NOT NULL, c07 int8 NOT
NULL, c08 int8 NOT NULL, c09 int8 NOT NULL, c10 int8 NOT NULL);
INSERT INTO lotsaints8 SELECT ((random() * 2^31) - 1)::int, ((random() * 2^31)
- 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int,
((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31)
- 1)::int, ((random() * 2^31) - 1)::int, ((random() * 2^31) - 1)::int,
((random() * 2^31) - 1)::int FROM generate_series(1, 2000000);
VACUUM FREEZE lotsaints8;
COPY lotsaints8 TO '/dev/null' WITH binary;
>From cd3f74953d7b343dfd6e68aa687cc2c0d0bd5bc0 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 12:46:20 -0800
Subject: [PATCH v1 1/6] stringinfo: Move more functions inline, provide
initStringInfoEx().
By moving the whole lifecycle of a stringinfo into inline functions,
a good compiler sometimes can be able to optimize away the existence
of the StringInfoData itself.
initStringInfoEx() allows stringinfo users to determine the initial
allocation size, avoiding over/under allocation when known.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/include/lib/stringinfo.h | 205 +++++++++++++++++++++++++++--------
src/common/stringinfo.c | 173 ++---------------------------
2 files changed, 171 insertions(+), 207 deletions(-)
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 5a2a3dbfbc0..38211323d9d 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -18,6 +18,9 @@
#ifndef STRINGINFO_H
#define STRINGINFO_H
+#include "common/int.h"
+#include "common/string.h"
+
/*-------------------------
* StringInfoData holds information about an extensible string.
* data is the current buffer for the string (allocated with palloc).
@@ -66,25 +69,137 @@ typedef StringInfoData *StringInfo;
*-------------------------
*/
-/*------------------------
- * makeStringInfo
- * Create an empty 'StringInfoData' & return a pointer to it.
- */
-extern StringInfo makeStringInfo(void);
-
-/*------------------------
- * initStringInfo
- * Initialize a StringInfoData struct (with previously undefined contents)
- * to describe an empty string.
- */
-extern void initStringInfo(StringInfo str);
/*------------------------
* resetStringInfo
* Clears the current content of the StringInfo, if any. The
* StringInfo remains valid.
*/
-extern void resetStringInfo(StringInfo str);
+static inline void
+resetStringInfo(StringInfoData *pg_restrict str)
+{
+ *(char *pg_restrict) (str->data) = '\0';
+ str->len = 0;
+ str->cursor = 0;
+}
+
+/*------------------------
+ * initStringInfo
+ * Initialize a StringInfoData struct (with previously undefined contents)
+ * to describe an empty string.
+ */
+static inline void
+initStringInfo(StringInfoData *pg_restrict str)
+{
+ int size = 1024; /* initial default buffer size */
+
+ str->data = (char *) palloc(size);
+ str->maxlen = size;
+ resetStringInfo(str);
+}
+
+/*------------------------
+ * initStringInfoEx
+ *
+ * Like initStringInfo(), but allows to specify the size of the initial
+ * allocation.
+ */
+static inline void
+initStringInfoEx(StringInfoData *pg_restrict str, int size)
+{
+ /*
+ * FIXME: Adding 1 to account for always present trailing \0 byte. Should
+ * that instead be done at the caller level? Should we round up?
+ */
+ str->data = (char *) palloc(size + 1);
+ str->maxlen = size + 1;
+ resetStringInfo(str);
+}
+
+/*------------------------
+ * makeStringInfo
+ * Create an empty 'StringInfoData' & return a pointer to it.
+ */
+static inline StringInfo
+makeStringInfo(void)
+{
+ StringInfo res;
+
+ res = (StringInfo) palloc(sizeof(StringInfoData));
+
+ initStringInfo(res);
+
+ return res;
+}
+
+/*------------------------
+ * enlargeStringInfoImpl
+ *
+ * Actually enlarge the string, only to be called by enlargeStringInfo().
+ */
+extern void enlargeStringInfoImpl(StringInfo str, int needed);
+
+/*------------------------
+ * enlargeStringInfo
+ * Make sure a StringInfo's buffer can hold at least 'needed' more bytes.
+ *
+ * External callers usually need not concern themselves with this, since
+ * all stringinfo.c routines do it automatically. However, if a caller
+ * knows that a StringInfo will eventually become X bytes large, it
+ * can save some palloc overhead by enlarging the buffer before starting
+ * to store data in it.
+ *
+ * NB: In the backend, because we use repalloc() to enlarge the buffer, the
+ * string buffer will remain allocated in the same memory context that was
+ * current when initStringInfo was called, even if another context is now
+ * current. This is the desired and indeed critical behavior!
+ */
+static inline void
+enlargeStringInfo(StringInfoData *pg_restrict str, int datalen)
+{
+ int res;
+
+ if (unlikely(pg_add_s32_overflow(str->len, datalen, &res)) ||
+ unlikely(res >= str->maxlen))
+ enlargeStringInfoImpl(str, datalen);
+}
+
+/*------------------------
+ * appendBinaryStringInfoNT
+ * Append arbitrary binary data to a StringInfo, allocating more space
+ * if necessary. Does not ensure a trailing null-byte exists.
+ */
+static inline void
+appendBinaryStringInfoNT(StringInfoData *pg_restrict str, const char *pg_restrict data, int datalen)
+{
+ Assert(str != NULL);
+
+ /* Make more room if needed */
+ enlargeStringInfo(str, datalen);
+
+ /* OK, append the data */
+ memcpy((char *pg_restrict) (str->data + str->len), data, datalen);
+ str->len += datalen;
+}
+
+/*------------------------
+ * appendBinaryStringInfo
+ * Append arbitrary binary data to a StringInfo, allocating more space
+ * if necessary. Ensures that a trailing null byte is present.
+ */
+static inline void
+appendBinaryStringInfo(StringInfoData *pg_restrict str, const char *pg_restrict data, int datalen)
+{
+ appendBinaryStringInfoNT(str, data, datalen);
+
+ /*
+ * Keep a trailing null in place, even though it's probably useless for
+ * binary data. (Some callers are dealing with text but call this because
+ * their input isn't null-terminated.)
+ */
+ *(char *pg_restrict) (str->data + str->len) = '\0';
+}
+
/*------------------------
* appendStringInfo
@@ -111,51 +226,49 @@ extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_
* Append a null-terminated string to str.
* Like appendStringInfo(str, "%s", s) but faster.
*/
-extern void appendStringInfoString(StringInfo str, const char *s);
+static inline void
+appendStringInfoString(StringInfoData *pg_restrict str, const char *pg_restrict s)
+{
+ appendBinaryStringInfo(str, s, strlen(s));
+}
/*------------------------
* appendStringInfoChar
* Append a single byte to str.
* Like appendStringInfo(str, "%c", ch) but much faster.
*/
-extern void appendStringInfoChar(StringInfo str, char ch);
+static inline void
+appendStringInfoChar(StringInfoData *pg_restrict str, char ch)
+{
+ /* Make more room if needed */
+ enlargeStringInfo(str, 1);
-/*------------------------
- * appendStringInfoCharMacro
- * As above, but a macro for even more speed where it matters.
- * Caution: str argument will be evaluated multiple times.
- */
-#define appendStringInfoCharMacro(str,ch) \
- (((str)->len + 1 >= (str)->maxlen) ? \
- appendStringInfoChar(str, ch) : \
- (void)((str)->data[(str)->len] = (ch), (str)->data[++(str)->len] = '\0'))
+ /* OK, append the character */
+ *(char *pg_restrict) (str->data + str->len) = ch;
+ str->len++;
+ *(char *pg_restrict) (str->data + str->len) = '\0';
+}
+
+/* backward compat for external code */
+#define appendStringInfoCharMacro appendStringInfoChar
/*------------------------
* appendStringInfoSpaces
* Append a given number of spaces to str.
*/
-extern void appendStringInfoSpaces(StringInfo str, int count);
+static inline void
+appendStringInfoSpaces(StringInfoData *pg_restrict str, int count)
+{
+ if (count > 0)
+ {
+ /* Make more room if needed */
+ enlargeStringInfo(str, count);
-/*------------------------
- * appendBinaryStringInfo
- * Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary.
- */
-extern void appendBinaryStringInfo(StringInfo str,
- const char *data, int datalen);
-
-/*------------------------
- * appendBinaryStringInfoNT
- * Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary. Does not ensure a trailing null-byte exists.
- */
-extern void appendBinaryStringInfoNT(StringInfo str,
- const char *data, int datalen);
-
-/*------------------------
- * enlargeStringInfo
- * Make sure a StringInfo's buffer can hold at least 'needed' more bytes.
- */
-extern void enlargeStringInfo(StringInfo str, int needed);
+ /* OK, append the spaces */
+ memset((char *pg_restrict) (str->data + str->len), ' ', count);
+ str->len += count;
+ *(char *pg_restrict) (str->data + str->len) = '\0';
+ }
+}
#endif /* STRINGINFO_H */
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index 0badc465545..0ea5e0abe56 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -32,53 +32,6 @@
#include "lib/stringinfo.h"
-/*
- * makeStringInfo
- *
- * Create an empty 'StringInfoData' & return a pointer to it.
- */
-StringInfo
-makeStringInfo(void)
-{
- StringInfo res;
-
- res = (StringInfo) palloc(sizeof(StringInfoData));
-
- initStringInfo(res);
-
- return res;
-}
-
-/*
- * initStringInfo
- *
- * Initialize a StringInfoData struct (with previously undefined contents)
- * to describe an empty string.
- */
-void
-initStringInfo(StringInfo str)
-{
- int size = 1024; /* initial default buffer size */
-
- str->data = (char *) palloc(size);
- str->maxlen = size;
- resetStringInfo(str);
-}
-
-/*
- * resetStringInfo
- *
- * Reset the StringInfo: the data buffer remains valid, but its
- * previous content, if any, is cleared.
- */
-void
-resetStringInfo(StringInfo str)
-{
- str->data[0] = '\0';
- str->len = 0;
- str->cursor = 0;
-}
-
/*
* appendStringInfo
*
@@ -167,120 +120,18 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args)
}
/*
- * appendStringInfoString
+ * enlargeStringInfoImpl
*
- * Append a null-terminated string to str.
- * Like appendStringInfo(str, "%s", s) but faster.
+ * Make enough space for 'needed' more bytes ('needed' does not include the
+ * terminating null). This is not for external consumption, it's only to be
+ * called by enlargeStringInfo() when more space is actually needed (including
+ * when we'd overflow the maximum size).
+ *
+ * As this normally shouldn't be the common case, mark as noinline, to avoid
+ * including the function into the fastpath.
*/
-void
-appendStringInfoString(StringInfo str, const char *s)
-{
- appendBinaryStringInfo(str, s, strlen(s));
-}
-
-/*
- * appendStringInfoChar
- *
- * Append a single byte to str.
- * Like appendStringInfo(str, "%c", ch) but much faster.
- */
-void
-appendStringInfoChar(StringInfo str, char ch)
-{
- /* Make more room if needed */
- if (str->len + 1 >= str->maxlen)
- enlargeStringInfo(str, 1);
-
- /* OK, append the character */
- str->data[str->len] = ch;
- str->len++;
- str->data[str->len] = '\0';
-}
-
-/*
- * appendStringInfoSpaces
- *
- * Append the specified number of spaces to a buffer.
- */
-void
-appendStringInfoSpaces(StringInfo str, int count)
-{
- if (count > 0)
- {
- /* Make more room if needed */
- enlargeStringInfo(str, count);
-
- /* OK, append the spaces */
- while (--count >= 0)
- str->data[str->len++] = ' ';
- str->data[str->len] = '\0';
- }
-}
-
-/*
- * appendBinaryStringInfo
- *
- * Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary. Ensures that a trailing null byte is present.
- */
-void
-appendBinaryStringInfo(StringInfo str, const char *data, int datalen)
-{
- Assert(str != NULL);
-
- /* Make more room if needed */
- enlargeStringInfo(str, datalen);
-
- /* OK, append the data */
- memcpy(str->data + str->len, data, datalen);
- str->len += datalen;
-
- /*
- * Keep a trailing null in place, even though it's probably useless for
- * binary data. (Some callers are dealing with text but call this because
- * their input isn't null-terminated.)
- */
- str->data[str->len] = '\0';
-}
-
-/*
- * appendBinaryStringInfoNT
- *
- * Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary. Does not ensure a trailing null-byte exists.
- */
-void
-appendBinaryStringInfoNT(StringInfo str, const char *data, int datalen)
-{
- Assert(str != NULL);
-
- /* Make more room if needed */
- enlargeStringInfo(str, datalen);
-
- /* OK, append the data */
- memcpy(str->data + str->len, data, datalen);
- str->len += datalen;
-}
-
-/*
- * enlargeStringInfo
- *
- * Make sure there is enough space for 'needed' more bytes
- * ('needed' does not include the terminating null).
- *
- * External callers usually need not concern themselves with this, since
- * all stringinfo.c routines do it automatically. However, if a caller
- * knows that a StringInfo will eventually become X bytes large, it
- * can save some palloc overhead by enlarging the buffer before starting
- * to store data in it.
- *
- * NB: In the backend, because we use repalloc() to enlarge the buffer, the
- * string buffer will remain allocated in the same memory context that was
- * current when initStringInfo was called, even if another context is now
- * current. This is the desired and indeed critical behavior!
- */
-void
-enlargeStringInfo(StringInfo str, int needed)
+pg_noinline void
+enlargeStringInfoImpl(StringInfo str, int needed)
{
int newlen;
@@ -317,8 +168,8 @@ enlargeStringInfo(StringInfo str, int needed)
/* Because of the above test, we now have needed <= MaxAllocSize */
- if (needed <= str->maxlen)
- return; /* got enough space already */
+ /* should only be called when needed */
+ Assert(needed > str->maxlen);
/*
* We don't want to allocate just a little more space with each append;
--
2.25.0.rc1
>From b71048bb26b561e9c5f022219c672705f130812f Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 12:58:00 -0800
Subject: [PATCH v1 2/6] stringinfo: Remove in-core use of
appendStringInfoCharMacro().
Kept seperate only to reduce size of patch moving to more inlining for
stringinfo.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/commands/copy.c | 2 +-
src/backend/commands/explain.c | 8 ++++----
src/backend/libpq/pqformat.c | 2 +-
src/backend/utils/adt/json.c | 6 +++---
src/backend/utils/adt/jsonb.c | 10 +++++-----
src/backend/utils/adt/jsonfuncs.c | 28 ++++++++++++++--------------
src/backend/utils/adt/jsonpath.c | 6 +++---
src/backend/utils/adt/rowtypes.c | 8 ++++----
src/backend/utils/adt/varlena.c | 4 ++--
src/backend/utils/adt/xml.c | 2 +-
src/backend/utils/error/elog.c | 12 ++++++------
src/backend/utils/mb/stringinfo_mb.c | 2 +-
contrib/tcn/tcn.c | 16 ++++++++--------
13 files changed, 53 insertions(+), 53 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index c93a788798d..88df90deb5b 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -510,7 +510,7 @@ CopySendString(CopyState cstate, const char *str)
static void
CopySendChar(CopyState cstate, char c)
{
- appendStringInfoCharMacro(cstate->fe_msgbuf, c);
+ appendStringInfoChar(cstate->fe_msgbuf, c);
}
static void
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index d189b8d573a..a603763579b 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3900,16 +3900,16 @@ ExplainXMLTag(const char *tagname, int flags, ExplainState *es)
if ((flags & X_NOWHITESPACE) == 0)
appendStringInfoSpaces(es->str, 2 * es->indent);
- appendStringInfoCharMacro(es->str, '<');
+ appendStringInfoChar(es->str, '<');
if ((flags & X_CLOSING) != 0)
- appendStringInfoCharMacro(es->str, '/');
+ appendStringInfoChar(es->str, '/');
for (s = tagname; *s; s++)
appendStringInfoChar(es->str, strchr(valid, *s) ? *s : '-');
if ((flags & X_CLOSE_IMMEDIATE) != 0)
appendStringInfoString(es->str, " /");
- appendStringInfoCharMacro(es->str, '>');
+ appendStringInfoChar(es->str, '>');
if ((flags & X_NOWHITESPACE) == 0)
- appendStringInfoCharMacro(es->str, '\n');
+ appendStringInfoChar(es->str, '\n');
}
/*
diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index a6f990c2d29..999252681c2 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -234,7 +234,7 @@ pq_send_ascii_string(StringInfo buf, const char *str)
if (IS_HIGHBIT_SET(ch))
ch = '?';
- appendStringInfoCharMacro(buf, ch);
+ appendStringInfoChar(buf, ch);
}
appendStringInfoChar(buf, '\0');
}
diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c
index 458505abfd8..1cdf75345cd 100644
--- a/src/backend/utils/adt/json.c
+++ b/src/backend/utils/adt/json.c
@@ -2484,7 +2484,7 @@ escape_json(StringInfo buf, const char *str)
{
const char *p;
- appendStringInfoCharMacro(buf, '"');
+ appendStringInfoChar(buf, '"');
for (p = str; *p; p++)
{
switch (*p)
@@ -2514,11 +2514,11 @@ escape_json(StringInfo buf, const char *str)
if ((unsigned char) *p < ' ')
appendStringInfo(buf, "\\u%04x", (int) *p);
else
- appendStringInfoCharMacro(buf, *p);
+ appendStringInfoChar(buf, *p);
break;
}
}
- appendStringInfoCharMacro(buf, '"');
+ appendStringInfoChar(buf, '"');
}
/*
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index c4a4ec78b0e..3234df61ef3 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -515,7 +515,7 @@ JsonbToCStringWorker(StringInfo out, JsonbContainer *in, int estimated_len, bool
if (!v.val.array.rawScalar)
{
add_indent(out, use_indent && !last_was_key, level);
- appendStringInfoCharMacro(out, '[');
+ appendStringInfoChar(out, '[');
}
else
raw_scalar = true;
@@ -528,7 +528,7 @@ JsonbToCStringWorker(StringInfo out, JsonbContainer *in, int estimated_len, bool
appendBinaryStringInfo(out, ", ", ispaces);
add_indent(out, use_indent && !last_was_key, level);
- appendStringInfoCharMacro(out, '{');
+ appendStringInfoChar(out, '{');
first = true;
level++;
@@ -576,14 +576,14 @@ JsonbToCStringWorker(StringInfo out, JsonbContainer *in, int estimated_len, bool
if (!raw_scalar)
{
add_indent(out, use_indent, level);
- appendStringInfoCharMacro(out, ']');
+ appendStringInfoChar(out, ']');
}
first = false;
break;
case WJB_END_OBJECT:
level--;
add_indent(out, use_indent, level);
- appendStringInfoCharMacro(out, '}');
+ appendStringInfoChar(out, '}');
first = false;
break;
default:
@@ -605,7 +605,7 @@ add_indent(StringInfo out, bool indent, int level)
{
int i;
- appendStringInfoCharMacro(out, '\n');
+ appendStringInfoChar(out, '\n');
for (i = 0; i < level; i++)
appendBinaryStringInfo(out, " ", 4);
}
diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c
index ab5a24a8584..95cbbbe1311 100644
--- a/src/backend/utils/adt/jsonfuncs.c
+++ b/src/backend/utils/adt/jsonfuncs.c
@@ -3860,7 +3860,7 @@ sn_object_start(void *state)
{
StripnullState *_state = (StripnullState *) state;
- appendStringInfoCharMacro(_state->strval, '{');
+ appendStringInfoChar(_state->strval, '{');
}
static void
@@ -3868,7 +3868,7 @@ sn_object_end(void *state)
{
StripnullState *_state = (StripnullState *) state;
- appendStringInfoCharMacro(_state->strval, '}');
+ appendStringInfoChar(_state->strval, '}');
}
static void
@@ -3876,7 +3876,7 @@ sn_array_start(void *state)
{
StripnullState *_state = (StripnullState *) state;
- appendStringInfoCharMacro(_state->strval, '[');
+ appendStringInfoChar(_state->strval, '[');
}
static void
@@ -3884,7 +3884,7 @@ sn_array_end(void *state)
{
StripnullState *_state = (StripnullState *) state;
- appendStringInfoCharMacro(_state->strval, ']');
+ appendStringInfoChar(_state->strval, ']');
}
static void
@@ -3904,7 +3904,7 @@ sn_object_field_start(void *state, char *fname, bool isnull)
}
if (_state->strval->data[_state->strval->len - 1] != '{')
- appendStringInfoCharMacro(_state->strval, ',');
+ appendStringInfoChar(_state->strval, ',');
/*
* Unfortunately we don't have the quoted and escaped string any more, so
@@ -3912,7 +3912,7 @@ sn_object_field_start(void *state, char *fname, bool isnull)
*/
escape_json(_state->strval, fname);
- appendStringInfoCharMacro(_state->strval, ':');
+ appendStringInfoChar(_state->strval, ':');
}
static void
@@ -3921,7 +3921,7 @@ sn_array_element_start(void *state, bool isnull)
StripnullState *_state = (StripnullState *) state;
if (_state->strval->data[_state->strval->len - 1] != '[')
- appendStringInfoCharMacro(_state->strval, ',');
+ appendStringInfoChar(_state->strval, ',');
}
static void
@@ -5178,7 +5178,7 @@ transform_string_values_object_start(void *state)
{
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
- appendStringInfoCharMacro(_state->strval, '{');
+ appendStringInfoChar(_state->strval, '{');
}
static void
@@ -5186,7 +5186,7 @@ transform_string_values_object_end(void *state)
{
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
- appendStringInfoCharMacro(_state->strval, '}');
+ appendStringInfoChar(_state->strval, '}');
}
static void
@@ -5194,7 +5194,7 @@ transform_string_values_array_start(void *state)
{
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
- appendStringInfoCharMacro(_state->strval, '[');
+ appendStringInfoChar(_state->strval, '[');
}
static void
@@ -5202,7 +5202,7 @@ transform_string_values_array_end(void *state)
{
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
- appendStringInfoCharMacro(_state->strval, ']');
+ appendStringInfoChar(_state->strval, ']');
}
static void
@@ -5211,14 +5211,14 @@ transform_string_values_object_field_start(void *state, char *fname, bool isnull
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
if (_state->strval->data[_state->strval->len - 1] != '{')
- appendStringInfoCharMacro(_state->strval, ',');
+ appendStringInfoChar(_state->strval, ',');
/*
* Unfortunately we don't have the quoted and escaped string any more, so
* we have to re-escape it.
*/
escape_json(_state->strval, fname);
- appendStringInfoCharMacro(_state->strval, ':');
+ appendStringInfoChar(_state->strval, ':');
}
static void
@@ -5227,7 +5227,7 @@ transform_string_values_array_element_start(void *state, bool isnull)
TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state;
if (_state->strval->data[_state->strval->len - 1] != '[')
- appendStringInfoCharMacro(_state->strval, ',');
+ appendStringInfoChar(_state->strval, ',');
}
static void
diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c
index 3c0dc38a7f8..43d5914477a 100644
--- a/src/backend/utils/adt/jsonpath.c
+++ b/src/backend/utils/adt/jsonpath.c
@@ -441,13 +441,13 @@ alignStringInfoInt(StringInfo buf)
switch (INTALIGN(buf->len) - buf->len)
{
case 3:
- appendStringInfoCharMacro(buf, 0);
+ appendStringInfoChar(buf, 0);
/* FALLTHROUGH */
case 2:
- appendStringInfoCharMacro(buf, 0);
+ appendStringInfoChar(buf, 0);
/* FALLTHROUGH */
case 1:
- appendStringInfoCharMacro(buf, 0);
+ appendStringInfoChar(buf, 0);
/* FALLTHROUGH */
default:
break;
diff --git a/src/backend/utils/adt/rowtypes.c b/src/backend/utils/adt/rowtypes.c
index 06ad83d7cae..fc80dfb1aea 100644
--- a/src/backend/utils/adt/rowtypes.c
+++ b/src/backend/utils/adt/rowtypes.c
@@ -424,17 +424,17 @@ record_out(PG_FUNCTION_ARGS)
/* And emit the string */
if (nq)
- appendStringInfoCharMacro(&buf, '"');
+ appendStringInfoChar(&buf, '"');
for (tmp = value; *tmp; tmp++)
{
char ch = *tmp;
if (ch == '"' || ch == '\\')
- appendStringInfoCharMacro(&buf, ch);
- appendStringInfoCharMacro(&buf, ch);
+ appendStringInfoChar(&buf, ch);
+ appendStringInfoChar(&buf, ch);
}
if (nq)
- appendStringInfoCharMacro(&buf, '"');
+ appendStringInfoChar(&buf, '"');
}
appendStringInfoChar(&buf, ')');
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 1b351cbc688..b11df3f92cd 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -5523,7 +5523,7 @@ text_format(PG_FUNCTION_ARGS)
*/
if (*cp != '%')
{
- appendStringInfoCharMacro(&str, *cp);
+ appendStringInfoChar(&str, *cp);
continue;
}
@@ -5532,7 +5532,7 @@ text_format(PG_FUNCTION_ARGS)
/* Easy case: %% outputs a single % */
if (*cp == '%')
{
- appendStringInfoCharMacro(&str, *cp);
+ appendStringInfoChar(&str, *cp);
continue;
}
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 3808c307f6f..8e71953831e 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -2391,7 +2391,7 @@ escape_xml(const char *str)
appendStringInfoString(&buf, "
");
break;
default:
- appendStringInfoCharMacro(&buf, *p);
+ appendStringInfoChar(&buf, *p);
break;
}
}
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index f5b0211f66b..55317881ea0 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2724,14 +2724,14 @@ appendCSVLiteral(StringInfo buf, const char *data)
if (p == NULL)
return;
- appendStringInfoCharMacro(buf, '"');
+ appendStringInfoChar(buf, '"');
while ((c = *p++) != '\0')
{
if (c == '"')
- appendStringInfoCharMacro(buf, '"');
- appendStringInfoCharMacro(buf, c);
+ appendStringInfoChar(buf, '"');
+ appendStringInfoChar(buf, c);
}
- appendStringInfoCharMacro(buf, '"');
+ appendStringInfoChar(buf, '"');
}
/*
@@ -3491,9 +3491,9 @@ append_with_tabs(StringInfo buf, const char *str)
while ((ch = *str++) != '\0')
{
- appendStringInfoCharMacro(buf, ch);
+ appendStringInfoChar(buf, ch);
if (ch == '\n')
- appendStringInfoCharMacro(buf, '\t');
+ appendStringInfoChar(buf, '\t');
}
}
diff --git a/src/backend/utils/mb/stringinfo_mb.c b/src/backend/utils/mb/stringinfo_mb.c
index c153b770076..1d7245b0a47 100644
--- a/src/backend/utils/mb/stringinfo_mb.c
+++ b/src/backend/utils/mb/stringinfo_mb.c
@@ -61,7 +61,7 @@ appendStringInfoStringQuoted(StringInfo str, const char *s, int maxlen)
ellipsis = false;
}
- appendStringInfoCharMacro(str, '\'');
+ appendStringInfoChar(str, '\'');
while ((chunk_end = strchr(chunk_search_start, '\'')) != NULL)
{
diff --git a/contrib/tcn/tcn.c b/contrib/tcn/tcn.c
index 552f107bf6b..d38d6e3bca8 100644
--- a/contrib/tcn/tcn.c
+++ b/contrib/tcn/tcn.c
@@ -32,15 +32,15 @@ PG_MODULE_MAGIC;
static void
strcpy_quoted(StringInfo r, const char *s, const char q)
{
- appendStringInfoCharMacro(r, q);
+ appendStringInfoChar(r, q);
while (*s)
{
if (*s == q)
- appendStringInfoCharMacro(r, q);
- appendStringInfoCharMacro(r, *s);
+ appendStringInfoChar(r, q);
+ appendStringInfoChar(r, *s);
s++;
}
- appendStringInfoCharMacro(r, q);
+ appendStringInfoChar(r, q);
}
/*
@@ -147,17 +147,17 @@ triggered_change_notification(PG_FUNCTION_ARGS)
foundPK = true;
strcpy_quoted(payload, RelationGetRelationName(rel), '"');
- appendStringInfoCharMacro(payload, ',');
- appendStringInfoCharMacro(payload, operation);
+ appendStringInfoChar(payload, ',');
+ appendStringInfoChar(payload, operation);
for (i = 0; i < indnkeyatts; i++)
{
int colno = index->indkey.values[i];
Form_pg_attribute attr = TupleDescAttr(tupdesc, colno - 1);
- appendStringInfoCharMacro(payload, ',');
+ appendStringInfoChar(payload, ',');
strcpy_quoted(payload, NameStr(attr->attname), '"');
- appendStringInfoCharMacro(payload, '=');
+ appendStringInfoChar(payload, '=');
strcpy_quoted(payload, SPI_getvalue(trigtuple, tupdesc, colno), '\'');
}
--
2.25.0.rc1
>From 93e27b84bef5232346d01cfffe60851434451c2b Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 12:50:06 -0800
Subject: [PATCH v1 3/6] pqformat: Move functions for type sending inline, add
pq_begintypsend_ex().
Combined with the previous commit inlining more functions in
stringinfo this allows many type send functions to be optimized
considerably better, optimizing away the StringInfoData entirely.
The new pq_begintypsend_ex() is useful to avoid over-allocating for
send functions that only output a small amount of data, e.g. int4send
now allocates 9 bytes, instead of 1024.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/include/libpq/pqformat.h | 52 ++++++++++++++++++++++++++++++++++++
src/backend/libpq/pqformat.c | 38 --------------------------
2 files changed, 52 insertions(+), 38 deletions(-)
diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h
index af31e9caba3..e0dbb783c6a 100644
--- a/src/include/libpq/pqformat.h
+++ b/src/include/libpq/pqformat.h
@@ -31,6 +31,58 @@ extern void pq_send_ascii_string(StringInfo buf, const char *str);
extern void pq_sendfloat4(StringInfo buf, float4 f);
extern void pq_sendfloat8(StringInfo buf, float8 f);
+
+/* --------------------------------
+ * pq_begintypsend - initialize for constructing a bytea result
+ * --------------------------------
+ */
+static inline void
+pq_begintypsend(StringInfo buf)
+{
+ initStringInfo(buf);
+ /* Reserve four bytes for the bytea length word */
+ appendStringInfoSpaces(buf, 4);
+}
+
+/* --------------------------------
+ * pq_begintypsend_ex - like pq_begintypesend, but with length hint
+ *
+ * This can be used over pq_begintypesend if the caller can cheaply determine
+ * how much data will be sent, reducing the initial size of the
+ * StringInfo. The passed in size need not include the overhead of the length
+ * word.
+ * --------------------------------
+ */
+static inline void
+pq_begintypsend_ex(StringInfo buf, int size)
+{
+ initStringInfoEx(buf, size + 4);
+ /* Reserve four bytes for the bytea length word */
+ appendStringInfoSpaces(buf, 4);
+}
+
+
+/* --------------------------------
+ * pq_endtypsend - finish constructing a bytea result
+ *
+ * The data buffer is returned as the palloc'd bytea value. (We expect
+ * that it will be suitably aligned for this because it has been palloc'd.)
+ * We assume the StringInfoData is just a local variable in the caller and
+ * need not be pfree'd.
+ * --------------------------------
+ */
+static inline bytea *
+pq_endtypsend(StringInfo buf)
+{
+ bytea *result = (bytea *) buf->data;
+
+ /* Insert correct length into bytea length word */
+ Assert(buf->len >= VARHDRSZ);
+ SET_VARSIZE(result, buf->len);
+
+ return result;
+}
+
/*
* Append a [u]int8 to a StringInfo buffer, which already has enough space
* preallocated.
diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index 999252681c2..347a0aa697a 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -319,44 +319,6 @@ pq_endmessage_reuse(StringInfo buf)
(void) pq_putmessage(buf->cursor, buf->data, buf->len);
}
-
-/* --------------------------------
- * pq_begintypsend - initialize for constructing a bytea result
- * --------------------------------
- */
-void
-pq_begintypsend(StringInfo buf)
-{
- initStringInfo(buf);
- /* Reserve four bytes for the bytea length word */
- appendStringInfoCharMacro(buf, '\0');
- appendStringInfoCharMacro(buf, '\0');
- appendStringInfoCharMacro(buf, '\0');
- appendStringInfoCharMacro(buf, '\0');
-}
-
-/* --------------------------------
- * pq_endtypsend - finish constructing a bytea result
- *
- * The data buffer is returned as the palloc'd bytea value. (We expect
- * that it will be suitably aligned for this because it has been palloc'd.)
- * We assume the StringInfoData is just a local variable in the caller and
- * need not be pfree'd.
- * --------------------------------
- */
-bytea *
-pq_endtypsend(StringInfo buf)
-{
- bytea *result = (bytea *) buf->data;
-
- /* Insert correct length into bytea length word */
- Assert(buf->len >= VARHDRSZ);
- SET_VARSIZE(result, buf->len);
-
- return result;
-}
-
-
/* --------------------------------
* pq_puttextmessage - generate a character set-converted message in one step
*
--
2.25.0.rc1
>From 86224c8f292adba4440f0a2cd4077afd7277c85c Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 12:53:33 -0800
Subject: [PATCH v1 4/6] WIP: Annotate palloc() with malloc and other compiler
attributes.
In particularly malloc is useful, allowing the compiler to realize
that the return value does not alias with other data, allowing other
optimizations.
If we were to do this, we'd obviously need to hide these behind
macros to support compilers without those attributes.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/include/utils/palloc.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h
index cc356a63728..a611148be67 100644
--- a/src/include/utils/palloc.h
+++ b/src/include/utils/palloc.h
@@ -74,7 +74,7 @@ extern void *MemoryContextAllocZeroAligned(MemoryContext context, Size size);
extern void *MemoryContextAllocExtended(MemoryContext context,
Size size, int flags);
-extern void *palloc(Size size);
+extern void *palloc(Size size) __attribute__((malloc, alloc_size(1), assume_aligned(MAXIMUM_ALIGNOF), returns_nonnull, warn_unused_result));
extern void *palloc0(Size size);
extern void *palloc_extended(Size size, int flags);
extern void *repalloc(void *pointer, Size size);
--
2.25.0.rc1
>From be4a71efa94e486e47b4e2ba1fb93149f1247b22 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 12:58:55 -0800
Subject: [PATCH v1 5/6] Move {int4,int8}send to use pq_begintypsend_ex.
If we were to introduce pq_begintypsend_ex(), we'd probably want to do
this to quite a few more functions.
---
src/backend/utils/adt/int.c | 2 +-
src/backend/utils/adt/int8.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c
index 583ce71e664..1c2528c3095 100644
--- a/src/backend/utils/adt/int.c
+++ b/src/backend/utils/adt/int.c
@@ -305,7 +305,7 @@ int4send(PG_FUNCTION_ARGS)
int32 arg1 = PG_GETARG_INT32(0);
StringInfoData buf;
- pq_begintypsend(&buf);
+ pq_begintypsend_ex(&buf, 4);
pq_sendint32(&buf, arg1);
PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
}
diff --git a/src/backend/utils/adt/int8.c b/src/backend/utils/adt/int8.c
index fcdf77331e7..be08530acbc 100644
--- a/src/backend/utils/adt/int8.c
+++ b/src/backend/utils/adt/int8.c
@@ -176,7 +176,7 @@ int8send(PG_FUNCTION_ARGS)
int64 arg1 = PG_GETARG_INT64(0);
StringInfoData buf;
- pq_begintypsend(&buf);
+ pq_begintypsend_ex(&buf, 8);
pq_sendint64(&buf, arg1);
PG_RETURN_BYTEA_P(pq_endtypsend(&buf));
}
--
2.25.0.rc1
>From d84bf0f4a4e0cc0ac088ea18162f0c8e94cfadf3 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 14 Jan 2020 13:00:26 -0800
Subject: [PATCH v1 6/6] copy: Use appendBinaryStringInfoNT() for sending
binary data.
Maintaining the trailing byte is useless overhead.
Author:
Reviewed-By:
Discussion: https://postgr.es/m/
Backpatch:
---
src/backend/commands/copy.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 88df90deb5b..9c0b38224d1 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -498,13 +498,13 @@ SendCopyEnd(CopyState cstate)
static void
CopySendData(CopyState cstate, const void *databuf, int datasize)
{
- appendBinaryStringInfo(cstate->fe_msgbuf, databuf, datasize);
+ appendBinaryStringInfoNT(cstate->fe_msgbuf, databuf, datasize);
}
static void
CopySendString(CopyState cstate, const char *str)
{
- appendBinaryStringInfo(cstate->fe_msgbuf, str, strlen(str));
+ appendBinaryStringInfoNT(cstate->fe_msgbuf, str, strlen(str));
}
static void
--
2.25.0.rc1
