In [1] I noticed a bit of a poor usage of appendStringInfoString which
just appends 4 spaces in a loop, one for each indent level of the
jsonb. It should be better just to use appendStringInfoSpaces and
just append all the spaces in one go rather than appending 4 spaces in
a loop. That'll save having to check enlargeStringInfo() once for each
loop.
I'm aiming this mostly as a cleanup patch, but after looking at the
appendStringInfoSpaces code, I thought it could be done a bit more
efficiently by using memset instead of using the while loop that keeps
track of 2 counters. memset has the option of doing more than a char
at a time, which should be useful for larger numbers of spaces.
It does seem a bit faster when appending 8 chars at least going by the
attached spaces.c file.
With -O1
$ ./spaces
while 0.536577 seconds
memset 0.326532 seconds
However, I'm not really expecting much of a performance increase from
this change. I do at least want to make sure I've not made anything
worse, so I used pgbench to run:
select jsonb_pretty(row_to_json(pg_class)::jsonb) from pg_class;
perf top says:
Master:
0.96% postgres [.] add_indent.part.0
Patched
0.25% postgres [.] add_indent.part.0
I can't really detect a certain enough TPS change over the noise. I
expect it might become more significant with more complex json that
has more than a single indent level.
I could only find 1 other instance where we use appendStringInfoString
to append spaces. I've adjusted that one too.
David
[1]
https://postgr.es/m/caaphdvrrfnsm8df24tmyozpvo-r5zp+0foqvo2xcyhrfteh...@mail.gmail.com
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index e4621ef8d6..5212a64b1e 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -3324,7 +3324,7 @@ show_hashagg_info(AggState *aggstate, ExplainState *es)
if (!gotone)
ExplainIndentText(es);
else
- appendStringInfoString(es->str, " ");
+ appendStringInfoSpaces(es->str, 2);
appendStringInfo(es->str, "Batches: %d Memory Usage: "
INT64_FORMAT "kB",
aggstate->hash_batches_used, memPeakKb);
diff --git a/src/backend/utils/adt/jsonb.c b/src/backend/utils/adt/jsonb.c
index 4ff2eced4c..0539f41c17 100644
--- a/src/backend/utils/adt/jsonb.c
+++ b/src/backend/utils/adt/jsonb.c
@@ -626,11 +626,8 @@ add_indent(StringInfo out, bool indent, int level)
{
if (indent)
{
- int i;
-
appendStringInfoCharMacro(out, '\n');
- for (i = 0; i < level; i++)
- appendBinaryStringInfo(out, " ", 4);
+ appendStringInfoSpaces(out, level * 4);
}
}
diff --git a/src/common/stringinfo.c b/src/common/stringinfo.c
index b3d3c99b8c..05b22b5c53 100644
--- a/src/common/stringinfo.c
+++ b/src/common/stringinfo.c
@@ -211,8 +211,8 @@ appendStringInfoSpaces(StringInfo str, int count)
enlargeStringInfo(str, count);
/* OK, append the spaces */
- while (--count >= 0)
- str->data[str->len++] = ' ';
+ memset(&str->data[str->len], ' ', count);
+ str->len += count;
str->data[str->len] = '\0';
}
}
#include <time.h>
#include <stdio.h>
#include <string.h>
int f1(char *data, int count)
{
int len = 0;
/* OK, append the spaces */
while (--count >= 0)
data[len++] = ' ';
data[len] = '\0';
return len;
}
int f2(char *data, int count)
{
memset(data, ' ', count);
data[count] = '\0';
return count;
}
#define NLOOPS 100000000
int main(void)
{
char buffer[8];
clock_t start, end;
unsigned long long x = 0;
start = clock();
for (int i = 0; i < NLOOPS; i++)
{
x += f1(buffer, sizeof(buffer) - 1);
}
end = clock();
printf("while %g seconds\n", (double) (end - start) / CLOCKS_PER_SEC);
start = clock();
for (int i = 0; i < NLOOPS; i++)
{
x += f2(buffer, sizeof(buffer) - 1);
}
end = clock();
printf("memset %g seconds\n", (double) (end - start) / CLOCKS_PER_SEC);
printf("%lld\n", x);
return 0;
}
