Re: Why is pq_begintypsend so slow?

David Fetter <david(at)fetter(dot)org> writes:
> On Sat, Jan 11, 2020 at 03:19:37PM -0500, Tom Lane wrote:
>> Jeff Janes <jeff(dot)janes(at)gmail(dot)com> writes:
>>> I saw that the hotspot was pq_begintypsend at 20%, which was twice the
>>> percentage as the next place winner (AllocSetAlloc).

>> I'd be inclined to replace the appendStringInfoCharMacro calls with
>> appendStringInfoSpaces(buf, 4) --- I don't think we care exactly what
>> is inserted into those bytes at this point. And maybe
>> appendStringInfoSpaces could stand some micro-optimization, too.
>> Use a memset and a single len adjustment, perhaps?

> Please find attached a patch that does it both of the things you
> suggested.

I've been fooling around with this here. On the test case Jeff
describes --- COPY BINARY tab TO '/dev/null' where tab contains
100 float8 columns filled from random() --- I can reproduce his
results. pq_begintypsend is the top hotspot and if perf's
localization is accurate, it's the instructions that fetch
str->len that hurt the most. Still not very clear why that is...

Converting pq_begintypsend to use appendStringInfoSpaces helps
a bit; it takes my test case from 91725 ms to 88847 ms, or about
3% savings. Noodling around with appendStringInfoSpaces doesn't
help noticeably; I tried memset, as well as open-coding (cf
patch below) but the results were all well within the noise
threshold.

I saw at this point that the remaining top spots were
enlargeStringInfo and appendBinaryStringInfo, so I experimented
with inlining them (again, see patch below). That *did* move
the needle: down to 72691 ms, or 20% better than HEAD. Of
course, that comes at a code-size cost, but it's only about
13kB growth:

before:
$ size src/backend/postgres
text data bss dec hex filename
7485285 58088 203328 7746701 76348d src/backend/postgres
after:
$ size src/backend/postgres
text data bss dec hex filename
7498652 58088 203328 7760068 7668c4 src/backend/postgres

That's under two-tenths of a percent. (This'd affect frontend
executables too, and I didn't check them.)

Seems like this is worth pursuing, especially if it can be
shown to improve any other cases noticeably. It might be
worth inlining some of the other trivial stringinfo functions,
though I'd tread carefully on that.

regards, tom lane