Re: Why is pq_begintypsend so slow?

From: Andres Freund <andres(at)anarazel(dot)de>
To: Sutou Kouhei <kou(at)clear-code(dot)com>
Cc: robertmhaas(at)gmail(dot)com, laurenz(dot)albe(at)cybertec(dot)at, tgl(at)sss(dot)pgh(dot)pa(dot)us, jack(at)jncsoftware(dot)com, david(at)fetter(dot)org, jeff(dot)janes(at)gmail(dot)com, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Why is pq_begintypsend so slow?
Date: 2024-02-19 19:53:51
Message-ID: 20240219195351.5vy7cdl3wxia66kg@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

On 2024-02-19 10:02:52 +0900, Sutou Kouhei wrote:
> In <20240218200906(dot)zvihkrs46yl2juzf(at)awork3(dot)anarazel(dot)de>
> "Re: Why is pq_begintypsend so slow?" on Sun, 18 Feb 2024 12:09:06 -0800,
> Andres Freund <andres(at)anarazel(dot)de> wrote:
>
> >> [1] https://www.postgresql.org/message-id/flat/20240215.153421.96888103784986803.kou%40clear-code.com#34df359e6d255795d16814ce138cc995
>
> >> Do we need one FunctionCallInfoBaseData for each attribute?
> >> How about sharing one FunctionCallInfoBaseData by all
> >> attributes like [1]?
> >
> > That makes no sense to me. You're throwing away most of the possible gains by
> > having to update the FunctionCallInfo fields on every call. You're saving
> > neglegible amounts of memory at a substantial runtime cost.
>
> The number of updated fields of your approach and [1] are
> same:
>
> Your approach: 6 (I think that "fcinfo->isnull = false" is
> needed though.)
>
> + fcinfo->args[0].value = CStringGetDatum(string);
> + fcinfo->args[0].isnull = false;
> + fcinfo->args[1].value = ObjectIdGetDatum(attr->typioparam);
> + fcinfo->args[1].isnull = false;
> + fcinfo->args[2].value = Int32GetDatum(attr->typmod);
> + fcinfo->args[2].isnull = false;
>
> [1]: 6 (including "fcinfo->isnull = false")
>
> + fcinfo->flinfo = flinfo;
> + fcinfo->context = escontext;
> + fcinfo->isnull = false;
> + fcinfo->args[0].value = CStringGetDatum(str);
> + fcinfo->args[1].value = ObjectIdGetDatum(typioparam);
> + fcinfo->args[2].value = Int32GetDatum(typmod);

If you want to do so you can elide the isnull assignments in my approach just
as well as yours. Assigning not just the value but also flinfo and context is
overhead. But you can't elide assigning flinfo and context, which is why
reusing one FunctionCallInfo isn't going to win

I don't think you necessarily need to assign fcinfo->isnull on every call,
these functions aren't allowed to return NULL IIRC. And if they do we'd error
out, so it could only happen once.

> I don't have a strong opinion how to implement this
> optimization as I said above. It seems that you like your
> approach. So I withdraw [1]. Could you complete this
> optimization? Can we proceed making COPY format extensible
> without this optimization? It seems that it'll take a little
> time to complete this optimization because your patch is
> still WIP. And it seems that you can work on it after making
> COPY format extensible.

I don't think optimizing this aspect needs to block making copy extensible.

I don't know how much time/energy I'll have to focus on this in the near
term. I really just reposted this because the earlier patches were relevant
for the discussion in another thread. If you want to pick the COPY part up,
feel free.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2024-02-19 20:47:22 Re: Patch: Add parse_type Function
Previous Message Andres Freund 2024-02-19 19:46:06 Re: PGC_SIGHUP shared_buffers?