Skip to content

Commit 1af8d9f

Browse files
authored
Rework checking if a year is a leap year (#60585)
This way is faster, saving about 8% on the microbenchmark that rounds to the nearest month. That is in the hot path for `date_histogram` which is a very popular aggregation so it seems worth it to at least try and speed it up a little.
1 parent 29e957e commit 1af8d9f

File tree

3 files changed

+57
-2
lines changed

3 files changed

+57
-2
lines changed

benchmarks/README.md

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,3 +63,29 @@ To get realistic results, you should exercise care when running benchmarks. Here
6363
* Blindly believe the numbers that your microbenchmark produces but verify them by measuring e.g. with `-prof perfasm`.
6464
* Run more threads than your number of CPU cores (in case you run multi-threaded microbenchmarks).
6565
* Look only at the `Score` column and ignore `Error`. Instead take countermeasures to keep `Error` low / variance explainable.
66+
67+
## Disassembling
68+
69+
Disassembling is fun! Maybe not always useful, but always fun! Generally, you'll want to install `perf` and FCML's `hsdis`.
70+
`perf` is generally available via `apg-get install perf` or `pacman -S perf`. FCML is a little more involved. This worked
71+
on 2020-08-01:
72+
73+
```
74+
wget https://github.com/swojtasiak/fcml-lib/releases/download/v1.2.2/fcml-1.2.2.tar.gz
75+
tar xf fcml*
76+
cd fcml*
77+
./configure
78+
make
79+
cd example/hsdis
80+
make
81+
cp .libs/libhsdis.so.0.0.0
82+
sudo cp .libs/libhsdis.so.0.0.0 /usr/lib/jvm/java-14-adoptopenjdk/lib/hsdis-amd64.so
83+
```
84+
85+
If you want to disassemble a single method do something like this:
86+
87+
```
88+
gradlew -p benchmarks run --args ' MemoryStatsBenchmark -jvmArgs "-XX:+UnlockDiagnosticVMOptions -XX:CompileCommand=print,*.yourMethodName -XX:PrintAssemblyOptions=intel"
89+
```
90+
91+
If you want `perf` to find the hot methods for you then do add `-prof:perfasm`.

server/src/main/java/org/elasticsearch/common/time/DateUtilsRounding.java

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,27 @@ static long utcMillisAtStartOfYear(final int year) {
9292
return (year * 365L + (leapYears - DAYS_0000_TO_1970)) * MILLIS_PER_DAY; // millis per day
9393
}
9494

95-
private static boolean isLeapYear(final int year) {
96-
return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0);
95+
static boolean isLeapYear(final int year) {
96+
// Joda had
97+
// return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0);
98+
// But we've replaced that with this:
99+
if ((year & 3) != 0) {
100+
return false;
101+
}
102+
if (year % 100 != 0) {
103+
return true;
104+
}
105+
return ((year / 100) & 3) == 0;
106+
/*
107+
* It is a little faster because it saves a division. We don't have good
108+
* measurements for this method on its own, but this change speeds up
109+
* rounding the nearest month by about 8%.
110+
*
111+
* Note: If you decompile this method to x86 assembly you won't see the
112+
* division you'd expect from % 100 and / 100. Instead you'll see a funny
113+
* sequence of bit twiddling operations which the jvm thinks is faster.
114+
* Division is slow so it almost certainly is.
115+
*/
97116
}
98117

99118
private static final long AVERAGE_MILLIS_PER_YEAR_DIVIDED_BY_TWO = MILLIS_PER_YEAR / 2;

server/src/test/java/org/elasticsearch/common/time/DateUtilsRoundingTests.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,14 @@ public void testDateUtilsRounding() {
4646
}
4747
}
4848
}
49+
50+
public void testIsLeapYear() {
51+
assertTrue(DateUtilsRounding.isLeapYear(2004));
52+
assertTrue(DateUtilsRounding.isLeapYear(2000));
53+
assertTrue(DateUtilsRounding.isLeapYear(1996));
54+
assertFalse(DateUtilsRounding.isLeapYear(2001));
55+
assertFalse(DateUtilsRounding.isLeapYear(1900));
56+
assertFalse(DateUtilsRounding.isLeapYear(-1000));
57+
assertTrue(DateUtilsRounding.isLeapYear(-996));
58+
}
4959
}

0 commit comments

Comments
 (0)