Skip to content

Commit

Permalink
Date/Time parsing: Use java time API instead of exception handling (#…
Browse files Browse the repository at this point in the history 
…37222)

* Add benchmark

* Use java time API instead of exception handling

when several formatters are used, the existing way of parsing those is
to throw an exception catch it, and try the next one. This is is
considerably slower than the approach taken in joda time, so that
indexing is reduced when a date format like `x||y` is used and y is the
date format being used.

This commit now uses the java API to parse the date by appending the
date time formatters to each other and does not rely on exception
handling.

* fix benchmark

* fix tests by changing formatter, also expose printer

* restore optional printing logic to fix tests

* fix tests

* incorporate review comments
  • Loading branch information
@spinscale
spinscale authored Jan 11, 2019
1 parent bbd0930 commit 9f3da01
Show file tree
Hide file tree
Showing 6 changed files with 129 additions and 208 deletions.
58 changes: 58 additions & 0 deletions benchmarks/src/main/java/org/elasticsearch/benchmark/time/DateFormatterBenchmark.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* 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.joda.Joda;
import org.elasticsearch.common.time.DateFormatter;
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 DateFormatterBenchmark {

private final DateFormatter javaFormatter = DateFormatter.forPattern("8year_month_day||ordinal_date||epoch_millis");
private final DateFormatter jodaFormatter = Joda.forPattern("year_month_day||ordinal_date||epoch_millis");

@Benchmark
public TemporalAccessor parseJavaDate() {
return javaFormatter.parse("1234567890");
}

@Benchmark
public TemporalAccessor parseJodaDate() {
return jodaFormatter.parse("1234567890");
}
}

2 changes: 1 addition & 1 deletion server/src/main/java/org/elasticsearch/common/time/DateFormatter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ static DateFormatter forPattern(String input) {
if (formatters.size() == 1) {
return formatters.get(0);
}
return new DateFormatters.MergedDateFormatter(input, formatters);

return DateFormatters.merge(input, formatters);
}
}
158 changes: 28 additions & 130 deletions server/src/main/java/org/elasticsearch/common/time/DateFormatters.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

package org.elasticsearch.common.time;

import org.elasticsearch.ElasticsearchParseException;
import org.elasticsearch.common.Strings;

import java.time.DateTimeException;
Expand All @@ -31,7 +30,6 @@
import java.time.ZonedDateTime;
import java.time.format.DateTimeFormatter;
import java.time.format.DateTimeFormatterBuilder;
import java.time.format.DateTimeParseException;
import java.time.format.ResolverStyle;
import java.time.format.SignStyle;
import java.time.temporal.ChronoField;
Expand All @@ -40,10 +38,9 @@
import java.time.temporal.TemporalAdjusters;
import java.time.temporal.TemporalQueries;
import java.time.temporal.WeekFields;
import java.util.Collections;
import java.util.ArrayList;
import java.util.List;
import java.util.Locale;
import java.util.stream.Collectors;

import static java.time.temporal.ChronoField.DAY_OF_MONTH;
import static java.time.temporal.ChronoField.DAY_OF_WEEK;
Expand Down Expand Up @@ -77,21 +74,17 @@ public class DateFormatters {
.appendValue(SECOND_OF_MINUTE, 2, 2, SignStyle.NOT_NEGATIVE)
.toFormatter(Locale.ROOT);

private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER_1 = new DateTimeFormatterBuilder()
private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_PRINTER = new DateTimeFormatterBuilder()
.append(STRICT_YEAR_MONTH_DAY_FORMATTER)
.optionalStart()
.appendLiteral('T')
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
.optionalStart()
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.optionalEnd()
.appendFraction(NANO_OF_SECOND, 3, 9, true)
.optionalStart()
.appendZoneOrOffsetId()
.optionalEnd()
.optionalEnd()
.toFormatter(Locale.ROOT);

private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER_2 = new DateTimeFormatterBuilder()
private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER = new DateTimeFormatterBuilder()
.append(STRICT_YEAR_MONTH_DAY_FORMATTER)
.optionalStart()
.appendLiteral('T')
Expand All @@ -100,7 +93,10 @@ public class DateFormatters {
.appendFraction(MILLI_OF_SECOND, 3, 3, true)
.optionalEnd()
.optionalStart()
.appendOffset("+HHmm", "Z")
.appendZoneOrOffsetId()
.optionalEnd()
.optionalStart()
.append(TIME_ZONE_FORMATTER_NO_COLON)
.optionalEnd()
.optionalEnd()
.toFormatter(Locale.ROOT);
Expand All @@ -109,10 +105,9 @@ public class DateFormatters {
* Returns a generic ISO datetime parser where the date is mandatory and the time is optional.
*/
private static final DateFormatter STRICT_DATE_OPTIONAL_TIME =
new JavaDateFormatter("strict_date_optional_time", STRICT_DATE_OPTIONAL_TIME_FORMATTER_1,
STRICT_DATE_OPTIONAL_TIME_FORMATTER_1, STRICT_DATE_OPTIONAL_TIME_FORMATTER_2);
new JavaDateFormatter("strict_date_optional_time", STRICT_DATE_OPTIONAL_TIME_PRINTER, STRICT_DATE_OPTIONAL_TIME_FORMATTER);

private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER_WITH_NANOS_1 = new DateTimeFormatterBuilder()
private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_FORMATTER_WITH_NANOS = new DateTimeFormatterBuilder()
.append(STRICT_YEAR_MONTH_DAY_FORMATTER)
.optionalStart()
.appendLiteral('T')
Expand All @@ -124,26 +119,17 @@ public class DateFormatters {
.appendZoneOrOffsetId()
.optionalEnd()
.optionalStart()
.appendOffset("+HHmm", "Z")
.append(TIME_ZONE_FORMATTER_NO_COLON)
.optionalEnd()
.optionalEnd()
.toFormatter(Locale.ROOT);

private static final DateTimeFormatter STRICT_DATE_OPTIONAL_TIME_PRINTER = new DateTimeFormatterBuilder()
.append(STRICT_YEAR_MONTH_DAY_FORMATTER)
.appendLiteral('T')
.append(STRICT_HOUR_MINUTE_SECOND_FORMATTER)
.appendFraction(NANO_OF_SECOND, 3, 9, true)
.optionalStart()
.appendZoneOrOffsetId()
.optionalEnd()
.toFormatter(Locale.ROOT);

/**
* Returns a generic ISO datetime parser where the date is mandatory and the time is optional with nanosecond resolution.
*/
private static final DateFormatter STRICT_DATE_OPTIONAL_TIME_NANOS = new JavaDateFormatter("strict_date_optional_time_nanos",
STRICT_DATE_OPTIONAL_TIME_PRINTER, STRICT_DATE_OPTIONAL_TIME_FORMATTER_WITH_NANOS_1);
STRICT_DATE_OPTIONAL_TIME_PRINTER, STRICT_DATE_OPTIONAL_TIME_FORMATTER_WITH_NANOS);

/////////////////////////////////////////
//
// BEGIN basic time formatters
Expand Down Expand Up @@ -818,7 +804,7 @@ public class DateFormatters {
* yyyy-MM-dd'T'HH:mm:ss.SSSZ
*/
private static final DateFormatter DATE_OPTIONAL_TIME = new JavaDateFormatter("date_optional_time",
STRICT_DATE_OPTIONAL_TIME_FORMATTER_1,
STRICT_DATE_OPTIONAL_TIME_PRINTER,
new DateTimeFormatterBuilder()
.append(DATE_FORMATTER)
.optionalStart()
Expand All @@ -836,26 +822,6 @@ public class DateFormatters {
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
.optionalEnd()
.optionalStart().appendZoneOrOffsetId().optionalEnd()
.optionalEnd()
.optionalEnd()
.optionalEnd()
.toFormatter(Locale.ROOT),
new DateTimeFormatterBuilder()
.append(DATE_FORMATTER)
.optionalStart()
.appendLiteral('T')
.optionalStart()
.appendValue(HOUR_OF_DAY, 1, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendLiteral(':')
.appendValue(MINUTE_OF_HOUR, 1, 2, SignStyle.NOT_NEGATIVE)
.optionalStart()
.appendLiteral(':')
.appendValue(SECOND_OF_MINUTE, 1, 2, SignStyle.NOT_NEGATIVE)
.optionalEnd()
.optionalStart()
.appendFraction(MILLI_OF_SECOND, 1, 3, true)
.optionalEnd()
.optionalStart().appendOffset("+HHmm", "Z").optionalEnd()
.optionalEnd()
.optionalEnd()
Expand Down Expand Up @@ -1006,7 +972,7 @@ public class DateFormatters {
* (yyyy-MM-dd'T'HH:mm:ss.SSSZZ).
*/
private static final DateFormatter DATE_TIME = new JavaDateFormatter("date_time",
STRICT_DATE_OPTIONAL_TIME_FORMATTER_1,
STRICT_DATE_OPTIONAL_TIME_PRINTER,
new DateTimeFormatterBuilder().append(DATE_TIME_FORMATTER).appendZoneOrOffsetId().toFormatter(Locale.ROOT),
new DateTimeFormatterBuilder().append(DATE_TIME_FORMATTER).append(TIME_ZONE_FORMATTER_NO_COLON).toFormatter(Locale.ROOT)
);
Expand Down Expand Up @@ -1483,90 +1449,22 @@ public static DateFormatter forPattern(String input) {
}
}

static class MergedDateFormatter implements DateFormatter {

private final String pattern;
// package private for tests
final List<DateFormatter> formatters;
private final List<DateMathParser> dateMathParsers;

MergedDateFormatter(String pattern, List<DateFormatter> formatters) {
assert formatters.size() > 0;
this.pattern = pattern;
this.formatters = Collections.unmodifiableList(formatters);
this.dateMathParsers = formatters.stream().map(DateFormatter::toDateMathParser).collect(Collectors.toList());
}

@Override
public TemporalAccessor parse(String input) {
IllegalArgumentException failure = null;
for (DateFormatter formatter : formatters) {
try {
return formatter.parse(input);
// TODO: remove DateTimeParseException when JavaDateFormatter throws IAE
} catch (IllegalArgumentException | DateTimeParseException e) {
if (failure == null) {
// wrap so the entire multi format is in the message
failure = new IllegalArgumentException("failed to parse date field [" + input + "] with format [" + pattern + "]",
e);
} else {
failure.addSuppressed(e);
}
}
static JavaDateFormatter merge(String pattern, List<DateFormatter> formatters) {
assert formatters.size() > 0;

List<DateTimeFormatter> dateTimeFormatters = new ArrayList<>(formatters.size());
DateTimeFormatter printer = null;
for (DateFormatter formatter : formatters) {
assert formatter instanceof JavaDateFormatter;
JavaDateFormatter javaDateFormatter = (JavaDateFormatter) formatter;
DateTimeFormatter dateTimeFormatter = javaDateFormatter.getParser();
if (printer == null) {
printer = javaDateFormatter.getPrinter();
}
throw failure;
}

@Override
public DateFormatter withZone(ZoneId zoneId) {
return new MergedDateFormatter(pattern, formatters.stream().map(f -> f.withZone(zoneId)).collect(Collectors.toList()));
}

@Override
public DateFormatter withLocale(Locale locale) {
return new MergedDateFormatter(pattern, formatters.stream().map(f -> f.withLocale(locale)).collect(Collectors.toList()));
}

@Override
public String format(TemporalAccessor accessor) {
return formatters.get(0).format(accessor);
dateTimeFormatters.add(dateTimeFormatter);
}

@Override
public String pattern() {
return pattern;
}

@Override
public Locale locale() {
return formatters.get(0).locale();
}

@Override
public ZoneId zone() {
return formatters.get(0).zone();
}

@Override
public DateMathParser toDateMathParser() {
return (text, now, roundUp, tz) -> {
ElasticsearchParseException failure = null;
for (DateMathParser parser : dateMathParsers) {
try {
return parser.parse(text, now, roundUp, tz);
} catch (ElasticsearchParseException e) {
if (failure == null) {
// wrap so the entire multi format is in the message
failure = new ElasticsearchParseException("failed to parse date field [" + text + "] with format ["
+ pattern + "]", e);
} else {
failure.addSuppressed(e);
}
}
}
throw failure;
};
}
return new JavaDateFormatter(pattern, printer, dateTimeFormatters.toArray(new DateTimeFormatter[0]));
}

private static final ZonedDateTime EPOCH_ZONED_DATE_TIME = Instant.EPOCH.atZone(ZoneOffset.UTC);
Expand Down
Loading

0 comments on commit 9f3da01

Please sign in to comment.