On Sat, 2024-02-17 at 12:24 -0800, Andres Freund wrote:
> On 2024-02-17 17:48:23 +0100, Laurenz Albe wrote:
> > - Patch 0001 speeds up pq_begintypsend with a custom macro.
> > That brought the binary copy down to 3.7 seconds, which is a
> > speed gain of 15%.
>
> Nice win, but I think we can do a bit better than this. Certainly it shouldn't
> be a macro, given the multiple evaluation risks. I don't think we actually
> need to zero those bytes, in fact that seems more likely to hide bugs than
> prevent them.
>
> I have an old patch series improving performance in this area.
The multiple evaluation danger could be worked around with an automatic
variable, or the macro could just carry a warning (like
appendStringInfoCharMacro).
But considering the thread in [1], I guess I'll retract that patch; I'm
sure the more invasive improvement you have in mind will do better.
> > - Patch 0001 speeds up uuid_out by avoiding the overhead of
> > a Stringinfo. This brings text mode COPY to 19.4 seconds,
> > which is speed gain of 21%.
>
> Nice!
>
> I wonder if we should move the core part for converting to hex to numutils.c,
> we already have code the for the inverse. There does seem to be further
> optimization potential in the conversion, and that seems better done somewhere
> central rather than one type's output function. OTOH, it might not be worth
> it, given the need to add the dashes.
I thought about it, but then decided not to do that.
Calling a function that converts the bytes to hex and then adding the
hyphens will slow down processing, and I think the code savings would be
minimal at best.
> > - Patch 0003 speeds up array_out a bit by avoiding some zero
> > byte writes. The measured speed gain is under 2%.
>
> Makes sense.
Thanks for your interest and approval. The attached patches are the
renamed, but unmodified patches 2 and 3 from before.
Yours,
Laurenz Albe
[1]:
https://postgr.es/m/CAMkU%3D1whbRDUwa4eayD9%2B59K-coxO9senDkPRbTn3cg0pUz4AQ%40mail.gmail.com
From de87d249e3f55c38062e563821af9fa32d15e341 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Sat, 17 Feb 2024 17:23:39 +0100
Subject: [PATCH v1 2/3] Speed up uuid_out
Since the size of the string representation of an uuid is
fixed, there is no benefit in using a StringInfo.
Avoiding the overhead makes the function substantially faster.
---
src/backend/utils/adt/uuid.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/src/backend/utils/adt/uuid.c b/src/backend/utils/adt/uuid.c
index 73dfd711c7..b48bbf01e1 100644
--- a/src/backend/utils/adt/uuid.c
+++ b/src/backend/utils/adt/uuid.c
@@ -53,10 +53,11 @@ uuid_out(PG_FUNCTION_ARGS)
{
pg_uuid_t *uuid = PG_GETARG_UUID_P(0);
static const char hex_chars[] = "0123456789abcdef";
- StringInfoData buf;
+ char *buf, *p;
int i;
- initStringInfo(&buf);
+ buf = palloc(2 * UUID_LEN + 5);
+ p = buf;
for (i = 0; i < UUID_LEN; i++)
{
int hi;
@@ -68,16 +69,17 @@ uuid_out(PG_FUNCTION_ARGS)
* ("-"). Therefore, add the hyphens at the appropriate places here.
*/
if (i == 4 || i == 6 || i == 8 || i == 10)
- appendStringInfoChar(&buf, '-');
+ *p++ = '-';
hi = uuid->data[i] >> 4;
lo = uuid->data[i] & 0x0F;
- appendStringInfoChar(&buf, hex_chars[hi]);
- appendStringInfoChar(&buf, hex_chars[lo]);
+ *p++ = hex_chars[hi];
+ *p++ = hex_chars[lo];
}
+ *p = '\0';
- PG_RETURN_CSTRING(buf.data);
+ PG_RETURN_CSTRING(buf);
}
/*
--
2.43.2
From e8346ea88785a763d2bd3f99800ae928b7469f64 Mon Sep 17 00:00:00 2001
From: Laurenz Albe <laurenz.a...@cybertec.at>
Date: Sat, 17 Feb 2024 17:24:19 +0100
Subject: [PATCH v1 3/3] Small speedup for array_out
Avoid writing zero bytes where it is not necessary.
This offers only a small, but measurable speed gain
for larger arrays.
---
src/backend/utils/adt/arrayfuncs.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/src/backend/utils/adt/arrayfuncs.c b/src/backend/utils/adt/arrayfuncs.c
index f3fee54e37..306c5062f7 100644
--- a/src/backend/utils/adt/arrayfuncs.c
+++ b/src/backend/utils/adt/arrayfuncs.c
@@ -1200,7 +1200,7 @@ array_out(PG_FUNCTION_ARGS)
p = retval;
#define APPENDSTR(str) (strcpy(p, (str)), p += strlen(p))
-#define APPENDCHAR(ch) (*p++ = (ch), *p = '\0')
+#define APPENDCHAR(ch) (*p++ = (ch))
if (needdims)
APPENDSTR(dims_str);
@@ -1222,10 +1222,9 @@ array_out(PG_FUNCTION_ARGS)
char ch = *tmp;
if (ch == '"' || ch == '\\')
- *p++ = '\\';
- *p++ = ch;
+ APPENDCHAR('\\');
+ APPENDCHAR(ch);
}
- *p = '\0';
APPENDCHAR('"');
}
else
@@ -1248,6 +1247,8 @@ array_out(PG_FUNCTION_ARGS)
j = i;
} while (j != -1);
+ *p = '\0';
+
#undef APPENDSTR
#undef APPENDCHAR
--
2.43.2
