[GitHub] activemq-artemis pull request #1829: ARTEMIS-1647 - Add plugin support for b...

[franz1981 <gi...@git.apache.org>]

Github user franz1981 commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/1829#discussion_r165317614
  
    --- Diff: artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java ---
    @@ -571,6 +571,8 @@ public AddressInfo getAddressInfo(SimpleString addressName) {
        // even though failover is complete
        @Override
        public synchronized void addBinding(final Binding binding) throws Exception {
    +      server.callBrokerPlugins(server.hasBrokerPlugins() ? plugin -> plugin.beforeAddBinding(binding) : null);
    --- End diff --
    
    Due to the `default` uses on `callBrokerPlugins` and other wierd reasons dependent by how the JIT works with compilations/inlining/interface optimization would be better at least to turn it into:
    ```
    if(server.hasBrokerPlugins()){
     ....
    }
    ```
    I've noticed it is used on most "cold path" of the broker so it is fine to left as it is in this place too: it doesn't seem a hot path. My suggestion is just for wider puporses :+1: 


---