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
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? |