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.gerasi...@oracle.com <mailto:ivan.gerasi...@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

Reply via email to