From dfd502f9caf3476d439c24b81fa894b0a82d567c Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Mon, 10 Aug 2020 12:45:34 -0400 Subject: [PATCH] Rework checking if a year is a leap year (#60585) (#60790) 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. Co-authored-by: Elastic Machine --- benchmarks/README.md | 26 +++++++++++++++++++ .../common/time/DateUtilsRounding.java | 23 ++++++++++++++-- .../common/time/DateUtilsRoundingTests.java | 10 +++++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/benchmarks/README.md b/benchmarks/README.md index 1e89c3f356f..1be31c4a38c 100644 --- a/benchmarks/README.md +++ b/benchmarks/README.md @@ -63,3 +63,29 @@ To get realistic results, you should exercise care when running benchmarks. Here * Blindly believe the numbers that your microbenchmark produces but verify them by measuring e.g. with `-prof perfasm`. * Run more threads than your number of CPU cores (in case you run multi-threaded microbenchmarks). * Look only at the `Score` column and ignore `Error`. Instead take countermeasures to keep `Error` low / variance explainable. + +## Disassembling + +Disassembling is fun! Maybe not always useful, but always fun! Generally, you'll want to install `perf` and FCML's `hsdis`. +`perf` is generally available via `apg-get install perf` or `pacman -S perf`. FCML is a little more involved. This worked +on 2020-08-01: + +``` +wget https://github.com/swojtasiak/fcml-lib/releases/download/v1.2.2/fcml-1.2.2.tar.gz +tar xf fcml* +cd fcml* +./configure +make +cd example/hsdis +make +cp .libs/libhsdis.so.0.0.0 +sudo cp .libs/libhsdis.so.0.0.0 /usr/lib/jvm/java-14-adoptopenjdk/lib/hsdis-amd64.so +``` + +If you want to disassemble a single method do something like this: + +``` +gradlew -p benchmarks run --args ' MemoryStatsBenchmark -jvmArgs "-XX:+UnlockDiagnosticVMOptions -XX:CompileCommand=print,*.yourMethodName -XX:PrintAssemblyOptions=intel" +``` + +If you want `perf` to find the hot methods for you then do add `-prof:perfasm`. diff --git a/server/src/main/java/org/elasticsearch/common/time/DateUtilsRounding.java b/server/src/main/java/org/elasticsearch/common/time/DateUtilsRounding.java index 773f89f0051..da980f588b7 100644 --- a/server/src/main/java/org/elasticsearch/common/time/DateUtilsRounding.java +++ b/server/src/main/java/org/elasticsearch/common/time/DateUtilsRounding.java @@ -92,8 +92,27 @@ class DateUtilsRounding { return (year * 365L + (leapYears - DAYS_0000_TO_1970)) * MILLIS_PER_DAY; // millis per day } - private static boolean isLeapYear(final int year) { - return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0); + static boolean isLeapYear(final int year) { + // Joda had + // return ((year & 3) == 0) && ((year % 100) != 0 || (year % 400) == 0); + // But we've replaced that with this: + if ((year & 3) != 0) { + return false; + } + if (year % 100 != 0) { + return true; + } + return ((year / 100) & 3) == 0; + /* + * It is a little faster because it saves a division. We don't have good + * measurements for this method on its own, but this change speeds up + * rounding the nearest month by about 8%. + * + * Note: If you decompile this method to x86 assembly you won't see the + * division you'd expect from % 100 and / 100. Instead you'll see a funny + * sequence of bit twiddling operations which the jvm thinks is faster. + * Division is slow so it almost certainly is. + */ } private static final long AVERAGE_MILLIS_PER_YEAR_DIVIDED_BY_TWO = MILLIS_PER_YEAR / 2; diff --git a/server/src/test/java/org/elasticsearch/common/time/DateUtilsRoundingTests.java b/server/src/test/java/org/elasticsearch/common/time/DateUtilsRoundingTests.java index 4ec1c261a2a..6d964009da1 100644 --- a/server/src/test/java/org/elasticsearch/common/time/DateUtilsRoundingTests.java +++ b/server/src/test/java/org/elasticsearch/common/time/DateUtilsRoundingTests.java @@ -46,4 +46,14 @@ public class DateUtilsRoundingTests extends ESTestCase { } } } + + public void testIsLeapYear() { + assertTrue(DateUtilsRounding.isLeapYear(2004)); + assertTrue(DateUtilsRounding.isLeapYear(2000)); + assertTrue(DateUtilsRounding.isLeapYear(1996)); + assertFalse(DateUtilsRounding.isLeapYear(2001)); + assertFalse(DateUtilsRounding.isLeapYear(1900)); + assertFalse(DateUtilsRounding.isLeapYear(-1000)); + assertTrue(DateUtilsRounding.isLeapYear(-996)); + } }