Skip to content

Commit b94acb6

Browse files
spinscalespinscale
authored
Speed up converting of temporal accessor to zoned date time (#37915)
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
1 parent 160d1bd commit b94acb6

File tree

22 files changed

+224
-143
lines changed

22 files changed

+224
-143
lines changed

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

Lines changed: 53 additions & 0 deletions
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‎

Lines changed: 1 addition & 1 deletion
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‎

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,23 @@
2828

2929
import java.time.Instant;
3030
import java.time.LocalDate;
31+
import java.time.ZoneId;
3132
import java.time.ZoneOffset;
3233
import java.time.ZonedDateTime;
34+
import java.time.temporal.ChronoField;
3335
import java.time.temporal.TemporalAccessor;
36+
import java.util.Arrays;
37+
import java.util.List;
3438
import java.util.Locale;
3539
import java.util.function.Function;
3640

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+
3748
enum DateFormat {
3849
Iso8601 {
3950
@Override
@@ -70,22 +81,37 @@ private long parseMillis(String date) {
7081
}
7182
},
7283
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+
7387
@Override
7488
Function<String, DateTime> getFunction(String format, DateTimeZone timezone, Locale locale) {
75-
7689
// support the 6.x BWC compatible way of parsing java 8 dates
7790
if (format.startsWith("8")) {
7891
format = format.substring(1);
7992
}
8093

94+
ZoneId zoneId = DateUtils.dateTimeZoneToZoneId(timezone);
8195
int year = LocalDate.now(ZoneOffset.UTC).getYear();
8296
DateFormatter formatter = DateFormatter.forPattern(format)
8397
.withLocale(locale)
84-
.withZone(DateUtils.dateTimeZoneToZoneId(timezone));
98+
.withZone(zoneId);
8599
return text -> {
86-
ZonedDateTime defaultZonedDateTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
87100
TemporalAccessor accessor = formatter.parse(text);
88-
long millis = DateFormatters.toZonedDateTime(accessor, defaultZonedDateTime).toInstant().toEpochMilli();
101+
// if there is no year, we fall back to the current one and
102+
// fill the rest of the date up with the parsed date
103+
if (accessor.isSupported(ChronoField.YEAR) == false) {
104+
ZonedDateTime newTime = Instant.EPOCH.atZone(ZoneOffset.UTC).withYear(year);
105+
for (ChronoField field : FIELDS) {
106+
if (accessor.isSupported(field)) {
107+
newTime = newTime.with(field, accessor.get(field));
108+
}
109+
}
110+
111+
accessor = newTime.withZoneSameLocal(zoneId);
112+
}
113+
114+
long millis = DateFormatters.from(accessor).toInstant().toEpochMilli();
89115
return new DateTime(millis, timezone);
90116
};
91117
}

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

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,29 +19,43 @@
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 {
3539

36-
public void testParseJoda() {
37-
Function<String, DateTime> jodaFunction = DateFormat.Java.getFunction("MMM dd HH:mm:ss Z",
40+
public void testParseJava() {
41+
Function<String, DateTime> javaFunction = DateFormat.Java.getFunction("MMM dd HH:mm:ss Z",
3842
DateTimeZone.forOffsetHours(-8), Locale.ENGLISH);
39-
assertThat(Instant.ofEpochMilli(jodaFunction.apply("Nov 24 01:29:01 -0800").getMillis())
43+
assertThat(Instant.ofEpochMilli(javaFunction.apply("Nov 24 01:29:01 -0800").getMillis())
4044
.atZone(ZoneId.of("GMT-8"))
4145
.format(DateTimeFormatter.ofPattern("MM dd HH:mm:ss", Locale.ENGLISH)),
4246
equalTo("11 24 01:29:01"));
4347
}
4448

49+
public void testParseJavaDefaultYear() {
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+
4559
public void testParseUnixMs() {
4660
assertThat(DateFormat.UnixMs.getFunction(null, DateTimeZone.UTC, null).apply("1000500").getMillis(), equalTo(1000500L));
4761
}

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

Lines changed: 75 additions & 87 deletions
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;
@@ -1550,105 +1552,91 @@ static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
15501552
dateTimeFormatters.toArray(new DateTimeFormatter[0]));
15511553
}
15521554

1553-
private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);
1555+
private static final LocalDate LOCALDATE_EPOCH = LocalDate.of(1970, 1, 1);
15541556

1555-
public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor) {
1556-
return toZonedDateTime(accessor, EPOCH_ZONED_DATE_TIME);
1557-
}
1558-
1559-
public static ZonedDateTime toZonedDateTime(TemporalAccessor accessor, ZonedDateTime defaults) {
1560-
try {
1561-
return ZonedDateTime.from(accessor);
1562-
} catch (DateTimeException e ) {
1557+
/**
1558+
* Convert a temporal accessor to a zoned date time object - as performant as possible.
1559+
* The .from() methods from the JDK are throwing exceptions when for example ZonedDateTime.from(accessor)
1560+
* or Instant.from(accessor). This results in a huge performance penalty and should be prevented
1561+
* This method prevents exceptions by querying the accessor for certain capabilities
1562+
* and then act on it accordingly
1563+
*
1564+
* This action assumes that we can reliably fall back to some defaults if not all parts of a
1565+
* zoned date time are set
1566+
*
1567+
* - If a zoned date time is passed, it is returned
1568+
* - If no timezone is found, ZoneOffset.UTC is used
1569+
* - If we find a time and a date, converting to a ZonedDateTime is straight forward,
1570+
* no defaults will be applied
1571+
* - If an accessor only containing of seconds and nanos is found (like epoch_millis/second)
1572+
* an Instant is created out of that, that becomes a ZonedDateTime with a time zone
1573+
* - If no time is given, the start of the day is used
1574+
* - If no month of the year is found, the first day of the year is used
1575+
* - If an iso based weekyear is found, but not week is specified, the first monday
1576+
* of the new year is chosen (reataining BWC to joda time)
1577+
* - If an iso based weekyear is found and an iso based weekyear week, the start
1578+
* of the day is used
1579+
*
1580+
* @param accessor The accessor returned from a parser
1581+
*
1582+
* @return The converted zoned date time
1583+
*/
1584+
public static ZonedDateTime from(TemporalAccessor accessor) {
1585+
if (accessor instanceof ZonedDateTime) {
1586+
return (ZonedDateTime) accessor;
15631587
}
15641588

1565-
ZonedDateTime result = defaults;
1566-
1567-
// special case epoch seconds
1568-
if (accessor.isSupported(ChronoField.INSTANT_SECONDS)) {
1569-
result = result.with(ChronoField.INSTANT_SECONDS, accessor.getLong(ChronoField.INSTANT_SECONDS));
1570-
if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
1571-
result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
1572-
}
1573-
return result;
1589+
ZoneId zoneId = accessor.query(TemporalQueries.zone());
1590+
if (zoneId == null) {
1591+
zoneId = ZoneOffset.UTC;
15741592
}
1575-
1576-
// try to set current year
1577-
if (accessor.isSupported(ChronoField.YEAR)) {
1578-
result = result.with(ChronoField.YEAR, accessor.getLong(ChronoField.YEAR));
1579-
} else if (accessor.isSupported(ChronoField.YEAR_OF_ERA)) {
1580-
result = result.with(ChronoField.YEAR_OF_ERA, accessor.getLong(ChronoField.YEAR_OF_ERA));
1593+
1594+
LocalDate localDate = accessor.query(TemporalQueries.localDate());
1595+
LocalTime localTime = accessor.query(TemporalQueries.localTime());
1596+
boolean isLocalDateSet = localDate != null;
1597+
boolean isLocalTimeSet = localTime != null;
1598+
1599+
// the first two cases are the most common, so this allows us to exit early when parsing dates
1600+
if (isLocalDateSet && isLocalTimeSet) {
1601+
return of(localDate, localTime, zoneId);
1602+
} else if (accessor.isSupported(ChronoField.INSTANT_SECONDS) && accessor.isSupported(NANO_OF_SECOND)) {
1603+
return Instant.from(accessor).atZone(zoneId);
1604+
} else if (isLocalDateSet) {
1605+
return localDate.atStartOfDay(zoneId);
1606+
} else if (isLocalTimeSet) {
1607+
return of(LOCALDATE_EPOCH, localTime, zoneId);
1608+
} else if (accessor.isSupported(ChronoField.YEAR)) {
1609+
if (accessor.isSupported(MONTH_OF_YEAR)) {
1610+
return getFirstOfMonth(accessor).atStartOfDay(zoneId);
1611+
} else {
1612+
return Year.of(accessor.get(ChronoField.YEAR)).atDay(1).atStartOfDay(zoneId);
1613+
}
15811614
} else if (accessor.isSupported(WeekFields.ISO.weekBasedYear())) {
15821615
if (accessor.isSupported(WeekFields.ISO.weekOfWeekBasedYear())) {
1583-
return LocalDate.from(result)
1584-
.with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
1585-
.withDayOfMonth(1) // makes this compatible with joda
1616+
return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
1617+
.atDay(1)
15861618
.with(WeekFields.ISO.weekOfWeekBasedYear(), accessor.getLong(WeekFields.ISO.weekOfWeekBasedYear()))
1587-
.atStartOfDay(ZoneOffset.UTC);
1619+
.atStartOfDay(zoneId);
15881620
} else {
1589-
return LocalDate.from(result)
1590-
.with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()))
1591-
// this exists solely to be BWC compatible with joda
1592-
// .with(TemporalAdjusters.nextOrSame(DayOfWeek.MONDAY))
1621+
return Year.of(accessor.get(WeekFields.ISO.weekBasedYear()))
1622+
.atDay(1)
15931623
.with(TemporalAdjusters.firstInMonth(DayOfWeek.MONDAY))
1594-
.atStartOfDay(defaults.getZone());
1595-
// return result.withHour(0).withMinute(0).withSecond(0)
1596-
// .with(WeekFields.ISO.weekBasedYear(), 0)
1597-
// .with(WeekFields.ISO.weekBasedYear(), accessor.getLong(WeekFields.ISO.weekBasedYear()));
1598-
// return ((ZonedDateTime) tmp).with(WeekFields.ISO.weekOfWeekBasedYear(), 1);
1599-
}
1600-
} else if (accessor.isSupported(IsoFields.WEEK_BASED_YEAR)) {
1601-
// special case weekbased year
1602-
result = result.with(IsoFields.WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_BASED_YEAR));
1603-
if (accessor.isSupported(IsoFields.WEEK_OF_WEEK_BASED_YEAR)) {
1604-
result = result.with(IsoFields.WEEK_OF_WEEK_BASED_YEAR, accessor.getLong(IsoFields.WEEK_OF_WEEK_BASED_YEAR));
1624+
.atStartOfDay(zoneId);
16051625
}
1606-
return result;
1607-
}
1608-
1609-
// month
1610-
if (accessor.isSupported(ChronoField.MONTH_OF_YEAR)) {
1611-
result = result.with(ChronoField.MONTH_OF_YEAR, accessor.getLong(ChronoField.MONTH_OF_YEAR));
16121626
}
16131627

1614-
// day of month
1615-
if (accessor.isSupported(ChronoField.DAY_OF_MONTH)) {
1616-
result = result.with(ChronoField.DAY_OF_MONTH, accessor.getLong(ChronoField.DAY_OF_MONTH));
1617-
}
1618-
1619-
// hour
1620-
if (accessor.isSupported(ChronoField.HOUR_OF_DAY)) {
1621-
result = result.with(ChronoField.HOUR_OF_DAY, accessor.getLong(ChronoField.HOUR_OF_DAY));
1622-
}
1623-
1624-
// minute
1625-
if (accessor.isSupported(ChronoField.MINUTE_OF_HOUR)) {
1626-
result = result.with(ChronoField.MINUTE_OF_HOUR, accessor.getLong(ChronoField.MINUTE_OF_HOUR));
1627-
}
1628-
1629-
// second
1630-
if (accessor.isSupported(ChronoField.SECOND_OF_MINUTE)) {
1631-
result = result.with(ChronoField.SECOND_OF_MINUTE, accessor.getLong(ChronoField.SECOND_OF_MINUTE));
1632-
}
1633-
1634-
if (accessor.isSupported(ChronoField.OFFSET_SECONDS)) {
1635-
result = result.withZoneSameLocal(ZoneOffset.ofTotalSeconds(accessor.get(ChronoField.OFFSET_SECONDS)));
1636-
}
1637-
1638-
// millis
1639-
if (accessor.isSupported(ChronoField.MILLI_OF_SECOND)) {
1640-
result = result.with(ChronoField.MILLI_OF_SECOND, accessor.getLong(ChronoField.MILLI_OF_SECOND));
1641-
}
1642-
1643-
if (accessor.isSupported(ChronoField.NANO_OF_SECOND)) {
1644-
result = result.with(ChronoField.NANO_OF_SECOND, accessor.getLong(ChronoField.NANO_OF_SECOND));
1645-
}
1628+
// we should not reach this piece of code, everything being parsed we should be able to
1629+
// convert to a zoned date time! If not, we have to extend the above methods
1630+
throw new IllegalArgumentException("temporal accessor [" + accessor + "] cannot be converted to zoned date time");
1631+
}
16461632

1647-
ZoneId zoneOffset = accessor.query(TemporalQueries.zone());
1648-
if (zoneOffset != null) {
1649-
result = result.withZoneSameLocal(zoneOffset);
1650-
}
1633+
@SuppressForbidden(reason = "ZonedDateTime.of is fine here")
1634+
private static ZonedDateTime of(LocalDate localDate, LocalTime localTime, ZoneId zoneId) {
1635+
return ZonedDateTime.of(localDate, localTime, zoneId);
1636+
}
16511637

1652-
return result;
1638+
@SuppressForbidden(reason = "LocalDate.of is fine here")
1639+
private static LocalDate getFirstOfMonth(TemporalAccessor accessor) {
1640+
return LocalDate.of(accessor.get(ChronoField.YEAR), accessor.get(MONTH_OF_YEAR), 1);
16531641
}
16541642
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,15 +218,15 @@ private Instant parseDateTime(String value, ZoneId timeZone, boolean roundUpIfNo
218218
DateTimeFormatter formatter = roundUpIfNoTime ? this.roundUpFormatter : this.formatter;
219219
try {
220220
if (timeZone == null) {
221-
return DateFormatters.toZonedDateTime(formatter.parse(value)).toInstant();
221+
return DateFormatters.from(formatter.parse(value)).toInstant();
222222
} else {
223223
TemporalAccessor accessor = formatter.parse(value);
224224
ZoneId zoneId = TemporalQueries.zone().queryFrom(accessor);
225225
if (zoneId != null) {
226226
timeZone = zoneId;
227227
}
228228

229-
return DateFormatters.toZonedDateTime(accessor).withZoneSameLocal(timeZone).toInstant();
229+
return DateFormatters.from(accessor).withZoneSameLocal(timeZone).toInstant();
230230
}
231231
} catch (DateTimeParseException e) {
232232
throw new ElasticsearchParseException("failed to parse date field [{}] with format [{}]: [{}]",

‎server/src/main/java/org/elasticsearch/index/mapper/DateFieldMapper.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,7 @@ protected DateMathParser dateMathParser() {
247247
}
248248

249249
long parse(String value) {
250-
return DateFormatters.toZonedDateTime(dateTimeFormatter().parse(value)).toInstant().toEpochMilli();
250+
return DateFormatters.from(dateTimeFormatter().parse(value)).toInstant().toEpochMilli();
251251
}
252252

253253
@Override

0 commit comments

Comments
 (0)