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

From: Sutou Kouhei <kou(at)clear-code(dot)com>
To: sawada(dot)mshk(at)gmail(dot)com
Cc: michael(at)paquier(dot)xyz, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Make COPY format extendable: Extract COPY TO format implementations
Date: 2024-11-14 07:19:48
Message-ID: 20241114.161948.1677325020727842666.kou@clear-code.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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?

Thanks,
--
kou

Attachment Content-Type Size
v20-intel-core-i7-3770-result.pdf application/pdf 122.6 KB
v22-intel-core-i7-3770-result.pdf application/pdf 142.4 KB
v24-intel-core-i7-3770-result.pdf application/pdf 132.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Ashutosh Bapat 2024-11-14 07:24:52 Re: logical replication: restart_lsn can go backwards (and more), seems broken since 9.4
Previous Message vignesh C 2024-11-14 06:52:44 Re: Disallow UPDATE/DELETE on table with unpublished generated column as REPLICA IDENTITY