Skip to content

Commit 1094d3b

Browse files
spinscalespinscale
authored
Speed up converting of temporal accessor to zoned date time (#38172)
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 approximately 50x longer. Relates #37826 This commit also backports #38171
1 parent f8191e6 commit 1094d3b

File tree

10 files changed

+210
-100
lines changed

10 files changed

+210
-100
lines changed

‎benchmarks/src/main/java/org/elasticsearch/benchmark/time/DateFormatterFromBenchmark.java

+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.elasticsearch.benchmark.time;
20+
21+
import org.elasticsearch.common.time.DateFormatter;
22+
import org.elasticsearch.common.time.DateFormatters;
23+
import org.openjdk.jmh.annotations.Benchmark;
24+
import org.openjdk.jmh.annotations.BenchmarkMode;
25+
import org.openjdk.jmh.annotations.Fork;
26+
import org.openjdk.jmh.annotations.Measurement;
27+
import org.openjdk.jmh.annotations.Mode;
28+
import org.openjdk.jmh.annotations.OutputTimeUnit;
29+
import org.openjdk.jmh.annotations.Scope;
30+
import org.openjdk.jmh.annotations.State;
31+
import org.openjdk.jmh.annotations.Warmup;
32+
33+
import java.time.temporal.TemporalAccessor;
34+
import java.util.concurrent.TimeUnit;
35+
36+
@Fork(3)
37+
@Warmup(iterations = 10)
38+
@Measurement(iterations = 10)
39+
@BenchmarkMode(Mode.AverageTime)
40+
@OutputTimeUnit(TimeUnit.NANOSECONDS)
41+
@State(Scope.Benchmark)
42+
@SuppressWarnings("unused") //invoked by benchmarking framework
43+
public class DateFormatterFromBenchmark {
44+
45+
private final TemporalAccessor accessor = DateFormatter.forPattern("epoch_millis").parse("1234567890");
46+
47+
@Benchmark
48+
public TemporalAccessor benchmarkFrom() {
49+
// benchmark an accessor that does not contain a timezone
50+
// this used to throw an exception earlier and thus was very very slow
51+
return DateFormatters.from(accessor);
52+
}
53+
}

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ public static Date parseTimeField(XContentParser parser, String fieldName) throw
3939
if (parser.currentToken() == XContentParser.Token.VALUE_NUMBER) {
4040
return new Date(parser.longValue());
4141
} else if (parser.currentToken() == XContentParser.Token.VALUE_STRING) {
42-
return new Date(DateFormatters.toZonedDateTime(DateTimeFormatter.ISO_INSTANT.parse(parser.text())).toInstant().toEpochMilli());
42+
return new Date(DateFormatters.from(DateTimeFormatter.ISO_INSTANT.parse(parser.text())).toInstant().toEpochMilli());
4343
}
4444
throw new IllegalArgumentException(
4545
"unexpected token [" + parser.currentToken() + "] for [" + fieldName + "]");

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

+27-2
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,20 @@
3131
import java.time.LocalDate;
3232
import java.time.ZoneOffset;
3333
import java.time.ZonedDateTime;
34+
import java.time.temporal.ChronoField;
3435
import java.time.temporal.TemporalAccessor;
36+
import java.util.Arrays;
37+
import java.util.List;
3538
import java.util.Locale;
3639
import java.util.function.Function;
3740

41+
import static java.time.temporal.ChronoField.DAY_OF_MONTH;
42+
import static java.time.temporal.ChronoField.HOUR_OF_DAY;
43+
import static java.time.temporal.ChronoField.MINUTE_OF_DAY;
44+
import static java.time.temporal.ChronoField.MONTH_OF_YEAR;
45+
import static java.time.temporal.ChronoField.NANO_OF_SECOND;
46+
import static java.time.temporal.ChronoField.SECOND_OF_DAY;
47+
3848
enum DateFormat {
3949
Iso8601 {
4050
@Override
@@ -71,6 +81,9 @@ private long parseMillis(String date) {
7181
}
7282
},
7383
Java {
84+
private final List<ChronoField> FIELDS =
85+
Arrays.asList(NANO_OF_SECOND, SECOND_OF_DAY, MINUTE_OF_DAY, HOUR_OF_DAY, DAY_OF_MONTH, MONTH_OF_YEAR);
86+
7487
@Override
7588
Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Locale locale) {
7689
// in case you are wondering why we do not call 'DateFormatter.forPattern(format)' for all cases here, but only for the
@@ -85,9 +98,21 @@ Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Loc
8598
.withLocale(locale)
8699
.withZone(DateUtils.dateTimeZoneToZoneId(timezone));
87100
return text -> {
88-
ZonedDateTime defaultZonedDateTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
89101
TemporalAccessor accessor = formatter.parse(text);
90-
long millis = DateFormatters.toZonedDateTime(accessor, defaultZonedDateTime).toInstant().toEpochMilli();
102+
// if there is no year, we fall back to the current one and
103+
// fill the rest of the date up with the parsed date
104+
if (accessor.isSupported(ChronoField.YEAR) == false) {
105+
ZonedDateTime newTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
106+
for (ChronoField field : FIELDS) {
107+
if (accessor.isSupported(field)) {
108+
newTime = newTime.with(field, accessor.get(field));
109+
}
110+
}
111+
112+
accessor = newTime.withZoneSameLocal(DateUtils.dateTimeZoneToZoneId(timezone));
113+
}
114+
115+
long millis = DateFormatters.from(accessor).toInstant().toEpochMilli();
91116
return new DateTime(millis, timezone);
92117
};
93118
} else {

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

+29
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,20 @@
1919

2020
package org.elasticsearch.ingest.common;
2121

22+
import org.elasticsearch.common.time.DateUtils;
2223
import org.elasticsearch.test.ESTestCase;
2324
import org.joda.time.DateTime;
2425
import org.joda.time.DateTimeZone;
2526

2627
import java.time.Instant;
2728
import java.time.ZoneId;
29+
import java.time.ZoneOffset;
30+
import java.time.ZonedDateTime;
2831
import java.time.format.DateTimeFormatter;
2932
import java.util.Locale;
3033
import java.util.function.Function;
3134

35+
import static org.hamcrest.Matchers.is;
3236
import static org.hamcrest.core.IsEqual.equalTo;
3337

3438
public class DateFormatTests extends ESTestCase {
@@ -42,6 +46,31 @@ public void testParseJoda() {
4246
equalTo("11 24 01:29:01"));
4347
}
4448

49+
public void testParseJodaDefaultYear() {
50+
String format = randomFrom("8dd/MM", "dd/MM");
51+
DateTimeZone timezone = DateUtils.zoneIdToDateTimeZone(ZoneId.of("Europe/Amsterdam"));
52+
Function<String, DateTime> javaFunction = DateFormat.Java.getFunction(format, timezone, Locale.ENGLISH);
53+
int year = ZonedDateTime.now(ZoneOffset.UTC).getYear();
54+
DateTime dateTime = javaFunction.apply("12/06");
55+
assertThat(dateTime.getYear(), is(year));
56+
assertThat(dateTime.toString(), is(year + "-06-12T00:00:00.000+02:00"));
57+
}
58+
59+
// if there is a time around end of year, which is different in UTC make sure the result is the same
60+
public void testParseDefaultYearBackwardsCompatible() {
61+
ZoneId zoneId = ZoneId.of("America/New_York");
62+
DateTimeZone timezone = DateUtils.zoneIdToDateTimeZone(zoneId);
63+
int year = ZonedDateTime.now(zoneId).getYear();
64+
int nextYear = year + 1;
65+
66+
DateTime javaDateTime = DateFormat.Java.getFunction("8dd/MM HH:mm", timezone, Locale.ENGLISH).apply("31/12 23:59");
67+
DateTime jodaDateTime = DateFormat.Java.getFunction("dd/MM HH:mm", timezone, Locale.ENGLISH).apply("31/12 23:59");
68+
assertThat(javaDateTime.getYear(), is(jodaDateTime.getYear()));
69+
assertThat(year, is(jodaDateTime.getYear()));
70+
assertThat(javaDateTime.withZone(DateTimeZone.UTC), is(jodaDateTime.withZone(DateTimeZone.UTC)));
71+
assertThat(nextYear, is(jodaDateTime.withZone(DateTimeZone.UTC).getYear()));
72+
}
73+
4574
public void testParseUnixMs() {
4675
assertThat(DateFormat.UnixMs.getFunction(null, DateTimeZone.UTC, null).apply("1000500").getMillis(), equalTo(1000500L));
4776
}

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

+86-83
Original file line numberDiff line numberDiff line change
@@ -20,11 +20,13 @@
2020
package org.elasticsearch.common.time;
2121

2222
import org.elasticsearch.common.Strings;
23+
import org.elasticsearch.common.SuppressForbidden;
2324

24-
import java.time.DateTimeException;
2525
import java.time.DayOfWeek;
2626
import java.time.Instant;
2727
import java.time.LocalDate;
28+
import java.time.LocalTime;
29+
import java.time.Year;
2830
import java.time.ZoneId;
2931
import java.time.ZoneOffset;
3032
import java.time.ZonedDateTime;
@@ -1503,105 +1505,106 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
15031505
dateTimeFormatters.toArray(new DateTimeFormatter[0]));
15041506
}
15051507

1506-
private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);
1508+
private static final LocalDate LOCALDATE_EPOCH = LocalDate.of(1970, 1, 1);
15071509

1508-
public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor) {
1509-
return toZonedDateTime(accessor, EPOCH_ZONED_DATE_TIME);
1510-
}
1511-
1512-
public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor, ZonedDateTime defaults) {
1513-
try {
1514-
return ZonedDateTime.from(accessor);
1515-
} catch (DateTimeException e ) {
1510+
/**
1511+
* Convert a temporal accessor to a zoned date time object - as performant as possible.
1512+
* The .from() methods from the JDK are throwing exceptions when for example ZonedDateTime.from(accessor)
1513+
* or Instant.from(accessor). This results in a huge performance penalty and should be prevented
1514+
* This method prevents exceptions by querying the accessor for certain capabilities
1515+
* and then act on it accordingly
1516+
*
1517+
* This action assumes that we can reliably fall back to some defaults if not all parts of a
1518+
* zoned date time are set
1519+
*
1520+
* - If a zoned date time is passed, it is returned
1521+
* - If no timezone is found, ZoneOffset.UTC is used
1522+
* - If we find a time and a date, converting to a ZonedDateTime is straight forward,
1523+
* no defaults will be applied
1524+
* - If an accessor only containing of seconds and nanos is found (like epoch_millis/second)
1525+
* an Instant is created out of that, that becomes a ZonedDateTime with a time zone
1526+
* - If no time is given, the start of the day is used
1527+
* - If no month of the year is found, the first day of the year is used
1528+
* - If an iso based weekyear is found, but not week is specified, the first monday
1529+
* of the new year is chosen (reataining BWC to joda time)
1530+
* - If an iso based weekyear is found and an iso based weekyear week, the start
1531+
* of the day is used
1532+
*
1533+
* @param accessor The accessor returned from a parser
1534+
*
1535+
* @return The converted zoned date time
1536+
*/
1537+
public static ZonedDateTime from(TemporalAccessor accessor) {
1538+
if (accessor instanceof ZonedDateTime) {
1539+
return (ZonedDateTime) accessor;
15161540
}
15171541

1518-
ZonedDateTime result = defaults;
1519-
1520-
// special case epoch seconds
1521-
if (accessor.isSupported(ChronoField.INSTANT_SECONDS)) {
1522-
result = result.with(ChronoField.INSTANT_SECONDS, accessor.getLong(ChronoField.INSTANT_SECONDS));
1523-
if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
1524-
result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
1525-
}
1526-
return result;
1542+
ZoneId zoneId = accessor.query(TemporalQueries.zone());
1543+
if (zoneId == null) {
1544+
zoneId = ZoneOffset.UTC;
15271545
}
15281546

1529-
// try to set current year
1530-
if (accessor.isSupported(ChronoField.YEAR)) {
1531-
result = result.with(ChronoField.YEAR, accessor.getLong(ChronoField.YEAR));
1532-
} else if (accessor.isSupported(ChronoField.YEAR_OF_ERA)) {
1533-
result = result.with(ChronoField.YEAR_OF_ERA, accessor.getLong(ChronoField.YEAR_OF_ERA));
1547+
LocalDate localDate = accessor.query(TemporalQueries.localDate());
1548+
LocalTime localTime = accessor.query(TemporalQueries.localTime());
1549+
boolean isLocalDateSet = localDate != null;
1550+
boolean isLocalTimeSet = localTime != null;
1551+
1552+
// the first two cases are the most common, so this allows us to exit early when parsing dates
1553+
if (isLocalDateSet && isLocalTimeSet) {
1554+
return of(localDate, localTime, zoneId);
1555+
} else if (accessor.isSupported(ChronoField.INSTANT_SECONDS) && accessor.isSupported(NANO_OF_SECOND)) {
1556+
return Instant.from(accessor).atZone(zoneId);
1557+
} else if (isLocalDateSet) {
1558+
return localDate.atStartOfDay(zoneId);
1559+
} else if (isLocalTimeSet) {
1560+
return of(getLocaldate(accessor), localTime, zoneId);
1561+
} else if (accessor.isSupported(ChronoField.YEAR)) {
1562+
if (accessor.isSupported(MONTH_OF_YEAR)) {
1563+
return getFirstOfMonth(accessor).atStartOfDay(zoneId);
1564+
} else {
1565+
return Year.of(accessor.get(ChronoField.YEAR)).atDay(1).atStartOfDay(zoneId);
1566+
}
1567+
} else if (accessor.isSupported(MONTH_OF_YEAR)) {
1568+
// missing year, falling back to the epoch and then filling
1569+
return getLocaldate(accessor).atStartOfDay(zoneId);
15341570
} else if (accessor.isSupported(WeekFields.ISO.weekBasedYear())) {
15351571
if (accessor.isSupported(WeekFields.ISO.weekOfWeekBasedYear())) {
1536-
return LocalDate.from(result)
1537-
.with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
1538-
.withDayOfMonth(1) // makes this compatible with joda
1572+
return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
1573+
.atDay(1)
15391574
.with(WeekFields.ISO.weekOfWeekBasedYear(), accessor.getLong(WeekFields.ISO.weekOfWeekBasedYear()))
1540-
.atStartOfDay(ZoneOffset.UTC);
1575+
.atStartOfDay(zoneId);
15411576
} else {
1542-
return LocalDate.from(result)
1543-
.with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
1544-
// this exists solely to be BWC compatible with joda
1545-
// .with(TemporalAdjusters.nextOrSame(DayOfWeek.MONDAY))
1577+
return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
1578+
.atDay(1)
15461579
.with(TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY))
1547-
.atStartOfDay(defaults.getZone());
1548-
// return result.withHour(0).withMinute(0).withSecond(0)
1549-
// .with(WeekFields.ISO.weekBasedYear(), 0)
1550-
// .with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()));
1551-
// return ((ZonedDateTime) tmp).with(WeekFields.ISO.weekOfWeekBasedYear(), 1);
1580+
.atStartOfDay(zoneId);
15521581
}
1553-
} else if (accessor.isSupported(IsoFields.WEEK_BASED_YEAR)) {
1554-
// special case weekbased year
1555-
result = result.with(IsoFields.WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_BASED_YEAR));
1556-
if (accessor.isSupported(IsoFields.WEEK_OF_WEEK_BASED_YEAR)) {
1557-
result = result.with(IsoFields.WEEK_OF_WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_OF_WEEK_BASED_YEAR));
1558-
}
1559-
return result;
1560-
}
1561-
1562-
// month
1563-
if (accessor.isSupported(ChronoField.MONTH_OF_YEAR)) {
1564-
result = result.with(ChronoField.MONTH_OF_YEAR, accessor.getLong(ChronoField.MONTH_OF_YEAR));
15651582
}
15661583

1567-
// day of month
1568-
if (accessor.isSupported(ChronoField.DAY_OF_MONTH)) {
1569-
result = result.with(ChronoField.DAY_OF_MONTH, accessor.getLong(ChronoField.DAY_OF_MONTH));
1570-
}
1571-
1572-
// hour
1573-
if (accessor.isSupported(ChronoField.HOUR_OF_DAY)) {
1574-
result = result.with(ChronoField.HOUR_OF_DAY, accessor.getLong(ChronoField.HOUR_OF_DAY));
1575-
}
1576-
1577-
// minute
1578-
if (accessor.isSupported(ChronoField.MINUTE_OF_HOUR)) {
1579-
result = result.with(ChronoField.MINUTE_OF_HOUR, accessor.getLong(ChronoField.MINUTE_OF_HOUR));
1580-
}
1581-
1582-
// second
1583-
if (accessor.isSupported(ChronoField.SECOND_OF_MINUTE)) {
1584-
result = result.with(ChronoField.SECOND_OF_MINUTE, accessor.getLong(ChronoField.SECOND_OF_MINUTE));
1585-
}
1586-
1587-
if (accessor.isSupported(ChronoField.OFFSET_SECONDS)) {
1588-
result = result.withZoneSameLocal(ZoneOffset.ofTotalSeconds(accessor.get(ChronoField.OFFSET_SECONDS)));
1589-
}
1584+
// we should not reach this piece of code, everything being parsed we should be able to
1585+
// convert to a zoned date time! If not, we have to extend the above methods
1586+
throw new IllegalArgumentException("temporal accessor [" + accessor + "] cannot be converted to zoned date time");
1587+
}
15901588

1591-
// millis
1592-
if (accessor.isSupported(ChronoField.MILLI_OF_SECOND)) {
1593-
result = result.with(ChronoField.MILLI_OF_SECOND, accessor.getLong(ChronoField.MILLI_OF_SECOND));
1589+
private static LocalDate getLocaldate(TemporalAccessor accessor) {
1590+
if (accessor.isSupported(MONTH_OF_YEAR)) {
1591+
if (accessor.isSupported(DAY_OF_MONTH)) {
1592+
return LocalDate.of(1970, accessor.get(MONTH_OF_YEAR), accessor.get(DAY_OF_MONTH));
1593+
} else {
1594+
return LocalDate.of(1970, accessor.get(MONTH_OF_YEAR), 1);
1595+
}
15941596
}
15951597

1596-
if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
1597-
result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
1598-
}
1598+
return LOCALDATE_EPOCH;
1599+
}
15991600

1600-
ZoneId zoneOffset = accessor.query(TemporalQueries.zone());
1601-
if (zoneOffset != null) {
1602-
result = result.withZoneSameLocal(zoneOffset);
1603-
}
1601+
@SuppressForbidden(reason = "ZonedDateTime.of is fine here")
1602+
private static ZonedDateTime of(LocalDate localDate, LocalTime localTime, ZoneId zoneId) {
1603+
return ZonedDateTime.of(localDate, localTime, zoneId);
1604+
}
16041605

1605-
return result;
1606+
@SuppressForbidden(reason = "LocalDate.of is fine here")
1607+
private static LocalDate getFirstOfMonth(TemporalAccessor accessor) {
1608+
return LocalDate.of(accessor.get(ChronoField.YEAR), accessor.get(MONTH_OF_YEAR), 1);
16061609
}
16071610
}

‎server/src/main/java/org/elasticsearch/common/time/JavaDateMathParser.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -217,15 +217,15 @@ private long parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNoTim
217217
DateTimeFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
218218
try {
219219
if (timeZone == null) {
220-
return DateFormatters.toZonedDateTime(formatter.parse(value)).toInstant().toEpochMilli();
220+
return DateFormatters.from(formatter.parse(value)).toInstant().toEpochMilli();
221221
} else {
222222
TemporalAccessor accessor = formatter.parse(value);
223223
ZoneId zoneId = TemporalQueries.zone().queryFrom(accessor);
224224
if (zoneId != null) {
225225
timeZone = zoneId;
226226
}
227227

228-
return DateFormatters.toZonedDateTime(accessor).withZoneSameLocal(timeZone).toInstant().toEpochMilli();
228+
return DateFormatters.from(accessor).withZoneSameLocal(timeZone).toInstant().toEpochMilli();
229229
}
230230
} catch (IllegalArgumentException | DateTimeException e) {
231231
throw new ElasticsearchParseException("failed to parse date field [{}] in format [{}]: [{}]", e, value, format, e.getMessage());

0 commit comments

Comments
 (0)