Re: Make COPY format extendable: Extract COPY TO format implementations

On Wed, Nov 13, 2024 at 11:19 PM Sutou Kouhei <kou(at)clear-code(dot)com> wrote:
>
> Hi,
>
> In <20241105(dot)174328(dot)1705956947135248653(dot)kou(at)clear-code(dot)com>
> "Re: Make COPY format extendable: Extract COPY TO format implementations" on Tue, 05 Nov 2024 17:43:28 +0900 (JST),
> Sutou Kouhei <kou(at)clear-code(dot)com> wrote:
>
> >> I've further investigated the performance regression, and found out it
> >> might be relevant that the compiler doesn't inline the
> >> CopyFromTextLikeOneRow() function. It might be worth testing with
> >> pg_attribute_always_inline instead of 'inline' as below:
> >
> > Wow! Good catch!
> >
> > I've rebased on the current master and updated the v20 and
> > v21 patch sets with "pg_attribute_always_inline" not
> > "inline".
> >
> > The v22 patch set is for the v20 patch set.
> > (TO/FROM changes are in one commit.)
> >
> > The v23 patch set is for the v21 patch set.
> > (TO/FROM changes are separated for easy to merge only FROM
> > or TO part.)
>
> I've run benchmark on my machine that has "Intel(R) Core(TM)
> i7-3770 CPU @ 3.40GHz".
>
> Summary:
> * "pg_attribute_always_inline" is effective for the "COPY
> FROM" part
> * "pg_attribute_always_inline" may not be needed for the
> "COPY TO" part
>
>
> v20-result.pdf: This is the same result PDF attached in
> https://www.postgresql.org/message-id/20241008.173918.995935870630354246.kou%40clear-code.com
> . This is the base line for "pg_attribute_always_inline"
> change.
>
> v22-result.pdf: This is a new result PDF for the v22 patch
> set.
>
> COPY FROM:
>
> 0001: The v22 patch set is slower than HEAD. This just
> introduces "routine" abstraction. It increases overhead. So
> this is expected.
>
> 0002-0005: The v22 patch set is faster than HEAD for all
> cases. The v20 patch set is slower than HEAD for smaller
> data. This shows that "pg_attribute_always_inline" for the
> "COPY FROM" part is effective on my machine too.
>
>
> COPY TO:
>
> 0001: The v22 patch set is slower than HEAD. This is
> as expected for the same reason as COPY FROM.
>
> 0002-0004: The v22 patch set is slower than HEAD. (The v20
> patch set is faster than HEAD.) This is not expected.
>
> 0005: The v22 patch set is faster than HEAD. This is
> expected. But 0005 just exports some functions. It doesn't
> change existing logic. So it's strange...
>
> This shows "pg_attribute_always_inline" is needless for the
> "COPY TO" part.
>
>
> I also tried the v24 patch set:
> * The "COPY FROM" part is same as the v22 patch set
> ("pg_attribute_always_inline" is used.)
> * The "COPY TO" part is same as the v20 patch set
> ("pg_attribute_always_inline" is NOT used.)
>
>
> (I think that the v24 patch set isn't useful for others. So
> I don't share it here. If you're interested in it, I'll
> share it here.)
>
> v24-result.pdf:
>
> COPY FROM: The same trend as the v22 patch set result. It's
> expected because the "COPY FROM" part is the same as the v22
> patch set.
>
> COPY TO: The v24 patch set is faster than the v22 patch set
> but the v24 patch set isn't same trend as the v20 patch
> set. This is not expected because the "COPY TO" part is the
> same as the v20 patch set.
>
>
> Anyway, the 0005 "COPY TO" parts are always faster than
> HEAD. So we can use either "pg_attribute_always_inline" or
> "inline".
>
>
> Summary:
> * "pg_attribute_always_inline" is effective for the "COPY
> FROM" part
> * "pg_attribute_always_inline" may not be needed for the
> "COPY TO" part
>
>
> Can we proceed this proposal with these results? Or should
> we wait for more benchmark results?

Thank you for sharing the benchmark test results! I think these
results are good for us to proceed. I'll closely look at COPY TO
results and review v22 patch sets.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com