From: Sutou Kouhei <kou@clear-code.com>
To: andres@anarazel.de
Cc: robertmhaas@gmail.com
Cc: laurenz.albe@cybertec.at
Cc: tgl@sss.pgh.pa.us
Cc: jack@jncsoftware.com
Cc: david@fetter.org
Cc: jeff.janes@gmail.com
Cc: pgsql-hackers@postgresql.org
Subject: Re: Why is pq_begintypsend so slow?
Date: Mon, 19 Feb 2024 10:02:52 +0900 (JST)
Message-ID: <20240219.100252.1610549091537921652.kou@clear-code.com> (raw)
In-Reply-To: <20240218200906.zvihkrs46yl2juzf@awork3.anarazel.de>
References: <20240218015955.rmw5mcmobt5hbene@awork3.anarazel.de>
<20240218.173809.707692226756628288.kou@clear-code.com>
<20240218200906.zvihkrs46yl2juzf@awork3.anarazel.de>
Hi,
In <20240218200906.zvihkrs46yl2juzf@awork3.anarazel.de>
"Re: Why is pq_begintypsend so slow?" on Sun, 18 Feb 2024 12:09:06 -0800,
Andres Freund <andres@anarazel.de> wrote:
>> [1] https://www.postgresql.org/message-id/flat/20240215.153421.96888103784986803.kou%40clear-code.com#34...>> 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);
>> Inlining InputFuncallCallSafe() here to use pre-initialized
>> fcinfo will decrease maintainability. Because we abstract
>> InputFunctionCall family in fmgr.c. If we define a
>> InputFunctionCall variant here, we need to change both of
>> fmgr.c and here when InputFunctionCall family is changed.
>> How about defining details in fmgr.c and call it here
>> instead like [1]?
>
> I'm not sure I buy that that is a problem. It's not like my approach was
> actually bypassing fmgr abstractions alltogether - instead it just used the
> lower level APIs, because it's a performance sensitive area.
[1] provides some optimized abstractions, which are
implemented with lower level APIs, without breaking the
abstractions.
Note that I don't have a strong opinion how to implement
this optimization. If other developers think this approach
makes sense for this optimization, I don't object it.
>> @@ -1966,7 +1992,7 @@ CopyReadBinaryAttribute(CopyFromState cstate, FmgrInfo *flinfo,
>> if (fld_size == -1)
>> {
>> *isnull = true;
>> - return ReceiveFunctionCall(flinfo, NULL, typioparam, typmod);
>> + return ReceiveFunctionCall(fcinfo->flinfo, NULL, attr->typioparam, attr->typmod);
>>
>> Why pre-initialized fcinfo isn't used here?
>
> Because it's a prototype and because I don't think it's a common path.
How about adding a comment why we don't need to optimize
this case?
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.
Thanks,
--
kou
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: kou@clear-code.com, andres@anarazel.de, robertmhaas@gmail.com, laurenz.albe@cybertec.at, tgl@sss.pgh.pa.us, jack@jncsoftware.com, david@fetter.org, jeff.janes@gmail.com
Subject: Re: Why is pq_begintypsend so slow?
In-Reply-To: <20240219.100252.1610549091537921652.kou@clear-code.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox