Skip to content

Commit bd4b9dd

Browse files
authored
Speed up time interval arounding around dst (backport elastic#56371) (elastic#56396)
When an index spans a daylight savings time transition we can't use our optimization that rewrites the requested time zone to a fixed time zone and instead we used to fall back to a java.util.time based rounding implementation. In elastic#55559 we optimized "time unit" rounding. This optimizes "time interval" rounding. The java.util.time based implementation is about 1650% slower than the rounding implementation for a fixed time zone. This replaces it with a similar optimization that is only about 30% slower than the fixed time zone. The java.util.time implementation allocates a ton of short lived objects but the optimized implementation doesn't. So it *might* end up being faster than the microbenchmarks imply.
1 parent 5b708f8 commit bd4b9dd

File tree

5 files changed

+280
-96
lines changed

5 files changed

+280
-96
lines changed

benchmarks/src/main/java/org/elasticsearch/common/RoundingBenchmark.java

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
package org.elasticsearch.common;
2121

2222
import org.elasticsearch.common.time.DateFormatter;
23+
import org.elasticsearch.common.unit.TimeValue;
24+
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder;
2325
import org.openjdk.jmh.annotations.Benchmark;
2426
import org.openjdk.jmh.annotations.BenchmarkMode;
2527
import org.openjdk.jmh.annotations.Fork;
@@ -60,8 +62,8 @@ public class RoundingBenchmark {
6062
@Param({ "UTC", "America/New_York" })
6163
public String zone;
6264

63-
@Param({ "MONTH_OF_YEAR", "HOUR_OF_DAY" })
64-
public String timeUnit;
65+
@Param({ "calendar year", "calendar hour", "10d", "5d", "1h" })
66+
public String interval;
6567

6668
@Param({ "1", "10000", "1000000", "100000000" })
6769
public int count;
@@ -86,7 +88,15 @@ public void buildDates() {
8688
dates[i] = date;
8789
date += diff;
8890
}
89-
Rounding rounding = Rounding.builder(Rounding.DateTimeUnit.valueOf(timeUnit)).timeZone(ZoneId.of(zone)).build();
91+
Rounding.Builder roundingBuilder;
92+
if (interval.startsWith("calendar ")) {
93+
roundingBuilder = Rounding.builder(
94+
DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval.substring("calendar ".length()))
95+
);
96+
} else {
97+
roundingBuilder = Rounding.builder(TimeValue.parseTimeValue(interval, "interval"));
98+
}
99+
Rounding rounding = roundingBuilder.timeZone(ZoneId.of(zone)).build();
90100
switch (rounder) {
91101
case "java time":
92102
rounderBuilder = rounding::prepareJavaTime;

server/src/main/java/org/elasticsearch/common/LocalTimeOffset.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ public static Lookup lookup(ZoneId zone, long minUtcMillis, long maxUtcMillis) {
9090
*
9191
* @return a lookup function of {@code null} if none could be built
9292
*/
93-
public static LocalTimeOffset lookupFixedOffset(ZoneId zone) {
93+
public static LocalTimeOffset fixedOffset(ZoneId zone) {
9494
return checkForFixedZone(zone, zone.getRules());
9595
}
9696

@@ -493,6 +493,10 @@ protected static Transition buildTransition(ZoneOffsetTransition transition, Loc
493493
long utcStart = transition.toEpochSecond() * 1000;
494494
long offsetBeforeMillis = transition.getOffsetBefore().getTotalSeconds() * 1000;
495495
long offsetAfterMillis = transition.getOffsetAfter().getTotalSeconds() * 1000;
496+
assert (false == previous instanceof Transition) || ((Transition) previous).startUtcMillis < utcStart :
497+
"transition list out of order at [" + previous + "] and [" + transition + "]";
498+
assert previous.millis != offsetAfterMillis :
499+
"transition list is has a duplicate at [" + previous + "] and [" + transition + "]";
496500
if (transition.isGap()) {
497501
long firstMissingLocalTime = utcStart + offsetBeforeMillis;
498502
long firstLocalTimeAfterGap = utcStart + offsetAfterMillis;
@@ -559,6 +563,11 @@ private static List<ZoneOffsetTransition> collectTransitions(ZoneId zone, ZoneRu
559563
if (false == itr.hasNext()) {
560564
if (minSecond < t.toEpochSecond() && t.toEpochSecond() < maxSecond) {
561565
transitions.add(t);
566+
/*
567+
* Sometimes the rules duplicate the transitions. And
568+
* duplicates confuse us. So we have to skip past them.
569+
*/
570+
minSecond = t.toEpochSecond() + 1;
562571
}
563572
transitions = buildTransitionsFromRules(transitions, zone, rules, minSecond, maxSecond);
564573
if (transitions != null && transitions.isEmpty()) {

server/src/main/java/org/elasticsearch/common/Rounding.java

Lines changed: 174 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ public Prepared prepare(long minUtcMillis, long maxUtcMillis) {
444444

445445
@Override
446446
public Prepared prepareForUnknown() {
447-
LocalTimeOffset offset = LocalTimeOffset.lookupFixedOffset(timeZone);
447+
LocalTimeOffset offset = LocalTimeOffset.fixedOffset(timeZone);
448448
if (offset != null) {
449449
if (unitRoundsToMidnight) {
450450
return new FixedToMidnightRounding(offset);
@@ -560,7 +560,7 @@ public long inGap(long localMillis, Gap gap) {
560560
@Override
561561
public long beforeGap(long localMillis, Gap gap) {
562562
return gap.previous().localToUtc(localMillis, this);
563-
};
563+
}
564564

565565
@Override
566566
public long inOverlap(long localMillis, Overlap overlap) {
@@ -570,7 +570,7 @@ public long inOverlap(long localMillis, Overlap overlap) {
570570
@Override
571571
public long beforeOverlap(long localMillis, Overlap overlap) {
572572
return overlap.previous().localToUtc(localMillis, this);
573-
};
573+
}
574574
}
575575

576576
private class NotToMidnightRounding extends AbstractNotToMidnightRounding implements LocalTimeOffset.Strategy {
@@ -744,21 +744,15 @@ public final long nextRoundingValue(long utcMillis) {
744744

745745
static class TimeIntervalRounding extends Rounding {
746746
static final byte ID = 2;
747-
/** Since, there is no offset of -1 ms, it is safe to use -1 for non-fixed timezones */
748-
private static final long TZ_OFFSET_NON_FIXED = -1;
749747

750748
private final long interval;
751749
private final ZoneId timeZone;
752-
/** For fixed offset timezones, this is the offset in milliseconds, otherwise TZ_OFFSET_NON_FIXED */
753-
private final long fixedOffsetMillis;
754750

755751
TimeIntervalRounding(long interval, ZoneId timeZone) {
756752
if (interval < 1)
757753
throw new IllegalArgumentException("Zero or negative time interval not supported");
758754
this.interval = interval;
759755
this.timeZone = timeZone;
760-
this.fixedOffsetMillis = timeZone.getRules().isFixedOffset() ?
761-
timeZone.getRules().getOffset(Instant.EPOCH).getTotalSeconds() * 1000 : TZ_OFFSET_NON_FIXED;
762756
}
763757

764758
TimeIntervalRounding(StreamInput in) throws IOException {
@@ -783,88 +777,32 @@ public byte id() {
783777

784778
@Override
785779
public Prepared prepare(long minUtcMillis, long maxUtcMillis) {
786-
return prepareForUnknown();
780+
long minLookup = minUtcMillis - interval;
781+
long maxLookup = maxUtcMillis;
782+
783+
LocalTimeOffset.Lookup lookup = LocalTimeOffset.lookup(timeZone, minLookup, maxLookup);
784+
if (lookup == null) {
785+
return prepareJavaTime();
786+
}
787+
LocalTimeOffset fixedOffset = lookup.fixedInRange(minLookup, maxLookup);
788+
if (fixedOffset != null) {
789+
return new FixedRounding(fixedOffset);
790+
}
791+
return new VariableRounding(lookup);
787792
}
788793

789794
@Override
790795
public Prepared prepareForUnknown() {
796+
LocalTimeOffset offset = LocalTimeOffset.fixedOffset(timeZone);
797+
if (offset != null) {
798+
return new FixedRounding(offset);
799+
}
791800
return prepareJavaTime();
792801
}
793802

794803
@Override
795804
Prepared prepareJavaTime() {
796-
return new Prepared() {
797-
@Override
798-
public long round(long utcMillis) {
799-
if (fixedOffsetMillis != TZ_OFFSET_NON_FIXED) {
800-
// This works as long as the tz offset doesn't change. It is worth getting this case out of the way first,
801-
// as the calculations for fixing things near to offset changes are a little expensive and unnecessary
802-
// in the common case of working with fixed offset timezones (such as UTC).
803-
long localMillis = utcMillis + fixedOffsetMillis;
804-
return (roundKey(localMillis, interval) * interval) - fixedOffsetMillis;
805-
}
806-
final Instant utcInstant = Instant.ofEpochMilli(utcMillis);
807-
final LocalDateTime rawLocalDateTime = LocalDateTime.ofInstant(utcInstant, timeZone);
808-
809-
// a millisecond value with the same local time, in UTC, as `utcMillis` has in `timeZone`
810-
final long localMillis = utcMillis + timeZone.getRules().getOffset(utcInstant).getTotalSeconds() * 1000;
811-
assert localMillis == rawLocalDateTime.toInstant(ZoneOffset.UTC).toEpochMilli();
812-
813-
final long roundedMillis = roundKey(localMillis, interval) * interval;
814-
final LocalDateTime roundedLocalDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(roundedMillis), ZoneOffset.UTC);
815-
816-
// Now work out what roundedLocalDateTime actually means
817-
final List<ZoneOffset> currentOffsets = timeZone.getRules().getValidOffsets(roundedLocalDateTime);
818-
if (currentOffsets.isEmpty() == false) {
819-
// There is at least one instant with the desired local time. In general the desired result is
820-
// the latest rounded time that's no later than the input time, but this could involve rounding across
821-
// a timezone transition, which may yield the wrong result
822-
final ZoneOffsetTransition previousTransition = timeZone.getRules().previousTransition(utcInstant.plusMillis(1));
823-
for (int offsetIndex = currentOffsets.size() - 1; 0 <= offsetIndex; offsetIndex--) {
824-
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(offsetIndex));
825-
final Instant offsetInstant = offsetTime.toInstant();
826-
if (previousTransition != null && offsetInstant.isBefore(previousTransition.getInstant())) {
827-
/*
828-
* Rounding down across the transition can yield the
829-
* wrong result. It's best to return to the transition
830-
* time and round that down.
831-
*/
832-
return round(previousTransition.getInstant().toEpochMilli() - 1);
833-
}
834-
835-
if (utcInstant.isBefore(offsetTime.toInstant()) == false) {
836-
return offsetInstant.toEpochMilli();
837-
}
838-
}
839-
840-
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(0));
841-
final Instant offsetInstant = offsetTime.toInstant();
842-
assert false : this + " failed to round " + utcMillis + " down: " + offsetInstant + " is the earliest possible";
843-
return offsetInstant.toEpochMilli(); // TODO or throw something?
844-
} else {
845-
// The desired time isn't valid because within a gap, so just return the gap time.
846-
ZoneOffsetTransition zoneOffsetTransition = timeZone.getRules().getTransition(roundedLocalDateTime);
847-
return zoneOffsetTransition.getInstant().toEpochMilli();
848-
}
849-
}
850-
851-
@Override
852-
public long nextRoundingValue(long time) {
853-
int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds();
854-
long millis = time + interval + offsetSeconds * 1000;
855-
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC)
856-
.withZoneSameLocal(timeZone)
857-
.toInstant().toEpochMilli();
858-
}
859-
860-
private long roundKey(long value, long interval) {
861-
if (value < 0) {
862-
return (value - interval + 1) / interval;
863-
} else {
864-
return value / interval;
865-
}
866-
}
867-
};
805+
return new JavaTimeRounding();
868806
}
869807

870808
@Override
@@ -898,6 +836,160 @@ public boolean equals(Object obj) {
898836
public String toString() {
899837
return "Rounding[" + interval + " in " + timeZone + "]";
900838
}
839+
840+
private long roundKey(long value, long interval) {
841+
if (value < 0) {
842+
return (value - interval + 1) / interval;
843+
} else {
844+
return value / interval;
845+
}
846+
}
847+
848+
/**
849+
* Rounds to down inside of a time zone with an "effectively fixed"
850+
* time zone. A time zone can be "effectively fixed" if:
851+
* <ul>
852+
* <li>It is UTC</li>
853+
* <li>It is a fixed offset from UTC at all times (UTC-5, America/Phoenix)</li>
854+
* <li>It is fixed over the entire range of dates that will be rounded</li>
855+
* </ul>
856+
*/
857+
private class FixedRounding implements Prepared {
858+
private final LocalTimeOffset offset;
859+
860+
FixedRounding(LocalTimeOffset offset) {
861+
this.offset = offset;
862+
}
863+
864+
@Override
865+
public long round(long utcMillis) {
866+
return offset.localToUtcInThisOffset(roundKey(offset.utcToLocalTime(utcMillis), interval) * interval);
867+
}
868+
869+
@Override
870+
public long nextRoundingValue(long utcMillis) {
871+
// TODO this is used in date range's collect so we should optimize it too
872+
return new JavaTimeRounding().nextRoundingValue(utcMillis);
873+
}
874+
}
875+
876+
/**
877+
* Rounds down inside of any time zone, even if it is not
878+
* "effectively fixed". See {@link FixedRounding} for a description of
879+
* "effectively fixed".
880+
*/
881+
private class VariableRounding implements Prepared, LocalTimeOffset.Strategy {
882+
private final LocalTimeOffset.Lookup lookup;
883+
884+
VariableRounding(LocalTimeOffset.Lookup lookup) {
885+
this.lookup = lookup;
886+
}
887+
888+
@Override
889+
public long round(long utcMillis) {
890+
LocalTimeOffset offset = lookup.lookup(utcMillis);
891+
return offset.localToUtc(roundKey(offset.utcToLocalTime(utcMillis), interval) * interval, this);
892+
}
893+
894+
@Override
895+
public long nextRoundingValue(long utcMillis) {
896+
// TODO this is used in date range's collect so we should optimize it too
897+
return new JavaTimeRounding().nextRoundingValue(utcMillis);
898+
}
899+
900+
@Override
901+
public long inGap(long localMillis, Gap gap) {
902+
return gap.startUtcMillis();
903+
}
904+
905+
@Override
906+
public long beforeGap(long localMillis, Gap gap) {
907+
return gap.previous().localToUtc(localMillis, this);
908+
}
909+
910+
@Override
911+
public long inOverlap(long localMillis, Overlap overlap) {
912+
// Convert the overlap at this offset because that'll produce the largest result.
913+
return overlap.localToUtcInThisOffset(localMillis);
914+
}
915+
916+
@Override
917+
public long beforeOverlap(long localMillis, Overlap overlap) {
918+
return overlap.previous().localToUtc(roundKey(overlap.firstNonOverlappingLocalTime() - 1, interval) * interval, this);
919+
}
920+
}
921+
922+
/**
923+
* Rounds down inside of any time zone using {@link LocalDateTime}
924+
* directly. It'll be slower than {@link VariableRounding} and much
925+
* slower than {@link FixedRounding}. We use it when we don' have an
926+
* "effectively fixed" time zone and we can't get a
927+
* {@link LocalTimeOffset.Lookup}. We might not be able to get one
928+
* because:
929+
* <ul>
930+
* <li>We don't know how to look up the minimum and maximum dates we
931+
* are going to round.</li>
932+
* <li>We expect to round over thousands and thousands of years worth
933+
* of dates with the same {@link Prepared} instance.</li>
934+
* </ul>
935+
*/
936+
private class JavaTimeRounding implements Prepared {
937+
@Override
938+
public long round(long utcMillis) {
939+
final Instant utcInstant = Instant.ofEpochMilli(utcMillis);
940+
final LocalDateTime rawLocalDateTime = LocalDateTime.ofInstant(utcInstant, timeZone);
941+
942+
// a millisecond value with the same local time, in UTC, as `utcMillis` has in `timeZone`
943+
final long localMillis = utcMillis + timeZone.getRules().getOffset(utcInstant).getTotalSeconds() * 1000;
944+
assert localMillis == rawLocalDateTime.toInstant(ZoneOffset.UTC).toEpochMilli();
945+
946+
final long roundedMillis = roundKey(localMillis, interval) * interval;
947+
final LocalDateTime roundedLocalDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(roundedMillis), ZoneOffset.UTC);
948+
949+
// Now work out what roundedLocalDateTime actually means
950+
final List<ZoneOffset> currentOffsets = timeZone.getRules().getValidOffsets(roundedLocalDateTime);
951+
if (currentOffsets.isEmpty() == false) {
952+
// There is at least one instant with the desired local time. In general the desired result is
953+
// the latest rounded time that's no later than the input time, but this could involve rounding across
954+
// a timezone transition, which may yield the wrong result
955+
final ZoneOffsetTransition previousTransition = timeZone.getRules().previousTransition(utcInstant.plusMillis(1));
956+
for (int offsetIndex = currentOffsets.size() - 1; 0 <= offsetIndex; offsetIndex--) {
957+
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(offsetIndex));
958+
final Instant offsetInstant = offsetTime.toInstant();
959+
if (previousTransition != null && offsetInstant.isBefore(previousTransition.getInstant())) {
960+
/*
961+
* Rounding down across the transition can yield the
962+
* wrong result. It's best to return to the transition
963+
* time and round that down.
964+
*/
965+
return round(previousTransition.getInstant().toEpochMilli() - 1);
966+
}
967+
968+
if (utcInstant.isBefore(offsetTime.toInstant()) == false) {
969+
return offsetInstant.toEpochMilli();
970+
}
971+
}
972+
973+
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(0));
974+
final Instant offsetInstant = offsetTime.toInstant();
975+
assert false : this + " failed to round " + utcMillis + " down: " + offsetInstant + " is the earliest possible";
976+
return offsetInstant.toEpochMilli(); // TODO or throw something?
977+
} else {
978+
// The desired time isn't valid because within a gap, so just return the start of the gap
979+
ZoneOffsetTransition zoneOffsetTransition = timeZone.getRules().getTransition(roundedLocalDateTime);
980+
return zoneOffsetTransition.getInstant().toEpochMilli();
981+
}
982+
}
983+
984+
@Override
985+
public long nextRoundingValue(long time) {
986+
int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds();
987+
long millis = time + interval + offsetSeconds * 1000;
988+
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC)
989+
.withZoneSameLocal(timeZone)
990+
.toInstant().toEpochMilli();
991+
}
992+
}
901993
}
902994

903995
static class OffsetRounding extends Rounding {

0 commit comments

Comments
 (0)