Thread: Libpq COPY optimization patch

Here is a patch against today's code 1/24. As discussed in -hackers
consumeInput/parse is removed from being called every single time. It's
still there for only when the data is sent to the server.

Alon.

"Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> Here is a patch against today's code 1/24. As discussed in -hackers
> consumeInput/parse is removed from being called every single time. It's
> still there for only when the data is sent to the server.

This appears to be the exact same patch you sent before.  Did you
test my suggestion of simply removing the PQconsumeInput call?
I see no reason to add it inside the loop.

            regards, tom lane

Tom,

>> Here is a patch against today's code 1/24. As discussed in -hackers
>> consumeInput/parse is removed from being called every single time. It's
>> still there for only when the data is sent to the server.
>
> This appears to be the exact same patch you sent before.  Did you
> test my suggestion of simply removing the PQconsumeInput call?
> I see no reason to add it inside the loop.

My mistake. I'll make the correction.

I guess that although parseInput is cheap we could still use a conditional
to see when data was sent and only then call it (without PQconsumeInput)
instead of calling it every single time PQputCopyData is called. Any
objection to that?

Alon.

"Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> I guess that although parseInput is cheap we could still use a conditional
> to see when data was sent and only then call it (without PQconsumeInput)
> instead of calling it every single time PQputCopyData is called. Any
> objection to that?

You mean something like

    if (input-buffer-not-empty)
        parseInput();

?  This still bothers me a bit since it's a mixing of logic levels;
PQputCopyData is an output routine, it shouldn't be getting its fingers
dirty with input buffer contents.  I'm willing to tolerate this if it
can be demonstrated that it provides a useful performance gain compared
to the unconditional parseInput call, but let's see some numbers.

            regards, tom lane

> You mean something like
>
> if (input-buffer-not-empty)
> parseInput();
>
> ?  This still bothers me a bit since it's a mixing of logic levels;
> PQputCopyData is an output routine, it shouldn't be getting its fingers
> dirty with input buffer contents.  I'm willing to tolerate this if it
> can be demonstrated that it provides a useful performance gain compared
> to the unconditional parseInput call, but let's see some numbers.

Yes, I understand. We'll see what the performance gain is like and see if
it's worth it, I'll report back.

Alon.

Libpq copy speedup patch attached. No input buffer logic involved, just
removing the expensive PQconsumeInput call per putCopyData call, and leaving
parseInput as is, as discussed before.

Alon.

"Alon Goldshuv" <agoldshuv@greenplum.com> writes:
> Libpq copy speedup patch attached. No input buffer logic involved, just
> removing the expensive PQconsumeInput call per putCopyData call, and leaving
> parseInput as is, as discussed before.

Applied (with a better comment) to HEAD and 8.1 branches.

            regards, tom lane