Making aggregate deserialization (and WAL receive) functions slightly faster
David Rowley <dgrowleyml@gmail.com>
From: David Rowley <dgrowleyml@gmail.com>
To: PostgreSQL Developers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-12T05:38:36Z
Lists: pgsql-hackers
Attachments
- string_agg_test.sh.txt (text/plain)
- string_agg_single_group_bench.png (image/png)
- v1-0001-Optimize-various-aggregate-deserialization-functi.patch (text/plain) patch v1-0001
- v1-0002-Add-more-usages-of-initStringInfoFromString.patch (text/plain) patch v1-0002
While working on 16fd03e95, I noticed that in each aggregate deserialization function, in order to "receive" the bytea value that is the serialized aggregate state, appendBinaryStringInfo is used to append the bytes of the bytea value onto a temporary StringInfoData. Using appendBinaryStringInfo seems a bit wasteful here. We could really just fake up a StringInfoData and point directly to the bytes of the bytea value. The best way I could think of to do this was to invent initStringInfoFromString() which initialises a StringInfoData and has the ->data field point directly at the specified buffer. This will mean that it would be unsafe to do any appendStringInfo* operations on the resulting StringInfoData as enlargeStringInfo would try to repalloc the data buffer, which might not even point to a palloc'd string. I thought it might be fine just to mention that in the comments for the function, but we could probably do a bit better and set maxlen to something like -1 and Assert() we never see -1 in the various append functions. I wasn't sure it was worth it, so didn't do that. I had a look around for other places that might be following the same pattern. I only found range_recv() and XLogWalRcvProcessMsg(). I didn't adjust the range_recv() one as I couldn't see how to do that without casting away a const. I did adjust the XLogWalRcvProcessMsg() one and got rid of a global variable in the process. I've attached the benchmark results I got after testing how the modification changed the performance of string_agg_deserialize(). I was hoping this would have a slightly more impressive performance impact, especially for string_agg() and array_agg() as the aggregate states of those can be large. However, in the test I ran, there's only a very slight performance gain. I may just not have found the best case, however. David
Commits
-
Make use of initReadOnlyStringInfo() in more places
- ac7d6f5f831e 17.0 landed
-
Optimize various aggregate deserialization functions, take 2
- 0c882a298881 17.0 landed
-
Introduce the concept of read-only StringInfos
- f0efa5aec193 17.0 landed
-
Revert "Optimize various aggregate deserialization functions"
- 4f3b56eea235 17.0 landed
-
Optimize various aggregate deserialization functions
- 608fd198def5 17.0 landed