[
https://issues.apache.org/jira/browse/STORM-1481?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15106858#comment-15106858
]
ASF GitHub Bot commented on STORM-1481:
---------------------------------------
Github user revans2 commented on the pull request:
https://github.com/apache/storm/pull/1023#issuecomment-172883855
OK so Math.abs(Integer.MIN_VALUE) actually returns Integer.MIN_VALUE
because of an integer overflow error. That is a good corner case to remember
in the future.
@arunmahadevan `Math.abs(val.hasCode() % numPartitions)` would work too.
The only reason to do one over the other would be performance vs readability.
In my very unscientific test
```
public class Test {
public static int toPositive(int number) {
return number & Integer.MAX_VALUE;
}
public static void main(String [] args) {
int count = Integer.valueOf(args[0]);
long start = System.nanoTime();
for (int i = 0; i < count; i++) {
int ret = Math.abs(i);
//int ret = toPositive(i);
}
long end = System.nanoTime();
System.out.println(end+" - "+ start+" = "+(end-start)+" rate:
"+(((double)count)/(end-start)));
}
}
```
I get about 291 ops per nanosecond for toPositive
`1453216134803090000 - 1453216134799657000 = 3433000 rate:
291.29041654529567`
and 289 ops per nanosecond for Math.abs
`1453216159561620000 - 1453216159558157000 = 3463000 rate:
288.7669650591972`
To me it is such a small difference About 1% over a part of the code where
computing the hash code dominates that it should come down to readability.
Both look equally as readable to me (because of the comments on toPositive).
I am +1 for this change.
> avoid Math.abs(Integer) get a negative value
> --------------------------------------------
>
> Key: STORM-1481
> URL: https://issues.apache.org/jira/browse/STORM-1481
> Project: Apache Storm
> Issue Type: Bug
> Components: storm-core
> Reporter: Xin Wang
> Assignee: Xin Wang
>
> before fix:
> {code:title=org.apache.storm.trident.partition.IndexHashGrouping}
> public static int objectToIndex(Object val, int numPartitions) {
> if(val==null) return 0;
> else {
> return Math.abs(val.hashCode()) % numPartitions;
> }
> }
> {code}
> If the hashcode is Integer.MIN_VALUE, then the result will be negative as
> well (since Math.abs(Integer.MIN_VALUE) == Integer.MIN_VALUE).
> after fix:
> Use toPositive replace Math.abs:
> {code}
> public static int toPositive(int number) {
> return number & Integer.MAX_VALUE;
> }
> {code}
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
