You are viewing a plain text version of this content. The canonical link for it is here.
Posted to proton@qpid.apache.org by Clebert Suconic <cs...@redhat.com> on 2014/07/02 17:40:10 UTC
Important!!! Please revert PROTON-597
commit ccb7b03b8089c2fea329d44cea5dc06da6345ea6
Author: Robert Gemmell <ro...@apache.org>
Date: Fri Jun 27 10:08:24 2014 +0000
PROTON-597: only release transport buffers if they are above a threshold capacity
git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1606012 13f79535-47bb-0310-9956-ffa450edef68
It makes Proton pretty much useless...
this code makes proton create a new buffer (a big buffer actually) every new message written.. even minor messages.
The previous version before this would always use the same buffer, recycling it... (making copies between my buffer and Proton), and then reseting the buffer ready for the next usage.
with this new change in place, a benchmark that I was already struggling to improve would make 2500 very small messages / second. With the new change you get it to 333 small messages / second...
If you want to diminish the weight of each connection, then you need a proper refactoring to Proton where you write to output buffers rather than internal buffers. Proton needs this buffer as it is now unless you do a proper refactoring.
Re: Important!!! Please revert PROTON-597
Posted by Clebert Suconic <cs...@redhat.com>.
Could you guys merge this commit?
https://github.com/clebertsuconic/qpid-proton/commit/744ee9ae731fd7180e35a1ed8f2ca56120649417
I'm really fiercely against static properties on this :P... even worse on requiring my users who won't probably have a clue we are using AMQP underneath to have to set a property in order to have things performing well.
On Jul 2, 2014, at 2:35 PM, Clebert Suconic <cs...@redhat.com> wrote:
>
> On Jul 2, 2014, at 12:28 PM, Robbie Gemmell <ro...@gmail.com> wrote:
>
>> Discussed with Clebert on IRC. There is a threshold where it changes
>> behaviour and that was set too low, so it has now been raised significantly
>> such that most people continue seeing the prior behaviour and those who
>> want it can opt in with the property.
>
>
> I don't really want to force my users to set any properties on the VM for stuff I would expect on my code. Any property should have a proper configuration so we can set it on the Transport please!
Re: Important!!! Please revert PROTON-597
Posted by Clebert Suconic <cs...@redhat.com>.
On Jul 2, 2014, at 12:28 PM, Robbie Gemmell <ro...@gmail.com> wrote:
> Discussed with Clebert on IRC. There is a threshold where it changes
> behaviour and that was set too low, so it has now been raised significantly
> such that most people continue seeing the prior behaviour and those who
> want it can opt in with the property.
I don't really want to force my users to set any properties on the VM for stuff I would expect on my code. Any property should have a proper configuration so we can set it on the Transport please!
Re: Important!!! Please revert PROTON-597
Posted by Clebert Suconic <cs...@redhat.com>.
Can you rather have a proper parameter on Transport such as setBufferReleaseThreshold than a static attribute for the whole VM?
Default is fine.. but I have no way to control such parameter.
(I actually think the default should be 2X maxFrameSize.. or something like that).
On Jul 2, 2014, at 12:28 PM, Robbie Gemmell <ro...@gmail.com> wrote:
> Discussed with Clebert on IRC. There is a threshold where it changes
> behaviour and that was set too low, so it has now been raised significantly
> such that most people continue seeing the prior behaviour and those who
> want it can opt in with the property.
>
> Robbie
>
> On 2 July 2014 16:42, Clebert Suconic <cs...@redhat.com> wrote:
>
>> Wrong commit...
>>
>>
>> this is the culprit:
>>
>> commit 8fe9a12b1ad8dc9cd35324f4ed53bea9cb37ce22
>> Author: Robert Gemmell <ro...@apache.org>
>> Date: Fri Jun 27 10:07:47 2014 +0000
>>
>> PROTON-597: update TransportOutputAdaptor and FrameParser to release
>> buffers after use, reducing memory consumption when using large frame sizes
>>
>> Change from Marcel Meulemans
>>
>> git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1606010
>> 13f79535-47bb-0310-9956-ffa450edef68
>>
>> On Jul 2, 2014, at 11:40 AM, Clebert Suconic <cs...@redhat.com> wrote:
>>
>>> commit ccb7b03b8089c2fea329d44cea5dc06da6345ea6
>>> Author: Robert Gemmell <ro...@apache.org>
>>> Date: Fri Jun 27 10:08:24 2014 +0000
>>>
>>> PROTON-597: only release transport buffers if they are above a
>> threshold capacity
>>>
>>> git-svn-id:
>> https://svn.apache.org/repos/asf/qpid/proton/trunk@1606012
>> 13f79535-47bb-0310-9956-ffa450edef68
>>>
>>>
>>>
>>> It makes Proton pretty much useless...
>>>
>>> this code makes proton create a new buffer (a big buffer actually) every
>> new message written.. even minor messages.
>>> The previous version before this would always use the same buffer,
>> recycling it... (making copies between my buffer and Proton), and then
>> reseting the buffer ready for the next usage.
>>>
>>>
>>> with this new change in place, a benchmark that I was already struggling
>> to improve would make 2500 very small messages / second. With the new
>> change you get it to 333 small messages / second...
>>>
>>>
>>>
>>>
>>> If you want to diminish the weight of each connection, then you need a
>> proper refactoring to Proton where you write to output buffers rather than
>> internal buffers. Proton needs this buffer as it is now unless you do a
>> proper refactoring.
>>
>>
Re: Important!!! Please revert PROTON-597
Posted by Robbie Gemmell <ro...@gmail.com>.
Discussed with Clebert on IRC. There is a threshold where it changes
behaviour and that was set too low, so it has now been raised significantly
such that most people continue seeing the prior behaviour and those who
want it can opt in with the property.
Robbie
On 2 July 2014 16:42, Clebert Suconic <cs...@redhat.com> wrote:
> Wrong commit...
>
>
> this is the culprit:
>
> commit 8fe9a12b1ad8dc9cd35324f4ed53bea9cb37ce22
> Author: Robert Gemmell <ro...@apache.org>
> Date: Fri Jun 27 10:07:47 2014 +0000
>
> PROTON-597: update TransportOutputAdaptor and FrameParser to release
> buffers after use, reducing memory consumption when using large frame sizes
>
> Change from Marcel Meulemans
>
> git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1606010
> 13f79535-47bb-0310-9956-ffa450edef68
>
> On Jul 2, 2014, at 11:40 AM, Clebert Suconic <cs...@redhat.com> wrote:
>
> > commit ccb7b03b8089c2fea329d44cea5dc06da6345ea6
> > Author: Robert Gemmell <ro...@apache.org>
> > Date: Fri Jun 27 10:08:24 2014 +0000
> >
> > PROTON-597: only release transport buffers if they are above a
> threshold capacity
> >
> > git-svn-id:
> https://svn.apache.org/repos/asf/qpid/proton/trunk@1606012
> 13f79535-47bb-0310-9956-ffa450edef68
> >
> >
> >
> > It makes Proton pretty much useless...
> >
> > this code makes proton create a new buffer (a big buffer actually) every
> new message written.. even minor messages.
> > The previous version before this would always use the same buffer,
> recycling it... (making copies between my buffer and Proton), and then
> reseting the buffer ready for the next usage.
> >
> >
> > with this new change in place, a benchmark that I was already struggling
> to improve would make 2500 very small messages / second. With the new
> change you get it to 333 small messages / second...
> >
> >
> >
> >
> > If you want to diminish the weight of each connection, then you need a
> proper refactoring to Proton where you write to output buffers rather than
> internal buffers. Proton needs this buffer as it is now unless you do a
> proper refactoring.
>
>
Re: Important!!! Please revert PROTON-597
Posted by Clebert Suconic <cs...@redhat.com>.
Wrong commit...
this is the culprit:
commit 8fe9a12b1ad8dc9cd35324f4ed53bea9cb37ce22
Author: Robert Gemmell <ro...@apache.org>
Date: Fri Jun 27 10:07:47 2014 +0000
PROTON-597: update TransportOutputAdaptor and FrameParser to release buffers after use, reducing memory consumption when using large frame sizes
Change from Marcel Meulemans
git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1606010 13f79535-47bb-0310-9956-ffa450edef68
On Jul 2, 2014, at 11:40 AM, Clebert Suconic <cs...@redhat.com> wrote:
> commit ccb7b03b8089c2fea329d44cea5dc06da6345ea6
> Author: Robert Gemmell <ro...@apache.org>
> Date: Fri Jun 27 10:08:24 2014 +0000
>
> PROTON-597: only release transport buffers if they are above a threshold capacity
>
> git-svn-id: https://svn.apache.org/repos/asf/qpid/proton/trunk@1606012 13f79535-47bb-0310-9956-ffa450edef68
>
>
>
> It makes Proton pretty much useless...
>
> this code makes proton create a new buffer (a big buffer actually) every new message written.. even minor messages.
> The previous version before this would always use the same buffer, recycling it... (making copies between my buffer and Proton), and then reseting the buffer ready for the next usage.
>
>
> with this new change in place, a benchmark that I was already struggling to improve would make 2500 very small messages / second. With the new change you get it to 333 small messages / second...
>
>
>
>
> If you want to diminish the weight of each connection, then you need a proper refactoring to Proton where you write to output buffers rather than internal buffers. Proton needs this buffer as it is now unless you do a proper refactoring.