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

On Wed, Feb 07, 2024 at 01:33:18PM +0900, Michael Paquier wrote:
> Hmm. That explains why I was not seeing any differences with this
> callback then. It seems to me that the order of actions to take is
> clear, like:
> - Revert 2889fd23be56 to keep a clean state of the tree, now done with
> 1aa8324b81fa.
> - Dive into the strlen() issue, as it really looks like this can
> create more simplifications for the patch discussed on this thread
> with COPY TO.

This has been done this morning with b619852086ed.

> - Revisit what we have here, looking at more profiles to see how HEAD
> an v13 compare. It looks like we are on a good path, but let's tackle
> things one step at a time.

And attached is a v14 that's rebased on HEAD. While on it, I've
looked at more profiles and did more runtime checks.

Some runtimes, in (ms), average of 15 runs, 30 int attributes on 5M
rows as mentioned above:
COPY FROM text binary
HEAD 6066 7110
v14 6087 7105
COPY TO text binary
HEAD 6591 10161
v14 6508 10189

And here are some profiles, where I'm not seeing an impact at
row-level with the addition of the callbacks:
COPY FROM, text, master:
- 66.59% 16.10% postgres postgres [.] NextCopyFrom ▒ - 50.50% NextCopyFrom
- 30.75% NextCopyFromRawFields
+ 15.93% CopyReadLine
13.73% CopyReadAttributesText
- 19.43% InputFunctionCallSafe
+ 13.49% int4in
0.77% pg_strtoint32_safe
+ 16.10% _start
COPY FROM, text, v14:
- 66.42% 0.74% postgres postgres [.] NextCopyFrom
- 65.67% NextCopyFrom
- 65.51% CopyFromTextOneRow
- 30.25% NextCopyFromRawFields
+ 16.14% CopyReadLine
13.40% CopyReadAttributesText
- 18.96% InputFunctionCallSafe
+ 13.15% int4in
0.70% pg_strtoint32_safe
+ 0.74% _start

COPY TO, binary, master
- 90.32% 7.14% postgres postgres [.] CopyOneRowTo
- 83.18% CopyOneRowTo
+ 60.30% SendFunctionCall
+ 10.99% appendBinaryStringInfo
+ 3.67% MemoryContextReset
+ 2.89% CopySendEndOfRow
0.89% memcpy(at)plt
0.66% 0xffffa052db5c
0.62% enlargeStringInfo
0.56% pgstat_progress_update_param
+ 7.14% _start
COPY TO, binary, v14
- 90.96% 0.21% postgres postgres [.] CopyOneRowTo
- 90.75% CopyOneRowTo
- 81.86% CopyToBinaryOneRow
+ 59.17% SendFunctionCall
+ 10.56% appendBinaryStringInfo
1.10% enlargeStringInfo
0.59% int4send
0.57% memcpy(at)plt
+ 3.68% MemoryContextReset
+ 2.83% CopySendEndOfRow
1.13% appendBinaryStringInfo
0.58% SendFunctionCall
0.58% pgstat_progress_update_param

Are there any comments about this v14? Sutou-san?

A next step I think we could take is to split the binary-only and the
text/csv-only data in each cstate into their own structure to make the
structure, with an opaque pointer that custom formats could use, but a
lot of fields are shared as well. This patch is already complicated
enough IMO, so I'm OK to leave it out for the moment, and focus on
making this infra pluggable as a next step.
--
Michael