Hi, I was reminded of the patchset I had posted in this thread by https://postgr.es/m/679d5455cbbb0af667ccb753da51a475bae1eaed.camel%40cybertec.at
On 2020-07-31 13:35:43 -0400, Robert Haas wrote: > On Fri, Jul 31, 2020 at 1:00 PM Andres Freund <and...@anarazel.de> wrote: > > > Perhaps we could add a comment about this, e.g. > > > Marking these pointers with pg_restrict tells the compiler that str > > > and str->data can't overlap, which may allow the compiler to optimize > > > better when this code is inlined. For example, it may be possible to > > > keep str->data in a register across consecutive appendStringInfoString > > > operations. > > > > > > Since pg_restrict is not widely used, I think it's worth adding this > > > kind of annotation, lest other hackers get confused. I'm probably not > > > the only one who isn't on top of this. > > > > Would it make more sense to have a bit of an explanation at > > pg_restrict's definition, instead of having it at (eventually) multiple > > places? > > I think, at least for the first few, it might be better to have a more > specific explanation at the point of use, as it may be easier to > understand in specific cases than in general. I imagine this only > really makes sense for places that are pretty hot. Whenever I looked at adding these comments, it felt wrong. We end up with repetitive boilerplate stuff as quite a few functions use it. I've thus not addressed this aspect in the attached rebased version. Perhaps a compromise would be to add such a comment to the top of stringinfo.h? > > Ah, I misunderstood. Yea, there's no reason not to do that. > > OK, then I vote for that version, as I think it looks nicer. Done. Greetings, Andres Freund
>From 3f43bf064afaa3a754fddfb9312256fb45820857 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 6 Feb 2024 17:32:00 -0800 Subject: [PATCH v3 01/10] stringinfo: Move more functions inline, provide initStringInfoWithSize() 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. initStringInfoWithSize() allows stringinfo users to determine the initial allocation size, avoiding over/under allocation when known. Author: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/20200603015559.qc7ry5jqmoxlp...@alap3.anarazel.de --- src/include/lib/stringinfo.h | 224 ++++++++++++++++++++++++++++------- src/common/stringinfo.c | 179 ++-------------------------- 2 files changed, 191 insertions(+), 212 deletions(-) diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 2cd636b01cf..409a2365cc3 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -18,6 +18,15 @@ #ifndef STRINGINFO_H #define STRINGINFO_H +#include "common/int.h" +#include "common/string.h" + +#ifdef FRONTEND +#include "common/fe_memutils.h" +#else +#include "utils/palloc.h" +#endif + /*------------------------- * StringInfoData holds information about an extensible string. * data is the current buffer for the string. @@ -99,18 +108,6 @@ 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); /*------------------------ * initReadOnlyStringInfo @@ -158,8 +155,141 @@ initStringInfoFromString(StringInfo str, char *data, int len) * resetStringInfo * Clears the current content of the StringInfo, if any. The * StringInfo remains valid. + * + * Read-only StringInfos as initialized by initReadOnlyStringInfo cannot be + * reset. */ -extern void resetStringInfo(StringInfo str); +static inline void +resetStringInfo(StringInfoData *pg_restrict str) +{ + /* don't allow resets of read-only StringInfos */ + Assert(str->maxlen != 0); + + *(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); +} + +/*------------------------ + * initStringInfoWithSize + * + * Like initStringInfo(), but allows to specify the size of the initial + * allocation. + */ +static inline void +initStringInfoWithSize(StringInfoData *pg_restrict str, int size) +{ + /* + * Note that maxlen is increased by 1 to account for the trailing \0 byte. + * Otherwise creating a stringinfo of size N and appending N bytes of data + * to it, would lead to a reallocation, to maintain the invariant that + * there always is space for the trailing \0 byte. + */ + 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 void *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 void *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 @@ -186,51 +316,55 @@ 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) +{ + char *pg_restrict ep; -/*------------------------ - * 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')) + /* Make more room if needed */ + enlargeStringInfo(str, 1); + + /* OK, append the character */ + ep = str->data + str->len; + ep[0] = ch; + ep[1] = '\0'; + str->len++; +} + +/* 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) + { + char *pg_restrict ep; -/*------------------------ - * appendBinaryStringInfo - * Append arbitrary binary data to a StringInfo, allocating more space - * if necessary. - */ -extern void appendBinaryStringInfo(StringInfo str, - const void *data, int datalen); + /* Make more room if needed */ + enlargeStringInfo(str, count); -/*------------------------ - * 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 void *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 */ + ep = str->data + str->len; + memset(ep, ' ', count); + str->len += count; + ep[count] = '\0'; + } +} #endif /* STRINGINFO_H */ diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index c61d5c58f30..92d168e71e5 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -32,59 +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. - * - * Read-only StringInfos as initialized by initReadOnlyStringInfo cannot be - * reset. - */ -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; -} - /* * appendStringInfo * @@ -173,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 */ - memset(&str->data[str->len], ' ', count); - str->len += count; - 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 void *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 void *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; @@ -326,8 +171,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.38.0
>From 8e3889d0667b5ce7e8e60066e7e29952d9ba5301 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 6 Feb 2024 17:34:42 -0800 Subject: [PATCH v3 02/10] stringinfo: Remove in-core use of appendStringInfoCharMacro() Kept seperate only to reduce size of patch moving to more inlining for stringinfo. Author: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/20200603015559.qc7ry5jqmoxlp...@alap3.anarazel.de --- src/backend/commands/copyto.c | 2 +- src/backend/commands/explain.c | 8 +++---- src/backend/libpq/pqformat.c | 10 ++++---- 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/csvlog.c | 8 +++---- src/backend/utils/error/elog.c | 4 ++-- src/backend/utils/mb/stringinfo_mb.c | 2 +- src/bin/pg_combinebackup/write_manifest.c | 6 ++--- src/bin/pg_rewind/libpq_source.c | 8 +++---- contrib/tcn/tcn.c | 16 ++++++------- 16 files changed, 64 insertions(+), 64 deletions(-) diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 20ffc90363d..b7f8b04ed8d 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -187,7 +187,7 @@ CopySendString(CopyToState cstate, const char *str) static void CopySendChar(CopyToState 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 83d00a46638..ddae81e5d7a 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -5098,16 +5098,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 a697ccfbbf9..4708a7988a6 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -235,7 +235,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'); } @@ -330,10 +330,10 @@ 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'); + appendStringInfoChar(buf, '\0'); + appendStringInfoChar(buf, '\0'); + appendStringInfoChar(buf, '\0'); + appendStringInfoChar(buf, '\0'); } /* -------------------------------- diff --git a/src/backend/utils/adt/json.c b/src/backend/utils/adt/json.c index d719a61f16b..a8826fd45c6 100644 --- a/src/backend/utils/adt/json.c +++ b/src/backend/utils/adt/json.c @@ -1550,7 +1550,7 @@ escape_json(StringInfo buf, const char *str) { const char *p; - appendStringInfoCharMacro(buf, '"'); + appendStringInfoChar(buf, '"'); for (p = str; *p; p++) { switch (*p) @@ -1580,11 +1580,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, '"'); } /* Semantic actions for key uniqueness check */ diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c index c10b3fbedf1..4dc820b92b2 100644 --- a/src/backend/utils/adt/jsonb.c +++ b/src/backend/utils/adt/jsonb.c @@ -533,7 +533,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; @@ -546,7 +546,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++; @@ -594,14 +594,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: @@ -621,7 +621,7 @@ add_indent(StringInfo out, bool indent, int level) { if (indent) { - appendStringInfoCharMacro(out, '\n'); + appendStringInfoChar(out, '\n'); appendStringInfoSpaces(out, level * 4); } } diff --git a/src/backend/utils/adt/jsonfuncs.c b/src/backend/utils/adt/jsonfuncs.c index 1b0f4943292..4a81bc15cf2 100644 --- a/src/backend/utils/adt/jsonfuncs.c +++ b/src/backend/utils/adt/jsonfuncs.c @@ -4311,7 +4311,7 @@ sn_object_start(void *state) { StripnullState *_state = (StripnullState *) state; - appendStringInfoCharMacro(_state->strval, '{'); + appendStringInfoChar(_state->strval, '{'); return JSON_SUCCESS; } @@ -4321,7 +4321,7 @@ sn_object_end(void *state) { StripnullState *_state = (StripnullState *) state; - appendStringInfoCharMacro(_state->strval, '}'); + appendStringInfoChar(_state->strval, '}'); return JSON_SUCCESS; } @@ -4331,7 +4331,7 @@ sn_array_start(void *state) { StripnullState *_state = (StripnullState *) state; - appendStringInfoCharMacro(_state->strval, '['); + appendStringInfoChar(_state->strval, '['); return JSON_SUCCESS; } @@ -4341,7 +4341,7 @@ sn_array_end(void *state) { StripnullState *_state = (StripnullState *) state; - appendStringInfoCharMacro(_state->strval, ']'); + appendStringInfoChar(_state->strval, ']'); return JSON_SUCCESS; } @@ -4363,7 +4363,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 @@ -4371,7 +4371,7 @@ sn_object_field_start(void *state, char *fname, bool isnull) */ escape_json(_state->strval, fname); - appendStringInfoCharMacro(_state->strval, ':'); + appendStringInfoChar(_state->strval, ':'); return JSON_SUCCESS; } @@ -4382,7 +4382,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, ','); return JSON_SUCCESS; } @@ -5785,7 +5785,7 @@ transform_string_values_object_start(void *state) { TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state; - appendStringInfoCharMacro(_state->strval, '{'); + appendStringInfoChar(_state->strval, '{'); return JSON_SUCCESS; } @@ -5795,7 +5795,7 @@ transform_string_values_object_end(void *state) { TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state; - appendStringInfoCharMacro(_state->strval, '}'); + appendStringInfoChar(_state->strval, '}'); return JSON_SUCCESS; } @@ -5805,7 +5805,7 @@ transform_string_values_array_start(void *state) { TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state; - appendStringInfoCharMacro(_state->strval, '['); + appendStringInfoChar(_state->strval, '['); return JSON_SUCCESS; } @@ -5815,7 +5815,7 @@ transform_string_values_array_end(void *state) { TransformJsonStringValuesState *_state = (TransformJsonStringValuesState *) state; - appendStringInfoCharMacro(_state->strval, ']'); + appendStringInfoChar(_state->strval, ']'); return JSON_SUCCESS; } @@ -5826,14 +5826,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, ':'); return JSON_SUCCESS; } @@ -5844,7 +5844,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, ','); return JSON_SUCCESS; } diff --git a/src/backend/utils/adt/jsonpath.c b/src/backend/utils/adt/jsonpath.c index 258ed8eb117..a760b3ba8de 100644 --- a/src/backend/utils/adt/jsonpath.c +++ b/src/backend/utils/adt/jsonpath.c @@ -484,13 +484,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 adc02702fca..d489dab9f54 100644 --- a/src/backend/utils/adt/rowtypes.c +++ b/src/backend/utils/adt/rowtypes.c @@ -452,17 +452,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 543afb66e58..01d5d0dbb63 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -5689,7 +5689,7 @@ text_format(PG_FUNCTION_ARGS) */ if (*cp != '%') { - appendStringInfoCharMacro(&str, *cp); + appendStringInfoChar(&str, *cp); continue; } @@ -5698,7 +5698,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 3e24aba546f..24967a1caf2 100644 --- a/src/backend/utils/adt/xml.c +++ b/src/backend/utils/adt/xml.c @@ -2655,7 +2655,7 @@ escape_xml(const char *str) appendStringInfoString(&buf, "
"); break; default: - appendStringInfoCharMacro(&buf, *p); + appendStringInfoChar(&buf, *p); break; } } diff --git a/src/backend/utils/error/csvlog.c b/src/backend/utils/error/csvlog.c index 1b62b07f231..9d6f1f661a7 100644 --- a/src/backend/utils/error/csvlog.c +++ b/src/backend/utils/error/csvlog.c @@ -45,14 +45,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, '"'); } /* diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c index bba00a0087f..4dce794a989 100644 --- a/src/backend/utils/error/elog.c +++ b/src/backend/utils/error/elog.c @@ -3689,9 +3689,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 f4af3909e88..b90aa9830a1 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/src/bin/pg_combinebackup/write_manifest.c b/src/bin/pg_combinebackup/write_manifest.c index 01deb82cc9f..fae1043d2d0 100644 --- a/src/bin/pg_combinebackup/write_manifest.c +++ b/src/bin/pg_combinebackup/write_manifest.c @@ -194,7 +194,7 @@ escape_json(StringInfo buf, const char *str) { const char *p; - appendStringInfoCharMacro(buf, '"'); + appendStringInfoChar(buf, '"'); for (p = str; *p; p++) { switch (*p) @@ -224,11 +224,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/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c index 7d898c3b501..476cc4249cd 100644 --- a/src/bin/pg_rewind/libpq_source.c +++ b/src/bin/pg_rewind/libpq_source.c @@ -613,19 +613,19 @@ process_queued_fetch_requests(libpq_source *src) static void appendArrayEscapedString(StringInfo buf, const char *str) { - appendStringInfoCharMacro(buf, '\"'); + appendStringInfoChar(buf, '\"'); while (*str) { char ch = *str; if (ch == '"' || ch == '\\') - appendStringInfoCharMacro(buf, '\\'); + appendStringInfoChar(buf, '\\'); - appendStringInfoCharMacro(buf, ch); + appendStringInfoChar(buf, ch); str++; } - appendStringInfoCharMacro(buf, '\"'); + appendStringInfoChar(buf, '\"'); } /* diff --git a/contrib/tcn/tcn.c b/contrib/tcn/tcn.c index 8d23c824c1b..754deae54e7 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.38.0
>From c877abe961024671beafe55dabdd716df6795578 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 14 Jan 2020 12:53:33 -0800 Subject: [PATCH v3 03/10] 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 | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/include/utils/palloc.h b/src/include/utils/palloc.h index 773a5d2c347..642b24b2bd2 100644 --- a/src/include/utils/palloc.h +++ b/src/include/utils/palloc.h @@ -75,7 +75,11 @@ extern void *MemoryContextAllocExtended(MemoryContext context, extern void *MemoryContextAllocAligned(MemoryContext context, Size size, Size alignto, int flags); -extern void *palloc(Size size); +extern void *palloc(Size size) +#if __has_attribute(malloc) && __has_attribute(assume_aligned) && __has_attribute(returns_nonnull) && __has_attribute(warn_unused_result) + __attribute__((malloc, assume_aligned(MAXIMUM_ALIGNOF), returns_nonnull, warn_unused_result)) +#endif + ; extern void *palloc0(Size size); extern void *palloc_extended(Size size, int flags); extern void *palloc_aligned(Size size, Size alignto, int flags); -- 2.38.0
>From 781f1855202683035ef0c6bddc8a69d4e323b1bc Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 6 Feb 2024 17:40:01 -0800 Subject: [PATCH v3 04/10] pqformat: Move type sending functions inline, add pq_begintypsend_with_size() 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_with_size() 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: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/20200603015559.qc7ry5jqmoxlp...@alap3.anarazel.de <> Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/include/libpq/pqformat.h | 53 ++++++++++++++++++++++++++++++++++++ src/backend/libpq/pqformat.c | 37 ------------------------- 2 files changed, 53 insertions(+), 37 deletions(-) diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index a710bc18687..62922f58747 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -16,6 +16,7 @@ #include "lib/stringinfo.h" #include "mb/pg_wchar.h" #include "port/pg_bswap.h" +#include "varatt.h" extern void pq_beginmessage(StringInfo buf, char msgtype); extern void pq_beginmessage_reuse(StringInfo buf, char msgtype); @@ -31,6 +32,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_with_size - 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_with_size(StringInfo buf, int size) +{ + initStringInfoWithSize(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 4708a7988a6..9275b241333 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -321,43 +321,6 @@ pq_endmessage_reuse(StringInfo buf) } -/* -------------------------------- - * pq_begintypsend - initialize for constructing a bytea result - * -------------------------------- - */ -void -pq_begintypsend(StringInfo buf) -{ - initStringInfo(buf); - /* Reserve four bytes for the bytea length word */ - appendStringInfoChar(buf, '\0'); - appendStringInfoChar(buf, '\0'); - appendStringInfoChar(buf, '\0'); - appendStringInfoChar(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.38.0
>From 953176ae5738758105860868e134858644dfac6c Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 14 Jan 2020 13:00:26 -0800 Subject: [PATCH v3 05/10] copy: Use appendBinaryStringInfoNT() for sending binary data Maintaining the trailing byte is useless overhead. Author: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/20200603015559.qc7ry5jqmoxlp...@alap3.anarazel.de --- src/backend/commands/copyto.c | 4 ++-- src/test/modules/test_copy_callbacks/test_copy_callbacks.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index b7f8b04ed8d..5857532f54e 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -175,13 +175,13 @@ SendCopyEnd(CopyToState cstate) static void CopySendData(CopyToState cstate, const void *databuf, int datasize) { - appendBinaryStringInfo(cstate->fe_msgbuf, databuf, datasize); + appendBinaryStringInfoNT(cstate->fe_msgbuf, databuf, datasize); } static void CopySendString(CopyToState cstate, const char *str) { - appendBinaryStringInfo(cstate->fe_msgbuf, str, strlen(str)); + appendBinaryStringInfoNT(cstate->fe_msgbuf, str, strlen(str)); } static void diff --git a/src/test/modules/test_copy_callbacks/test_copy_callbacks.c b/src/test/modules/test_copy_callbacks/test_copy_callbacks.c index 0bbd2aa6bb9..1ba205675af 100644 --- a/src/test/modules/test_copy_callbacks/test_copy_callbacks.c +++ b/src/test/modules/test_copy_callbacks/test_copy_callbacks.c @@ -25,8 +25,8 @@ static void to_cb(void *data, int len) { ereport(NOTICE, - (errmsg("COPY TO callback called with data \"%s\" and length %d", - (char *) data, len))); + (errmsg("COPY TO callback called with data \"%.*s\" and length %d", + len, (char *) data, len))); } PG_FUNCTION_INFO_V1(test_copy_to_callback); -- 2.38.0
>From 1f7ac47f5b6281bd010b5f6451be3f48f8a57151 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 14 Jan 2020 12:58:55 -0800 Subject: [PATCH v3 06/10] Use pq_begintypsend_with_size() in a number of send functions If we were to introduce pq_begintypsend_with_size(), we'd probably want to do this to quite a few more functions. Author: Andres Freund <and...@anarazel.de> Discussion: https://postgr.es/m/20200603015559.qc7ry5jqmoxlp...@alap3.anarazel.de <> Author: Reviewed-by: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/utils/adt/bool.c | 2 +- src/backend/utils/adt/float.c | 4 ++-- src/backend/utils/adt/int.c | 4 ++-- src/backend/utils/adt/int8.c | 2 +- src/backend/utils/adt/uuid.c | 2 +- src/backend/utils/adt/varlena.c | 2 +- 6 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/backend/utils/adt/bool.c b/src/backend/utils/adt/bool.c index 85e6786563e..fe188f4f03c 100644 --- a/src/backend/utils/adt/bool.c +++ b/src/backend/utils/adt/bool.c @@ -189,7 +189,7 @@ boolsend(PG_FUNCTION_ARGS) bool arg1 = PG_GETARG_BOOL(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, 1); pq_sendbyte(&buf, arg1 ? 1 : 0); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/backend/utils/adt/float.c b/src/backend/utils/adt/float.c index 901edcc8961..4f29c435e2e 100644 --- a/src/backend/utils/adt/float.c +++ b/src/backend/utils/adt/float.c @@ -351,7 +351,7 @@ float4send(PG_FUNCTION_ARGS) float4 num = PG_GETARG_FLOAT4(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, 4); pq_sendfloat4(&buf, num); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } @@ -568,7 +568,7 @@ float8send(PG_FUNCTION_ARGS) float8 num = PG_GETARG_FLOAT8(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, 8); pq_sendfloat8(&buf, num); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 234f20796b7..761dc4acce9 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -100,7 +100,7 @@ int2send(PG_FUNCTION_ARGS) int16 arg1 = PG_GETARG_INT16(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, 2); pq_sendint16(&buf, arg1); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } @@ -324,7 +324,7 @@ int4send(PG_FUNCTION_ARGS) int32 arg1 = PG_GETARG_INT32(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&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 ede14086aee..5423cb2ecde 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -97,7 +97,7 @@ int8send(PG_FUNCTION_ARGS) int64 arg1 = PG_GETARG_INT64(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, 8); pq_sendint64(&buf, arg1); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c index 73dfd711c73..5e04640bce3 100644 --- a/src/backend/utils/adt/uuid.c +++ b/src/backend/utils/adt/uuid.c @@ -153,7 +153,7 @@ uuid_send(PG_FUNCTION_ARGS) pg_uuid_t *uuid = PG_GETARG_UUID_P(0); StringInfoData buffer; - pq_begintypsend(&buffer); + pq_begintypsend_with_size(&buffer, UUID_LEN); pq_sendbytes(&buffer, uuid->data, UUID_LEN); PG_RETURN_BYTEA_P(pq_endtypsend(&buffer)); } diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index 01d5d0dbb63..ea696c1dc71 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -621,7 +621,7 @@ textsend(PG_FUNCTION_ARGS) text *t = PG_GETARG_TEXT_PP(0); StringInfoData buf; - pq_begintypsend(&buf); + pq_begintypsend_with_size(&buf, VARSIZE_ANY_EXHDR(t)); pq_sendtext(&buf, VARDATA_ANY(t), VARSIZE_ANY_EXHDR(t)); PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); } -- 2.38.0
>From bf6ae654768063f95e381145e9cf44969138c5dd Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 2 Jun 2020 16:47:26 -0700 Subject: [PATCH v3 07/10] wip: make send calls in printtup.c cheaper Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/backend/access/common/printtup.c | 40 +++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c index 8a4953ea033..b4c031f67e7 100644 --- a/src/backend/access/common/printtup.c +++ b/src/backend/access/common/printtup.c @@ -49,6 +49,15 @@ typedef struct bool typisvarlena; /* is it varlena (ie possibly toastable)? */ int16 format; /* format code for this column */ FmgrInfo finfo; /* Precomputed call info for output fn */ + + /* use union with FunctionCallInfoBaseData to guarantee alignment */ + union + { + FunctionCallInfoBaseData fcinfo; + /* ensure enough space for nargs args is available */ + char fcinfo_data[SizeForFunctionCallInfo(1)]; + } fcinfo_data; + } PrinttupAttrInfo; typedef struct @@ -278,6 +287,9 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs) &thisState->typoutput, &thisState->typisvarlena); fmgr_info(thisState->typoutput, &thisState->finfo); + InitFunctionCallInfoData(thisState->fcinfo_data.fcinfo, + &thisState->finfo, 1, InvalidOid, + NULL, NULL); } else if (format == 1) { @@ -285,6 +297,9 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs) &thisState->typsend, &thisState->typisvarlena); fmgr_info(thisState->typsend, &thisState->finfo); + InitFunctionCallInfoData(thisState->fcinfo_data.fcinfo, + &thisState->finfo, 1, InvalidOid, + NULL, NULL); } else ereport(ERROR, @@ -361,11 +376,28 @@ printtup(TupleTableSlot *slot, DestReceiver *self) { /* Binary output */ bytea *outputbytes; + int outputlen; + Datum result; + FunctionCallInfo fcinfo = &thisState->fcinfo_data.fcinfo; - outputbytes = SendFunctionCall(&thisState->finfo, attr); - pq_sendint32(buf, VARSIZE(outputbytes) - VARHDRSZ); - pq_sendbytes(buf, VARDATA(outputbytes), - VARSIZE(outputbytes) - VARHDRSZ); + fcinfo->args[0].value = attr; + fcinfo->args[0].isnull = false; + result = FunctionCallInvoke(fcinfo); + + /* + * Check for null result, since caller is clearly not expecting + * one + */ + if (unlikely(fcinfo->isnull)) + elog(ERROR, "send function return null"); + + outputbytes = DatumGetByteaP(result); + outputlen = VARSIZE(outputbytes) - VARHDRSZ; + + Assert(outputlen > 0); + + pq_sendint32(buf, outputlen); + pq_sendbytes(buf, VARDATA(outputbytes), outputlen); } } -- 2.38.0
>From e0b35e463349a8dc8bdce3139ba7b20d4d4b3ca1 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Tue, 2 Jun 2020 16:47:26 -0700 Subject: [PATCH v3 08/10] wip: make in/out send/recv calls in copy.c cheaper Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/include/commands/copyfrom_internal.h | 22 +++++- src/backend/access/common/printtup.c | 2 +- src/backend/commands/copyfrom.c | 35 +++++---- src/backend/commands/copyfromparse.c | 96 ++++++++++++++++-------- src/backend/commands/copyto.c | 83 ++++++++++++++++---- src/tools/pgindent/typedefs.list | 70 ++++++++--------- 6 files changed, 212 insertions(+), 96 deletions(-) diff --git a/src/include/commands/copyfrom_internal.h b/src/include/commands/copyfrom_internal.h index cad52fcc783..1bc51d8a0fc 100644 --- a/src/include/commands/copyfrom_internal.h +++ b/src/include/commands/copyfrom_internal.h @@ -52,6 +52,23 @@ typedef enum CopyInsertMethod * ExecForeignBatchInsert only if valid */ } CopyInsertMethod; +typedef struct CopyInAttributeInfo +{ + AttrNumber num; + const char *name; + + Oid typioparam; + int32 typmod; + + FmgrInfo in_finfo; + union + { + FunctionCallInfoBaseData fcinfo; + char fcinfo_data[SizeForFunctionCallInfo(3)]; + } in_fcinfo; + +} CopyInAttributeInfo; + /* * This struct contains all the state variables used throughout a COPY FROM * operation. @@ -93,9 +110,8 @@ typedef struct CopyFromStateData AttrNumber num_defaults; /* count of att that are missing and have * default value */ - FmgrInfo *in_functions; /* array of input functions for each attrs */ - Oid *typioparams; /* array of element types for in_functions */ - ErrorSaveContext *escontext; /* soft error trapper during in_functions + CopyInAttributeInfo *in_attributes; + ErrorSaveContext *escontext; /* soft error trapper during in_attributes * execution */ uint64 num_errors; /* total number of rows which contained soft * errors */ diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c index b4c031f67e7..bee3fc26220 100644 --- a/src/backend/access/common/printtup.c +++ b/src/backend/access/common/printtup.c @@ -394,7 +394,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self) outputbytes = DatumGetByteaP(result); outputlen = VARSIZE(outputbytes) - VARHDRSZ; - Assert(outputlen > 0); + Assert(outputlen >= 0); pq_sendint32(buf, outputlen); pq_sendbytes(buf, VARDATA(outputbytes), outputlen); diff --git a/src/backend/commands/copyfrom.c b/src/backend/commands/copyfrom.c index 1fe70b91338..96e56d3c128 100644 --- a/src/backend/commands/copyfrom.c +++ b/src/backend/commands/copyfrom.c @@ -1384,8 +1384,7 @@ BeginCopyFrom(ParseState *pstate, TupleDesc tupDesc; AttrNumber num_phys_attrs, num_defaults; - FmgrInfo *in_functions; - Oid *typioparams; + CopyInAttributeInfo *in_attributes; Oid in_func_oid; int *defmap; ExprState **defexprs; @@ -1608,27 +1607,36 @@ BeginCopyFrom(ParseState *pstate, * the input function), and info about defaults and constraints. (Which * input function we use depends on text/binary format choice.) */ - in_functions = (FmgrInfo *) palloc(num_phys_attrs * sizeof(FmgrInfo)); - typioparams = (Oid *) palloc(num_phys_attrs * sizeof(Oid)); + in_attributes = (CopyInAttributeInfo *) + palloc0(num_phys_attrs * sizeof(CopyInAttributeInfo)); defmap = (int *) palloc(num_phys_attrs * sizeof(int)); defexprs = (ExprState **) palloc(num_phys_attrs * sizeof(ExprState *)); for (int attnum = 1; attnum <= num_phys_attrs; attnum++) { - Form_pg_attribute att = TupleDescAttr(tupDesc, attnum - 1); + CopyInAttributeInfo *attr = &in_attributes[attnum - 1]; + Form_pg_attribute pgatt = TupleDescAttr(tupDesc, attnum - 1); /* We don't need info for dropped attributes */ - if (att->attisdropped) + if (pgatt->attisdropped) continue; + attr->num = attnum; + attr->typmod = pgatt->atttypmod; + attr->name = NameStr(pgatt->attname); + /* Fetch the input function and typioparam info */ if (cstate->opts.binary) - getTypeBinaryInputInfo(att->atttypid, - &in_func_oid, &typioparams[attnum - 1]); + getTypeBinaryInputInfo(pgatt->atttypid, + &in_func_oid, &attr->typioparam); else - getTypeInputInfo(att->atttypid, - &in_func_oid, &typioparams[attnum - 1]); - fmgr_info(in_func_oid, &in_functions[attnum - 1]); + getTypeInputInfo(pgatt->atttypid, + &in_func_oid, &attr->typioparam); + fmgr_info(in_func_oid, &attr->in_finfo); + InitFunctionCallInfoData(attr->in_fcinfo.fcinfo, &attr->in_finfo, 3, + InvalidOid, (fmNodePtr) cstate->escontext, + NULL); + /* Get default info if available */ defexprs[attnum - 1] = NULL; @@ -1640,7 +1648,7 @@ BeginCopyFrom(ParseState *pstate, */ if ((cstate->opts.default_print != NULL || !list_member_int(cstate->attnumlist, attnum)) && - !att->attgenerated) + !pgatt->attgenerated) { Expr *defexpr = (Expr *) build_column_default(cstate->rel, attnum); @@ -1689,8 +1697,7 @@ BeginCopyFrom(ParseState *pstate, cstate->bytes_processed = 0; /* We keep those variables in cstate. */ - cstate->in_functions = in_functions; - cstate->typioparams = typioparams; + cstate->in_attributes = in_attributes; cstate->defmap = defmap; cstate->defexprs = defexprs; cstate->volatile_defexprs = volatile_defexprs; diff --git a/src/backend/commands/copyfromparse.c b/src/backend/commands/copyfromparse.c index 7cacd0b752c..7cf9d10e7ae 100644 --- a/src/backend/commands/copyfromparse.c +++ b/src/backend/commands/copyfromparse.c @@ -154,8 +154,8 @@ static bool CopyReadLine(CopyFromState cstate); static bool CopyReadLineText(CopyFromState cstate); static int CopyReadAttributesText(CopyFromState cstate); static int CopyReadAttributesCSV(CopyFromState cstate); -static Datum CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, - Oid typioparam, int32 typmod, +static Datum CopyReadBinaryAttribute(CopyFromState cstate, + CopyInAttributeInfo *att, bool *isnull); @@ -859,8 +859,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, AttrNumber num_phys_attrs, attr_count, num_defaults = cstate->num_defaults; - FmgrInfo *in_functions = cstate->in_functions; - Oid *typioparams = cstate->typioparams; + CopyInAttributeInfo *in_attributes = cstate->in_attributes; int i; int *defmap = cstate->defmap; ExprState **defexprs = cstate->defexprs; @@ -899,13 +898,14 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, { int attnum = lfirst_int(cur); int m = attnum - 1; - Form_pg_attribute att = TupleDescAttr(tupDesc, m); + CopyInAttributeInfo *attr = &in_attributes[m]; + FunctionCallInfo fcinfo = &attr->in_fcinfo.fcinfo; if (fieldno >= fldct) ereport(ERROR, (errcode(ERRCODE_BAD_COPY_FILE_FORMAT), errmsg("missing data for column \"%s\"", - NameStr(att->attname)))); + attr->name))); string = field_strings[fieldno++]; if (cstate->convert_select_flags && @@ -939,7 +939,7 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, } } - cstate->cur_attname = NameStr(att->attname); + cstate->cur_attname = attr->name; cstate->cur_attval = string; if (string != NULL) @@ -956,20 +956,47 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, values[m] = ExecEvalExpr(defexprs[m], econtext, &nulls[m]); } - - /* - * If ON_ERROR is specified with IGNORE, skip rows with soft - * errors - */ - else if (!InputFunctionCallSafe(&in_functions[m], - string, - typioparams[m], - att->atttypmod, - (Node *) cstate->escontext, - &values[m])) + else if (string == NULL && fcinfo->flinfo->fn_strict) { - cstate->num_errors++; - return true; + /* FIXME: shouldn't be reachable */ + nulls[m] = true; + } + else + { + fcinfo->args[0].value = CStringGetDatum(string); + fcinfo->args[0].isnull = false; + fcinfo->args[1].value = ObjectIdGetDatum(attr->typioparam); + fcinfo->args[1].isnull = false; + fcinfo->args[2].value = Int32GetDatum(attr->typmod); + fcinfo->args[2].isnull = false; + + values[m] = FunctionCallInvoke(fcinfo); + + if (SOFT_ERROR_OCCURRED(cstate->escontext)) + { + /* + * If ON_ERROR is specified with IGNORE, skip rows with + * soft errors + */ + cstate->num_errors++; + return true; + } + + /* Should get null result if and only if str is NULL */ + if (string == NULL) + { + if (unlikely(!fcinfo->isnull)) + elog(ERROR, "input function %u returned non-NULL", + fcinfo->flinfo->fn_oid); + } + else + { + if (unlikely(fcinfo->isnull)) + elog(ERROR, "input function %u returned NULL", + fcinfo->flinfo->fn_oid); + } + + nulls[m] = string == NULL; } cstate->cur_attname = NULL; @@ -1021,13 +1048,11 @@ NextCopyFrom(CopyFromState cstate, ExprContext *econtext, { int attnum = lfirst_int(cur); int m = attnum - 1; - Form_pg_attribute att = TupleDescAttr(tupDesc, m); + CopyInAttributeInfo *attr = &in_attributes[m]; - cstate->cur_attname = NameStr(att->attname); + cstate->cur_attname = attr->name; values[m] = CopyReadBinaryAttribute(cstate, - &in_functions[m], - typioparams[m], - att->atttypmod, + attr, &nulls[m]); cstate->cur_attname = NULL; } @@ -1952,12 +1977,13 @@ endfield: * Read a binary attribute */ static Datum -CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, - Oid typioparam, int32 typmod, +CopyReadBinaryAttribute(CopyFromState cstate, + CopyInAttributeInfo *attr, bool *isnull) { int32 fld_size; Datum result; + FunctionCallInfo fcinfo = &attr->in_fcinfo.fcinfo; if (!CopyGetInt32(cstate, &fld_size)) ereport(ERROR, @@ -1966,7 +1992,7 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, if (fld_size == -1) { *isnull = true; - return ReceiveFunctionCall(flinfo, NULL, typioparam, typmod); + return ReceiveFunctionCall(fcinfo->flinfo, NULL, attr->typioparam, attr->typmod); } if (fld_size < 0) ereport(ERROR, @@ -1987,8 +2013,18 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo, cstate->attribute_buf.data[fld_size] = '\0'; /* Call the column type's binary input converter */ - result = ReceiveFunctionCall(flinfo, &cstate->attribute_buf, - typioparam, typmod); + fcinfo->args[0].value = PointerGetDatum(&cstate->attribute_buf); + fcinfo->args[0].isnull = false; + fcinfo->args[1].value = ObjectIdGetDatum(attr->typioparam); + fcinfo->args[1].isnull = false; + fcinfo->args[2].value = Int32GetDatum(attr->typmod); + fcinfo->args[2].isnull = false; + + result = FunctionCallInvoke(fcinfo); + + if (unlikely(fcinfo->isnull)) + elog(ERROR, "receive function %u returned NULL", + fcinfo->flinfo->fn_oid); /* Trouble if it didn't eat the whole buffer */ if (cstate->attribute_buf.cursor != cstate->attribute_buf.len) diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index 5857532f54e..b5cada5cb75 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -54,6 +54,20 @@ typedef enum CopyDest COPY_CALLBACK, /* to callback function */ } CopyDest; +typedef struct CopyOutAttributeInfo +{ + AttrNumber num; + const char *name; + int32 typid; + + FmgrInfo out_finfo; + union + { + FunctionCallInfoBaseData fcinfo; + char fcinfo_data[SizeForFunctionCallInfo(1)]; + } out_fcinfo; +} CopyOutAttributeInfo; + /* * This struct contains all the state variables used throughout a COPY TO * operation. @@ -96,7 +110,8 @@ typedef struct CopyToStateData */ MemoryContext copycontext; /* per-copy execution context */ - FmgrInfo *out_functions; /* lookup info for output functions */ + CopyOutAttributeInfo *out_attributes; + MemoryContext rowcontext; /* per-row evaluation context */ uint64 bytes_processed; /* number of bytes processed so far */ } CopyToStateData; @@ -768,23 +783,34 @@ DoCopyTo(CopyToState cstate) cstate->fe_msgbuf = makeStringInfo(); /* Get info about the columns we need to process. */ - cstate->out_functions = (FmgrInfo *) palloc(num_phys_attrs * sizeof(FmgrInfo)); + cstate->out_attributes = + (CopyOutAttributeInfo *) palloc(num_phys_attrs * sizeof(CopyOutAttributeInfo)); foreach(cur, cstate->attnumlist) { int attnum = lfirst_int(cur); + CopyOutAttributeInfo *attr = &cstate->out_attributes[attnum - 1]; + Form_pg_attribute pgatt; Oid out_func_oid; bool isvarlena; - Form_pg_attribute attr = TupleDescAttr(tupDesc, attnum - 1); + + pgatt = TupleDescAttr(tupDesc, attnum - 1); + attr->num = attnum; + attr->name = NameStr(pgatt->attname); + attr->typid = pgatt->atttypid; if (cstate->opts.binary) - getTypeBinaryOutputInfo(attr->atttypid, + getTypeBinaryOutputInfo(attr->typid, &out_func_oid, &isvarlena); else - getTypeOutputInfo(attr->atttypid, + getTypeOutputInfo(attr->typid, &out_func_oid, &isvarlena); - fmgr_info(out_func_oid, &cstate->out_functions[attnum - 1]); + + fmgr_info(out_func_oid, &attr->out_finfo); + InitFunctionCallInfoData(attr->out_fcinfo.fcinfo, &attr->out_finfo, + 1, InvalidOid, + NULL, NULL); } /* @@ -908,7 +934,7 @@ static void CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) { bool need_delim = false; - FmgrInfo *out_functions = cstate->out_functions; + CopyOutAttributeInfo *out_attributes = cstate->out_attributes; MemoryContext oldcontext; ListCell *cur; char *string; @@ -928,6 +954,8 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) foreach(cur, cstate->attnumlist) { int attnum = lfirst_int(cur); + CopyOutAttributeInfo *attr = &out_attributes[attnum - 1]; + FunctionCallInfo fcinfo = &attr->out_fcinfo.fcinfo; Datum value = slot->tts_values[attnum - 1]; bool isnull = slot->tts_isnull[attnum - 1]; @@ -949,8 +977,22 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) { if (!cstate->opts.binary) { - string = OutputFunctionCall(&out_functions[attnum - 1], - value); + Datum result; + + fcinfo->args[0].value = value; + fcinfo->args[0].isnull = false; + + result = FunctionCallInvoke(fcinfo); + + /* + * Check for null result, since caller is clearly not + * expecting one + */ + if (unlikely(fcinfo->isnull)) + elog(ERROR, "send function return null"); + + string = DatumGetCString(result); + if (cstate->opts.csv_mode) CopyAttributeOutCSV(cstate, string, cstate->opts.force_quote_flags[attnum - 1]); @@ -959,13 +1001,26 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) } else { + int outputlen; + Datum result; bytea *outputbytes; - outputbytes = SendFunctionCall(&out_functions[attnum - 1], - value); - CopySendInt32(cstate, VARSIZE(outputbytes) - VARHDRSZ); - CopySendData(cstate, VARDATA(outputbytes), - VARSIZE(outputbytes) - VARHDRSZ); + fcinfo->args[0].value = value; + fcinfo->args[0].isnull = false; + result = FunctionCallInvoke(fcinfo); + + /* + * Check for null result, since caller is clearly not + * expecting one + */ + if (unlikely(fcinfo->isnull)) + elog(ERROR, "send function return null"); + + outputbytes = DatumGetByteaP(result); + outputlen = VARSIZE(outputbytes) - VARHDRSZ; + + CopySendInt32(cstate, outputlen); + CopySendData(cstate, VARDATA(outputbytes), outputlen); } } } diff --git a/src/tools/pgindent/typedefs.list b/src/tools/pgindent/typedefs.list index d808aad8b05..2bd1028a5f4 100644 --- a/src/tools/pgindent/typedefs.list +++ b/src/tools/pgindent/typedefs.list @@ -273,6 +273,13 @@ BlockId BlockIdData BlockInfoRecord BlockNumber +BlockRefTable +BlockRefTableBuffer +BlockRefTableEntry +BlockRefTableKey +BlockRefTableReader +BlockRefTableSerializedEntry +BlockRefTableWriter BlockSampler BlockSamplerData BlockedProcData @@ -373,7 +380,6 @@ CatalogId CatalogIdMapEntry CatalogIndexState ChangeVarNodes_context -ReplaceVarnoContext CheckPoint CheckPointStmt CheckpointStatsData @@ -476,10 +482,12 @@ CopyFormatOptions CopyFromState CopyFromStateData CopyHeaderChoice +CopyInAttributeInfo CopyInsertMethod CopyMultiInsertBuffer CopyMultiInsertInfo CopyOnErrorChoice +CopyOutAttributeInfo CopySource CopyStmt CopyToState @@ -552,6 +560,8 @@ DR_intorel DR_printtup DR_sqlfunction DR_transientrel +DSMRegistryCtxStruct +DSMRegistryEntry DWORD DataDirSyncMethod DataDumperPtr @@ -612,8 +622,6 @@ DropSubscriptionStmt DropTableSpaceStmt DropUserMappingStmt DropdbStmt -DSMRegistryCtxStruct -DSMRegistryEntry DumpComponents DumpId DumpOptions @@ -748,6 +756,7 @@ FetchStmt FieldSelect FieldStore File +FileBackupMethod FileFdwExecutionState FileFdwPlanState FileNameMap @@ -1157,6 +1166,7 @@ InProgressEnt IncludeWal InclusionOpaque IncrementVarSublevelsUp_context +IncrementalBackupInfo IncrementalSort IncrementalSortExecutionStatus IncrementalSortGroupInfo @@ -1291,9 +1301,9 @@ JsonManifestWALRangeField JsonObjectAgg JsonObjectConstructor JsonOutput -JsonParseExpr JsonParseContext JsonParseErrorType +JsonParseExpr JsonPath JsonPathBool JsonPathExecContext @@ -2008,6 +2018,7 @@ PathClauseUsage PathCostComparison PathHashStack PathKey +PathKeyInfo PathKeysComparison PathTarget PatternInfo @@ -2359,6 +2370,7 @@ ReorderBufferUpdateProgressTxnCB ReorderTuple RepOriginId ReparameterizeForeignPathByChild_function +ReplaceVarnoContext ReplaceVarsFromTargetList_context ReplaceVarsNoMatchOption ReplicaIdentityStmt @@ -2697,6 +2709,7 @@ SubscriptingRefState Subscription SubscriptionInfo SubscriptionRelState +SummarizerReadLocalXLogPrivate SupportRequestCost SupportRequestIndexCondition SupportRequestOptimizeWindowClause @@ -2846,7 +2859,6 @@ TocEntry TokenAuxData TokenizedAuthLine TrackItem -TransamVariablesData TransApplyAction TransInvalidationInfo TransState @@ -2855,6 +2867,7 @@ TransactionState TransactionStateData TransactionStmt TransactionStmtKind +TransamVariablesData TransformInfo TransformJsonStringValuesState TransitionCaptureState @@ -2898,8 +2911,8 @@ TupleTableSlotOps TuplesortClusterArg TuplesortDatumArg TuplesortIndexArg -TuplesortIndexBrinArg TuplesortIndexBTreeArg +TuplesortIndexBrinArg TuplesortIndexHashArg TuplesortInstrumentation TuplesortMethod @@ -2945,12 +2958,14 @@ UnicodeNormalizationQC Unique UniquePath UniquePathMethod +UniqueRelInfo UniqueState UnlistenStmt UnresolvedTup UnresolvedTupData UpdateContext UpdateStmt +UploadManifestCmd UpperRelationKind UpperUniquePath UserAuth @@ -2999,7 +3014,6 @@ VolatileFunctionStatus Vsrt WAIT_ORDER WALAvailability -WalInsertClass WALInsertLock WALInsertLockPadded WALOpenSegment @@ -3032,6 +3046,7 @@ WaitEventTimeout WaitPMResult WalCloseMethod WalCompression +WalInsertClass WalLevel WalRcvData WalRcvExecResult @@ -3045,6 +3060,9 @@ WalSnd WalSndCtlData WalSndSendDataCallback WalSndState +WalSummarizerData +WalSummaryFile +WalSummaryIO WalSyncMethod WalTimeSample WalUsage @@ -3201,8 +3219,10 @@ avl_node avl_tree avw_dbase backslashResult +backup_file_entry backup_manifest_info backup_manifest_option +backup_wal_range base_yy_extra_type basebackup_options bbsink @@ -3245,6 +3265,10 @@ cached_re_str canonicalize_state cashKEY catalogid_hash +cb_cleanup_dir +cb_options +cb_tablespace +cb_tablespace_mapping check_agg_arguments_context check_function_callback check_network_data @@ -3498,10 +3522,12 @@ macKEY macaddr macaddr8 macaddr_sortsupport_state +manifest_data manifest_file manifest_files_hash manifest_files_iterator manifest_wal_range +manifest_writer map_variable_attnos_context max_parallel_hazard_context mb2wchar_with_len_converter @@ -3733,6 +3759,7 @@ ret_type rewind_source rewrite_event rf_context +rfile rm_detail_t role_auth_extra rolename_hash @@ -3866,7 +3893,6 @@ unicodeStyleColumnFormat unicodeStyleFormat unicodeStyleRowFormat unicode_linestyle -UniqueRelInfo unit_conversion unlogged_relation_entry utf_local_conversion_func @@ -3907,6 +3933,8 @@ worker_spi_state worker_state worktable wrap +ws_file_info +ws_options xl_brin_createidx xl_brin_desummarize xl_brin_insert @@ -4026,29 +4054,3 @@ yyscan_t z_stream z_streamp zic_t -BlockRefTable -BlockRefTableBuffer -BlockRefTableEntry -BlockRefTableKey -BlockRefTableReader -BlockRefTableSerializedEntry -BlockRefTableWriter -SummarizerReadLocalXLogPrivate -WalSummarizerData -WalSummaryFile -WalSummaryIO -FileBackupMethod -IncrementalBackupInfo -UploadManifestCmd -backup_file_entry -backup_wal_range -cb_cleanup_dir -cb_options -cb_tablespace -cb_tablespace_mapping -manifest_data -manifest_writer -rfile -ws_options -ws_file_info -PathKeyInfo -- 2.38.0
>From ce78f7462b933ba9d648280ed34e2f90eb0c17e6 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Sat, 17 Feb 2024 13:17:56 -0800 Subject: [PATCH v3 09/10] inline pq_sendbytes, stop maintaining trailing null byte Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/include/libpq/pqformat.h | 12 +++++++++++- src/backend/libpq/pqformat.c | 11 ----------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index 62922f58747..bc35bf3f28a 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -23,7 +23,6 @@ extern void pq_beginmessage_reuse(StringInfo buf, char msgtype); extern void pq_endmessage(StringInfo buf); extern void pq_endmessage_reuse(StringInfo buf); -extern void pq_sendbytes(StringInfo buf, const void *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); @@ -216,6 +215,17 @@ pq_sendbyte(StringInfo buf, uint8 byt) pq_sendint8(buf, byt); } +static inline void +pq_sendbytes(StringInfo buf, const void *data, int datalen) +{ + /* + * FIXME: used to say: use variant that maintains a trailing null-byte, + * out of caution but that doesn't make much sense to me, and proves to be + * a performance issue. + */ + appendBinaryStringInfoNT(buf, data, datalen); +} + /* * Append a binary integer to a StringInfo buffer * diff --git a/src/backend/libpq/pqformat.c b/src/backend/libpq/pqformat.c index 9275b241333..807e143cef2 100644 --- a/src/backend/libpq/pqformat.c +++ b/src/backend/libpq/pqformat.c @@ -118,17 +118,6 @@ pq_beginmessage_reuse(StringInfo buf, char msgtype) buf->cursor = msgtype; } -/* -------------------------------- - * pq_sendbytes - append raw data to a StringInfo buffer - * -------------------------------- - */ -void -pq_sendbytes(StringInfo buf, const void *data, int datalen) -{ - /* use variant that maintains a trailing null-byte, out of caution */ - appendBinaryStringInfo(buf, data, datalen); -} - /* -------------------------------- * pq_sendcountedtext - append a counted text string (with character set conversion) * -- 2.38.0
>From c06aa2636a47812ef17d8be7d7e43a8a49e88403 Mon Sep 17 00:00:00 2001 From: Andres Freund <and...@anarazel.de> Date: Sat, 17 Feb 2024 17:38:47 -0800 Subject: [PATCH v3 10/10] heavy-wip: Allow string buffer reuse in send functions Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/include/libpq/pqformat.h | 19 +++++++++++++++++-- src/backend/access/common/printtup.c | 9 +++++++-- src/backend/commands/copyto.c | 8 +++++++- src/backend/utils/adt/int.c | 10 ++++++---- src/backend/utils/adt/int8.c | 9 +++++---- src/backend/utils/adt/varlena.c | 10 ++++++---- src/backend/utils/fmgr/fmgr.c | 17 ++++++++++++++++- 7 files changed, 64 insertions(+), 18 deletions(-) diff --git a/src/include/libpq/pqformat.h b/src/include/libpq/pqformat.h index bc35bf3f28a..7792691b4cf 100644 --- a/src/include/libpq/pqformat.h +++ b/src/include/libpq/pqformat.h @@ -40,8 +40,16 @@ static inline void pq_begintypsend(StringInfo buf) { initStringInfo(buf); - /* Reserve four bytes for the bytea length word */ - appendStringInfoSpaces(buf, 4); + + /* + * Reserve four bytes for the bytea length word. We don't need to fill + * them with anything (pq_endtypsend will do that), and this function is + * enough of a hot spot that it's worth cheating to save some cycles. Note + * in particular that we don't bother to guarantee that the buffer is + * null-terminated. + */ + Assert(buf->maxlen > 4); + buf->len = 4; } /* -------------------------------- @@ -61,6 +69,13 @@ pq_begintypsend_with_size(StringInfo buf, int size) appendStringInfoSpaces(buf, 4); } +static inline void +pq_begintypsend_res(StringInfo buf) +{ + Assert(buf && buf->data && buf->len == 0); + + buf->len = 4; +} /* -------------------------------- * pq_endtypsend - finish constructing a bytea result diff --git a/src/backend/access/common/printtup.c b/src/backend/access/common/printtup.c index bee3fc26220..d7d6d12f242 100644 --- a/src/backend/access/common/printtup.c +++ b/src/backend/access/common/printtup.c @@ -69,6 +69,7 @@ typedef struct int nattrs; PrinttupAttrInfo *myinfo; /* Cached info about each attr */ StringInfoData buf; /* output buffer (*not* in tmpcontext) */ + StringInfoData fieldbuf; /* FIXME */ MemoryContext tmpcontext; /* Memory context for per-row workspace */ } DR_printtup; @@ -128,6 +129,8 @@ printtup_startup(DestReceiver *self, int operation, TupleDesc typeinfo) */ initStringInfo(&myState->buf); + initStringInfo(&myState->fieldbuf); + /* * Create a temporary memory context that we can reset once per row to * recover palloc'd memory. This avoids any problems with leaks inside @@ -289,7 +292,7 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs) fmgr_info(thisState->typoutput, &thisState->finfo); InitFunctionCallInfoData(thisState->fcinfo_data.fcinfo, &thisState->finfo, 1, InvalidOid, - NULL, NULL); + (Node *) &myState->fieldbuf, NULL); } else if (format == 1) { @@ -299,7 +302,7 @@ printtup_prepare_info(DR_printtup *myState, TupleDesc typeinfo, int numAttrs) fmgr_info(thisState->typsend, &thisState->finfo); InitFunctionCallInfoData(thisState->fcinfo_data.fcinfo, &thisState->finfo, 1, InvalidOid, - NULL, NULL); + (Node *) &myState->fieldbuf, NULL); } else ereport(ERROR, @@ -319,6 +322,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self) DR_printtup *myState = (DR_printtup *) self; MemoryContext oldcontext; StringInfo buf = &myState->buf; + StringInfo fieldbuf = &myState->fieldbuf; int natts = typeinfo->natts; int i; @@ -398,6 +402,7 @@ printtup(TupleTableSlot *slot, DestReceiver *self) pq_sendint32(buf, outputlen); pq_sendbytes(buf, VARDATA(outputbytes), outputlen); + resetStringInfo(fieldbuf); } } diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c index b5cada5cb75..dd552db4828 100644 --- a/src/backend/commands/copyto.c +++ b/src/backend/commands/copyto.c @@ -110,6 +110,8 @@ typedef struct CopyToStateData */ MemoryContext copycontext; /* per-copy execution context */ + StringInfoData attribute_buf; + CopyOutAttributeInfo *out_attributes; MemoryContext rowcontext; /* per-row evaluation context */ @@ -782,6 +784,8 @@ DoCopyTo(CopyToState cstate) /* We use fe_msgbuf as a per-row buffer regardless of copy_dest */ cstate->fe_msgbuf = makeStringInfo(); + initStringInfo(&cstate->attribute_buf); + /* Get info about the columns we need to process. */ cstate->out_attributes = (CopyOutAttributeInfo *) palloc(num_phys_attrs * sizeof(CopyOutAttributeInfo)); @@ -810,7 +814,7 @@ DoCopyTo(CopyToState cstate) fmgr_info(out_func_oid, &attr->out_finfo); InitFunctionCallInfoData(attr->out_fcinfo.fcinfo, &attr->out_finfo, 1, InvalidOid, - NULL, NULL); + (Node *) &cstate->attribute_buf, NULL); } /* @@ -938,6 +942,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) MemoryContext oldcontext; ListCell *cur; char *string; + StringInfo attribute_buf = &cstate->attribute_buf; MemoryContextReset(cstate->rowcontext); oldcontext = MemoryContextSwitchTo(cstate->rowcontext); @@ -1021,6 +1026,7 @@ CopyOneRowTo(CopyToState cstate, TupleTableSlot *slot) CopySendInt32(cstate, outputlen); CopySendData(cstate, VARDATA(outputbytes), outputlen); + resetStringInfo(attribute_buf); } } } diff --git a/src/backend/utils/adt/int.c b/src/backend/utils/adt/int.c index 761dc4acce9..2fdf2b247de 100644 --- a/src/backend/utils/adt/int.c +++ b/src/backend/utils/adt/int.c @@ -322,11 +322,13 @@ Datum int4send(PG_FUNCTION_ARGS) { int32 arg1 = PG_GETARG_INT32(0); - StringInfoData buf; + StringInfo buf; - pq_begintypsend_with_size(&buf, 4); - pq_sendint32(&buf, arg1); - PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); + buf = (StringInfo) fcinfo->context; + + pq_begintypsend_res(buf); + 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 5423cb2ecde..a171125b110 100644 --- a/src/backend/utils/adt/int8.c +++ b/src/backend/utils/adt/int8.c @@ -95,11 +95,12 @@ Datum int8send(PG_FUNCTION_ARGS) { int64 arg1 = PG_GETARG_INT64(0); - StringInfoData buf; + StringInfo buf; - pq_begintypsend_with_size(&buf, 8); - pq_sendint64(&buf, arg1); - PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); + buf = (StringInfo) fcinfo->context; + pq_begintypsend_res(buf); + pq_sendint64(buf, arg1); + PG_RETURN_BYTEA_P(pq_endtypsend(buf)); } diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c index ea696c1dc71..387e368726e 100644 --- a/src/backend/utils/adt/varlena.c +++ b/src/backend/utils/adt/varlena.c @@ -619,11 +619,13 @@ Datum textsend(PG_FUNCTION_ARGS) { text *t = PG_GETARG_TEXT_PP(0); - StringInfoData buf; + StringInfo buf = (StringInfo) fcinfo->context; - pq_begintypsend_with_size(&buf, VARSIZE_ANY_EXHDR(t)); - pq_sendtext(&buf, VARDATA_ANY(t), VARSIZE_ANY_EXHDR(t)); - PG_RETURN_BYTEA_P(pq_endtypsend(&buf)); + Assert(fcinfo->context); + + pq_begintypsend_res(buf); + pq_sendtext(buf, VARDATA_ANY(t), VARSIZE_ANY_EXHDR(t)); + PG_RETURN_BYTEA_P(pq_endtypsend(buf)); } diff --git a/src/backend/utils/fmgr/fmgr.c b/src/backend/utils/fmgr/fmgr.c index e48a86be54b..5677a9a6bdf 100644 --- a/src/backend/utils/fmgr/fmgr.c +++ b/src/backend/utils/fmgr/fmgr.c @@ -1743,7 +1743,22 @@ ReceiveFunctionCall(FmgrInfo *flinfo, StringInfo buf, bytea * SendFunctionCall(FmgrInfo *flinfo, Datum val) { - return DatumGetByteaP(FunctionCall1(flinfo, val)); + StringInfoData buf; + Datum result; + + LOCAL_FCINFO(fcinfo, 1); + + initStringInfo(&buf); + + InitFunctionCallInfoData(*fcinfo, flinfo, 1, InvalidOid, (Node *) &buf, NULL); + fcinfo->args[0].value = val; + fcinfo->args[0].isnull = false; + result = FunctionCallInvoke(fcinfo); + + if (fcinfo->isnull) + elog(ERROR, "function %u returned NULL", flinfo->fn_oid); + + return DatumGetByteaP(result); } /* -- 2.38.0
