Re: SendRowDescriptionMessage() is slow for queries with a lot of columns
Alvaro Herrera <alvherre@alvh.no-ip.org>
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Andres Freund <andres@anarazel.de>
Cc: Robert Haas <robertmhaas@gmail.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2017-10-11T16:05:32Z
Lists: pgsql-hackers
Andres Freund wrote: > On 2017-10-11 10:53:56 +0200, Alvaro Herrera wrote: > > I wonder if it'd be a good idea to nag external users about pq_sendint > > usage (is a #warning possible?). > > Think we'd need some separate infrastructure, i.e. for gcc ending up > with __attribute__((deprecated)). I don't quite see this being worth > adding it, but ... Probably not. > > OTOH, do we really need to keep it > > around? Maybe we should ditch it, since obviously the compat shim can > > be installed locally in each extension that really needs it (thinking > > that most external code can simply be adjusted to the new functions). > > That seems like causing unnecessary pain - we're talking about a few > lines in a header here, right? It's not like they'll be trivially > converting to pq_sendint$width anytime soon, unless we backpatch this. Well, my concern is to ensure that extension authors take advantage of the optimized implementation. If we never let them know that we've rewritten things, most are not going to realize they can make their extensions faster with a very simple code change. On the other hand, I suppose that for the vast majority of extensions, doing the change is unlikely to have a noticeable in performance, so perhaps we should just keep the shim and move along. If do nothing, it's unlikely we'd ever get rid of the compat function. Maybe add a comment suggesting to remove once pg10 is out of support? > > I'm scared about the not-null-terminated stringinfo stuff. Is it > > possible to create bugs by polluting a stringinfo with it, then having > > the stringinfo be used by unsuspecting code? Admittedly, you can break > > things already with the binary appends, so probably not an issue. > > All of the converted sites already add integers into the StringInfo - > and most of the those integers consist out of a couple bytes of 0, > because they're lengths. So I don't think there's a huge danger here. Right, agreed on that. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Commits
-
Replace remaining uses of pq_sendint with pq_sendint{8,16,32}.
- 31079a4a8e66 11.0 landed
-
Improve performance of SendRowDescriptionMessage.
- 4c119fbcd49b 11.0 landed
-
Use one stringbuffer for all rows printed in printtup.c.
- f2dec34e19d3 11.0 landed
-
Add configure infrastructure to detect support for C99's restrict.
- 0b974dba2d6b 11.0 landed
-
Add more efficient functions to pqformat API.
- 1de09ad8eb1f 11.0 landed
-
Allow to avoid NUL-byte management for stringinfos and use in format.c.
- 70c2d1be2b1e 11.0 landed
-
Replace most usages of ntoh[ls] and hton[sl] with pg_bswap.h.
- 0ba99c84e8c7 11.0 landed
-
Extend & revamp pg_bswap.h infrastructure.
- 510b8cbff15f 11.0 landed