Hi,
Attached is a revised version of this patchset. I'd like to get some
input on two points:
1) Does anybody have a better idea than the static buffer in
SendRowDescriptionMessage()? That's not particularly pretty, but
there's not really a convenient stringbuffer to use when called from
exec_describe_portal_message(). We could instead create a local
buffer for exec_describe_portal_message().
An alternative idea would be to have one reeusable buffer created for
each transaction command, but I'm not sure that's really better.
2) There's a lot of remaining pq_sendint() callers in other parts of the
tree. If others are ok with that, I'd do a separate pass over them.
I'd say that even after doing that, we should keep pq_sendint(),
because a lot of extension code is using that.
3) The use of restrict, with a configure based fallback, is something
we've not done before, but it's C99 and delivers significantly more
efficient code. Any arguments against?
Regards,
Andres
>From ff8c4128a46199beab2beb09c1ad0627bbc18b94 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 20 Sep 2017 13:01:22 -0700
Subject: [PATCH 1/6] Add configure infrastructure to detect support for C99's
restrict.
Will be used in later commits improving performance for a few key
routines where information about aliasing allows for significantly
better code generation.
This allows to use the C99 'restrict' keyword without breaking C89, or
for that matter C++, compilers. If not supported it's defined to be
empty.
Author: Andres Freund
---
configure | 46 +++++++++++++++++++++++++++++++++++++++++++
configure.in | 1 +
src/include/pg_config.h.in | 14 +++++++++++++
src/include/pg_config.h.win32 | 6 ++++++
4 files changed, 67 insertions(+)
diff --git a/configure b/configure
index 216447e739..5fa7a61025 100755
--- a/configure
+++ b/configure
@@ -11545,6 +11545,52 @@ _ACEOF
;;
esac
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for C/C++ restrict keyword" >&5
+$as_echo_n "checking for C/C++ restrict keyword... " >&6; }
+if ${ac_cv_c_restrict+:} false; then :
+ $as_echo_n "(cached) " >&6
+else
+ ac_cv_c_restrict=no
+ # The order here caters to the fact that C++ does not require restrict.
+ for ac_kw in __restrict __restrict__ _Restrict restrict; do
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+/* end confdefs.h. */
+typedef int * int_ptr;
+ int foo (int_ptr $ac_kw ip) {
+ return ip[0];
+ }
+int
+main ()
+{
+int s[1];
+ int * $ac_kw t = s;
+ t[0] = 0;
+ return foo(t)
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile "$LINENO"; then :
+ ac_cv_c_restrict=$ac_kw
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+ test "$ac_cv_c_restrict" != no && break
+ done
+
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_c_restrict" >&5
+$as_echo "$ac_cv_c_restrict" >&6; }
+
+ case $ac_cv_c_restrict in
+ restrict) ;;
+ no) $as_echo "#define restrict /**/" >>confdefs.h
+ ;;
+ *) cat >>confdefs.h <<_ACEOF
+#define restrict $ac_cv_c_restrict
+_ACEOF
+ ;;
+ esac
+
{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for printf format archetype" >&5
$as_echo_n "checking for printf format archetype... " >&6; }
if ${pgac_cv_printf_archetype+:} false; then :
diff --git a/configure.in b/configure.in
index a2e3d8331a..bebbd11af9 100644
--- a/configure.in
+++ b/configure.in
@@ -1299,6 +1299,7 @@ fi
m4_defun([AC_PROG_CC_STDC], []) dnl We don't want that.
AC_C_BIGENDIAN
AC_C_INLINE
+AC_C_RESTRICT
PGAC_PRINTF_ARCHETYPE
AC_C_FLEXIBLE_ARRAY_MEMBER
PGAC_C_SIGNED
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 368a297e6d..b7ae9a0702 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -916,6 +916,20 @@
if such a type exists, and if the system does not define it. */
#undef intptr_t
+/* Define to the equivalent of the C99 'restrict' keyword, or to
+ nothing if this is not supported. Do not define if restrict is
+ supported directly. */
+#undef restrict
+/* Work around a bug in Sun C++: it does not support _Restrict or
+ __restrict__, even though the corresponding Sun C compiler ends up with
+ "#define restrict _Restrict" or "#define restrict __restrict__" in the
+ previous line. Perhaps some future version of Sun C++ will work with
+ restrict; if so, hopefully it defines __RESTRICT like Sun C does. */
+#if defined __SUNPRO_CC && !defined __RESTRICT
+# define _Restrict
+# define __restrict__
+#endif
+
/* Define to empty if the C compiler does not understand signed types. */
#undef signed
diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 3537b6f704..e6b3c5d551 100644
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -674,6 +674,12 @@
#define inline __inline
#endif
+/* Define to the equivalent of the C99 'restrict' keyword, or to
+ nothing if this is not supported. Do not define if restrict is
+ supported directly. */
+/* works for C an C++ in msvc */
+#define restrict __restrict
+
/* Define to empty if the C compiler does not understand signed types. */
/* #undef signed */
--
2.14.1.536.g6867272d5b.dirty
>From e30ff518e30f923e4705f0817192bbbe77c82d2c Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 3 Oct 2017 00:16:15 -0700
Subject: [PATCH 2/6] Allow to avoid NUL-byte management for stringinfos and
use in format.c.
In a lot of the places having appendBinaryStringInfo() maintain a
trailing NUL byte wasn't actually meaningful, e.g. when appending an
integer which can contain 0 in one of its bytes.
Removing this yields some small speedup, but more importantly will be
more consistent when providing faster variants of pq_sendint etc.
---
src/backend/lib/stringinfo.c | 21 ++++++++++++++++++++-
src/backend/libpq/pqformat.c | 18 +++++++++---------
src/include/lib/stringinfo.h | 8 ++++++++
3 files changed, 37 insertions(+), 10 deletions(-)
diff --git a/src/backend/lib/stringinfo.c b/src/backend/lib/stringinfo.c
index fd15567144..ecb45c982f 100644
--- a/src/backend/lib/stringinfo.c
+++ b/src/backend/lib/stringinfo.c
@@ -202,7 +202,7 @@ appendStringInfoSpaces(StringInfo str, int count)
* appendBinaryStringInfo
*
* Append arbitrary binary data to a StringInfo, allocating more space
- * if necessary.
+ * if necessary. Ensures that a trailing null byte is present.
*/
void
appendBinaryStringInfo(StringInfo str, const char *data, int datalen)
@@ -224,6 +224,25 @@ appendBinaryStringInfo(StringInfo str, const char *data, int datalen)
str->data[str->len] = '\0';
}
+/*
+ * appendBinaryStringInfoNT
+ *
+ * Append arbitrary binary data to a StringInfo, allocating more space
+ * if necessary. Does not ensure 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
*
diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index f27a04f834..2414d0d8e9 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -138,13 +138,13 @@ pq_sendcountedtext(StringInfo buf, const char *str, int slen,
{
slen = strlen(p);
pq_sendint(buf, slen + extra, 4);
- appendBinaryStringInfo(buf, p, slen);
+ appendBinaryStringInfoNT(buf, p, slen);
pfree(p);
}
else
{
pq_sendint(buf, slen + extra, 4);
- appendBinaryStringInfo(buf, str, slen);
+ appendBinaryStringInfoNT(buf, str, slen);
}
}
@@ -191,11 +191,11 @@ pq_sendstring(StringInfo buf, const char *str)
if (p != str) /* actual conversion has been done? */
{
slen = strlen(p);
- appendBinaryStringInfo(buf, p, slen + 1);
+ appendBinaryStringInfoNT(buf, p, slen + 1);
pfree(p);
}
else
- appendBinaryStringInfo(buf, str, slen + 1);
+ appendBinaryStringInfoNT(buf, str, slen + 1);
}
/* --------------------------------
@@ -242,15 +242,15 @@ pq_sendint(StringInfo buf, int i, int b)
{
case 1:
n8 = (unsigned char) i;
- appendBinaryStringInfo(buf, (char *) &n8, 1);
+ appendBinaryStringInfoNT(buf, (char *) &n8, 1);
break;
case 2:
n16 = pg_hton16((uint16) i);
- appendBinaryStringInfo(buf, (char *) &n16, 2);
+ appendBinaryStringInfoNT(buf, (char *) &n16, 2);
break;
case 4:
n32 = pg_hton32((uint32) i);
- appendBinaryStringInfo(buf, (char *) &n32, 4);
+ appendBinaryStringInfoNT(buf, (char *) &n32, 4);
break;
default:
elog(ERROR, "unsupported integer size %d", b);
@@ -271,7 +271,7 @@ pq_sendint64(StringInfo buf, int64 i)
{
uint64 n64 = pg_hton64(i);
- appendBinaryStringInfo(buf, (char *) &n64, sizeof(n64));
+ appendBinaryStringInfoNT(buf, (char *) &n64, sizeof(n64));
}
/* --------------------------------
@@ -297,7 +297,7 @@ pq_sendfloat4(StringInfo buf, float4 f)
swap.f = f;
swap.i = pg_hton32(swap.i);
- appendBinaryStringInfo(buf, (char *) &swap.i, 4);
+ appendBinaryStringInfoNT(buf, (char *) &swap.i, 4);
}
/* --------------------------------
diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h
index 9694ea3f21..49be6b8e16 100644
--- a/src/include/lib/stringinfo.h
+++ b/src/include/lib/stringinfo.h
@@ -143,6 +143,14 @@ extern void appendStringInfoSpaces(StringInfo str, int count);
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 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.
--
2.14.1.536.g6867272d5b.dirty
>From 0dbd72b02cb187ffcaade8a3a47c51249b411d42 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 13 Sep 2017 18:39:24 -0700
Subject: [PATCH 3/6] Add more efficient functions to pqformat API.
New inline functions allow to add data to a stringbuf in a more
efficient manner by pre-allocating memory ahead of time.
The newly added pq_beginmessage_pre/pq_endmessage_keep allow reuse of
a stringbuffer across multiple message cycles.
---
src/backend/libpq/pqformat.c | 86 +++++++++--------------
src/backend/utils/mb/mbutils.c | 11 ---
src/include/libpq/pqformat.h | 155 +++++++++++++++++++++++++++++++++++++++--
src/include/mb/pg_wchar.h | 11 +++
4 files changed, 195 insertions(+), 68 deletions(-)
diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c
index 2414d0d8e9..a7657d5e37 100644
--- a/src/backend/libpq/pqformat.c
+++ b/src/backend/libpq/pqformat.c
@@ -97,13 +97,24 @@ pq_beginmessage(StringInfo buf, char msgtype)
}
/* --------------------------------
- * pq_sendbyte - append a raw byte to a StringInfo buffer
+
+ * pq_beginmessage_pre - initialize for sending a message, reuse buffer
+ *
+ * This requires the buffer to be allocated in an sufficiently long-lived
+ * memory context.
* --------------------------------
*/
void
-pq_sendbyte(StringInfo buf, int byt)
+pq_beginmessage_pre(StringInfo buf, char msgtype)
{
- appendStringInfoCharMacro(buf, byt);
+ resetStringInfo(buf);
+
+ /*
+ * We stash the message type into the buffer's cursor field, expecting
+ * that the pq_sendXXX routines won't touch it. We could alternatively
+ * make it the first byte of the buffer contents, but this seems easier.
+ */
+ buf->cursor = msgtype;
}
/* --------------------------------
@@ -113,6 +124,7 @@ pq_sendbyte(StringInfo buf, int byt)
void
pq_sendbytes(StringInfo buf, const char *data, int datalen)
{
+ /* use variant that maintains a trailing null-byte, out of caution */
appendBinaryStringInfo(buf, data, datalen);
}
@@ -137,13 +149,13 @@ pq_sendcountedtext(StringInfo buf, const char *str, int slen,
if (p != str) /* actual conversion has been done? */
{
slen = strlen(p);
- pq_sendint(buf, slen + extra, 4);
+ pq_sendint32(buf, slen + extra);
appendBinaryStringInfoNT(buf, p, slen);
pfree(p);
}
else
{
- pq_sendint(buf, slen + extra, 4);
+ pq_sendint32(buf, slen + extra);
appendBinaryStringInfoNT(buf, str, slen);
}
}
@@ -227,53 +239,6 @@ pq_send_ascii_string(StringInfo buf, const char *str)
appendStringInfoChar(buf, '\0');
}
-/* --------------------------------
- * pq_sendint - append a binary integer to a StringInfo buffer
- * --------------------------------
- */
-void
-pq_sendint(StringInfo buf, int i, int b)
-{
- unsigned char n8;
- uint16 n16;
- uint32 n32;
-
- switch (b)
- {
- case 1:
- n8 = (unsigned char) i;
- appendBinaryStringInfoNT(buf, (char *) &n8, 1);
- break;
- case 2:
- n16 = pg_hton16((uint16) i);
- appendBinaryStringInfoNT(buf, (char *) &n16, 2);
- break;
- case 4:
- n32 = pg_hton32((uint32) i);
- appendBinaryStringInfoNT(buf, (char *) &n32, 4);
- break;
- default:
- elog(ERROR, "unsupported integer size %d", b);
- break;
- }
-}
-
-/* --------------------------------
- * pq_sendint64 - append a binary 8-byte int to a StringInfo buffer
- *
- * It is tempting to merge this with pq_sendint, but we'd have to make the
- * argument int64 for all data widths --- that could be a big performance
- * hit on machines where int64 isn't efficient.
- * --------------------------------
- */
-void
-pq_sendint64(StringInfo buf, int64 i)
-{
- uint64 n64 = pg_hton64(i);
-
- appendBinaryStringInfoNT(buf, (char *) &n64, sizeof(n64));
-}
-
/* --------------------------------
* pq_sendfloat4 - append a float4 to a StringInfo buffer
*
@@ -297,7 +262,7 @@ pq_sendfloat4(StringInfo buf, float4 f)
swap.f = f;
swap.i = pg_hton32(swap.i);
- appendBinaryStringInfoNT(buf, (char *) &swap.i, 4);
+ pq_sendint32(buf, swap.i);
}
/* --------------------------------
@@ -341,6 +306,21 @@ pq_endmessage(StringInfo buf)
buf->data = NULL;
}
+/* --------------------------------
+ * pq_endmessage_keep - send the completed message to the frontend
+ *
+ * The data buffer is *not* freed, allowing to reuse the buffer with
+ * pg_beginmessage_pre.
+ --------------------------------
+ */
+
+void
+pq_endmessage_keep(StringInfo buf)
+{
+ /* msgtype was saved in cursor field */
+ (void) pq_putmessage(buf->cursor, buf->data, buf->len);
+}
+
/* --------------------------------
* pq_begintypsend - initialize for constructing a bytea result
diff --git a/src/backend/utils/mb/mbutils.c b/src/backend/utils/mb/mbutils.c
index c4fbe0903b..56f4dc1453 100644
--- a/src/backend/utils/mb/mbutils.c
+++ b/src/backend/utils/mb/mbutils.c
@@ -41,17 +41,6 @@
#include "utils/memutils.h"
#include "utils/syscache.h"
-/*
- * When converting strings between different encodings, we assume that space
- * for converted result is 4-to-1 growth in the worst case. The rate for
- * currently supported encoding pairs are within 3 (SJIS JIS X0201 half width
- * kanna -> UTF8 is the worst case). So "4" should be enough for the moment.
- *
- * Note that this is not the same as the maximum character width in any
- * particular encoding.
- */
-#define MAX_CONVERSION_GROWTH 4
-
/*
* We maintain a simple linked list caching the fmgr lookup info for the
* currently selected conversion functions, as well as any that have been
diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h
index 32112547a0..030b376810 100644
--- a/src/include/libpq/pqformat.h
+++ b/src/include/libpq/pqformat.h
@@ -14,20 +14,167 @@
#define PQFORMAT_H
#include "lib/stringinfo.h"
+#include "mb/pg_wchar.h"
+#include "port/pg_bswap.h"
extern void pq_beginmessage(StringInfo buf, char msgtype);
-extern void pq_sendbyte(StringInfo buf, int byt);
+extern void pq_beginmessage_pre(StringInfo buf, char msgtype);
+extern void pq_endmessage(StringInfo buf);
+extern void pq_endmessage_keep(StringInfo buf);
+
extern void pq_sendbytes(StringInfo buf, const char *data, int datalen);
extern void pq_sendcountedtext(StringInfo buf, const char *str, int slen,
bool countincludesself);
extern void pq_sendtext(StringInfo buf, const char *str, int slen);
extern void pq_sendstring(StringInfo buf, const char *str);
extern void pq_send_ascii_string(StringInfo buf, const char *str);
-extern void pq_sendint(StringInfo buf, int i, int b);
-extern void pq_sendint64(StringInfo buf, int64 i);
extern void pq_sendfloat4(StringInfo buf, float4 f);
extern void pq_sendfloat8(StringInfo buf, float8 f);
-extern void pq_endmessage(StringInfo buf);
+
+extern void pq_sendfloat4(StringInfo buf, float4 f);
+extern void pq_sendfloat8(StringInfo buf, float8 f);
+
+/*
+ * Append a int8 to a StringInfo buffer, which already has enough space
+ * preallocated.
+ */
+static inline void
+pq_sendint8_pre(StringInfo restrict buf, int8 i)
+{
+ Assert(buf->len + sizeof(i) <= buf->maxlen);
+ *(int8* restrict) (buf->data + buf->len) = i;
+ buf->len += sizeof(i);
+}
+
+/*
+ * Append a int16 to a StringInfo buffer, which already has enough space
+ * preallocated.
+ */
+static inline void
+pq_sendint16_pre(StringInfo restrict buf, int16 i)
+{
+ Assert(buf->len + sizeof(i) <= buf->maxlen);
+ *(int16* restrict) (buf->data + buf->len) = pg_hton16(i);
+ buf->len += sizeof(i);
+}
+
+/*
+ * Append a int32 to a StringInfo buffer, which already has enough space
+ * preallocated.
+ */
+static inline void
+pq_sendint32_pre(StringInfo restrict buf, int32 i)
+{
+ Assert(buf->len + sizeof(i) <= buf->maxlen);
+ *(int32* restrict) (buf->data + buf->len) = pg_hton32(i);
+ buf->len += sizeof(i);
+}
+
+/*
+ * Append a int64 to a StringInfo buffer, which already has enough space
+ * preallocated.
+ */
+static inline void
+pq_sendint64_pre(StringInfo restrict buf, int64 i)
+{
+ Assert(buf->len + sizeof(i) <= buf->maxlen);
+ *(int64* restrict) (buf->data + buf->len) = pg_hton64(i);
+ buf->len += sizeof(i);
+}
+
+/*
+ * Append a null-terminated text string (with conversion) to a buffer with
+ * preallocated space.
+ *
+ * NB: The pre-allocated space needs to be sufficient for the string after
+ * converting to client encoding.
+ *
+ * NB: passed text string must be null-terminated, and so is the data
+ * sent to the frontend.
+ */
+static inline void
+pq_sendstring_pre(StringInfo restrict buf, const char* restrict str)
+{
+ int slen = strlen(str);
+ char *p;
+
+ p = pg_server_to_client(str, slen);
+ if (p != str) /* actual conversion has been done? */
+ slen = strlen(p);
+
+ Assert(buf->len + slen + 1 <= buf->maxlen);
+
+ memcpy(((char* restrict) buf->data + buf->len), p, slen + 1);
+ buf->len += slen + 1;
+
+ if (p != str)
+ pfree(p);
+}
+
+/* append a binary int8 to a StringInfo buffer */
+static inline void
+pq_sendint8(StringInfo buf, int8 i)
+{
+ enlargeStringInfo(buf, sizeof(i));
+ pq_sendint8_pre(buf, i);
+}
+
+/* append a binary int16 to a StringInfo buffer */
+static inline void
+pq_sendint16(StringInfo buf, int16 i)
+{
+ enlargeStringInfo(buf, sizeof(i));
+ pq_sendint16_pre(buf, i);
+}
+
+/* append a binary int32 to a StringInfo buffer */
+static inline void
+pq_sendint32(StringInfo buf, int32 i)
+{
+ enlargeStringInfo(buf, sizeof(i));
+ pq_sendint32_pre(buf, i);
+}
+
+/* append a binary int64 to a StringInfo buffer */
+static inline void
+pq_sendint64(StringInfo buf, int64 i)
+{
+ enlargeStringInfo(buf, sizeof(i));
+ pq_sendint64_pre(buf, i);
+}
+
+/* append a binary byte to a StringInfo buffer */
+static inline void
+pq_sendbyte(StringInfo buf, int8 byt)
+{
+ pq_sendint8(buf, byt);
+}
+
+/*
+ * Append a binary integer to a StringInfo buffer
+ *
+ * This function is deprecated.
+ */
+static inline void
+pq_sendint(StringInfo buf, int i, int b)
+{
+ switch (b)
+ {
+ case 1:
+ pq_sendint8(buf, (int8) i);
+ break;
+ case 2:
+ pq_sendint16(buf, (int16) i);
+ break;
+ case 4:
+ pq_sendint32(buf, (int32) i);
+ break;
+ default:
+ elog(ERROR, "unsupported integer size %d", b);
+ break;
+ }
+}
+
extern void pq_begintypsend(StringInfo buf);
extern bytea *pq_endtypsend(StringInfo buf);
diff --git a/src/include/mb/pg_wchar.h b/src/include/mb/pg_wchar.h
index d57ef017cb..9227d634f6 100644
--- a/src/include/mb/pg_wchar.h
+++ b/src/include/mb/pg_wchar.h
@@ -304,6 +304,17 @@ typedef enum pg_enc
/* On FE are possible all encodings */
#define PG_VALID_FE_ENCODING(_enc) PG_VALID_ENCODING(_enc)
+/*
+ * When converting strings between different encodings, we assume that space
+ * for converted result is 4-to-1 growth in the worst case. The rate for
+ * currently supported encoding pairs are within 3 (SJIS JIS X0201 half width
+ * kanna -> UTF8 is the worst case). So "4" should be enough for the moment.
+ *
+ * Note that this is not the same as the maximum character width in any
+ * particular encoding.
+ */
+#define MAX_CONVERSION_GROWTH 4
+
/*
* Table for mapping an encoding number to official encoding name and
* possibly other subsidiary data. Be careful to check encoding number
--
2.14.1.536.g6867272d5b.dirty
>From 799bf9c14b1c7e023e0436a2f6509cc1acca5df5 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 3 Oct 2017 00:36:42 -0700
Subject: [PATCH 4/6] Use one stringbuffer for all rows printed in printtup.c.
This avoids newly allocating, and then possibly growing, the
stringbuffer for every row. For wide rows this can substantially
reduce memory allocator overhead, at the price of not reducing memory
usage after outputting an especially wide row.
---
src/backend/access/common/printtup.c | 19 +++++++++++--------
1 file changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 20d20e623e..07bebd7033 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -57,6 +57,7 @@ typedef struct
typedef struct
{
DestReceiver pub; /* publicly-known function pointers */
+ StringInfoData buf; /* output buffer */
Portal portal; /* the Portal we are printing from */
bool sendDescrip; /* send RowDescription at startup? */
TupleDesc attrinfo; /* The attr info we are set up for */
@@ -251,6 +252,8 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs)
int16 *formats = myState->portal->formats;
int i;
+ initStringInfo(&myState->buf);
+
/* get rid of any old data */
if (myState->myinfo)
pfree(myState->myinfo);
@@ -302,7 +305,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
TupleDesc typeinfo = slot->tts_tupleDescriptor;
DR_printtup *myState = (DR_printtup *) self;
MemoryContext oldcontext;
- StringInfoData buf;
+ StringInfo buf = &myState->buf;
int natts = typeinfo->natts;
int i;
@@ -319,9 +322,9 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
/*
* Prepare a DataRow message (note buffer is in per-row context)
*/
- pq_beginmessage(&buf, 'D');
+ pq_beginmessage_pre(buf, 'D');
- pq_sendint(&buf, natts, 2);
+ pq_sendint(buf, natts, 2);
/*
* send the attributes of this tuple
@@ -333,7 +336,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
if (slot->tts_isnull[i])
{
- pq_sendint(&buf, -1, 4);
+ pq_sendint(buf, -1, 4);
continue;
}
@@ -354,7 +357,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
char *outputstr;
outputstr = OutputFunctionCall(&thisState->finfo, attr);
- pq_sendcountedtext(&buf, outputstr, strlen(outputstr), false);
+ pq_sendcountedtext(buf, outputstr, strlen(outputstr), false);
}
else
{
@@ -362,13 +365,13 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
bytea *outputbytes;
outputbytes = SendFunctionCall(&thisState->finfo, attr);
- pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
- pq_sendbytes(&buf, VARDATA(outputbytes),
+ pq_sendint(buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
+ pq_sendbytes(buf, VARDATA(outputbytes),
VARSIZE(outputbytes) - VARHDRSZ);
}
}
- pq_endmessage(&buf);
+ pq_endmessage_keep(buf);
/* Return to caller's context, and flush row's temporary memory */
MemoryContextSwitchTo(oldcontext);
--
2.14.1.536.g6867272d5b.dirty
>From 50382e56a47dd77900a864a12b7502e3a91fbd73 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 3 Oct 2017 00:39:16 -0700
Subject: [PATCH 5/6] Improve performance of SendRowDescriptionMessage.
Using the new pqformat functions yields performance for statements
with a noticeable number of rows. The function itself is more than
twice as fast.
---
src/backend/access/common/printtup.c | 159 ++++++++++++++++++++++++++---------
1 file changed, 121 insertions(+), 38 deletions(-)
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 07bebd7033..62a3fc3d6d 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -32,6 +32,8 @@ static bool printtup_internal_20(TupleTableSlot *slot, DestReceiver *self);
static void printtup_shutdown(DestReceiver *self);
static void printtup_destroy(DestReceiver *self);
+static void SendRowDescriptionCols_2(StringInfo buf, TupleDesc typeinfo, List *targetlist, int16 *formats);
+static void SendRowDescriptionCols_3(StringInfo buf, TupleDesc typeinfo, List *targetlist, int16 *formats);
/* ----------------------------------------------------------------
* printtup / debugtup support
@@ -190,12 +192,121 @@ SendRowDescriptionMessage(TupleDesc typeinfo, List *targetlist, int16 *formats)
{
int natts = typeinfo->natts;
int proto = PG_PROTOCOL_MAJOR(FrontendProtocol);
+ static StringInfo rowDescriptionBuf = NULL;
+
+ /*
+ * As this routine is executed for every single query, it can be a
+ * bottleneck. To avoid unnecessary allocator overhead reuse a single
+ * buffer. That means we'll never shrink below the largest row-description
+ * sent, but that seems acceptable given the limited size.
+ */
+ if (unlikely(!rowDescriptionBuf))
+ {
+ MemoryContext oldContext = MemoryContextSwitchTo(TopMemoryContext);
+
+ rowDescriptionBuf = makeStringInfo();
+ MemoryContextSwitchTo(oldContext);
+ }
+
+ /* tuple descriptor message type */
+ pq_beginmessage_pre(rowDescriptionBuf, 'T');
+ /* # of attrs in tuples */
+ pq_sendint16(rowDescriptionBuf, natts);
+
+ if (proto >= 3)
+ SendRowDescriptionCols_3(rowDescriptionBuf, typeinfo, targetlist,
+ formats);
+ else
+ SendRowDescriptionCols_2(rowDescriptionBuf, typeinfo, targetlist,
+ formats);
+
+ pq_endmessage_keep(rowDescriptionBuf);
+}
+
+/*
+ * Send description for each column when using v3+ protocol
+ */
+static void
+SendRowDescriptionCols_3(StringInfo buf, TupleDesc typeinfo, List *targetlist, int16 *formats)
+{
+ int natts = typeinfo->natts;
int i;
- StringInfoData buf;
ListCell *tlist_item = list_head(targetlist);
- pq_beginmessage(&buf, 'T'); /* tuple descriptor message type */
- pq_sendint(&buf, natts, 2); /* # of attrs in tuples */
+ /*
+ * Preallocate memory for the entire message to be sent. That allows to
+ * use the significantly faster inline pqformat.h functions and to avoid
+ * reallocations.
+ *
+ * Have to overestimate the size of the column-names, to account for
+ * character set overhead.
+ */
+ enlargeStringInfo(buf, (NAMEDATALEN * MAX_CONVERSION_GROWTH /* attname */
+ + sizeof(int32) /* attlen */
+ + sizeof(int32) /* resorigtbl */
+ + sizeof(int16) /* resorigcol */
+ + sizeof(Oid) /* atttypid */
+ + sizeof(int16) /* attlen */
+ + sizeof(int32) /* attypmod */
+ ) * natts);
+
+ for (i = 0; i < natts; ++i)
+ {
+ Form_pg_attribute att = TupleDescAttr(typeinfo, i);
+ Oid atttypid = att->atttypid;
+ int32 atttypmod = att->atttypmod;
+ int32 resorigtbl;
+ int32 resorigcol;
+ int16 format;
+
+ /*
+ * If column is a domain, send the base type and typmod
+ * instead. Lookup before sending any ints, for efficiency.
+ */
+ atttypid = getBaseTypeAndTypmod(atttypid, &atttypmod);
+
+ /* Do we have a non-resjunk tlist item? */
+ while (tlist_item &&
+ ((TargetEntry *) lfirst(tlist_item))->resjunk)
+ tlist_item = lnext(tlist_item);
+ if (tlist_item)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(tlist_item);
+
+ resorigtbl = tle->resorigtbl;
+ resorigcol = tle->resorigcol;
+ tlist_item = lnext(tlist_item);
+ }
+ else
+ {
+ /* No info available, so send zeroes */
+ resorigtbl = 0;
+ resorigcol = 0;
+ }
+
+ if (formats)
+ format = formats[i];
+ else
+ format = 0;
+
+ pq_sendstring_pre(buf, NameStr(att->attname));
+ pq_sendint32_pre(buf, resorigtbl);
+ pq_sendint16_pre(buf, resorigcol);
+ pq_sendint32_pre(buf, atttypid);
+ pq_sendint16_pre(buf, att->attlen);
+ pq_sendint32_pre(buf, atttypmod);
+ pq_sendint16_pre(buf, format);
+ }
+}
+
+/*
+ * Send description for each column when using v2 protocol
+ */
+static void
+SendRowDescriptionCols_2(StringInfo buf, TupleDesc typeinfo, List *targetlist, int16 *formats)
+{
+ int natts = typeinfo->natts;
+ int i;
for (i = 0; i < natts; ++i)
{
@@ -203,44 +314,16 @@ SendRowDescriptionMessage(TupleDesc typeinfo, List *targetlist, int16 *formats)
Oid atttypid = att->atttypid;
int32 atttypmod = att->atttypmod;
- pq_sendstring(&buf, NameStr(att->attname));
- /* column ID info appears in protocol 3.0 and up */
- if (proto >= 3)
- {
- /* Do we have a non-resjunk tlist item? */
- while (tlist_item &&
- ((TargetEntry *) lfirst(tlist_item))->resjunk)
- tlist_item = lnext(tlist_item);
- if (tlist_item)
- {
- TargetEntry *tle = (TargetEntry *) lfirst(tlist_item);
-
- pq_sendint(&buf, tle->resorigtbl, 4);
- pq_sendint(&buf, tle->resorigcol, 2);
- tlist_item = lnext(tlist_item);
- }
- else
- {
- /* No info available, so send zeroes */
- pq_sendint(&buf, 0, 4);
- pq_sendint(&buf, 0, 2);
- }
- }
/* If column is a domain, send the base type and typmod instead */
atttypid = getBaseTypeAndTypmod(atttypid, &atttypmod);
- pq_sendint(&buf, (int) atttypid, sizeof(atttypid));
- pq_sendint(&buf, att->attlen, sizeof(att->attlen));
- pq_sendint(&buf, atttypmod, sizeof(atttypmod));
- /* format info appears in protocol 3.0 and up */
- if (proto >= 3)
- {
- if (formats)
- pq_sendint(&buf, formats[i], 2);
- else
- pq_sendint(&buf, 0, 2);
- }
+
+ pq_sendstring(buf, NameStr(att->attname));
+ /* column ID only info appears in protocol 3.0 and up */
+ pq_sendint32(buf, atttypid);
+ pq_sendint16(buf, att->attlen);
+ pq_sendint32(buf, atttypmod);
+ /* format info only appears in protocol 3.0 and up */
}
- pq_endmessage(&buf);
}
/*
--
2.14.1.536.g6867272d5b.dirty
>From dececc7ac2eb7a1639938e39541560c711aa4de9 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Tue, 3 Oct 2017 00:44:57 -0700
Subject: [PATCH 6/6] Replace remaining printtup uses of pq_sendint with
pq_sendintXX.
It'd probably be a good idea to convert all remaining uses in the
tree, but these are fairly commonly used and in one file, so ...
---
src/backend/access/common/printtup.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c
index 62a3fc3d6d..5197682e70 100644
--- a/src/backend/access/common/printtup.c
+++ b/src/backend/access/common/printtup.c
@@ -407,7 +407,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
*/
pq_beginmessage_pre(buf, 'D');
- pq_sendint(buf, natts, 2);
+ pq_sendint16(buf, natts);
/*
* send the attributes of this tuple
@@ -419,7 +419,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
if (slot->tts_isnull[i])
{
- pq_sendint(buf, -1, 4);
+ pq_sendint32(buf, -1);
continue;
}
@@ -448,7 +448,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self)
bytea *outputbytes;
outputbytes = SendFunctionCall(&thisState->finfo, attr);
- pq_sendint(buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
+ pq_sendint32(buf, VARSIZE(outputbytes) - VARHDRSZ);
pq_sendbytes(buf, VARDATA(outputbytes),
VARSIZE(outputbytes) - VARHDRSZ);
}
@@ -506,13 +506,13 @@ printtup_20(TupleTableSlot *slot, DestReceiver *self)
k >>= 1;
if (k == 0) /* end of byte? */
{
- pq_sendint(&buf, j, 1);
+ pq_sendint8(&buf, j);
j = 0;
k = 1 << 7;
}
}
if (k != (1 << 7)) /* flush last partial byte */
- pq_sendint(&buf, j, 1);
+ pq_sendint8(&buf, j);
/*
* send the attributes of this tuple
@@ -691,13 +691,13 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
k >>= 1;
if (k == 0) /* end of byte? */
{
- pq_sendint(&buf, j, 1);
+ pq_sendint8(&buf, j);
j = 0;
k = 1 << 7;
}
}
if (k != (1 << 7)) /* flush last partial byte */
- pq_sendint(&buf, j, 1);
+ pq_sendint8(&buf, j);
/*
* send the attributes of this tuple
@@ -714,7 +714,7 @@ printtup_internal_20(TupleTableSlot *slot, DestReceiver *self)
Assert(thisState->format == 1);
outputbytes = SendFunctionCall(&thisState->finfo, attr);
- pq_sendint(&buf, VARSIZE(outputbytes) - VARHDRSZ, 4);
+ pq_sendint32(&buf, VARSIZE(outputbytes) - VARHDRSZ);
pq_sendbytes(&buf, VARDATA(outputbytes),
VARSIZE(outputbytes) - VARHDRSZ);
}
--
2.14.1.536.g6867272d5b.dirty
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
