ARTEMIS-1606 - Change AddressInfo RoutingType Set to use EnumSet

[michaelandrepearce]

Change all use from Set< RoutingType > to EnumSet<RoutingType>
Deprecating any old exposed interfaces but keeping for back compatibility.
Address info to avoid iterator on getRoutingType hotpath, like wise can be avoided where single RoutingType is passed in.

[michaelandrepearce]

@franz1981 ping.

[franz1981]

@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

[michaelandrepearce]

@franz1981 did you get a chance to do your local check? you happy for this to merge?

[franz1981]

@michaelandrepearce I'm looking at it locally right now 👍

[cshannon]

This looks good to me too

[jbertram]

Any metrics to quantify the benefit and justify this change?

[michaelandrepearce]

Sure, using JMH to measure performance of the two AddressInfo Implementations.

Benchmark Mode Cnt Score Error 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 over 8x.

Also from a java memory footprint this is far better, note the constant size, and that even with one object its more than half the size (nearly almost a third).

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

[jbertram]

Thanks, @michaelandrepearce. Nice results!

[franz1981]

Very good results! A lil OT but seems that we really start need a JMH folder with all the benchs on Artemis eh? :P

[michaelandrepearce]

@franz1981 i was waiting on you, to give a thumbs up before i merged, is that a thumbs up?

@jbertram cheers.

[franz1981]

@michaelandrepearce I'm running a test before and after, but first can you try add .jvmArgs("-XX:+UseG1GC") and .forks(2) to your benchmark?

[franz1981]

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 💯

[michaelandrepearce]

Great thanks, ill merge then.