From ae981295c55e7b02f81e85171ad840b6a3d5b330 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Tue, 14 Jan 2020 12:46:20 -0800 Subject: [PATCH v2 1/9] stringinfo: Move more functions inline, provide initStringInfoEx(). By moving the whole lifecycle of a stringinfo into inline functions, a good compiler sometimes can be able to optimize away the existence of the StringInfoData itself. initStringInfoEx() allows stringinfo users to determine the initial allocation size, avoiding over/under allocation when known. Author: Reviewed-By: Discussion: https://postgr.es/m/ Backpatch: --- src/include/lib/stringinfo.h | 205 +++++++++++++++++++++++++++-------- src/common/stringinfo.c | 173 ++--------------------------- 2 files changed, 171 insertions(+), 207 deletions(-) diff --git a/src/include/lib/stringinfo.h b/src/include/lib/stringinfo.h index 5a2a3dbfbc0..38211323d9d 100644 --- a/src/include/lib/stringinfo.h +++ b/src/include/lib/stringinfo.h @@ -18,6 +18,9 @@ #ifndef STRINGINFO_H #define STRINGINFO_H +#include "common/int.h" +#include "common/string.h" + /*------------------------- * StringInfoData holds information about an extensible string. * data is the current buffer for the string (allocated with palloc). @@ -66,25 +69,137 @@ typedef StringInfoData *StringInfo; *------------------------- */ -/*------------------------ - * makeStringInfo - * Create an empty 'StringInfoData' & return a pointer to it. - */ -extern StringInfo makeStringInfo(void); - -/*------------------------ - * initStringInfo - * Initialize a StringInfoData struct (with previously undefined contents) - * to describe an empty string. - */ -extern void initStringInfo(StringInfo str); /*------------------------ * resetStringInfo * Clears the current content of the StringInfo, if any. The * StringInfo remains valid. */ -extern void resetStringInfo(StringInfo str); +static inline void +resetStringInfo(StringInfoData *pg_restrict str) +{ + *(char *pg_restrict) (str->data) = '\0'; + str->len = 0; + str->cursor = 0; +} + +/*------------------------ + * initStringInfo + * Initialize a StringInfoData struct (with previously undefined contents) + * to describe an empty string. + */ +static inline void +initStringInfo(StringInfoData *pg_restrict str) +{ + int size = 1024; /* initial default buffer size */ + + str->data = (char *) palloc(size); + str->maxlen = size; + resetStringInfo(str); +} + +/*------------------------ + * initStringInfoEx + * + * Like initStringInfo(), but allows to specify the size of the initial + * allocation. + */ +static inline void +initStringInfoEx(StringInfoData *pg_restrict str, int size) +{ + /* + * FIXME: Adding 1 to account for always present trailing \0 byte. Should + * that instead be done at the caller level? Should we round up? + */ + str->data = (char *) palloc(size + 1); + str->maxlen = size + 1; + resetStringInfo(str); +} + +/*------------------------ + * makeStringInfo + * Create an empty 'StringInfoData' & return a pointer to it. + */ +static inline StringInfo +makeStringInfo(void) +{ + StringInfo res; + + res = (StringInfo) palloc(sizeof(StringInfoData)); + + initStringInfo(res); + + return res; +} + +/*------------------------ + * enlargeStringInfoImpl + * + * Actually enlarge the string, only to be called by enlargeStringInfo(). + */ +extern void enlargeStringInfoImpl(StringInfo str, int needed); + +/*------------------------ + * enlargeStringInfo + * Make sure a StringInfo's buffer can hold at least 'needed' more bytes. + * + * External callers usually need not concern themselves with this, since + * all stringinfo.c routines do it automatically. However, if a caller + * knows that a StringInfo will eventually become X bytes large, it + * can save some palloc overhead by enlarging the buffer before starting + * to store data in it. + * + * NB: In the backend, because we use repalloc() to enlarge the buffer, the + * string buffer will remain allocated in the same memory context that was + * current when initStringInfo was called, even if another context is now + * current. This is the desired and indeed critical behavior! + */ +static inline void +enlargeStringInfo(StringInfoData *pg_restrict str, int datalen) +{ + int res; + + if (unlikely(pg_add_s32_overflow(str->len, datalen, &res)) || + unlikely(res >= str->maxlen)) + enlargeStringInfoImpl(str, datalen); +} + +/*------------------------ + * appendBinaryStringInfoNT + * Append arbitrary binary data to a StringInfo, allocating more space + * if necessary. Does not ensure a trailing null-byte exists. + */ +static inline void +appendBinaryStringInfoNT(StringInfoData *pg_restrict str, const char *pg_restrict data, int datalen) +{ + Assert(str != NULL); + + /* Make more room if needed */ + enlargeStringInfo(str, datalen); + + /* OK, append the data */ + memcpy((char *pg_restrict) (str->data + str->len), data, datalen); + str->len += datalen; +} + +/*------------------------ + * appendBinaryStringInfo + * Append arbitrary binary data to a StringInfo, allocating more space + * if necessary. Ensures that a trailing null byte is present. + */ +static inline void +appendBinaryStringInfo(StringInfoData *pg_restrict str, const char *pg_restrict data, int datalen) +{ + appendBinaryStringInfoNT(str, data, datalen); + + /* + * Keep a trailing null in place, even though it's probably useless for + * binary data. (Some callers are dealing with text but call this because + * their input isn't null-terminated.) + */ + *(char *pg_restrict) (str->data + str->len) = '\0'; +} + /*------------------------ * appendStringInfo @@ -111,51 +226,49 @@ extern int appendStringInfoVA(StringInfo str, const char *fmt, va_list args) pg_ * Append a null-terminated string to str. * Like appendStringInfo(str, "%s", s) but faster. */ -extern void appendStringInfoString(StringInfo str, const char *s); +static inline void +appendStringInfoString(StringInfoData *pg_restrict str, const char *pg_restrict s) +{ + appendBinaryStringInfo(str, s, strlen(s)); +} /*------------------------ * appendStringInfoChar * Append a single byte to str. * Like appendStringInfo(str, "%c", ch) but much faster. */ -extern void appendStringInfoChar(StringInfo str, char ch); +static inline void +appendStringInfoChar(StringInfoData *pg_restrict str, char ch) +{ + /* Make more room if needed */ + enlargeStringInfo(str, 1); -/*------------------------ - * appendStringInfoCharMacro - * As above, but a macro for even more speed where it matters. - * Caution: str argument will be evaluated multiple times. - */ -#define appendStringInfoCharMacro(str,ch) \ - (((str)->len + 1 >= (str)->maxlen) ? \ - appendStringInfoChar(str, ch) : \ - (void)((str)->data[(str)->len] = (ch), (str)->data[++(str)->len] = '\0')) + /* OK, append the character */ + *(char *pg_restrict) (str->data + str->len) = ch; + str->len++; + *(char *pg_restrict) (str->data + str->len) = '\0'; +} + +/* backward compat for external code */ +#define appendStringInfoCharMacro appendStringInfoChar /*------------------------ * appendStringInfoSpaces * Append a given number of spaces to str. */ -extern void appendStringInfoSpaces(StringInfo str, int count); +static inline void +appendStringInfoSpaces(StringInfoData *pg_restrict str, int count) +{ + if (count > 0) + { + /* Make more room if needed */ + enlargeStringInfo(str, count); -/*------------------------ - * appendBinaryStringInfo - * Append arbitrary binary data to a StringInfo, allocating more space - * if necessary. - */ -extern void appendBinaryStringInfo(StringInfo str, - const char *data, int datalen); - -/*------------------------ - * appendBinaryStringInfoNT - * Append arbitrary binary data to a StringInfo, allocating more space - * if necessary. Does not ensure a trailing null-byte exists. - */ -extern void appendBinaryStringInfoNT(StringInfo str, - const char *data, int datalen); - -/*------------------------ - * enlargeStringInfo - * Make sure a StringInfo's buffer can hold at least 'needed' more bytes. - */ -extern void enlargeStringInfo(StringInfo str, int needed); + /* OK, append the spaces */ + memset((char *pg_restrict) (str->data + str->len), ' ', count); + str->len += count; + *(char *pg_restrict) (str->data + str->len) = '\0'; + } +} #endif /* STRINGINFO_H */ diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c index 0badc465545..0ea5e0abe56 100644 --- a/src/common/stringinfo.c +++ b/src/common/stringinfo.c @@ -32,53 +32,6 @@ #include "lib/stringinfo.h" -/* - * makeStringInfo - * - * Create an empty 'StringInfoData' & return a pointer to it. - */ -StringInfo -makeStringInfo(void) -{ - StringInfo res; - - res = (StringInfo) palloc(sizeof(StringInfoData)); - - initStringInfo(res); - - return res; -} - -/* - * initStringInfo - * - * Initialize a StringInfoData struct (with previously undefined contents) - * to describe an empty string. - */ -void -initStringInfo(StringInfo str) -{ - int size = 1024; /* initial default buffer size */ - - str->data = (char *) palloc(size); - str->maxlen = size; - resetStringInfo(str); -} - -/* - * resetStringInfo - * - * Reset the StringInfo: the data buffer remains valid, but its - * previous content, if any, is cleared. - */ -void -resetStringInfo(StringInfo str) -{ - str->data[0] = '\0'; - str->len = 0; - str->cursor = 0; -} - /* * appendStringInfo * @@ -167,120 +120,18 @@ appendStringInfoVA(StringInfo str, const char *fmt, va_list args) } /* - * appendStringInfoString + * enlargeStringInfoImpl * - * Append a null-terminated string to str. - * Like appendStringInfo(str, "%s", s) but faster. + * Make enough space for 'needed' more bytes ('needed' does not include the + * terminating null). This is not for external consumption, it's only to be + * called by enlargeStringInfo() when more space is actually needed (including + * when we'd overflow the maximum size). + * + * As this normally shouldn't be the common case, mark as noinline, to avoid + * including the function into the fastpath. */ -void -appendStringInfoString(StringInfo str, const char *s) -{ - appendBinaryStringInfo(str, s, strlen(s)); -} - -/* - * appendStringInfoChar - * - * Append a single byte to str. - * Like appendStringInfo(str, "%c", ch) but much faster. - */ -void -appendStringInfoChar(StringInfo str, char ch) -{ - /* Make more room if needed */ - if (str->len + 1 >= str->maxlen) - enlargeStringInfo(str, 1); - - /* OK, append the character */ - str->data[str->len] = ch; - str->len++; - str->data[str->len] = '\0'; -} - -/* - * appendStringInfoSpaces - * - * Append the specified number of spaces to a buffer. - */ -void -appendStringInfoSpaces(StringInfo str, int count) -{ - if (count > 0) - { - /* Make more room if needed */ - enlargeStringInfo(str, count); - - /* OK, append the spaces */ - while (--count >= 0) - str->data[str->len++] = ' '; - str->data[str->len] = '\0'; - } -} - -/* - * appendBinaryStringInfo - * - * Append arbitrary binary data to a StringInfo, allocating more space - * if necessary. Ensures that a trailing null byte is present. - */ -void -appendBinaryStringInfo(StringInfo str, const char *data, int datalen) -{ - Assert(str != NULL); - - /* Make more room if needed */ - enlargeStringInfo(str, datalen); - - /* OK, append the data */ - memcpy(str->data + str->len, data, datalen); - str->len += datalen; - - /* - * Keep a trailing null in place, even though it's probably useless for - * binary data. (Some callers are dealing with text but call this because - * their input isn't null-terminated.) - */ - str->data[str->len] = '\0'; -} - -/* - * appendBinaryStringInfoNT - * - * Append arbitrary binary data to a StringInfo, allocating more space - * if necessary. Does not ensure a trailing null-byte exists. - */ -void -appendBinaryStringInfoNT(StringInfo str, const char *data, int datalen) -{ - Assert(str != NULL); - - /* Make more room if needed */ - enlargeStringInfo(str, datalen); - - /* OK, append the data */ - memcpy(str->data + str->len, data, datalen); - str->len += datalen; -} - -/* - * enlargeStringInfo - * - * Make sure there is enough space for 'needed' more bytes - * ('needed' does not include the terminating null). - * - * External callers usually need not concern themselves with this, since - * all stringinfo.c routines do it automatically. However, if a caller - * knows that a StringInfo will eventually become X bytes large, it - * can save some palloc overhead by enlarging the buffer before starting - * to store data in it. - * - * NB: In the backend, because we use repalloc() to enlarge the buffer, the - * string buffer will remain allocated in the same memory context that was - * current when initStringInfo was called, even if another context is now - * current. This is the desired and indeed critical behavior! - */ -void -enlargeStringInfo(StringInfo str, int needed) +pg_noinline void +enlargeStringInfoImpl(StringInfo str, int needed) { int newlen; @@ -317,8 +168,8 @@ enlargeStringInfo(StringInfo str, int needed) /* Because of the above test, we now have needed <= MaxAllocSize */ - if (needed <= str->maxlen) - return; /* got enough space already */ + /* should only be called when needed */ + Assert(needed > str->maxlen); /* * We don't want to allocate just a little more space with each append; -- 2.25.0.114.g5b0ca878e0