Re: SendRowDescriptionMessage() is slow for queries with a lot of columns

On Tue, Oct 3, 2017 at 3:55 AM, Andres Freund <andres@anarazel.de> wrote:
> Attached is a revised version of this patchset.

I don't much like the functions with "_pre" affixed to their names.
It's not at all clear that "pre" means "preallocated"; it sounds more
like you're doing something ahead of time.  I wonder about maybe
calling these e.g. pq_writeint16, with "write" meaning "assume
preallocation" and "send" meaning "don't assume preallocation".  There
could be other ideas, too.

> I'd like to get some
> input on two points:
>
> 1) Does anybody have a better idea than the static buffer in
>    SendRowDescriptionMessage()? That's not particularly pretty, but
>    there's not really a convenient stringbuffer to use when called from
>    exec_describe_portal_message(). We could instead create a local
>    buffer for exec_describe_portal_message().
>
>    An alternative idea would be to have one reeusable buffer created for
>    each transaction command, but I'm not sure that's really better.

I don't have a better idea.

> 2) There's a lot of remaining pq_sendint() callers in other parts of the
>    tree. If others are ok with that, I'd do a separate pass over them.
>    I'd say that even after doing that, we should keep pq_sendint(),
>    because a lot of extension code is using that.

I think we should change everything to the new style and I wouldn't
object to removing pq_sendint() either.  However, if we want to keep
it with a note that only extension code should use it, that's OK with
me, too.

> 3) The use of restrict, with a configure based fallback, is something
>    we've not done before, but it's C99 and delivers significantly more
>    efficient code. Any arguments against?

It's pretty unobvious why it helps here.  I think you should add
comments.  Also, unless I'm missing something, there's nothing to keep
pq_sendintXX_pre from causing an alignment fault except unbridled
optimism...

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company