On Thu, 21 Mar 2024 at 22:44, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:
>
> On Thu, 21 Mar 2024 at 01:45, David Rowley <dgrowleyml@gmail.com> wrote:
> > As I understand the code, there's no problem calling
> > internal_flush_buffer() when the buffer is empty and I suspect that if
> > we're sending a few buffers with "len > PqSendBufferSize" that it's
> > just so unlikely that the buffer is empty that we should just do the
> > function call and let internal_flush_buffer() handle doing nothing if
> > the buffer really is empty. I think the chances of
> > internal_flush_buffer() having to do exactly nothing here is less than
> > 1 in 8192, so I just don't think the check is worthwhile.
>
> I think you're missing the exact case that we're trying to improve
> here: Calls to internal_putbytes with a very large len, e.g. 1MB.
> With the new code the buffer will be empty ~50% of the time (not less
> than 1 in 8192) with such large buffers, because the flow that will
> happen:
It was the code I misread. I understand what the aim is. I failed to
notice the while loop in internal_putbytes(). So what I mentioned
about trying to fill the buffer before flushing already happens. I
now agree that the PqSendStart == PqSendPointer test. I'd say since
the reported regression was with 100 byte rows that testing "len >=
PqSendBufferSize" before PqSendStart == PqSendPointer makes sense.
David