[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Great thanks, ill merge then. ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 These are the flamegraph of a couple of profiled tests, one on master:  Where the violet bars are the `AddressInfo::new` and the `AddressInfo::getRoutingType` (+ iterator) while with this PR:  There isn't anymore any cost associated with `AddressInfo`: for me is a thumbs up! Althouh it seems negligible (at a first look) the impact of this change has changed the garbage produced and the CPU time required to call `doSend`: indeed the latencies are better too. Well done :100: ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue:
https://github.com/apache/activemq-artemis/pull/1777
@michaelandrepearce I'm running a test before and after, but first can you
try add ".jvmArgs("-XX:+UseG1GC")" and `.forks(2)` to your benchmark?
---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @franz1981 i was waiting on you, to give a thumbs up before i merged, is that a thumbs up? ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Very good results! A lil OT but seems that we really start need a JMH folder with all the benchs on Artemis eh? :P ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Thanks, @michaelandrepearce. Nice results! ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Sure, using JMH to measure performance of the two AddressInfo Implementations. BenchmarkMode Cnt ScoreError Units MyBenchmark.testEnumSetAddressInfo thrpt 200 198607467.630 ± 507344.268 ops/s MyBenchmark.testHashSetAddressInfo thrpt 200 22849376.205 ± 216480.582 ops/s This is simulating what happens in the hotpath of sending a message in doSend within ServerSessionImpl. Which is creating an AddressInfo and then getRoutingType. As you note the throughput is an order of magnitude of 8x. Also from a java memory footprint this is far better. AddressInfo using HashSet with one routing type -> 208 bytes AddressInfo using HashSet with two routing types -> 240 bytes AddressInfo using EnumSet with one routing type -> 72 bytes AddressInfo using EnumSet with two routing types -> 72 bytes ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user jbertram commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 Any metrics to quantify the benefit and justify this change? ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user cshannon commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 This looks good to me too ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @michaelandrepearce I'm looking at it locally right now :+1: ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @franz1981 you happy for this to merge? ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user franz1981 commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @michaelandrepearce Looking at the changes via Github it seems clean and effective and I admit I'm a fan of using the "right" collection when needed, but I want to use some time tomorrow to check the code locally too and run some CI tests on it: similar to `SimpleString` changes I prefer to be 100% that isn't impacting negatively with hidden behaviours ---
[GitHub] activemq-artemis issue #1777: ARTEMIS-1606 - Change AddressInfo RoutingType ...
Github user michaelandrepearce commented on the issue: https://github.com/apache/activemq-artemis/pull/1777 @franz1981 ping. ---
