Re: Why is pq_begintypsend so slow?

Hi,

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);

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