Em seg., 8 de abr. de 2024 às 09:27, Jelte Fennema-Nio
escreveu:
> On Sun, 7 Apr 2024 at 11:34, David Rowley wrote:
> > That seems to require modifying the following function signatures:
> > secure_write(), be_tls_write(), be_gssapi_write(). That's not an area
> > I'm familiar with, however.
>
On Sun, 7 Apr 2024 at 11:34, David Rowley wrote:
> That seems to require modifying the following function signatures:
> secure_write(), be_tls_write(), be_gssapi_write(). That's not an area
> I'm familiar with, however.
Attached is a new patchset where 0003 does exactly that. The only
place wher
Em seg., 8 de abr. de 2024 às 07:42, Jelte Fennema-Nio
escreveu:
> On Sun, 7 Apr 2024 at 14:41, David Rowley wrote:
> > Looking at the code in socket_putmessage_noblock(), I don't understand
> > why it's ok for PqSendBufferSize to be int but "required" must be
> > size_t. There's a line that do
On Sun, 7 Apr 2024 at 14:41, David Rowley wrote:
> Looking at the code in socket_putmessage_noblock(), I don't understand
> why it's ok for PqSendBufferSize to be int but "required" must be
> size_t. There's a line that does "PqSendBufferSize = required;". It
> kinda looks like they both should b
David Rowley , 6 Nis 2024 Cmt, 04:34 tarihinde şunu
yazdı:
> Does anyone else want to try the attached script on the v5 patch to
> see if their numbers are better?
>
I'm seeing the below results with your script on my machine (). I ran it
several times, results were almost similar each time.
mas
Em sáb., 6 de abr. de 2024 às 22:39, Andres Freund
escreveu:
> Hi,
>
> On 2024-04-07 00:45:31 +0200, Jelte Fennema-Nio wrote:
> > On Sat, 6 Apr 2024 at 22:21, Andres Freund wrote:
> > > The small regression for small results is still kinda visible, I
> haven't yet
> > > tested the patch downthre
On Sun, 7 Apr 2024 at 22:05, Jelte Fennema-Nio wrote:
>
> On Sun, 7 Apr 2024 at 03:39, Andres Freund wrote:
> > Changing the global vars to size_t seems mildly bogus to me. All it's
> > achieving is to use slightly more memory. It also just seems unrelated to
> > the
> > change.
>
> I took a clo
On Sun, 7 Apr 2024 at 03:39, Andres Freund wrote:
> Changing the global vars to size_t seems mildly bogus to me. All it's
> achieving is to use slightly more memory. It also just seems unrelated to the
> change.
I took a closer look at this. I agree that changing PqSendBufferSize
to size_t is unn
On Sun, 7 Apr 2024 at 08:21, Andres Freund wrote:
> I added WITH BINARY, SET STORAGE EXTERNAL and tested both unix socket and
> localhost. I also reduced row counts and iteration counts, because I am
> impatient, and I don't think it matters much here. Attached the modified
> version.
Thanks for
Hi,
On 2024-04-07 00:45:31 +0200, Jelte Fennema-Nio wrote:
> On Sat, 6 Apr 2024 at 22:21, Andres Freund wrote:
> > The small regression for small results is still kinda visible, I haven't yet
> > tested the patch downthread.
>
> Thanks a lot for the faster test script, I'm also impatient. I stil
On Sat, 6 Apr 2024 at 22:21, Andres Freund wrote:
> The small regression for small results is still kinda visible, I haven't yet
> tested the patch downthread.
Thanks a lot for the faster test script, I'm also impatient. I still
saw the small regression with David his patch. Here's a v6 where I
t
Hi,
On Fri, 5 Apr 2024 at 03:28, Melih Mutlu
wrote:
>Right. It was a mistake, forgot to remove that. Fixed it in v5.
If you don't mind, I have some suggestions for patch v5.
1. Shouldn't PqSendBufferSize be of type size_t?
There are several comparisons with other size_t variables.
static size_
Hi,
On 2024-04-06 14:34:17 +1300, David Rowley wrote:
> I don't see any issues with v5, so based on the performance numbers
> shown on this thread for the latest patch, it would make sense to push
> it. The problem is, I just can't recreate the performance numbers.
>
> I've tried both on my AMD 3
On Sat, 6 Apr 2024 at 23:17, Jelte Fennema-Nio wrote:
> Weird that on your machines you don't see a difference. Are you sure
> you didn't make a silly mistake, like not restarting postgres or
> something?
I'm sure. I spent quite a long time between the AMD and an Apple m2 trying.
I did see the s
On Sat, 6 Apr 2024 at 03:34, David Rowley wrote:
> Does anyone else want to try the attached script on the v5 patch to
> see if their numbers are better?
On my machine (i9-10900X, in Ubuntu 22.04 on WSL on Windows) v5
consistently beats master by ~0.25 seconds:
master:
Run 100 100 500: 1.948
On Fri, 5 Apr 2024 at 03:28, Melih Mutlu wrote:
>
> Jelte Fennema-Nio , 4 Nis 2024 Per, 16:34 tarihinde şunu
> yazdı:
>>
>> On Thu, 4 Apr 2024 at 13:08, Melih Mutlu wrote:
>> > I changed internal_flush() to an inline function, results look better this
>> > way.
>>
>> It seems you also change in
Jelte Fennema-Nio , 4 Nis 2024 Per, 16:34 tarihinde
şunu yazdı:
> On Thu, 4 Apr 2024 at 13:08, Melih Mutlu wrote:
> > I changed internal_flush() to an inline function, results look better
> this way.
>
> It seems you also change internal_flush_buffer to be inline (but only
> in the actual functio
On Thu, 4 Apr 2024 at 13:08, Melih Mutlu wrote:
> I changed internal_flush() to an inline function, results look better this
> way.
It seems you also change internal_flush_buffer to be inline (but only
in the actual function definition, not declaration at the top). I
don't think inlining interna
Hi,
Melih Mutlu , 28 Mar 2024 Per, 22:44 tarihinde şunu
yazdı:
>
> On Wed, Mar 27, 2024 at 14:39 David Rowley wrote:
>>
>> On Fri, 22 Mar 2024 at 12:46, Melih Mutlu wrote:
>> can you confirm if the test was done in debug with casserts on? If
>> so, it would be much better to have asserts off a
On Wed, Mar 27, 2024 at 18:54 Robert Haas wrote:
> On Wed, Mar 27, 2024 at 7:39 AM David Rowley wrote:
> > Robert, I understand you'd like a bit more from this patch. I'm
> > wondering if you planning on blocking another committer from going
> > ahead with this? Or if you have a reason why the c
On Wed, Mar 27, 2024 at 14:39 David Rowley wrote:
> On Fri, 22 Mar 2024 at 12:46, Melih Mutlu wrote:
> > I did all of the above changes and it seems like those resolved the
> regression issue.
>
> Thanks for adjusting the patch. The numbers do look better, but on
> looking at your test.sh scri
On Wed, Mar 27, 2024 at 7:39 AM David Rowley wrote:
> Robert, I understand you'd like a bit more from this patch. I'm
> wondering if you planning on blocking another committer from going
> ahead with this? Or if you have a reason why the current state of the
> patch is not a meaningful enough impr
On Fri, 22 Mar 2024 at 12:46, Melih Mutlu wrote:
> I did all of the above changes and it seems like those resolved the
> regression issue.
Thanks for adjusting the patch. The numbers do look better, but on
looking at your test.sh script from [1], I see:
meson setup --buildtype debug -Dcassert
Hi,
PSA v3.
Jelte Fennema-Nio , 21 Mar 2024 Per, 12:58 tarihinde
şunu yazdı:
> On Thu, 21 Mar 2024 at 01:24, Melih Mutlu wrote:
> > What if I do a simple comparison like PqSendStart == PqSendPointer
> instead of calling pq_is_send_pending()
>
> Yeah, that sounds worth trying out. So the new sug
Heikki Linnakangas , 14 Mar 2024 Per, 15:46 tarihinde şunu
yazdı:
> On 14/03/2024 13:22, Melih Mutlu wrote:
> > @@ -1282,14 +1283,32 @@ internal_putbytes(const char *s, size_t len)
> > if (internal_flush())
> > return EOF;
> > }
> >
On Thu, 21 Mar 2024 at 22:44, Jelte Fennema-Nio wrote:
>
> On Thu, 21 Mar 2024 at 01:45, David Rowley 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 > PqSendBuffer
On Thu, 21 Mar 2024 at 01:24, Melih Mutlu wrote:
> What if I do a simple comparison like PqSendStart == PqSendPointer instead of
> calling pq_is_send_pending()
Yeah, that sounds worth trying out. So the new suggestions to fix the
perf issues on small message sizes would be:
1. add "inline" to i
On Thu, 21 Mar 2024 at 01:45, David Rowley 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 w
On Thu, 21 Mar 2024 at 13:24, Melih Mutlu wrote:
> 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?
As I understand the code, there's no pro
David Rowley , 21 Mar 2024 Per, 00:54 tarihinde şunu
yazdı:
> On Fri, 15 Mar 2024 at 02:03, Jelte Fennema-Nio
> wrote:
> >
> > On Thu, 14 Mar 2024 at 13:12, Robert Haas wrote:
> > >
> > > On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu
> wrote:
> > > > 1- Even though I expect both the patch and HEA
On Fri, 15 Mar 2024 at 01:46, Heikki Linnakangas wrote:
> - the "(int *) &len)" cast is not ok, and will break visibly on
> big-endian systems where sizeof(int) != sizeof(size_t).
I think fixing this requires adjusting the signature of
internal_flush_buffer() to use size_t instead of int. That
On Fri, 15 Mar 2024 at 02:03, Jelte Fennema-Nio wrote:
>
> On Thu, 14 Mar 2024 at 13:12, Robert Haas wrote:
> >
> > On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu wrote:
> > > 1- Even though I expect both the patch and HEAD behave similarly in case
> > > of small data (case 1: 100 bytes), the patc
On Thu, 14 Mar 2024 at 13:12, Robert Haas wrote:
>
> On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu 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
On 14/03/2024 13:22, Melih Mutlu wrote:
@@ -1282,14 +1283,32 @@ internal_putbytes(const char *s, size_t len)
if (internal_flush())
return EOF;
}
- amount = PqSendBufferSize - PqSendPointer;
- if (a
On Thu, 14 Mar 2024 at 12:22, Melih Mutlu wrote:
> I did some experiments with this patch, after previous discussions
One thing I noticed is that the buffer sizes don't seem to matter much
in your experiments, even though Andres his expectation was that 1400
would be better. I think I know the re
On Thu, Mar 14, 2024 at 7:22 AM Melih Mutlu 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.
--
Robert H
Hi hackers,
I did some experiments with this patch, after previous discussions. This
probably does not answer all questions, but would be happy to do more if
needed.
First, I updated the patch according to what suggested here [1]. PSA v2.
I tweaked the master branch a bit to not allow any buffer
Hi,
On 2024-02-01 15:02:57 -0500, Robert Haas wrote:
> On Thu, Feb 1, 2024 at 10:52 AM Robert Haas wrote:
> There was probably a better way to phrase this email ... the sentiment
> is sincere, but there was almost certainly a way of writing it that
> didn't sound like I'm super-annoyed.
NP - I c
On Thu, Feb 1, 2024 at 10:52 AM Robert Haas wrote:
> On Wed, Jan 31, 2024 at 10:24 PM Andres Freund wrote:
> > While not perfect - e.g. because networks might use jumbo packets / large
> > MTUs
> > and we don't know how many outstanding bytes there are locally, I think a
> > decent heuristic cou
On Wed, Jan 31, 2024 at 10:24 PM Andres Freund wrote:
> While not perfect - e.g. because networks might use jumbo packets / large MTUs
> and we don't know how many outstanding bytes there are locally, I think a
> decent heuristic could be to always try to send at least one packet worth of
> data a
Hi,
On 2024-01-31 14:57:35 -0500, Robert Haas wrote:
> > You're right and I'm open to doing more legwork. I'd also appreciate any
> > suggestion about how to test this properly and/or useful scenarios to
> > test. That would be really helpful.
>
> I think experimenting to see whether the long-shor
On Wed, Jan 31, 2024 at 2:23 PM Melih Mutlu wrote:
>> That seems like it might be a useful refinement of Melih Mutlu's
>> original proposal, but consider a message stream that consists of
>> messages exactly 8kB in size. If that message stream begins when the
>> buffer is empty, all messages are s
Robert Haas , 31 Oca 2024 Çar, 20:23 tarihinde şunu
yazdı:
> On Tue, Jan 30, 2024 at 6:39 PM Jelte Fennema-Nio
> wrote:
> > I agree that it's hard to prove that such heuristics will always be
> > better in practice than the status quo. But I feel like we shouldn't
> > let perfect be the enemy of
On Wed, Jan 31, 2024 at 12:49 PM Jelte Fennema-Nio wrote:
> Testing a bunch of scenarios to find a good one sounds like a good
> idea, which can probably give us a more optimal heuristic. But it also
> sounds like a lot of work, and probably results in a lot of
> discussion. That extra effort migh
On Wed, 31 Jan 2024 at 18:23, Robert Haas wrote:
> That's kind of an odd artifact, but maybe it's fine in
> practice.
I agree it's an odd artifact, but it's not a regression over the
status quo. Achieving that was the intent of my suggestion: A change
that improves some cases, but regresses nowhe
On Tue, Jan 30, 2024 at 6:39 PM Jelte Fennema-Nio wrote:
> I agree that it's hard to prove that such heuristics will always be
> better in practice than the status quo. But I feel like we shouldn't
> let perfect be the enemy of good here.
Sure, I agree.
> I one approach that is a clear
> improve
On Tue, 30 Jan 2024 at 19:48, Robert Haas wrote:
>
> On Tue, Jan 30, 2024 at 12:58 PM Melih Mutlu wrote:
> > Sounds like it's difficult to come up with a heuristic that would work well
> > enough for most cases.
> > One thing with sending data instead of copying it if the buffer is empty is
> >
On Tue, Jan 30, 2024 at 12:58 PM Melih Mutlu wrote:
> Sounds like it's difficult to come up with a heuristic that would work well
> enough for most cases.
> One thing with sending data instead of copying it if the buffer is empty is
> that initially the buffer is empty. I believe it will stay em
Hi Robert,
Robert Haas , 29 Oca 2024 Pzt, 20:48 tarihinde şunu
yazdı:
> > If there's already some data in PqSendBuffer, I wonder if it would be
> > better to fill it up with data, flush it, and then send the rest of the
> > data directly. Instead of flushing the partial data first. I'm afraid
> >
Hi Heikki,
Heikki Linnakangas , 29 Oca 2024 Pzt, 19:12 tarihinde şunu
yazdı:
> > Proposed change modifies socket_putmessage to send any data larger than
> > 8K immediately without copying it into the send buffer. Assuming that
> > the send buffer would be flushed anyway due to reaching its limit,
On Mon, Jan 29, 2024 at 11:12 AM Heikki Linnakangas wrote:
> Agreed, that's silly.
+1.
> If there's already some data in PqSendBuffer, I wonder if it would be
> better to fill it up with data, flush it, and then send the rest of the
> data directly. Instead of flushing the partial data first. I'
On 20/11/2023 14:21, Melih Mutlu wrote:
Hi hackers
I've been looking into ways to reduce the overhead we're having in
pqcomm and I'd like to propose a small patch to modify how
socket_putmessage works.
Currently socket_putmessage copies any input data into the pqcomm send
buffer (PqSendBuff
Hi hackers
I've been looking into ways to reduce the overhead we're having in pqcomm
and I'd like to propose a small patch to modify how socket_putmessage works.
Currently socket_putmessage copies any input data into the pqcomm send
buffer (PqSendBuffer) and the size of this buffer is 8K. When th
53 matches
Mail list logo
