On Fri, 23 Dec 2022 at 22:04, Peter Eisentraut <peter.eisentr...@enterprisedb.com> wrote: > > On 19.12.22 23:48, David Rowley wrote: > > On Tue, 20 Dec 2022 at 11:42, Tom Lane <t...@sss.pgh.pa.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
