On Mon, Nov 18, 2024 at 5:31 PM Sutou Kouhei <k...@clear-code.com> wrote: > > Hi, > > In <CAD21AoC=DX5QQVb27C6UdpPfY-F=-PGnQ1u6rWo69DV=4et...@mail.gmail.com> > "Re: Make COPY format extendable: Extract COPY TO format implementations" > on Mon, 18 Nov 2024 17:02:41 -0800, > Masahiko Sawada <sawada.m...@gmail.com> wrote: > > > I have a question about v22. We use pg_attribute_always_inline for > > some functions to avoid function call overheads. Applying it to > > CopyToTextLikeOneRow() and CopyFromTextLikeOneRow() are legitimate as > > we've discussed. But there are more function where the patch applied > > it to: > > > > -bool > > -NextCopyFromRawFields(CopyFromState cstate, char ***fields, int *nfields) > > +static pg_attribute_always_inline bool > > +NextCopyFromRawFields(CopyFromState cstate, char ***fields, int > > *nfields, bool is_csv) > > > > -static bool > > -CopyReadLineText(CopyFromState cstate) > > +static pg_attribute_always_inline bool > > +CopyReadLineText(CopyFromState cstate, bool is_csv) > > > > +static pg_attribute_always_inline void > > +CopyToTextLikeSendEndOfRow(CopyToState cstate) > > > > I think it's out of scope of this patch even if these changes are > > legitimate. Is there any reason for these changes? > > Yes for NextCopyFromRawFields() and CopyReadLineText(). > No for CopyToTextLikeSendEndOfRow(). > > NextCopyFromRawFields() and CopyReadLineText() have "bool > is_csv". So I think that we should use > pg_attribute_always_inline (or inline) like > CopyToTextLikeOneRow() and CopyFromTextLikeOneRow(). I think > that it's not out of scope of this patch because it's a part > of CopyToTextLikeOneRow() and CopyFromTextLikeOneRow() > optimization. > > Note: The optimization is based on "bool is_csv" parameter > and constant "true"/"false" argument function call. If we > can inline this function call, all "if (is_csv)" checks in > the function are removed.
Understood, thank you for pointing this out. > > pg_attribute_always_inline (or inline) for > CopyToTextLikeSendEndOfRow() is out of scope of this > patch. You're right. > > I think that inlining CopyToTextLikeSendEndOfRow() is better > because it's called per row. But it's not related to the > optimization. > > > Should I create a new patch set without > pg_attribute_always_inline/inline for > CopyToTextLikeSendEndOfRow()? Or could you remove it when > you push? Since I'm reviewing the patch and the patch organization I'll include it. Regards, -- Masahiko Sawada Amazon Web Services: https://aws.amazon.com
