[GitHub] activemq-artemis issue #1753: ARTEMIS-1573 Improve UTF translation allowing ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1753 @franz1981 LGTM. The only comment i would have, is it worth adding the code to handle the offheap now also, just while its fresh in your mind, and then it be more

[GitHub] activemq-artemis issue #1775: ARTEMIS-1587 Add setting to control the queue ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1775 Why is this needed? If an address is allowed for queues to be auto created, the client describes if the queue should be durable or not. e.g. See JMS Subscription (which maps

[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @franz1981 ping. ---

[GitHub] activemq-artemis pull request #1777: ARTEMIS-1606 - Change AddressInfo Routi...

2018-01-13 Thread michaelandrepearce
GitHub user michaelandrepearce opened a pull request: https://github.com/apache/activemq-artemis/pull/1777 ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet Change all use from Set to EnumSet Deprecating any old exposed interfaces but keeping for back

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @franz1981 re the checkProperties change, its not thread safe. And i'd rather not put synchronised block in. Found this on running larger test suite. On checking

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 Just rebased branch ready for merge. ---

[GitHub] activemq-artemis pull request #1776: ARTEMIS-1596 - Fix checkstyle.

2018-01-13 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/activemq-artemis/pull/1776 ---

[GitHub] activemq-artemis issue #1776: ARTEMIS-1596 - Fix checkstyle.

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1776 This is to fix master, checkstyle fail which was merged in from https://github.com/apache/activemq-artemis/pull/1766 ---

[GitHub] activemq-artemis pull request #1776: ARTEMIS-1596 - Fix checkstyle.

2018-01-13 Thread michaelandrepearce
GitHub user michaelandrepearce opened a pull request: https://github.com/apache/activemq-artemis/pull/1776 ARTEMIS-1596 - Fix checkstyle. You can merge this pull request into a Git repository by running: $ git pull https://github.com/michaelandrepearce/activemq-artemis

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @franz1981 just pushed to this brach that last change. What else you think we need do to merge all this? Be good to get it in, before we crack on with the next thing

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @franz1981 indeed it does looks like its improved things, obviously we have to note this is an artifical test case, but it does look good. Hats off to you, for thinking about

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 Here is more evidence we really want bits like routing type not as a typed property but as a top level header, really dont want to be creating typed properties if we can not

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @michaelandrepearce I can only say that Christmas is far away, but this is looking just like a Christmas present to me eheh What makes me (even) more happy is that without

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @franz1981 i stupidly didn't screenshot it, but yes there was slight improvement for the same test case, about a 0.5k uplift on average, 10.5 -> 11k for 2producers, 2consumers

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 The results seems great: it is a huge drop on latencies! So "Post-Change (2.5-SNAPSHOT local build - Using old 2.4 client)" is including the pooling too? And most importantly,

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 Pre-Change https://user-images.githubusercontent.com/1387822/34908377-877bb67e-f886-11e7-83cc-850b56167c7d.png;> Post-Change including the remove of the

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread franz1981
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @michaelandrepearce > Do we want to look address this also (NOT for this PR but something id like to discuss with you), as its not helping matters on the front of GC, as obviously

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 Also another thing spotted, on decoding the typed properties a duplicate of the buffer is made ``` final ByteBuf byteBuf =

[GitHub] activemq-artemis issue #1757: ARTEMIS-1586 Reduce GC pressure due to String ...

2018-01-13 Thread michaelandrepearce
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1757 @franz1981 so i was looking at this, agree things look better, one thing ive noticed though is TypedProperties is being created server side due to routingType "ouch" this is