Copilot commented on code in PR #15633:
URL: https://github.com/apache/dubbo/pull/15633#discussion_r2280048803
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/DefaultServiceInstance.java:
##########
@@ -25,14 +25,17 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
+import java.util.HashSet;
Review Comment:
The HashSet import is only used in the equals method for tracking checked
keys. Consider using a more efficient approach like early termination or
restructuring the comparison logic to avoid the overhead of creating and
maintaining a HashSet for each equals call.
```suggestion
```
##########
dubbo-registry/dubbo-registry-api/src/main/java/org/apache/dubbo/registry/client/DefaultServiceInstance.java:
##########
@@ -305,28 +315,47 @@ public boolean equals(Object o) {
return false;
}
DefaultServiceInstance that = (DefaultServiceInstance) o;
- boolean equals = Objects.equals(getServiceName(),
that.getServiceName())
- && Objects.equals(getHost(), that.getHost())
- && Objects.equals(getPort(), that.getPort());
+ if (!Objects.equals(getServiceName(), that.getServiceName())
+ || !Objects.equals(getHost(), that.getHost())
+ || !Objects.equals(getPort(), that.getPort())) {
+ return false;
+ }
+
+ Set<String> checkedKeys = new HashSet<>(this.getMetadata().size());
for (Map.Entry<String, String> entry : this.getMetadata().entrySet()) {
- if
(entry.getKey().equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME)) {
+ String key = entry.getKey();
+ checkedKeys.add(key);
+ if (key.equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME) ||
key.equals(TIMESTAMP_KEY)) {
continue;
}
- if (entry.getValue() == null) {
- equals = equals && (entry.getValue() ==
that.getMetadata().get(entry.getKey()));
- } else {
- equals = equals &&
entry.getValue().equals(that.getMetadata().get(entry.getKey()));
+ if (!Objects.equals(entry.getValue(),
that.getMetadata().get(key))) {
+ return false;
}
}
- return equals;
+ for (Map.Entry<String, String> thatEntry :
that.getMetadata().entrySet()) {
+ String thatKey = thatEntry.getKey();
+ if (checkedKeys.contains(thatKey)
+ || thatKey.equals(EXPORTED_SERVICES_REVISION_PROPERTY_NAME)
+ || thatKey.equals(TIMESTAMP_KEY)) {
+ continue;
+ }
+ // Return false if 'that' contains a key not present in 'this'
regardless of the entry value.
+ // This ensures consistency with hashCode, as any extra key in
'that' would result in a different hashCode.
+ return false;
+ }
+
Review Comment:
Creating a HashSet for every equals() call can be inefficient, especially
for frequently compared objects. Consider restructuring the comparison logic to
avoid this allocation, perhaps by iterating through both metadata maps in a
single pass or using a different comparison strategy.
--
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: notifications-unsubscr...@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@dubbo.apache.org
For additional commands, e-mail: notifications-h...@dubbo.apache.org
