RFR: 8155795: Optimize Integer/Long.reverse by using reverseBytes

[Claes Redestad]

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