Re: Why is pq_begintypsend so slow?

Hi,

On 2020-01-11 22:32:45 -0500, Tom Lane wrote:
> I wrote:
> > 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.
>
> Oh ... marking the test in the inline part of enlargeStringInfo()
> as unlikely() helps quite a bit more: 66100 ms, a further 9% gain.
> Might be over-optimizing for this particular case, perhaps
>
> (But ... I'm not finding these numbers to be super reproducible
> across different ASLR layouts. So take it with a grain of salt.)

FWIW, I've also observed, in another thread (the node func generation
thing [1]), that inlining enlargeStringInfo() helps a lot, especially
when inlining some of its callers. Moving e.g. appendStringInfo() inline
allows the compiler to sometimes optimize away the strlen. But if
e.g. an inlined appendBinaryStringInfo() still calls enlargeStringInfo()
unconditionally, successive appends cannot optimize away memory accesses
for ->len/->data.

For the case of send functions, we really ought to have at least
pq_begintypsend(), enlargeStringInfo() and pq_endtypsend() inline. That
way the compiler ought to be able to avoid repeatedly loading/storing
->len, after the initial initStringInfo() call. Might even make sense to
also have initStringInfo() inline, because then the compiler would
probably never actually materialize the StringInfoData (and would
automatically have good aliasing information too).

The commit referenced above is obviously quite WIP-ey, and contains
things that should be split into separate commits. But I think it might
be worth moving more functions into the header, like I've done in that
commit.

The commit also adds appendStringInfoU?Int(32,64) operations - I've
unsuprisingly found these to be *considerably* faster than going through
appendStringInfoString().

> but I think that's a reasonable marking given that we overallocate
> the stringinfo buffer for most uses.

Wonder if it's worth providing a function to initialize the stringinfo
differently for the many cases where we have at least a very good idea
of how long the string will be. It's sad to allocate 1kb just for
e.g. int4send to send an integer plus length.

Greetings,

Andres Freund

[1] https://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=commit;h=127e860cf65f50434e0bb97acbba4b0ea6f38cfd