Re: Speedup generation of command completion tags

From: Andres Freund <andres(at)anarazel(dot)de>
To: David Rowley <dgrowley(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Speedup generation of command completion tags
Date: 2022-12-10 22:05:01
Message-ID: 20221210220501.h7xriuvq72jnbdn6@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2022-12-10 20:32:06 +1300, David Rowley wrote:
> @@ -20,13 +20,14 @@
> typedef struct CommandTagBehavior
> {
> const char *name;
> + const uint8 namelen;

Perhaps worth adding a comment noting that namelen is the length without the
null byte?

> +static inline Size
> +make_completion_tag(char *buff, const QueryCompletion *qc,
> + bool force_undecorated_output)
> +{
> + CommandTag tag = qc->commandTag;
> + Size taglen;
> + const char *tagname = GetCommandTagNameAndLen(tag, &taglen);
> + char *bufp;
> +
> + /*
> + * We assume the tagname is plain ASCII and therefore requires no encoding
> + * conversion.
> + */
> + memcpy(buff, tagname, taglen + 1);
> + bufp = buff + taglen;
> +
> + /* ensure that the tagname isn't long enough to overrun the buffer */
> + Assert(taglen <= COMPLETION_TAG_BUFSIZE - MAXINT8LEN - 4);
> +
> + /*
> + * In PostgreSQL versions 11 and earlier, it was possible to create a
> + * table WITH OIDS. When inserting into such a table, INSERT used to
> + * include the Oid of the inserted record in the completion tag. To
> + * maintain compatibility in the wire protocol, we now write a "0" (for
> + * InvalidOid) in the location where we once wrote the new record's Oid.
> + */
> + if (command_tag_display_rowcount(tag) && !force_undecorated_output)

This does another external function call to cmdtag.c...

What about moving make_completion_tag() to cmdtag.c? Then we could just get
the entire CommandTagBehaviour struct at once. It's not super pretty to pass
QueryCompletion to a routine in cmdtag.c, but it's not awful. And if we deem
it problematic, we could just pass qc->commandTag, qc->nprocessed as a
separate arguments.

I wonder if any of the other GetCommandTagName() would benefit noticably from
not having to compute the length. I guess the calls
set_ps_display(GetCommandTagName()) calls in exec_simple_query() and
exec_execute_message() might, although set_ps_display() isn't exactly zero
overhead. But I do see it show up as a few percent in profiles, with the
biggest contributor being the call to strlen.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2022-12-10 23:11:35 Re: Error-safe user functions
Previous Message Tom Lane 2022-12-10 21:01:24 Re: Error-safe user functions