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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
