MIME-Version: 1.0
References: <CA+zULE5AYd+78w7Uu7jN=5N3w-9vsKuK2WrpjkKwS65hyhWfbw@mail.gmail.com>
 <CALj2ACXiC3Wy7o=81j8jpY=nQq3Hs4y+06gXDkOBrmoO+JDrrw@mail.gmail.com>
 <CA+zULE6bV9yg9+=u0KAhjz-C_YpDr6bh5YknkffgqDHiKWugOg@mail.gmail.com>
 <CA+zULE56RADYr4R=ss=bvrzA2r6PyzWE98Gkkwr_7nOzKsSd-A@mail.gmail.com>
 <CALj2ACVaBEbFnBwJ44Uuy7xwr50M+wwTJoEz0C9dFOc=iZeTDw@mail.gmail.com>
 <CA+zULE6aB9Y_CgT_rmGFPpXUuEANmVXi7_RbrZLH7EPhzy3-OA@mail.gmail.com>
 <CA+zULE5BoZtkYMf9yPGH+BnPEPspXENVCZd_b4s=ayewU6DHZg@mail.gmail.com>
 <4FCF6DCF-231C-456A-99D0-8A7FE701AD17@yesql.se> <CA+zULE5M8CujFba55g4NkGSSgRhxLeUhZbX+Q2oBivBcrmYyzQ@mail.gmail.com>
 <CAGECzQQYS4V5wR27DV8x8oQosMAkW1D6gZDUETpc1jkCw6vSGQ@mail.gmail.com>
In-Reply-To: <CAGECzQQYS4V5wR27DV8x8oQosMAkW1D6gZDUETpc1jkCw6vSGQ@mail.gmail.com>
From: Jeroen Vermeulen <jtvjtv@gmail.com>
Date: Fri, 3 Mar 2023 17:16:05 +0100
Message-ID: <CA+zULE5aADii2Ju9sZ=J4idQ7Ey=Z=r3zgAQN4jeKZ=VcNt8rw@mail.gmail.com>
Subject: Re: libpq: PQgetCopyData() and allocation overhead
To: Jelte Fennema <postgres@jeltef.nl>
Cc: daniel@yesql.se, 
	Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000c8b50805f60143dd"
Archived-At: <https://www.postgresql.org/message-id/CA%2BzULE5aADii2Ju9sZ%3DJ4idQ7Ey%3DZ%3Dr3zgAQN4jeKZ%3DVcNt8rw%40mail.gmail.com>
Precedence: bulk

--000000000000c8b50805f60143dd
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Thank you.

I meant to try PQExpBuffer =E2=80=94 as you say, it fits much better with e=
xisting
libpq style.  But then I hit on the callback idea which was even more
efficient, by a fair margin.  It was also considerably simpler both inside
libpq and in the client code, eliminating all sorts of awkward questions
about who deallocates the buffer in which situations.  So I ditched the
"dynamic buffer" concept and went with the callback.

One other possible alternative suggests itself: instead of taking a
callback and a context pointer, the function could probably just return a
struct: status/size, and buffer.  Then the caller would have to figure out
whether there's a line in the buffer, and if so, process it.  It seems like
more work for the client code, but it may make the compiler's optimisation
work easier.


Jeroen

On Fri, 3 Mar 2023 at 16:52, Jelte Fennema <postgres@jeltef.nl> wrote:

> On Thu, 2 Mar 2023 at 20:45, Jeroen Vermeulen <jtvjtv@gmail.com> wrote:
> > I'm attaching a diff now.  This is not a patch, it's just a discussion
> piece.
>
> Did you try with PQExpBuffer? I still think that probably fits better
> in the API design of libpq. Obviously if it's significantly slower
> than the callback approach in this patch then it's worth considering
> using the callback approach. Overall it definitely seems reasonable to
> me to have an API that doesn't do an allocation per row.
>

--000000000000c8b50805f60143dd
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div>Thank you.</div><div><br></div><div>I meant to try PQ=
ExpBuffer =E2=80=94 as you say, it fits much better with existing libpq sty=
le.=C2=A0 But then I hit on the callback idea which was even more efficient=
, by a fair margin.=C2=A0 It was also considerably simpler both inside libp=
q and in the client code, eliminating all sorts of awkward questions about =
who deallocates the buffer in which situations.=C2=A0 So I ditched the  &qu=
ot;dynamic buffer&quot; concept and went with the callback.<br></div><div><=
br></div><div>One other possible alternative suggests itself: instead of ta=
king a callback and a context pointer, the function could probably just ret=
urn a struct: status/size, and buffer.=C2=A0 Then the caller would have to =
figure out whether there&#39;s a line in the buffer, and if so, process it.=
=C2=A0 It seems like more work for the client code, but it may make the com=
piler&#39;s optimisation work easier.<br></div><div><br></div><div><br></di=
v><div>Jeroen<br></div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr=
" class=3D"gmail_attr">On Fri, 3 Mar 2023 at 16:52, Jelte Fennema &lt;<a hr=
ef=3D"mailto:postgres@jeltef.nl">postgres@jeltef.nl</a>&gt; wrote:<br></div=
><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border=
-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, 2 Mar 2023 at 20=
:45, Jeroen Vermeulen &lt;<a href=3D"mailto:jtvjtv@gmail.com" target=3D"_bl=
ank">jtvjtv@gmail.com</a>&gt; wrote:<br>
&gt; I&#39;m attaching a diff now.=C2=A0 This is not a patch, it&#39;s just=
 a discussion piece.<br>
<br>
Did you try with PQExpBuffer? I still think that probably fits better<br>
in the API design of libpq. Obviously if it&#39;s significantly slower<br>
than the callback approach in this patch then it&#39;s worth considering<br=
>
using the callback approach. Overall it definitely seems reasonable to<br>
me to have an API that doesn&#39;t do an allocation per row.<br>
</blockquote></div>

--000000000000c8b50805f60143dd--