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

On Thu, Oct 10, 2024 at 3:55 PM Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> wrote:
>
> On Tue, Oct 8, 2024 at 8:34 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
> >
> > On Mon, Oct 07, 2024 at 03:23:08PM -0700, Masahiko Sawada wrote:
> > > In the benchmark, I've applied the v20 patch set and 'master' in the
> > > result refers to a19f83f87966. And I disabled CPU turbo boost where
> > > possible. Overall, v20 patch got a similar or better performance in
> > > both COPY FROM and COPY TO compared to master except for on MacOS.
> > > I'm not sure that changes made to master since the last benchmark run by
> > > Tomas and Suto-san might contribute to these results.
> >
> > Don't think so. FWIW, I have been looking at the set of tests with
> > previous patch versions around v7 and v10 I have done, and did notice
> > a similar pattern where COPY FROM was getting slightly better for text
> > and binary. It did not look like only noise involved, and it was
> > kind of reproducible. As long as we avoid the function pointer
> > redirection for the per-row processing when dealing with in-core
> > formats, we should be fine as far as I understand. That's what the
> > latest patch set is doing based on a read of v21.
>
> Yeah, what v21 patch is doing makes sense to me.

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:

diff --git a/src/backend/commands/copyfromparse.c
b/src/backend/commands/copyfromparse.c
index 1a5e1ef711..9f8f839d6c 100644
--- a/src/backend/commands/copyfromparse.c
+++ b/src/backend/commands/copyfromparse.c
@@ -861,7 +861,7 @@ NextCopyFromRawFields(CopyFromState cstate, char
***fields, int *nfields, bool i
*
* Workhorse for CopyFromTextOneRow() and CopyFromCSVOneRow().
*/
-static inline bool
+static pg_attribute_always_inline bool
CopyFromTextLikeOneRow(CopyFromState cstate,
ExprContext *econtext,
Datum *values,
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index aa99459b26..fde2d41316 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -170,7 +170,7 @@ CopyToTextLikeOutFunc(CopyToState cstate, Oid
atttypid, FmgrInfo *finfo)
*
* Workhorse for CopyToTextOneRow() and CopyToCSVOneRow().
*/
-static inline void
+static pg_attribute_always_inline void
CopyToTextLikeOneRow(CopyToState cstate,
TupleTableSlot *slot,
bool is_csv)

In my environment (RHEL 8.9, Intel Xeon Platinum 8375C, EC2 m6id.metal
instance, GCC 12.2.0), the performance got better with
pg_attribute_always_inline. I've confirmed that for the case where
there is a performance regression, that function was not actually
inlined by checking the object file.

Regards,

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