David Rowley <dgrowle...@gmail.com>, 21 Mar 2024 Per, 00:54 tarihinde şunu yazdı:
> On Fri, 15 Mar 2024 at 02:03, Jelte Fennema-Nio <postg...@jeltef.nl> > wrote: > > > > On Thu, 14 Mar 2024 at 13:12, Robert Haas <robertmh...@gmail.com> wrote: > > > > > > On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu <m.melihmu...@gmail.com> > wrote: > > > > 1- Even though I expect both the patch and HEAD behave similarly in > case of small data (case 1: 100 bytes), the patch runs slightly slower than > HEAD. > > > > > > I wonder why this happens. It seems like maybe something that could be > fixed. > > > > some wild guesses: > > 1. maybe it's the extra call overhead of the new internal_flush > > implementation. What happens if you make that an inline function? > > 2. maybe swap these conditions around (the call seems heavier than a > > simple comparison): !pq_is_send_pending() && len >= PqSendBufferSize > > I agree these are both worth trying. For #2, I wonder if the > pq_is_send_pending() call is even worth checking at all. It seems to > me that the internal_flush_buffer() code will just do nothing if > nothing is pending. Also, isn't there almost always going to be > something pending when the "len >= PqSendBufferSize" condition is met? > We've just added the msgtype and number of bytes to the buffer which > is 5 bytes. If the previous message was also more than > PqSendBufferSize, then the buffer is likely to have 5 bytes due to the > previous flush, otherwise isn't it a 1 in 8192 chance that the buffer > is empty? > > If that fails to resolve the regression, maybe it's worth memcpy()ing > enough bytes out of the message to fill the buffer then flush it and > check if we still have > PqSendBufferSize remaining and skip the > memcpy() for the rest. That way there are no small flushes of just 5 > bytes and only ever the possibility of reducing the flushes as no > pattern should cause the number of flushes to increase. > In len > PqSendBufferSize cases, the buffer should be filled as much as possible if we're sure that it will be flushed at some point. Otherwise we might end up with small flushes. The cases where we're sure that the buffer will be flushed is when the buffer is not empty. If it's empty, there is no need to fill it unnecessarily as it might cause an additional flush. AFAIU from what you said, we shouldn't be worried about such a case since it's unlikely to have the buffer empty due to the first 5 bytes. I guess the only case where the buffer can be empty is when the buffer has PqSendBufferSize-5 bytes from previous messages and adding 5 bytes of the current message will flush the buffer. I'm not sure if removing the check may cause any regression in any case, but it's just there to be safe. What if I do a simple comparison like PqSendStart == PqSendPointer instead of calling pq_is_send_pending() as Heikki suggested, then this check should not hurt that much. Right? Does that make sense? -- Melih Mutlu Microsoft
