RFR 8209171 : Simplify Java implementation of Integer/Long.numberOfTrailingZeros()

[Ivan Gerasimov]

Hi Martin!

Thanks for taking a look!


On 8/9/18 5:13 PM, Martin Buchholz wrote:
>
>
> On Thu, Aug 9, 2018 at 4:15 PM, Ivan Gerasimov 
> <ivan.gerasimov at oracle.com <mailto:ivan.gerasimov at oracle.com>> wrote:
>
>     Hello!
>
>     Integer.numberOfTrailingZeros() and Long.numberOfTrailingZeros()
>     are intrinsified by Hotspot, so the Java implementation of these
>     is not too important.
>     However, they still can be improved by a tiny bit :)
>
>     Could you please help review the fix?
>
>     Bug: https://bugs.openjdk.java.net/browse/JDK-8209171
>     <https://bugs.openjdk.java.net/browse/JDK-8209171>
>     Webrev: http://cr.openjdk.java.net/~igerasim/8209171/00/webrev/
>     <http://cr.openjdk.java.net/%7Eigerasim/8209171/00/webrev/>
>     Benchmark for Integer:
>     http://cr.openjdk.java.net/~igerasim/8209171/00/bench/int/MyBenchmark.java
>     <http://cr.openjdk.java.net/%7Eigerasim/8209171/00/bench/int/MyBenchmark.java>
>     Benchmark for Long:
>     http://cr.openjdk.java.net/~igerasim/8209171/00/bench/long/MyBenchmark.java
>     <http://cr.openjdk.java.net/%7Eigerasim/8209171/00/bench/long/MyBenchmark.java>
>
>     The resulting code is both smaller and faster.  It may also help
>     to warm up Integer.numberOfLeadingZeros() quicker.
>
>     On average, the new Integer.numberOfTrailingZeros() has got +11%
>     to performance for -client and +1% for -server.
>     Long.numberOfTrailingZeros() is faster on 17% for -client and +20%
>     for -server.
>
>
> It seems to me the benchmark is not entirely fair to the old java 
> implementation, since the new one gets the benefit of the 
> intrinsification of  numberOfLeadingZeros.  A fairer comparison would 
> use turn off intrinsification of both.
I did not use the intrinsified variants of numberOfLeadingZeros in the 
benchmark.

In the first (Integer) benchmark I copied/pasted the Java implementation 
of both Integer.numberOfLeadingZeros and Integer.bitCount.
In the second (Long) one, Long.numberOfLeadingZeros and Long.bitCount 
were also copied into the test with their original names, and Integer 
versions of the functions were copied as Integer_numberOfLeadingZeros 
and Integer_numberOfTrailingZeros.

The later one was not the original function 
Integer.numberOfTrailingZeros, but the new one, so the numbers of the 
second benchmark reflect changes in both classes: Integer and Long.
>
> I guess benchmarking on 32-bit platforms is  no longer relevant, given 
> that they are all legacy now.
I ran them on x64 (Intel Core i7 to be precise).

-- 
With kind regards,
Ivan Gerasimov