Speed up time interval arounding around dst (backport #56371) (#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 #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.
This commit is contained in:
Nik Everett 2020-05-08 13:39:27 -04:00 committed by GitHub
parent 5b708f846c
commit bd4b9dd10e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
5 changed files with 280 additions and 96 deletions

View File

@ -20,6 +20,8 @@
package org.elasticsearch.common;
import org.elasticsearch.common.time.DateFormatter;
import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramAggregationBuilder;
import org.openjdk.jmh.annotations.Benchmark;
import org.openjdk.jmh.annotations.BenchmarkMode;
import org.openjdk.jmh.annotations.Fork;
@ -60,8 +62,8 @@ public class RoundingBenchmark {
@Param({ "UTC", "America/New_York" })
public String zone;
@Param({ "MONTH_OF_YEAR", "HOUR_OF_DAY" })
public String timeUnit;
@Param({ "calendar year", "calendar hour", "10d", "5d", "1h" })
public String interval;
@Param({ "1", "10000", "1000000", "100000000" })
public int count;
@ -86,7 +88,15 @@ public class RoundingBenchmark {
dates[i] = date;
date += diff;
}
Rounding rounding = Rounding.builder(Rounding.DateTimeUnit.valueOf(timeUnit)).timeZone(ZoneId.of(zone)).build();
Rounding.Builder roundingBuilder;
if (interval.startsWith("calendar ")) {
roundingBuilder = Rounding.builder(
DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval.substring("calendar ".length()))
);
} else {
roundingBuilder = Rounding.builder(TimeValue.parseTimeValue(interval, "interval"));
}
Rounding rounding = roundingBuilder.timeZone(ZoneId.of(zone)).build();
switch (rounder) {
case "java time":
rounderBuilder = rounding::prepareJavaTime;

View File

@ -90,7 +90,7 @@ public abstract class LocalTimeOffset {
*
* @return a lookup function of {@code null} if none could be built
*/
public static LocalTimeOffset lookupFixedOffset(ZoneId zone) {
public static LocalTimeOffset fixedOffset(ZoneId zone) {
return checkForFixedZone(zone, zone.getRules());
}
@ -493,6 +493,10 @@ public abstract class LocalTimeOffset {
long utcStart = transition.toEpochSecond() * 1000;
long offsetBeforeMillis = transition.getOffsetBefore().getTotalSeconds() * 1000;
long offsetAfterMillis = transition.getOffsetAfter().getTotalSeconds() * 1000;
assert (false == previous instanceof Transition) || ((Transition) previous).startUtcMillis < utcStart :
"transition list out of order at [" + previous + "] and [" + transition + "]";
assert previous.millis != offsetAfterMillis :
"transition list is has a duplicate at [" + previous + "] and [" + transition + "]";
if (transition.isGap()) {
long firstMissingLocalTime = utcStart + offsetBeforeMillis;
long firstLocalTimeAfterGap = utcStart + offsetAfterMillis;
@ -559,6 +563,11 @@ public abstract class LocalTimeOffset {
if (false == itr.hasNext()) {
if (minSecond < t.toEpochSecond() && t.toEpochSecond() < maxSecond) {
transitions.add(t);
/*
* Sometimes the rules duplicate the transitions. And
* duplicates confuse us. So we have to skip past them.
*/
minSecond = t.toEpochSecond() + 1;
}
transitions = buildTransitionsFromRules(transitions, zone, rules, minSecond, maxSecond);
if (transitions != null && transitions.isEmpty()) {

View File

@ -444,7 +444,7 @@ public abstract class Rounding implements Writeable {
@Override
public Prepared prepareForUnknown() {
LocalTimeOffset offset = LocalTimeOffset.lookupFixedOffset(timeZone);
LocalTimeOffset offset = LocalTimeOffset.fixedOffset(timeZone);
if (offset != null) {
if (unitRoundsToMidnight) {
return new FixedToMidnightRounding(offset);
@ -560,7 +560,7 @@ public abstract class Rounding implements Writeable {
@Override
public long beforeGap(long localMillis, Gap gap) {
return gap.previous().localToUtc(localMillis, this);
};
}
@Override
public long inOverlap(long localMillis, Overlap overlap) {
@ -570,7 +570,7 @@ public abstract class Rounding implements Writeable {
@Override
public long beforeOverlap(long localMillis, Overlap overlap) {
return overlap.previous().localToUtc(localMillis, this);
};
}
}
private class NotToMidnightRounding extends AbstractNotToMidnightRounding implements LocalTimeOffset.Strategy {
@ -744,21 +744,15 @@ public abstract class Rounding implements Writeable {
static class TimeIntervalRounding extends Rounding {
static final byte ID = 2;
/** Since, there is no offset of -1 ms, it is safe to use -1 for non-fixed timezones */
private static final long TZ_OFFSET_NON_FIXED = -1;
private final long interval;
private final ZoneId timeZone;
/** For fixed offset timezones, this is the offset in milliseconds, otherwise TZ_OFFSET_NON_FIXED */
private final long fixedOffsetMillis;
TimeIntervalRounding(long interval, ZoneId timeZone) {
if (interval < 1)
throw new IllegalArgumentException("Zero or negative time interval not supported");
this.interval = interval;
this.timeZone = timeZone;
this.fixedOffsetMillis = timeZone.getRules().isFixedOffset() ?
timeZone.getRules().getOffset(Instant.EPOCH).getTotalSeconds() * 1000 : TZ_OFFSET_NON_FIXED;
}
TimeIntervalRounding(StreamInput in) throws IOException {
@ -783,88 +777,32 @@ public abstract class Rounding implements Writeable {
@Override
public Prepared prepare(long minUtcMillis, long maxUtcMillis) {
return prepareForUnknown();
long minLookup = minUtcMillis - interval;
long maxLookup = maxUtcMillis;
LocalTimeOffset.Lookup lookup = LocalTimeOffset.lookup(timeZone, minLookup, maxLookup);
if (lookup == null) {
return prepareJavaTime();
}
LocalTimeOffset fixedOffset = lookup.fixedInRange(minLookup, maxLookup);
if (fixedOffset != null) {
return new FixedRounding(fixedOffset);
}
return new VariableRounding(lookup);
}
@Override
public Prepared prepareForUnknown() {
LocalTimeOffset offset = LocalTimeOffset.fixedOffset(timeZone);
if (offset != null) {
return new FixedRounding(offset);
}
return prepareJavaTime();
}
@Override
Prepared prepareJavaTime() {
return new Prepared() {
@Override
public long round(long utcMillis) {
if (fixedOffsetMillis != TZ_OFFSET_NON_FIXED) {
// This works as long as the tz offset doesn't change. It is worth getting this case out of the way first,
// as the calculations for fixing things near to offset changes are a little expensive and unnecessary
// in the common case of working with fixed offset timezones (such as UTC).
long localMillis = utcMillis + fixedOffsetMillis;
return (roundKey(localMillis, interval) * interval) - fixedOffsetMillis;
}
final Instant utcInstant = Instant.ofEpochMilli(utcMillis);
final LocalDateTime rawLocalDateTime = LocalDateTime.ofInstant(utcInstant, timeZone);
// a millisecond value with the same local time, in UTC, as `utcMillis` has in `timeZone`
final long localMillis = utcMillis + timeZone.getRules().getOffset(utcInstant).getTotalSeconds() * 1000;
assert localMillis == rawLocalDateTime.toInstant(ZoneOffset.UTC).toEpochMilli();
final long roundedMillis = roundKey(localMillis, interval) * interval;
final LocalDateTime roundedLocalDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(roundedMillis), ZoneOffset.UTC);
// Now work out what roundedLocalDateTime actually means
final List<ZoneOffset> currentOffsets = timeZone.getRules().getValidOffsets(roundedLocalDateTime);
if (currentOffsets.isEmpty() == false) {
// There is at least one instant with the desired local time. In general the desired result is
// the latest rounded time that's no later than the input time, but this could involve rounding across
// a timezone transition, which may yield the wrong result
final ZoneOffsetTransition previousTransition = timeZone.getRules().previousTransition(utcInstant.plusMillis(1));
for (int offsetIndex = currentOffsets.size() - 1; 0 <= offsetIndex; offsetIndex--) {
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(offsetIndex));
final Instant offsetInstant = offsetTime.toInstant();
if (previousTransition != null && offsetInstant.isBefore(previousTransition.getInstant())) {
/*
* Rounding down across the transition can yield the
* wrong result. It's best to return to the transition
* time and round that down.
*/
return round(previousTransition.getInstant().toEpochMilli() - 1);
}
if (utcInstant.isBefore(offsetTime.toInstant()) == false) {
return offsetInstant.toEpochMilli();
}
}
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(0));
final Instant offsetInstant = offsetTime.toInstant();
assert false : this + " failed to round " + utcMillis + " down: " + offsetInstant + " is the earliest possible";
return offsetInstant.toEpochMilli(); // TODO or throw something?
} else {
// The desired time isn't valid because within a gap, so just return the gap time.
ZoneOffsetTransition zoneOffsetTransition = timeZone.getRules().getTransition(roundedLocalDateTime);
return zoneOffsetTransition.getInstant().toEpochMilli();
}
}
@Override
public long nextRoundingValue(long time) {
int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds();
long millis = time + interval + offsetSeconds * 1000;
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC)
.withZoneSameLocal(timeZone)
.toInstant().toEpochMilli();
}
private long roundKey(long value, long interval) {
if (value < 0) {
return (value - interval + 1) / interval;
} else {
return value / interval;
}
}
};
return new JavaTimeRounding();
}
@Override
@ -898,6 +836,160 @@ public abstract class Rounding implements Writeable {
public String toString() {
return "Rounding[" + interval + " in " + timeZone + "]";
}
private long roundKey(long value, long interval) {
if (value < 0) {
return (value - interval + 1) / interval;
} else {
return value / interval;
}
}
/**
* Rounds to down inside of a time zone with an "effectively fixed"
* time zone. A time zone can be "effectively fixed" if:
* <ul>
* <li>It is UTC</li>
* <li>It is a fixed offset from UTC at all times (UTC-5, America/Phoenix)</li>
* <li>It is fixed over the entire range of dates that will be rounded</li>
* </ul>
*/
private class FixedRounding implements Prepared {
private final LocalTimeOffset offset;
FixedRounding(LocalTimeOffset offset) {
this.offset = offset;
}
@Override
public long round(long utcMillis) {
return offset.localToUtcInThisOffset(roundKey(offset.utcToLocalTime(utcMillis), interval) * interval);
}
@Override
public long nextRoundingValue(long utcMillis) {
// TODO this is used in date range's collect so we should optimize it too
return new JavaTimeRounding().nextRoundingValue(utcMillis);
}
}
/**
* Rounds down inside of any time zone, even if it is not
* "effectively fixed". See {@link FixedRounding} for a description of
* "effectively fixed".
*/
private class VariableRounding implements Prepared, LocalTimeOffset.Strategy {
private final LocalTimeOffset.Lookup lookup;
VariableRounding(LocalTimeOffset.Lookup lookup) {
this.lookup = lookup;
}
@Override
public long round(long utcMillis) {
LocalTimeOffset offset = lookup.lookup(utcMillis);
return offset.localToUtc(roundKey(offset.utcToLocalTime(utcMillis), interval) * interval, this);
}
@Override
public long nextRoundingValue(long utcMillis) {
// TODO this is used in date range's collect so we should optimize it too
return new JavaTimeRounding().nextRoundingValue(utcMillis);
}
@Override
public long inGap(long localMillis, Gap gap) {
return gap.startUtcMillis();
}
@Override
public long beforeGap(long localMillis, Gap gap) {
return gap.previous().localToUtc(localMillis, this);
}
@Override
public long inOverlap(long localMillis, Overlap overlap) {
// Convert the overlap at this offset because that'll produce the largest result.
return overlap.localToUtcInThisOffset(localMillis);
}
@Override
public long beforeOverlap(long localMillis, Overlap overlap) {
return overlap.previous().localToUtc(roundKey(overlap.firstNonOverlappingLocalTime() - 1, interval) * interval, this);
}
}
/**
* Rounds down inside of any time zone using {@link LocalDateTime}
* directly. It'll be slower than {@link VariableRounding} and much
* slower than {@link FixedRounding}. We use it when we don' have an
* "effectively fixed" time zone and we can't get a
* {@link LocalTimeOffset.Lookup}. We might not be able to get one
* because:
* <ul>
* <li>We don't know how to look up the minimum and maximum dates we
* are going to round.</li>
* <li>We expect to round over thousands and thousands of years worth
* of dates with the same {@link Prepared} instance.</li>
* </ul>
*/
private class JavaTimeRounding implements Prepared {
@Override
public long round(long utcMillis) {
final Instant utcInstant = Instant.ofEpochMilli(utcMillis);
final LocalDateTime rawLocalDateTime = LocalDateTime.ofInstant(utcInstant, timeZone);
// a millisecond value with the same local time, in UTC, as `utcMillis` has in `timeZone`
final long localMillis = utcMillis + timeZone.getRules().getOffset(utcInstant).getTotalSeconds() * 1000;
assert localMillis == rawLocalDateTime.toInstant(ZoneOffset.UTC).toEpochMilli();
final long roundedMillis = roundKey(localMillis, interval) * interval;
final LocalDateTime roundedLocalDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(roundedMillis), ZoneOffset.UTC);
// Now work out what roundedLocalDateTime actually means
final List<ZoneOffset> currentOffsets = timeZone.getRules().getValidOffsets(roundedLocalDateTime);
if (currentOffsets.isEmpty() == false) {
// There is at least one instant with the desired local time. In general the desired result is
// the latest rounded time that's no later than the input time, but this could involve rounding across
// a timezone transition, which may yield the wrong result
final ZoneOffsetTransition previousTransition = timeZone.getRules().previousTransition(utcInstant.plusMillis(1));
for (int offsetIndex = currentOffsets.size() - 1; 0 <= offsetIndex; offsetIndex--) {
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(offsetIndex));
final Instant offsetInstant = offsetTime.toInstant();
if (previousTransition != null && offsetInstant.isBefore(previousTransition.getInstant())) {
/*
* Rounding down across the transition can yield the
* wrong result. It's best to return to the transition
* time and round that down.
*/
return round(previousTransition.getInstant().toEpochMilli() - 1);
}
if (utcInstant.isBefore(offsetTime.toInstant()) == false) {
return offsetInstant.toEpochMilli();
}
}
final OffsetDateTime offsetTime = roundedLocalDateTime.atOffset(currentOffsets.get(0));
final Instant offsetInstant = offsetTime.toInstant();
assert false : this + " failed to round " + utcMillis + " down: " + offsetInstant + " is the earliest possible";
return offsetInstant.toEpochMilli(); // TODO or throw something?
} else {
// The desired time isn't valid because within a gap, so just return the start of the gap
ZoneOffsetTransition zoneOffsetTransition = timeZone.getRules().getTransition(roundedLocalDateTime);
return zoneOffsetTransition.getInstant().toEpochMilli();
}
}
@Override
public long nextRoundingValue(long time) {
int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds();
long millis = time + interval + offsetSeconds * 1000;
return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC)
.withZoneSameLocal(timeZone)
.toInstant().toEpochMilli();
}
}
}
static class OffsetRounding extends Rounding {

View File

@ -28,6 +28,7 @@ import java.time.Instant;
import java.time.ZoneId;
import java.time.ZoneOffset;
import java.time.zone.ZoneOffsetTransition;
import java.time.zone.ZoneRules;
import java.util.List;
import java.util.concurrent.TimeUnit;
@ -46,7 +47,7 @@ public class LocalTimeOffsetTests extends ESTestCase {
public void testNotFixed() {
ZoneId zone = ZoneId.of("America/New_York");
assertThat(LocalTimeOffset.lookupFixedOffset(zone), nullValue());
assertThat(LocalTimeOffset.fixedOffset(zone), nullValue());
}
public void testUtc() {
@ -59,7 +60,7 @@ public class LocalTimeOffsetTests extends ESTestCase {
}
private void assertFixOffset(ZoneId zone, long offsetMillis) {
LocalTimeOffset fixed = LocalTimeOffset.lookupFixedOffset(zone);
LocalTimeOffset fixed = LocalTimeOffset.fixedOffset(zone);
assertThat(fixed, notNullValue());
LocalTimeOffset.Lookup lookup = LocalTimeOffset.lookup(zone, Long.MIN_VALUE, Long.MAX_VALUE);
@ -76,11 +77,10 @@ public class LocalTimeOffsetTests extends ESTestCase {
assertRoundingAtOffset(randomBoolean() ? fixed : fixedInRange, randomLong(), offsetMillis);
}
private void assertRoundingAtOffset(LocalTimeOffset offset, long time, long offsetMillis) {
assertThat(offset.utcToLocalTime(time), equalTo(time + offsetMillis));
assertThat(offset.localToUtcInThisOffset(time + offsetMillis), equalTo(time));
assertThat(offset.localToUtc(time + offsetMillis, unusedStrategy()), equalTo(time));
}
public void testJustTransitions() {
@ -155,6 +155,26 @@ public class LocalTimeOffsetTests extends ESTestCase {
assertRoundingAtOffset(lookup.lookup(time), time, TimeUnit.MINUTES.toMillis(345));
}
/**
* America/Tijuana's
* {@link ZoneRules#getTransitions() fully defined transitions} overlap
* with its {@link ZoneRules#getTransitionRules() future rules} and if
* we're not careful we can end up with duplicate transitions because we
* have to collect them independently. That will trip assertions, failing
* this test real fast. If they don't trip the assertions then trying to
* use the transitions will produce incorrect results, failing the
* size assertion.
*/
public void testLastTransitionOverlapsRules() {
ZoneId zone = ZoneId.of("America/Tijuana");
long min = utcTime("2011-11-06T08:31:57.091Z");
long max = utcTime("2011-11-06T09:02:57.091Z");
LocalTimeOffset.Lookup lookup = LocalTimeOffset.lookup(zone, min, max);
assertThat(lookup.size(), equalTo(2));
assertRoundingAtOffset(lookup.lookup(min), min, -TimeUnit.HOURS.toMillis(7));
assertRoundingAtOffset(lookup.lookup(max), max, -TimeUnit.HOURS.toMillis(8));
}
public void testOverlap() {
/*
* Europe/Rome turn their clocks back an hour 1978 which is totally

View File

@ -387,23 +387,56 @@ public class RoundingTests extends ESTestCase {
public void testRandomTimeIntervalRounding() {
for (int i = 0; i < 1000; i++) {
int unitCount = randomIntBetween(1, 365);
TimeUnit unit = randomFrom(TimeUnit.MINUTES, TimeUnit.HOURS, TimeUnit.DAYS);
long interval = unit.toMillis(randomIntBetween(1, 365));
long interval = unit.toMillis(unitCount);
ZoneId tz = randomZone();
Rounding rounding = new Rounding.TimeIntervalRounding(interval, tz);
long mainDate = Math.abs(randomLong() % (2 * (long) 10e11)); // 1970-01-01T00:00:00Z - 2033-05-18T05:33:20.000+02:00
long mainDate = randomDate();
if (randomBoolean()) {
mainDate = nastyDate(mainDate, tz, interval);
}
long min = mainDate - 2 * interval;
long max = mainDate + 2 * interval;
/*
* Prepare a rounding with one extra interval of range because
* in the tests far below we call round(round(min)). The first
* round might spit out a time below the min if min is near a
* daylight savings time transition. So we request an extra big
* range just in case.
*/
Rounding.Prepared prepared = rounding.prepare(min - interval, max);
// Round a whole bunch of dates and make sure they line up with the known good java time implementation
Rounding.Prepared javaTimeRounding = rounding.prepareJavaTime();
for (int d = 0; d < 1000; d++) {
long date = dateBetween(min, max);
long javaRounded = javaTimeRounding.round(date);
long esRounded = prepared.round(date);
if (javaRounded != esRounded) {
fail("Expected [" + unitCount + " " + unit + " in " + tz + "] to round [" + Instant.ofEpochMilli(date) + "] to ["
+ Instant.ofEpochMilli(javaRounded) + "] but instead rounded to [" + Instant.ofEpochMilli(esRounded) + "]");
}
long javaNextRoundingValue = javaTimeRounding.nextRoundingValue(date);
long esNextRoundingValue = prepared.nextRoundingValue(date);
if (javaNextRoundingValue != esNextRoundingValue) {
fail("Expected [" + unitCount + " " + unit + " in " + tz + "] to round [" + Instant.ofEpochMilli(date) + "] to ["
+ Instant.ofEpochMilli(esRounded) + "] and nextRoundingValue to be ["
+ Instant.ofEpochMilli(javaNextRoundingValue) + "] but instead was to ["
+ Instant.ofEpochMilli(esNextRoundingValue) + "]");
}
}
// check two intervals around date
long previousRoundedValue = Long.MIN_VALUE;
for (long date = mainDate - 2 * interval; date < mainDate + 2 * interval; date += interval / 2) {
for (long date = min; date < max; date += interval / 2) {
try {
final long roundedDate = rounding.round(date);
final long nextRoundingValue = rounding.nextRoundingValue(roundedDate);
assertThat("Rounding should be idempotent", roundedDate, equalTo(rounding.round(roundedDate)));
final long nextRoundingValue = prepared.nextRoundingValue(roundedDate);
assertThat("Rounding should be idempotent", roundedDate, equalTo(prepared.round(roundedDate)));
assertThat("Rounded value smaller or equal than unrounded", roundedDate, lessThanOrEqualTo(date));
assertThat("Values smaller than rounded value should round further down", rounding.round(roundedDate - 1),
assertThat("Values smaller than rounded value should round further down", prepared.round(roundedDate - 1),
lessThan(roundedDate));
assertThat("Rounding should be >= previous rounding value", roundedDate, greaterThanOrEqualTo(previousRoundedValue));
@ -778,6 +811,26 @@ public class RoundingTests extends ESTestCase {
assertThat(prepared.round(time("9000-03-31T15:25:15.148Z")), isDate(time("9000-03-31T15:00:00Z"), tz));
}
/**
* Example of when we round past when local clocks were wound forward.
*/
public void testIntervalBeforeGap() {
ZoneId tz = ZoneId.of("Africa/Cairo");
Rounding rounding = Rounding.builder(TimeValue.timeValueDays(257)).timeZone(tz).build();
assertThat(rounding.round(time("1969-07-08T09:00:14.599Z")), isDate(time("1969-04-18T22:00:00Z"), tz));
}
/**
* Example of when we round past when local clocks were wound backwards,
* <strong>and</strong> then past the time they were wound forwards before
* that. So, we jumped back a long, long way.
*/
public void testIntervalTwoTransitions() {
ZoneId tz = ZoneId.of("America/Detroit");
Rounding rounding = Rounding.builder(TimeValue.timeValueDays(279)).timeZone(tz).build();
assertThat(rounding.round(time("1982-11-10T02:51:22.662Z")), isDate(time("1982-03-23T05:00:00Z"), tz));
}
private void assertInterval(long rounded, long nextRoundingValue, Rounding rounding, int minutes,
ZoneId tz) {
assertInterval(rounded, dateBetween(rounded, nextRoundingValue), nextRoundingValue, rounding, tz);