You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@activemq.apache.org by clebertsuconic <gi...@git.apache.org> on 2017/10/02 19:41:31 UTC
[GitHub] activemq-artemis pull request #1567: ARTEMIS-1397 Removing Netty Copied clas...
GitHub user clebertsuconic opened a pull request:
https://github.com/apache/activemq-artemis/pull/1567
ARTEMIS-1397 Removing Netty Copied classes
instead of duplicating a buffer from Netty, this will use an existing Wrapped Unpooled Buffer
Which will in turn use Unsafe Properly.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clebertsuconic/activemq-artemis ARTEMIS-1397
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/activemq-artemis/pull/1567.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #1567
----
commit 67a0e5d8bf72403ed570608483558d9765606f4d
Author: Clebert Suconic <cl...@apache.org>
Date: 2017-10-02T19:40:11Z
ARTEMIS-1397 Removing Netty Copied classes
instead of duplicating a buffer from Netty, this will use an existing Wrapped Unpooled Buffer
Which will in turn use Unsafe Properly.
----
---
[GitHub] activemq-artemis pull request #1567: ARTEMIS-1397 Removing Netty Copied clas...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/activemq-artemis/pull/1567
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@michaelandrepearce fancy doing your first merge?
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@franz1981 i agree with you, any chance we can get netty guys, to either take your class, or to get netty to make the methods you use in UnsafeByteBufUtil set to public? So we don't have to have a full copy of UnsafeByteBufUtil in artemis just to sort some OSGI issue?
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@clebertsuconic yup lets, step me through the process, good chance to learn.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@franz1981 +100 thanks for doing that really appreciated it’s a good feature.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@michaelandrepearce / @franz1981 I don't really see much gain on saving the ByteBuf instance. Maybe page.write would create a few more instances.. but I doubt it would really matter here.
I would rather accept the little weight caused then having to maintain Unsafe just for that.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@franz1981 we can have it back.. just would need to have it properly at Netty exposed.. either have your class, a super class we can easily extend.. or whatever is needed.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@clebertsuconic @michaelandrepearce I've just posted on Netty's gitter what we have on Artemis, to see if there is any chance to have it directly on Netty (I will do the PR if I'll receive positive feedbacks) :+1:
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@michaelandrepearce this is following up after #1561
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by clebertsuconic <gi...@git.apache.org>.
Github user clebertsuconic commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@michaelandrepearce I think the right thing is to use the Netty Buffer Like I am doing now... reusing the instance.. just to call wrap.. not even Netty guys mind about that...
If we really need it.. we could maybe convince Netty guys to provide a wrap method on a buffer, so we could always reuse the same instance.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by michaelandrepearce <gi...@git.apache.org>.
Github user michaelandrepearce commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@clebertsuconic just for record, i don't have issue with the class @franz1981 made, its more the very recent change that brought in the UnsafeByteBufUtil.java just so they could move package, for OSGI. Id much rather if that class is needed we request Netty to make its method public.
---
[GitHub] activemq-artemis issue #1567: ARTEMIS-1397 Removing Netty Copied classes
Posted by franz1981 <gi...@git.apache.org>.
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1567
@clebertsuconic @michaelandrepearce the reason behind that class was to have more stable latencies on hot paths with no data-sync enabled: if not reused it creates a big amount of garbage when you have bursts of writes.
With data-sync it won't make a big difference if you don't have a very fast disk...
Anyway I trust your the decision about it guys :)
---