refactor: isChangedServiceInfo method in ServiceInfoHolder has been r…

[E1se2t]

The isChangedServiceInfo method primarily aims to compare instance information for any changes and then categorize them into three separate collections: new, deleted, and modified collections.

In the original code, adding and modifying instances were processed within one loop, while deleting instances were handled in another loop, resulting in unnecessary iterations. In the refactored code, I combined the keys of the newHostMap and oldHostMap collections and used a HashSet to remove duplicates. Then, within the loop of the allHostMapKeys collection, the following conditions were checked sequentially:

1. newInstance ！= oldInstance
   add modHosts

2. newInstance ！= null && oldInstance == null
   add newHosts

3. newInstance == null && oldInstance ！= null
   add remvHosts

Add them to the ArrayList, and there is no longer a need for HashSet to handle duplicate results，The improvement has enhanced the efficiency and code readability of the method, while also eliminating redundant processing.

[codecov-commenter]

Codecov Report

Merging #10882 (66d723d) into develop (910c205) will decrease coverage by 0.03%.
Report is 7 commits behind head on develop.
The diff coverage is 69.23%.

Additional details and impacted files

@@              Coverage Diff              @@
##             develop   #10882      +/-   ##
=============================================
- Coverage      53.29%   53.27%   -0.03%     
- Complexity      5621     5625       +4     
=============================================
  Files            927      927              
  Lines          29555    29538      -17     
  Branches        3249     3246       -3     
=============================================
- Hits           15751    15735      -16     
- Misses         12420    12434      +14     
+ Partials        1384     1369      -15     

Files Changed	Coverage Δ
...a/nacos/client/naming/cache/ServiceInfoHolder.java	75.24% <69.23%> (-1.95%)	⬇️

... and 24 files with indirect coverage changes

---

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 74e740c...66d723d. Read the comment docs.

[KomachiSion]

I think the change will lead to increased time complexity.

The old implementation: loop newHosts and oldHosts twice for each.

But new implementation loop newHosts and oldHosts third times(host to map, map key set merge, and merged key set loop).

[E1se2t]

I think the change will lead to increased time complexity.

The old implementation: loop newHosts and oldHosts twice for each.

But new implementation loop newHosts and oldHosts third times(host to map, map key set merge, and merged key set loop).

If I merge the keys of the map during the loop that converts hosts to a map, it can avoid the issue of three loops that you mentioned. Is this change supported?

[KomachiSion]

Why not do enhance directly in this issue?

The situation prefer performance I think. And the old one I think the readability has no much different for new implementaton.

[E1se2t]

Why not do enhance directly in this issue?

The situation prefer performance I think. And the old one I think the readability has no much different for new implementaton.

Why not do enhance directly in this issue?

The situation prefer performance I think. And the old one I think the readability has no much different for new implementaton.

@KomachiSion I have reworked the code again, this time removing the step where I converted newService.getHosts() into a map. Within the loop iterating over newService.getHosts(), I've completed the comparison tasks for Instance objects:

if oldHost ！= null && oldHost == newHost
oldHostMap.remove(host);

if oldHost ！= null && newHost != oldHost
oldHostMap.remove(key);
and
modHosts.add(newHost);

if oldHost == null
newHosts.add(newHost);

After filtering oldHostMap within the loop, at the end of the iteration, only data where newHost == null and oldHosts != null remain in oldHostMap. These data are directly placed in the remvHosts.

Through this approach, there is no need to pre-convert newHosts into a map. Furthermore, it also reduces the requirement for two separate comparison and filtering loops.