Speed up converting of temporal accessor to zoned date time

The existing implementation was slow due to exceptions being thrown if
an accessor did not have a time zone. This implementation queries for
having a timezone, local time and local date and also checks for an
instant preventing to throw an exception and thus speeding up the conversion.

This removes the existing method and create a new one named
DateFormatters.from(TemporalAccessor accessor) to resemble the naming of
the java time ones.

Before this change an epoch millis parser using the toZonedDateTime
method took

DateFormatterToZonedTimeBenchmark.benchmarkToZonedDateTime  avgt   30  3970,202 ± 71,260  ns/op

With this change

DateFormatterToZonedTimeBenchmark.benchmarkToZonedDateTime  avgt   30  67,863 ± 1,162  ns/op

Closes elastic#37826

benchmarks/src/main/java/org/elasticsearch/benchmark/time/DateFormatterToZonedTimeBenchmark.java

@@ -0,0 +1,53 @@
+/*
+ * Licensed to Elasticsearch under one or more contributor
+ * license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright
+ * ownership. Elasticsearch licenses this file to you under
+ * the Apache License, Version 2.0 (the "License"); you may
+ * not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.elasticsearch.benchmark.time;
+
+import org.elasticsearch.common.time.DateFormatter;
+import org.elasticsearch.common.time.DateFormatters;
+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.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.time.temporal.TemporalAccessor;
+import java.util.concurrent.TimeUnit;
+
+@Fork(3)
+@Warmup(iterations = 10)
+@Measurement(iterations = 10)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@State(Scope.Benchmark)
+@SuppressWarnings("unused") //invoked by benchmarking framework
+public class DateFormatterToZonedTimeBenchmark {
+
+    private final TemporalAccessor accessor = DateFormatter.forPattern("epoch_millis").parse("1234567890");
+
+    @Benchmark
+    public TemporalAccessor benchmarkToZonedDateTime() {
+        // benchmark an accessor that does not contain a timezone
+        // this used to throw an exception earlier and thus was very very slow
+        return DateFormatters.from(accessor);
+    }
+}

client/rest-high-level/src/main/java/org/elasticsearch/client/ml/job/util/TimeUtil.java

@@ -39,7 +39,7 @@ public static Date parseTimeField(XContentParser parser, String fieldName) throw
         if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) {
             return new Date(parser.longValue());
         } else if (parser.currentToken() == XContentParser.Token.VALUE_STRING) {
-            return new Date(DateFormatters.toZonedDateTime(DateTimeFormatter.ISO_INSTANT.parse(parser.text())).toInstant().toEpochMilli());
+            return new Date(DateFormatters.from(DateTimeFormatter.ISO_INSTANT.parse(parser.text())).toInstant().toEpochMilli());
         }
         throw new IllegalArgumentException(
             "unexpected token [" + parser.currentToken() + "] for [" + fieldName + "]");

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/DateFormat.java

@@ -29,12 +29,22 @@
 
 import java.time.Instant;
 import java.time.LocalDate;
+import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
+import java.time.temporal.ChronoField;
 import java.time.temporal.TemporalAccessor;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Locale;
 import java.util.function.Function;
 
+import static java.time.temporal.ChronoField.DAY_OF_MONTH;
+import static java.time.temporal.ChronoField.HOUR_OF_DAY;
+import static java.time.temporal.ChronoField.MINUTE_OF_DAY;
+import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
+import static java.time.temporal.ChronoField.NANO_OF_SECOND;
+
 enum DateFormat {
     Iso8601 {
         @Override
@@ -71,6 +81,9 @@ private long parseMillis(String date) {
         }
     },
     Java {
+        private final List<ChronoField> FIELDS =
+            Arrays.asList(NANO_OF_SECOND, MINUTE_OF_DAY, HOUR_OF_DAY, DAY_OF_MONTH, MONTH_OF_YEAR);
+
         @Override
         Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Locale locale) {
             // in case you are wondering why we do not call 'DateFormatter.forPattern(format)' for all cases here, but only for the
@@ -80,20 +93,33 @@ Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Loc
             // This means that we leave the existing parsing logic in place, but will fall back to the new java date parsing logic, if an
             // "8" is prepended to the date format string
             int year = LocalDate.now(ZoneOffset.UTC).getYear();
+            ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(timezone);
             if (format.startsWith("8")) {
                 DateFormatter formatter = DateFormatter.forPattern(format)
                     .withLocale(locale)
-                    .withZone(DateUtils.dateTimeZoneToZoneId(timezone));
+                    .withZone(zoneId);
                 return text -> {
-                    ZonedDateTime defaultZonedDateTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
                     TemporalAccessor accessor = formatter.parse(text);
-                    long millis = DateFormatters.toZonedDateTime(accessor, defaultZonedDateTime).toInstant().toEpochMilli();
+                    // if there is no year, we fall back to the current one and
+                    // fill the rest of the date up with the parsed date
+                    if (accessor.isSupported(ChronoField.YEAR) == false) {
+                        ZonedDateTime newTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
+                        for (ChronoField field : FIELDS) {
+                            if (accessor.isSupported(field)) {
+                                newTime = newTime.with(field, accessor.get(field));
+                            }
+                        }
+
+                        accessor = newTime.withZoneSameLocal(zoneId);
+                    }
+
+                    long millis = DateFormatters.from(accessor).toInstant().toEpochMilli();
                     return new DateTime(millis, timezone);
                 };
             } else {
                 DateFormatter formatter = Joda.forPattern(format)
                     .withYear(year)
-                    .withZone(DateUtils.dateTimeZoneToZoneId(timezone))
+                    .withZone(zoneId)
                     .withLocale(locale);
                 return text -> new DateTime(formatter.parseMillis(text), timezone);
             }

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/DateFormatTests.java

@@ -19,29 +19,53 @@
 
 package org.elasticsearch.ingest.common;
 
+import org.elasticsearch.common.time.DateUtils;
 import org.elasticsearch.test.ESTestCase;
 import org.joda.time.DateTime;
 import org.joda.time.DateTimeZone;
 
 import java.time.Instant;
 import java.time.ZoneId;
+import java.time.ZoneOffset;
+import java.time.ZonedDateTime;
 import java.time.format.DateTimeFormatter;
 import java.util.Locale;
 import java.util.function.Function;
 
+import static org.hamcrest.Matchers.is;
 import static org.hamcrest.core.IsEqual.equalTo;
 
 public class DateFormatTests extends ESTestCase {
 
-    public void testParseJoda() {
-        Function<String, DateTime> jodaFunction = DateFormat.Java.getFunction("MMM dd HH:mm:ss Z",
+    public void testParseJava() {
+        Function<String, DateTime> javaFunction = DateFormat.Java.getFunction("MMM dd HH:mm:ss Z",
                 DateTimeZone.forOffsetHours(-8), Locale.ENGLISH);
-        assertThat(Instant.ofEpochMilli(jodaFunction.apply("Nov 24 01:29:01 -0800").getMillis())
+        assertThat(Instant.ofEpochMilli(javaFunction.apply("Nov 24 01:29:01 -0800").getMillis())
                         .atZone(ZoneId.of("GMT-8"))
                         .format(DateTimeFormatter.ofPattern("MM dd HH:mm:ss", Locale.ENGLISH)),
                 equalTo("11 24 01:29:01"));
     }
 
+    public void testParseJodaDefaultYear() {
+        String format = randomFrom("dd/MM");
+        DateTimeZone timezone = DateUtils.zoneIdToDateTimeZone(ZoneId.of("Europe/Amsterdam"));
+        Function<String, DateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ENGLISH);
+        int year = ZonedDateTime.now(ZoneOffset.UTC).getYear();
+        DateTime dateTime = javaFunction.apply("12/06");
+        assertThat(dateTime.getYear(), is(year));
+        assertThat(dateTime.toString(), is(year + "-06-12T00:00:00.000+02:00"));
+    }
+
+    public void testParseJavaDefaultYear() {
+        String format = randomFrom("8dd/MM");
+        DateTimeZone timezone = DateUtils.zoneIdToDateTimeZone(ZoneId.of("Europe/Amsterdam"));
+        Function<String, DateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ENGLISH);
+        int year = ZonedDateTime.now(ZoneOffset.UTC).getYear();
+        DateTime dateTime = javaFunction.apply("12/06");
+        assertThat(dateTime.getYear(), is(year));
+        assertThat(dateTime.toString(), is(year + "-06-12T00:00:00.000+02:00"));
+    }
+
     public void testParseUnixMs() {
         assertThat(DateFormat.UnixMs.getFunction(null, DateTimeZone.UTC, null).apply("1000500").getMillis(), equalTo(1000500L));
     }

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

@@ -20,11 +20,13 @@
 package org.elasticsearch.common.time;
 
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.SuppressForbidden;
 
-import java.time.DateTimeException;
 import java.time.DayOfWeek;
 import java.time.Instant;
 import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.Year;
 import java.time.ZoneId;
 import java.time.ZoneOffset;
 import java.time.ZonedDateTime;
@@ -53,6 +55,12 @@
 
 public class DateFormatters {
 
+    private static final DateTimeFormatter PARSE_DEFAULT_EPOCH_PARSER = new DateTimeFormatterBuilder()
+        .parseDefaulting(ChronoField.YEAR, 1970)
+        .parseDefaulting(MONTH_OF_YEAR, 1)
+        .parseDefaulting(DAY_OF_YEAR, 1)
+        .toFormatter(Locale.ROOT);
+
     private static final DateTimeFormatter TIME_ZONE_FORMATTER_NO_COLON = new DateTimeFormatterBuilder()
         .appendOffset("+HHmm", "Z")
         .toFormatter(Locale.ROOT);
@@ -1544,105 +1552,82 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
             dateTimeFormatters.toArray(new DateTimeFormatter[0]));
     }
 
-    private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);
+    private static final LocalDate LOCALDATE_EPOCH = LocalDate.of(1970, 1, 1);
 
-    public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor) {
-        return toZonedDateTime(accessor, EPOCH_ZONED_DATE_TIME);
-    }
-
-    public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor, ZonedDateTime defaults) {
-        try {
-            return ZonedDateTime.from(accessor);
-        } catch (DateTimeException e ) {
+    /**
+     * Convert a temporal accessor to a zoned date time object - as performant as possible.
+     * The .from() methods from the JDK are throwing exceptions when for example ZonedDateTime.from(accessor)
+     * or Instant.from(accessor). This results in a huge performance penalty and should be prevented
+     * This method prevents exceptions by querying the accessor for certain capabilities
+     * and then act on it accordingly
+     *
+     * This action assumes that we can reliably fall back to some defaults if not all parts of a
+     * zoned date time are set
+     *
+     * - If a zoned date time is passed, it is returned
+     * - If no timezone is found, ZoneOffset.UTC is used
+     * - If we find a time and a date, converting to a ZonedDateTime is straight forward,
+     *   no defaults will be applied
+     * - If an accessor only containing of seconds and nanos is found (like epoch_millis/second)
+     *   an Instant is created out of that, that becomes a ZonedDateTime with a time zone
+     * - If no time is given, the start of the day is used
+     * - If no month of the year is found, the first day of the year is used
+     * - If an iso based weekyear is found, but not week is specified, the first monday
+     *   of the new year is chosen (reataining BWC to joda time)
+     * - If an iso based weekyear is found and an iso based weekyear week, the start
+     *   of the day is used
+     *
+     * @param accessor The accessor returned from a parser
+     *
+     * @return The converted zoned date time
+     */
+    @SuppressForbidden(reason = "LocalDate.of is fine here")
+    public static ZonedDateTime from(TemporalAccessor accessor) {
+        if (accessor instanceof ZonedDateTime) {
+            return (ZonedDateTime) accessor;
         }
 
-        ZonedDateTime result = defaults;
-
-        // special case epoch seconds
-        if (accessor.isSupported(ChronoField.INSTANT_SECONDS)) {
-            result = result.with(ChronoField.INSTANT_SECONDS, accessor.getLong(ChronoField.INSTANT_SECONDS));
-            if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
-                result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
-            }
-            return result;
+        ZoneId zoneId = accessor.query(TemporalQueries.zone());
+        if (zoneId == null) {
+            zoneId = ZoneOffset.UTC;
         }
-
-        // try to set current year
-        if (accessor.isSupported(ChronoField.YEAR)) {
-            result = result.with(ChronoField.YEAR, accessor.getLong(ChronoField.YEAR));
-        } else if (accessor.isSupported(ChronoField.YEAR_OF_ERA)) {
-            result = result.with(ChronoField.YEAR_OF_ERA, accessor.getLong(ChronoField.YEAR_OF_ERA));
+
+        LocalDate localDate = accessor.query(TemporalQueries.localDate());
+        LocalTime localTime = accessor.query(TemporalQueries.localTime());
+        boolean isLocalDateSet = localDate != null;
+        boolean isLocalTimeSet = localTime != null;
+
+        // the first two cases are the most common, so this allows us to exit early when parsing dates
+        if (isLocalDateSet && isLocalTimeSet) {
+            return ZonedDateTime.of(localDate, localTime, zoneId);
+        } else if (accessor.isSupported(ChronoField.INSTANT_SECONDS) && accessor.isSupported(NANO_OF_SECOND)) {
+            return Instant.from(accessor).atZone(zoneId);
+        } else if (isLocalDateSet) {
+            return localDate.atStartOfDay(zoneId);
+        } else if (isLocalTimeSet) {
+            return ZonedDateTime.of(LOCALDATE_EPOCH, localTime, zoneId);
+        } else if (accessor.isSupported(ChronoField.YEAR)) {
+            if (accessor.isSupported(MONTH_OF_YEAR)) {
+                return LocalDate.of(accessor.get(ChronoField.YEAR), accessor.get(MONTH_OF_YEAR), 1).atStartOfDay(zoneId);
+            } else {
+                return Year.of(accessor.get(ChronoField.YEAR)).atDay(1).atStartOfDay(zoneId);
+            }
         } else if (accessor.isSupported(WeekFields.ISO.weekBasedYear())) {
             if (accessor.isSupported(WeekFields.ISO.weekOfWeekBasedYear())) {
-                return LocalDate.from(result)
-                    .with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
-                    .withDayOfMonth(1) // makes this compatible with joda
+                return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
+                    .atDay(1)
                     .with(WeekFields.ISO.weekOfWeekBasedYear(), accessor.getLong(WeekFields.ISO.weekOfWeekBasedYear()))
-                    .atStartOfDay(ZoneOffset.UTC);
+                    .atStartOfDay(zoneId);
             } else {
-                return LocalDate.from(result)
-                    .with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
-                    // this exists solely to be BWC compatible with joda
-//                    .with(TemporalAdjusters.nextOrSame(DayOfWeek.MONDAY))
+                return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
+                    .atDay(1)
                     .with(TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY))
-                    .atStartOfDay(defaults.getZone());
-//                return result.withHour(0).withMinute(0).withSecond(0)
-//                    .with(WeekFields.ISO.weekBasedYear(), 0)
-//                    .with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()));
-//                return ((ZonedDateTime) tmp).with(WeekFields.ISO.weekOfWeekBasedYear(), 1);
+                    .atStartOfDay(zoneId);
             }
-        } else if (accessor.isSupported(IsoFields.WEEK_BASED_YEAR)) {
-            // special case weekbased year
-            result = result.with(IsoFields.WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_BASED_YEAR));
-            if (accessor.isSupported(IsoFields.WEEK_OF_WEEK_BASED_YEAR)) {
-                result = result.with(IsoFields.WEEK_OF_WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_OF_WEEK_BASED_YEAR));
-            }
-            return result;
-        }
-
-        // month
-        if (accessor.isSupported(ChronoField.MONTH_OF_YEAR)) {
-            result = result.with(ChronoField.MONTH_OF_YEAR, accessor.getLong(ChronoField.MONTH_OF_YEAR));
-        }
-
-        // day of month
-        if (accessor.isSupported(ChronoField.DAY_OF_MONTH)) {
-            result = result.with(ChronoField.DAY_OF_MONTH, accessor.getLong(ChronoField.DAY_OF_MONTH));
-        }
-
-        // hour
-        if (accessor.isSupported(ChronoField.HOUR_OF_DAY)) {
-            result = result.with(ChronoField.HOUR_OF_DAY, accessor.getLong(ChronoField.HOUR_OF_DAY));
-        }
-
-        // minute
-        if (accessor.isSupported(ChronoField.MINUTE_OF_HOUR)) {
-            result = result.with(ChronoField.MINUTE_OF_HOUR, accessor.getLong(ChronoField.MINUTE_OF_HOUR));
-        }
-
-        // second
-        if (accessor.isSupported(ChronoField.SECOND_OF_MINUTE)) {
-            result = result.with(ChronoField.SECOND_OF_MINUTE, accessor.getLong(ChronoField.SECOND_OF_MINUTE));
-        }
-
-        if (accessor.isSupported(ChronoField.OFFSET_SECONDS)) {
-            result = result.withZoneSameLocal(ZoneOffset.ofTotalSeconds(accessor.get(ChronoField.OFFSET_SECONDS)));
-        }
-
-        // millis
-        if (accessor.isSupported(ChronoField.MILLI_OF_SECOND)) {
-            result = result.with(ChronoField.MILLI_OF_SECOND, accessor.getLong(ChronoField.MILLI_OF_SECOND));
-        }
-
-        if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
-            result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
-        }
-
-        ZoneId zoneOffset = accessor.query(TemporalQueries.zone());
-        if (zoneOffset != null) {
-            result = result.withZoneSameLocal(zoneOffset);
         }
 
-        return result;
+        // we should not reach this piece of code, everything being parsed we should be able to
+        // convert to a zoned date time! If not, we have to extend the above methods
+        throw new IllegalArgumentException("temporal accessor [" + accessor + "] cannot be converted to zoned date time");
     }
 }