Skip to content

Commit 7b215a1

Browse files
original-brownbearoriginal-brownbear
authored
Cleanup some Inefficiencies in JavaDateFormatter (#84922)
Fixes a couple of smaller inefficiencies that I found in profiling and otherwise. * Iteration over the parsers is faster if we just use an array. * Make some of the object construction more efficient since formats aren't always constants, like in e.g. the index name resolver. * Fix non-static initializer block running useless puts into the static mutable map on every instantiation * no need to even have the map, just inline what it does and save some code, indirection and the iteration over the mutable map
1 parent 0bd269d commit 7b215a1

File tree

2 files changed

+79
-76
lines changed

2 files changed

+79
-76
lines changed

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

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
package org.elasticsearch.common.time;
1010

1111
import org.elasticsearch.common.Strings;
12-
import org.elasticsearch.common.logging.DeprecationLogger;
13-
import org.elasticsearch.common.util.LazyInitializable;
1412
import org.elasticsearch.core.SuppressForbidden;
1513

1614
import java.time.Instant;
@@ -43,13 +41,6 @@
4341
import static java.time.temporal.ChronoField.SECOND_OF_MINUTE;
4442

4543
public class DateFormatters {
46-
// DateFormatters is being used even before the logging is initialized.
47-
// If LogManager.getLogger is called before logging config is loaded
48-
// it results in errors sent to status logger and startup to fail.
49-
// Hence a lazy initialization.
50-
private static final LazyInitializable<DeprecationLogger, RuntimeException> deprecationLogger = new LazyInitializable<>(
51-
() -> DeprecationLogger.getLogger(FormatNames.class)
52-
);
5344

5445
public static final WeekFields WEEK_FIELDS_ROOT = WeekFields.of(Locale.ROOT);
5546

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

Lines changed: 79 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
package org.elasticsearch.common.time;
1010

1111
import org.elasticsearch.common.Strings;
12-
import org.elasticsearch.common.util.Maps;
1312

1413
import java.text.ParsePosition;
1514
import java.time.ZoneId;
@@ -18,44 +17,25 @@
1817
import java.time.format.DateTimeParseException;
1918
import java.time.temporal.ChronoField;
2019
import java.time.temporal.TemporalAccessor;
21-
import java.time.temporal.TemporalField;
2220
import java.util.ArrayList;
23-
import java.util.Arrays;
24-
import java.util.Collection;
2521
import java.util.Collections;
2622
import java.util.List;
2723
import java.util.Locale;
28-
import java.util.Map;
2924
import java.util.Objects;
3025
import java.util.function.Consumer;
26+
import java.util.function.UnaryOperator;
3127

3228
class JavaDateFormatter implements DateFormatter {
3329

34-
// base fields which should be used for default parsing, when we round up for date math
35-
private static final Map<TemporalField, Long> ROUND_UP_BASE_FIELDS = Maps.newMapWithExpectedSize(6);
36-
37-
{
38-
ROUND_UP_BASE_FIELDS.put(ChronoField.MONTH_OF_YEAR, 1L);
39-
ROUND_UP_BASE_FIELDS.put(ChronoField.DAY_OF_MONTH, 1L);
40-
ROUND_UP_BASE_FIELDS.put(ChronoField.HOUR_OF_DAY, 23L);
41-
ROUND_UP_BASE_FIELDS.put(ChronoField.MINUTE_OF_HOUR, 59L);
42-
ROUND_UP_BASE_FIELDS.put(ChronoField.SECOND_OF_MINUTE, 59L);
43-
ROUND_UP_BASE_FIELDS.put(ChronoField.NANO_OF_SECOND, 999_999_999L);
44-
}
45-
4630
private final String format;
4731
private final DateTimeFormatter printer;
48-
private final List<DateTimeFormatter> parsers;
49-
private final JavaDateFormatter roundupParser;
32+
private final DateTimeFormatter[] parsers;
33+
private final RoundUpFormatter roundupParser;
5034

51-
static class RoundUpFormatter extends JavaDateFormatter {
35+
private static final class RoundUpFormatter extends JavaDateFormatter {
5236

53-
RoundUpFormatter(String format, List<DateTimeFormatter> roundUpParsers) {
54-
super(format, firstFrom(roundUpParsers), null, roundUpParsers);
55-
}
56-
57-
private static DateTimeFormatter firstFrom(List<DateTimeFormatter> roundUpParsers) {
58-
return roundUpParsers.get(0);
37+
RoundUpFormatter(String format, DateTimeFormatter[] roundUpParsers) {
38+
super(format, roundUpParsers[0], (RoundUpFormatter) null, roundUpParsers);
5939
}
6040

6141
@Override
@@ -66,7 +46,18 @@ JavaDateFormatter getRoundupParser() {
6646

6747
// named formatters use default roundUpParser
6848
JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter... parsers) {
69-
this(format, printer, builder -> ROUND_UP_BASE_FIELDS.forEach(builder::parseDefaulting), parsers);
49+
this(
50+
format,
51+
printer,
52+
// set up base fields which should be used for default parsing, when we round up for date math
53+
builder -> builder.parseDefaulting(ChronoField.MONTH_OF_YEAR, 1L)
54+
.parseDefaulting(ChronoField.DAY_OF_MONTH, 1L)
55+
.parseDefaulting(ChronoField.HOUR_OF_DAY, 23L)
56+
.parseDefaulting(ChronoField.MINUTE_OF_HOUR, 59L)
57+
.parseDefaulting(ChronoField.SECOND_OF_MINUTE, 59L)
58+
.parseDefaulting(ChronoField.NANO_OF_SECOND, 999_999_999L),
59+
parsers
60+
);
7061
}
7162

7263
// subclasses override roundUpParser
@@ -79,24 +70,28 @@ JavaDateFormatter getRoundupParser() {
7970
if (printer == null) {
8071
throw new IllegalArgumentException("printer may not be null");
8172
}
82-
long distinctZones = Arrays.stream(parsers).map(DateTimeFormatter::getZone).distinct().count();
83-
if (distinctZones > 1) {
84-
throw new IllegalArgumentException("formatters must have the same time zone");
85-
}
86-
long distinctLocales = Arrays.stream(parsers).map(DateTimeFormatter::getLocale).distinct().count();
87-
if (distinctLocales > 1) {
88-
throw new IllegalArgumentException("formatters must have the same locale");
89-
}
9073
this.printer = printer;
9174
this.format = format;
75+
this.parsers = parsersArray(printer, parsers);
76+
this.roundupParser = createRoundUpParser(format, roundupParserConsumer, locale(), this.parsers);
77+
}
9278

79+
private static DateTimeFormatter[] parsersArray(DateTimeFormatter printer, DateTimeFormatter... parsers) {
9380
if (parsers.length == 0) {
94-
this.parsers = Collections.singletonList(printer);
95-
} else {
96-
this.parsers = Arrays.asList(parsers);
81+
return new DateTimeFormatter[] { printer };
82+
}
83+
final ZoneId zoneId = parsers[0].getZone();
84+
final Locale locale = parsers[0].getLocale();
85+
for (int i = 1; i < parsers.length; i++) {
86+
final DateTimeFormatter parser = parsers[i];
87+
if (Objects.equals(parser.getZone(), zoneId) == false) {
88+
throw new IllegalArgumentException("formatters must have the same time zone");
89+
}
90+
if (Objects.equals(parser.getLocale(), locale) == false) {
91+
throw new IllegalArgumentException("formatters must have the same locale");
92+
}
9793
}
98-
List<DateTimeFormatter> roundUp = createRoundUpParser(format, roundupParserConsumer);
99-
this.roundupParser = new RoundUpFormatter(format, roundUp);
94+
return parsers;
10095
}
10196

10297
/**
@@ -108,16 +103,19 @@ JavaDateFormatter getRoundupParser() {
108103
* <code>DateFormatters</code>.
109104
* This means that we need to also have multiple RoundUp parsers.
110105
*/
111-
private List<DateTimeFormatter> createRoundUpParser(String format, Consumer<DateTimeFormatterBuilder> roundupParserConsumer) {
106+
private static RoundUpFormatter createRoundUpParser(
107+
String format,
108+
Consumer<DateTimeFormatterBuilder> roundupParserConsumer,
109+
Locale locale,
110+
DateTimeFormatter[] parsers
111+
) {
112112
if (format.contains("||") == false) {
113-
List<DateTimeFormatter> roundUpParsers = new ArrayList<>();
114-
for (DateTimeFormatter parser : this.parsers) {
113+
return new RoundUpFormatter(format, mapParsers(parser -> {
115114
DateTimeFormatterBuilder builder = new DateTimeFormatterBuilder();
116115
builder.append(parser);
117116
roundupParserConsumer.accept(builder);
118-
roundUpParsers.add(builder.toFormatter(locale()));
119-
}
120-
return roundUpParsers;
117+
return builder.toFormatter(locale);
118+
}, parsers));
121119
}
122120
return null;
123121
}
@@ -135,22 +133,26 @@ public static DateFormatter combined(String input, List<DateFormatter> formatter
135133
if (printer == null) {
136134
printer = javaDateFormatter.getPrinter();
137135
}
138-
parsers.addAll(javaDateFormatter.getParsers());
139-
roundUpParsers.addAll(javaDateFormatter.getRoundupParser().getParsers());
136+
Collections.addAll(parsers, javaDateFormatter.parsers);
137+
Collections.addAll(roundUpParsers, javaDateFormatter.getRoundupParser().parsers);
140138
}
141139

142-
return new JavaDateFormatter(input, printer, roundUpParsers, parsers);
140+
return new JavaDateFormatter(
141+
input,
142+
printer,
143+
roundUpParsers.toArray(DateTimeFormatter[]::new),
144+
parsers.toArray(DateTimeFormatter[]::new)
145+
);
143146
}
144147

145-
private JavaDateFormatter(
146-
String format,
147-
DateTimeFormatter printer,
148-
List<DateTimeFormatter> roundUpParsers,
149-
List<DateTimeFormatter> parsers
150-
) {
148+
private JavaDateFormatter(String format, DateTimeFormatter printer, DateTimeFormatter[] roundUpParsers, DateTimeFormatter[] parsers) {
149+
this(format, printer, new RoundUpFormatter(format, roundUpParsers), parsers);
150+
}
151+
152+
private JavaDateFormatter(String format, DateTimeFormatter printer, RoundUpFormatter roundupParser, DateTimeFormatter[] parsers) {
151153
this.format = format;
152154
this.printer = printer;
153-
this.roundupParser = roundUpParsers != null ? new RoundUpFormatter(format, roundUpParsers) : null;
155+
this.roundupParser = roundupParser;
154156
this.parsers = parsers;
155157
}
156158

@@ -190,7 +192,7 @@ public TemporalAccessor parse(String input) {
190192
* @throws DateTimeParseException when unable to parse with any parsers
191193
*/
192194
private TemporalAccessor doParse(String input) {
193-
if (parsers.size() > 1) {
195+
if (parsers.length > 1) {
194196
for (DateTimeFormatter formatter : parsers) {
195197
ParsePosition pos = new ParsePosition(0);
196198
Object object = formatter.toFormat().parseObject(input, pos);
@@ -200,7 +202,7 @@ private TemporalAccessor doParse(String input) {
200202
}
201203
throw new DateTimeParseException("Failed to parse with all enclosed parsers", input, 0);
202204
}
203-
return this.parsers.get(0).parse(input);
205+
return this.parsers[0].parse(input);
204206
}
205207

206208
private boolean parsingSucceeded(Object object, String input, ParsePosition pos) {
@@ -213,9 +215,7 @@ public DateFormatter withZone(ZoneId zoneId) {
213215
if (zoneId.equals(zone())) {
214216
return this;
215217
}
216-
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withZone(zoneId)).toList();
217-
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers().stream().map(p -> p.withZone(zoneId)).toList();
218-
return new JavaDateFormatter(format, printer.withZone(zoneId), roundUpParsers, parsers);
218+
return mapParsers(p -> p.withZone(zoneId));
219219
}
220220

221221
@Override
@@ -224,9 +224,24 @@ public DateFormatter withLocale(Locale locale) {
224224
if (locale.equals(locale())) {
225225
return this;
226226
}
227-
List<DateTimeFormatter> parsers = this.parsers.stream().map(p -> p.withLocale(locale)).toList();
228-
List<DateTimeFormatter> roundUpParsers = this.roundupParser.getParsers().stream().map(p -> p.withLocale(locale)).toList();
229-
return new JavaDateFormatter(format, printer.withLocale(locale), roundUpParsers, parsers);
227+
return mapParsers(p -> p.withLocale(locale));
228+
}
229+
230+
private JavaDateFormatter mapParsers(UnaryOperator<DateTimeFormatter> mapping) {
231+
return new JavaDateFormatter(
232+
format,
233+
mapping.apply(printer),
234+
mapParsers(mapping, ((JavaDateFormatter) this.roundupParser).parsers),
235+
mapParsers(mapping, this.parsers)
236+
);
237+
}
238+
239+
private static DateTimeFormatter[] mapParsers(UnaryOperator<DateTimeFormatter> mapping, DateTimeFormatter[] parsers) {
240+
DateTimeFormatter[] res = new DateTimeFormatter[parsers.length];
241+
for (int i = 0; i < parsers.length; i++) {
242+
res[i] = mapping.apply(parsers[i]);
243+
}
244+
return res;
230245
}
231246

232247
@Override
@@ -276,7 +291,4 @@ public String toString() {
276291
return String.format(Locale.ROOT, "format[%s] locale[%s]", format, locale());
277292
}
278293

279-
Collection<DateTimeFormatter> getParsers() {
280-
return parsers;
281-
}
282294
}

0 commit comments

Comments
 (0)