MIME-Version: 1.0
References: <CAD21AoBfLWbe3GtD3E8zLJtzvk49=ho21j8drfp6GwdbhLD=LQ@mail.gmail.com>
 <CAD21AoAdj-EJOH1o2fTLke-uskSvuenT--fKW9nkLzYcLwU_eg@mail.gmail.com>
 <20241105.174328.1705956947135248653.kou@clear-code.com> <20241114.161948.1677325020727842666.kou@clear-code.com>
In-Reply-To: <20241114.161948.1677325020727842666.kou@clear-code.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Thu, 14 Nov 2024 16:04:41 -0800
Message-ID: <CAD21AoBhzGa0XyeOv5bCy5LxC+WrpufA+pfrrdZeK4XnTzF7kw@mail.gmail.com>
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
To: Sutou Kouhei <kou@clear-code.com>
Cc: michael@paquier.xyz, pgsql-hackers@postgresql.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAD21AoBhzGa0XyeOv5bCy5LxC%2BWrpufA%2BpfrrdZeK4XnTzF7kw%40mail.gmail.com>
Precedence: bulk

On Wed, Nov 13, 2024 at 11:19=E2=80=AFPM Sutou Kouhei <kou@clear-code.com> =
wrote:
>
> Hi,
>
> In <20241105.174328.1705956947135248653.kou@clear-code.com>
>   "Re: Make COPY format extendable: Extract COPY TO format implementation=
s" on Tue, 05 Nov 2024 17:43:28 +0900 (JST),
>   Sutou Kouhei <kou@clear-code.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,


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