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