Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/java.base/share/classes/java/time/Year.java
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,10 @@ public static Year parse(CharSequence text, DateTimeFormatter formatter) {
* @return true if the year is leap, false otherwise
*/
public static boolean isLeap(long year) {
return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0);
// A year that is a multiple of 100, 200 and 300 is not divisible by 16, but 400 is.
// So for a year that's divisible by 4, checking that it's also divisible by 16
// is sufficient to determine it must be a leap year.
return (year & 15) == 0 ? (year & 3) == 0 : (year & 3) == 0 && year % 100 != 0;
Copy link
Member

@merykitty merykitty Nov 4, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 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) to save one instruction (ror edi 2) is replacing 100 by 25:

  return (year & 15) == 0 || ((year & 3) == 0 && year % 25 != 0);

}

//-----------------------------------------------------------------------
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -475,7 +475,7 @@ public LocalDate dateNow(Clock clock) {
*/
@Override
public boolean isLeapYear(long prolepticYear) {
return ((prolepticYear & 3) == 0) && ((prolepticYear % 100) != 0 || (prolepticYear % 400) == 0);
return Year.isLeap(prolepticYear);
}

@Override
Expand Down
7 changes: 5 additions & 2 deletions src/java.base/share/classes/java/util/GregorianCalendar.java
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,10 @@ public boolean isLeapYear(int year) {
}

if (year > gregorianCutoverYear) {
return (year%100 != 0) || (year%400 == 0); // Gregorian
// A multiple of 100, 200 and 300 is not divisible by 16, but 400 is.
// So for a year that's divisible by 4, checking that it's also divisible by 16
// is sufficient to determine it must be a leap year.
return (year & 15) == 0 || (year % 100 != 0); // Gregorian
}
if (year < gregorianCutoverYearJulian) {
return true; // Julian
Expand All @@ -840,7 +843,7 @@ public boolean isLeapYear(int year) {
} else {
gregorian = year == gregorianCutoverYear;
}
return gregorian ? (year%100 != 0) || (year%400 == 0) : true;
return !gregorian || (year & 15) == 0 || (year % 100 != 0);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,12 @@ private CalendarUtils() {}
* @see CalendarDate#isLeapYear
*/
public static boolean isGregorianLeapYear(int gregorianYear) {
return (((gregorianYear % 4) == 0) && (((gregorianYear % 100) != 0)
|| ((gregorianYear % 400) == 0)));
// A year that is a multiple of 100, 200 and 300 is not divisible by 16, but 400 is.
// So for a year that's divisible by 4, checking that it's also divisible by 16
// is sufficient to determine it must be a leap year.
return (gregorianYear & 15) == 0
? (gregorianYear & 3) == 0
: (gregorianYear & 3) == 0 && gregorianYear % 100 != 0;
Comment on lines +46 to +48

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly,

Suggested change
return (gregorianYear & 15) == 0
? (gregorianYear & 3) == 0
: (gregorianYear & 3) == 0 && gregorianYear % 100 != 0;
return ((gregorianYear & 15) == 0) || ((gregorianYear & 3) == 0 && gregorianYear % 25 != 0);

}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ long getTransitionEpochSecond(int year) {
}

static final boolean isLeapYear(int year) {
return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0);
return CalendarUtils.isGregorianLeapYear(year);
}

static final int lengthOfMonth(int year, int month) {
Expand Down
108 changes: 108 additions & 0 deletions test/micro/org/openjdk/bench/java/time/LeapYearBench.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
/*
* Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

package org.openjdk.bench.java.time;

import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
import org.openjdk.jmh.annotations.Measurement;
import org.openjdk.jmh.annotations.Mode;
import org.openjdk.jmh.annotations.OutputTimeUnit;
import org.openjdk.jmh.annotations.Scope;
import org.openjdk.jmh.annotations.Setup;
import org.openjdk.jmh.annotations.State;
import org.openjdk.jmh.annotations.Warmup;
import org.openjdk.jmh.infra.Blackhole;

import java.time.Duration;
import java.time.Instant;
import java.time.Year;
import java.time.ZonedDateTime;
import java.time.chrono.IsoChronology;
import java.time.temporal.ChronoUnit;
import java.util.GregorianCalendar;
import java.util.Random;
import java.util.concurrent.TimeUnit;
import java.util.stream.IntStream;

/**
* Examine Year.leapYear-related operations
*/
@BenchmarkMode(Mode.Throughput)
@OutputTimeUnit(TimeUnit.MICROSECONDS)
@Warmup(iterations = 5, time = 1)
@Measurement(iterations = 5, time = 1)
@Fork(3)
@State(Scope.Thread)
public class LeapYearBench {

private long[] years;

private GregorianCalendar calendar;

@Setup
public void createInstants() {
// Large enough number of years to guarantee that the distribution of
// leap years is reasonably realistic
years = new long[2048];
final Random random = new Random(0);
for (int i = 0; i < years.length; i++) {
years[i] = random.nextLong(2000) + 2000;
}
calendar = GregorianCalendar.from(ZonedDateTime.now());
}

@Benchmark
public void isLeapYear(Blackhole bh) {
for (long year : years) {
bh.consume(Year.isLeap(year));
}
}

@Benchmark
public void isLeapYearChrono(Blackhole bh) {
for (long year : years) {
bh.consume(IsoChronology.INSTANCE.isLeapYear(year));
}
}

@Benchmark
public void isLeapYearGregorian(Blackhole bh) {
for (long year : years) {
bh.consume(calendar.isLeapYear((int)year));
}
}

public static boolean isLeapNeriSchneider(long year) {
int d = year % 100 != 0 ? 4 : 16;
return (year & (d - 1)) == 0;
}

@Benchmark
public void isLeapYearNS(Blackhole bh) {
for (long year : years) {
bh.consume(isLeapNeriSchneider(year));
}
}
}