Re: appendBinaryStringInfo stuff

On Fri, 23 Dec 2022 at 22:04, Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> On 19.12.22 23:48, David Rowley wrote:
> > On Tue, 20 Dec 2022 at 11:42, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >> I think Peter is entirely right to question whether *this* type's
> >> output function is performance-critical. Who's got large tables with
> >> jsonpath columns? It seems to me the type would mostly only exist
> >> as constants within queries.
> >
> > The patch touches code in the path of jsonb's output function too. I
> > don't think you could claim the same for that.
>
> Ok, let's leave the jsonb output alone. The jsonb output code also
> won't change a lot, but there is a bunch of stuff for jsonpath on the
> horizon, so having some more robust coding style to imitate there seems
> useful. Here is another patch set with the jsonb changes omitted.

Maybe if there's concern that inlining appendStringInfoString is going
to bloat the binary too much, then how about we just invent an inlined
version of it using some other name that we can use when performance
matters? We could then safely replace the offending
appendBinaryStringInfos from both places without any concern for
regressing performance.

FWIW, I just did a few compilation runs of our supported versions to
see how much postgres binary grew release to release:

branch postgres binary size growth bytes
REL_10_STABLE 8230232 0
REL_11_STABLE 8586024 355792
REL_12_STABLE 8831664 245640
REL_13_STABLE 8990824 159160
REL_14_STABLE 9484848 494024
REL_15_STABLE 9744680 259832
master 9977896 233216
inline_asis 10004032 26136

(inlined_asis = inlined appendStringInfoString)

On the other hand, if we went with inlining the existing function,
then it looks to be about 10% of the growth we saw between v14 and
v15. That seems quite large.

David