Re: Improvements in Copy From

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: David Rowley <dgrowleyml(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Improvements in Copy From
Date: 2020-09-09 06:53:31
Message-ID: CAHut+PsPhcpXtcojEh5KOdZTg7eVVSfMh3b8yLkwUKnRSRAUSA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

My basic understanding of first part of your patch is that by
adjusting the "minread" it now allows it to loop multiple times
internally within the CopyGetData rather than calling CopyLoadRawBuf
for every N lines. There doesn't seem to be much change to what other
code gets executed so the saving is essentially whatever is the cost
of making 2 x function calls (CopyLoadRawBuff + CopyGetData) x N. Is
that understanding correct?

But with that change there seems to be opportunity for yet another
tiny saving possible. IIUC, now you are processing a lot more data
within the CopyGetData so it is now very likely that you will also
gobble the COPY_NEW_FE's 'c' marker. So cstate->reached_eof will be
set. So this means the calling code of CopyReadLineText may no longer
need to call the CopyLoadRawBuf one last time just to discover there
are no more bytes to read - something that it already knows if
cstate->reached_eof == true.

For example, with your change can't you also modify CopyReadLineText like below:

BEFORE
if (!CopyLoadRawBuf(cstate))
hit_eof = true;

AFTER
if (cstate->reached_eof)
{
cstate->raw_buf[0] = '\0';
cstate->raw_buf_index = cstate->raw_buf_len = 0;
hit_eof = true;
}
else if (!CopyLoadRawBuf(cstate))
{
hit_eof = true;
}

Whether such a micro-optimisation is worth doing is another question.

---

Kind Regards,
Peter Smith.
Fujitsu Australia

On Sun, Aug 30, 2020 at 5:25 PM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, Aug 27, 2020 at 11:02 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >
> > Hello.
> >
> > FYI - that patch has conflicts when applied.
> >
>
> Thanks for letting me know. Attached new patch which is rebased on top of head.
>
> Regards,
> VIgnesh
> EnterpriseDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Darafei Komяpa Praliaskouski 2020-09-09 07:05:04 Re: Yet another fast GiST build
Previous Message Andrey M. Borodin 2020-09-09 06:43:40 Re: Yet another fast GiST build