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

    https://github.com/apache/activemq-artemis/pull/2427#discussion_r232498771
  
    --- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java
 ---
    @@ -318,6 +320,33 @@ public synchronized boolean containsProperty(final 
SimpleString key) {
           }
        }
     
    +   public synchronized boolean 
cleanupInternalProperties(Predicate<SimpleString> propertyNamePredicate) {
    +      if (!internalProperties) {
    --- End diff --
    
    The biggest improvement here is exactly due to the fact that cleaning up 
properties atomically is a duty of who own the data ie TypedProperties. 
    About the message particulars I do believe that `TypedProperty` already 
contains some of them:
    
    - internal property name prefix ie `TypedProperties.AMQ_PROPNAME`
    - internal properties presence checks ie 
`TypedProperties::hasInternalProperties`
    
    I have used the `Predicate` to allow decoupling, but I understand your 
point here: we can move it outer (as it is, algoritmically), but I'm not fully 
sure that it will bring the same benefit (eg enter/exit synchronize 
TypedProperty once).
    Just to give some reason of this change: with this PR it is producing less 
then 1/4 of the garbage while having more then twice the throughput.


---

Reply via email to