[Minor] Optimize ShuffleServerInfo#hashCode #538
Conversation
Codecov Report
@@ Coverage Diff @@
## master #538 +/- ##
============================================
+ Coverage 60.21% 60.26% +0.05%
- Complexity 1784 1785 +1
============================================
Files 205 205
Lines 11557 11557
Branches 1042 1042
============================================
+ Hits 6959 6965 +6
+ Misses 4190 4186 -4
+ Partials 408 406 -2
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| @@ -54,7 +54,7 @@ public int getPort() { | |||
|
|
|||
| @Override | |||
| public int hashCode() { | |||
| return host.hashCode(); | |||
| return id.hashCode(); | |||
There was a problem hiding this comment.
Why not include all three fields (id, host, port) in hashCode()?
For example:
Objects.hash(id, host, port)There was a problem hiding this comment.
By default id = host + "-" + port, so id is enough.
There was a problem hiding this comment.
By default
id = host + "-" + port, so id is enough.
Should we remove the constructor at line 37?
There was a problem hiding this comment.
No. In some cases, the id does not need to be recalculated.
There was a problem hiding this comment.
OK. Can you add some comments here to make future reviewer be aware of it.
What changes were proposed in this pull request?
Optimize ShuffleServerInfo#hashCode
Why are the changes needed?
For better performence.
Does this PR introduce any user-facing change?
No
How was this patch tested?
No need