Re: Speedup generation of command completion tags - Mailing list pgsql-hackers

From Andres Freund
Subject Re: Speedup generation of command completion tags
Date
Msg-id 20221210220501.h7xriuvq72jnbdn6@awork3.anarazel.de
Whole thread Raw
In response to Speedup generation of command completion tags  (David Rowley <dgrowley@gmail.com>)
Responses Re: Speedup generation of command completion tags  (David Rowley <dgrowleyml@gmail.com>)
List 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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: Error-safe user functions
Next
From: Andrew Dunstan
Date:
Subject: Re: Error-safe user functions