On Sat, 4 Nov 2023 16:01:44 GMT, Claes Redestad <redes...@openjdk.org> wrote:
>> src/java.base/share/classes/java/time/Year.java line 321: >> >>> 319: // So for a year that's divisible by 4, checking that it's >>> also divisible by 16 >>> 320: // is sufficient to determine it must be a leap year. >>> 321: return (year & 15) == 0 ? (year & 3) == 0 : (year & 3) == 0 && >>> year % 100 != 0; >> >> I think `(year & 3) == 0 && ((year & 15) == 0) || (year % 25) != 0` would be >> better simply because the common path will be a little bit shorter. > > Benchmark Mode Cnt Score Error Units > LeapYearBench.isLeapYear thrpt 15 0,735 ± 0,004 ops/us > LeapYearBench.isLeapYearChrono thrpt 15 0,734 ± 0,006 ops/us > > > So equal to or even slightly worse than baseline. I tested a few variants > before submitting the PR - some that looked simpler or better - but the > ternary variant in this PR always came out on top. On top of my general comments made earlier, let me give my two cents on this line: return (year & 15) == 0 ? (year & 3) == 0 : (year & 3) == 0 && year % 100 != 0; If `(year & 15) == 0`, then the last *four* bits of `year` are zeros. In particular, the last *two* bits of `year` are zeros, that is, `(year & 3) == 0`. The same conclusion can be obtained in terms of divisibility: if `year % 16 == 0`, i.e., `year` is a multiple of 16, then `year % 4 == 0`, i.e., `year` is multiple of 4. What I'm trying to say is that the line above can be simplified to: return (year & 15) == 0 ? true : (year & 3) == 0 && year % 100 != 0; But now it becomes clear that the above is also equivalent to: return (year & 15) == 0 || ((year & 3) == 0 && year % 100 != 0); Which is the simplest form of all the above. It's possible, but I'm not sure, that the Java compiler makes this simplification for you. (FWIW: the GCC C compiler does. Indeed, as seen [here](https://godbolt.org/z/P8sMv3Yno) the three expressions above generate exactly the same assembly instructions.) As I explained in my earlier post, for this particular expression a further simplification, that makes the compiler (at least the C compiler [above](https://godbolt.org/z/P8sMv3Yno)) to save one instruction (`ror edi 2`) is replacing 100 by 25: return (year & 15) == 0 || ((year & 3) == 0 && year % 25 != 0); ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16491#discussion_r1382469177
