Closed richardstartin closed 2 years ago
@vigna Since there doesn't appear to be CI running on this change, I feel it's worth mentioning I've run the test suite and everything passed.
Very interesting how it handles the case 0. OK, but, just out of curiosity, did you try to benchmark with GraalVM?
Very interesting how it handles the case 0. OK, but, just out of curiosity, did you try to benchmark with GraalVM?
I did not, but I can try that and let you know.
I got roughly the same scores (JDK 17.0.5, OpenJDK 64-Bit Server VM, 17.0.5+8-jvmci-22.3-b08, on an M1):
Benchmark Mode Cnt Score Error Units
NextPowerOf2Benchmark.after avgt 5 0.647 ± 0.109 ns/op
NextPowerOf2Benchmark.before avgt 5 0.965 ± 0.002 ns/op
OK thanks!
Oh no! I thought this was an elegant solution (looking for a problem, of course) :(
Why no? I just merged...
Ah OK for some weird reason I did not merge properly.
The integer version of this method showed up in a profile, and I was curious enough to look at its implementation. This provides a slightly faster implementation of
HashCommon.nextPowerOf2
which takes advantage ofnumberOfLeadingZeros
intrinsics.I wrote a very simple benchmark to justify the change. While it shows that the existing code is hardly worth optimising, the new code is indeed faster: