On 2016-05-02 16:48, Aleksey Shipilev wrote:
On 05/02/2016 05:07 PM, Claes Redestad wrote:
I'd like to sponsor this patch proposed by Jaroslav Kameník here:
http://mail.openjdk.java.net/pipermail/core-libs-dev/2016-April/040660.html
Bug https://bugs.openjdk.java.net/browse/JDK-8155795
Webrev: http://cr.openjdk.java.net/~redestad/8155795/webrev.01/
*) I wonder if Integer.reverseBytes is better spelled as:
return (i >>> 24) |
((i & 0xFF0000) >> 8) |
((i & 0xFF00) << 8) |
(i << 24);
The reverseBytes changes were motivated by seeing slightly better
performance on the micro with the suggested changes, but after
discussing this a bit I think we should revert to the original code alone
and investigate if there's some compiler quirk lurking here separately.
I'll file a bug.
*) The test should probably follow the same randomized testing pattern
as other tests:
for (int i = 0; i < N; i++) {
int x = rnd.nextInt();
String expected = new StringBuilder().
.append(leftpad(Integer.toBinaryString(x), 32))
.reverse().toString();
String actual =
leftpad(Integer.toBinaryString(Integer.reverse(x)), 32);
if (!expected.equals(actual)) {
throw new RuntimeException("reverse: \n" +
expected + " \n" + actual);
}
// That's how development looks like in 2016.
private static String leftpad(String s, int width) {
String r = s;
for (int c = 0; c < width - s.length(); c++) {
r = "0" + r;
}
return r;
}
...and should probably precede any other test that uses reverse().
Seems reasonable.
Jaroslav, do you mind improving the test as per Aleksey's
suggestions?
Thanks!
/Claes
