Skip to content

Commit e00afd2

Browse files
spinscalespinscale
committed
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 approximately 50x longer. Relates elastic#37826 This commit also backports elastic#38171
1 parent d64541e commit e00afd2

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)