jbertram commented on code in PR #5757:
URL: https://github.com/apache/activemq-artemis/pull/5757#discussion_r2138551748


##########
artemis-protocols/artemis-mqtt-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/mqtt/MQTTSessionState.java:
##########
@@ -587,8 +588,8 @@ public Integer getMatchingId(String topic) {
 
       private void update(MqttTopicSubscription newSub, Integer newId) {
          if (newId != null && !newId.equals(id)) {
-            if (this.topicFilterPattern == null || 
!subscription.topicFilter().equals(newSub.topicFilter())) {
-               topicFilterPattern = Match.createPattern(newSub.topicFilter(), 
MQTTUtil.MQTT_WILDCARD, true);
+            if (this.address == null || 
!subscription.topicFilter().equals(newSub.topicFilter())) {
+               address = new 
AddressImpl(SimpleString.of(newSub.topicFilter()), MQTTUtil.MQTT_WILDCARD);

Review Comment:
   This particular `AddressImpl` _is_ re-used between multiple invocations of 
`matches`. Were you referring to this line in `getMatchingId`:
   ```java
   if (id != null && new AddressImpl(SimpleString.of(topic), 
MQTTUtil.MQTT_WILDCARD).matches(address)) {
   ```
   I wasn't keen on creating a new instance of `AddressImpl` here for every 
invocation of `matches`, but the constructor performs a `split` that would be 
necessary either way so I thought it made some sense to just re-use that. Is 
your concern here GC pressure or something else?
   
   In any case, I'll look at creating a `static` version of `matches` that will 
fit with this as well as the existing use-cases.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@activemq.apache.org
For additional commands, e-mail: gitbox-h...@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact


Reply via email to