Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore date aggregation performance in UTC case #38221

Merged
merged 29 commits into from Feb 11, 2019
Merged
Changes from 14 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
242df6f
Restore date aggregation performance in UTC case
spinscale Feb 1, 2019
1326aa1
fix tests, fast path is only with UTC and small date time units though
spinscale Feb 2, 2019
1c5fb71
add back benchmark tests
spinscale Feb 2, 2019
6eeeb55
Add more tests
spinscale Feb 2, 2019
66c8106
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 2, 2019
4399661
Speed up year of century parsing.
spinscale Feb 2, 2019
ed263c9
Bring quarter of year for UTC case up to speed again
spinscale Feb 2, 2019
6dd0d6e
Fix month of year benchmark, where joda is still much faster
spinscale Feb 2, 2019
59db0ae
speed up month of year
spinscale Feb 2, 2019
e6f7255
minor refactoring
spinscale Feb 3, 2019
67688df
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 4, 2019
4fca036
refactorings, more docs
spinscale Feb 4, 2019
1ecaa93
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 4, 2019
35bd640
remove unused import
spinscale Feb 4, 2019
72e315d
fix typo
spinscale Feb 4, 2019
056fe59
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 4, 2019
368dd69
review comment, calculate quarter of year without using month
spinscale Feb 4, 2019
b8c8b57
work on review comments
spinscale Feb 4, 2019
00308c8
incorporate review comments, add tests
spinscale Feb 4, 2019
f6acf7c
split joda time code into separate class
spinscale Feb 4, 2019
58f58c0
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 5, 2019
3d9cb8b
incorporate review comments, add joda time to NOTICE.txt according to…
spinscale Feb 5, 2019
5503ce5
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 5, 2019
7b84ee2
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 5, 2019
6f2060b
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 5, 2019
16dadc6
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 7, 2019
b50f840
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 8, 2019
203be57
Merge branch 'master' into 1902-fix-utc-aggs-performance
spinscale Feb 9, 2019
82b8f76
remove merge header
spinscale Feb 9, 2019
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -34,8 +34,14 @@
import org.openjdk.jmh.annotations.Warmup;

import java.time.ZoneId;
import java.time.ZoneOffset;
import java.util.concurrent.TimeUnit;

import static org.elasticsearch.common.Rounding.DateTimeUnit.DAY_OF_MONTH;
import static org.elasticsearch.common.Rounding.DateTimeUnit.MONTH_OF_YEAR;
import static org.elasticsearch.common.Rounding.DateTimeUnit.QUARTER_OF_YEAR;
import static org.elasticsearch.common.Rounding.DateTimeUnit.YEAR_OF_CENTURY;

@Fork(3)
@Warmup(iterations = 10)
@Measurement(iterations = 10)
@@ -48,23 +54,13 @@
private final ZoneId zoneId = ZoneId.of("Europe/Amsterdam");
private final DateTimeZone timeZone = DateUtils.zoneIdToDateTimeZone(zoneId);

private final long timestamp = 1548879021354L;

This comment has been minimized.

Copy link
@danielmitterdorfer

danielmitterdorfer Feb 4, 2019

Member

Using a constant value (+ declaring it as final) might allow for additional compiler optimizations that are unrealistic. We also see that some of the results are very low. For example, timeUnitRoundingUtcYearOfCenturyJava takes just 3 nanoseconds which is only around 10 CPU cycles on a 3GHz core. Given the amount of work that the respective method is doing (it's basic arithmetic) it does not seem completely unreasonable but if we just consider the microbenchmark results this would still need a bit further investigation. However, the macrobenchmark results that you posted look fine and give me confidence that performance has improved substantially indeed.


private final org.elasticsearch.common.rounding.Rounding jodaRounding =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(timeZone).build();
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.HOUR_OF_DAY).timeZone(timeZone).build();
private final Rounding javaRounding = Rounding.builder(Rounding.DateTimeUnit.HOUR_OF_DAY)
.timeZone(zoneId).build();

private final org.elasticsearch.common.rounding.Rounding jodaDayOfMonthRounding =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(timeZone).build();
private final Rounding javaDayOfMonthRounding = Rounding.builder(TimeValue.timeValueMinutes(60))
.timeZone(zoneId).build();

private final org.elasticsearch.common.rounding.Rounding timeIntervalRoundingJoda =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(timeZone).build();
private final Rounding timeIntervalRoundingJava = Rounding.builder(TimeValue.timeValueMinutes(60))
.timeZone(zoneId).build();

private final long timestamp = 1548879021354L;

@Benchmark
public long timeRoundingDateTimeUnitJoda() {
return jodaRounding.round(timestamp);
@@ -75,6 +71,11 @@ public long timeRoundingDateTimeUnitJava() {
return javaRounding.round(timestamp);
}

private final org.elasticsearch.common.rounding.Rounding jodaDayOfMonthRounding =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(timeZone).build();
private final Rounding javaDayOfMonthRounding = Rounding.builder(DAY_OF_MONTH)
.timeZone(zoneId).build();

@Benchmark
public long timeRoundingDateTimeUnitDayOfMonthJoda() {
return jodaDayOfMonthRounding.round(timestamp);
@@ -85,6 +86,11 @@ public long timeRoundingDateTimeUnitDayOfMonthJava() {
return javaDayOfMonthRounding.round(timestamp);
}

private final org.elasticsearch.common.rounding.Rounding timeIntervalRoundingJoda =
org.elasticsearch.common.rounding.Rounding.builder(TimeValue.timeValueMinutes(60)).timeZone(timeZone).build();
private final Rounding timeIntervalRoundingJava = Rounding.builder(TimeValue.timeValueMinutes(60))
.timeZone(zoneId).build();

@Benchmark
public long timeIntervalRoundingJava() {
return timeIntervalRoundingJava.round(timestamp);
@@ -94,4 +100,65 @@ public long timeIntervalRoundingJava() {
public long timeIntervalRoundingJoda() {
return timeIntervalRoundingJoda.round(timestamp);
}

private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcDayOfMonthJoda =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.DAY_OF_MONTH).timeZone(DateTimeZone.UTC).build();
private final Rounding timeUnitRoundingUtcDayOfMonthJava = Rounding.builder(DAY_OF_MONTH)
.timeZone(ZoneOffset.UTC).build();

@Benchmark
public long timeUnitRoundingUtcDayOfMonthJava() {
return timeUnitRoundingUtcDayOfMonthJava.round(timestamp);
}

@Benchmark
public long timeUnitRoundingUtcDayOfMonthJoda() {
return timeUnitRoundingUtcDayOfMonthJoda.round(timestamp);
}

private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcQuarterOfYearJoda =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.QUARTER).timeZone(DateTimeZone.UTC).build();
private final Rounding timeUnitRoundingUtcQuarterOfYearJava = Rounding.builder(QUARTER_OF_YEAR)
.timeZone(ZoneOffset.UTC).build();

@Benchmark
public long timeUnitRoundingUtcQuarterOfYearJava() {
return timeUnitRoundingUtcQuarterOfYearJava.round(timestamp);
}

@Benchmark
public long timeUnitRoundingUtcQuarterOfYearJoda() {
return timeUnitRoundingUtcQuarterOfYearJoda.round(timestamp);
}

private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcMonthOfYearJoda =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.MONTH_OF_YEAR).timeZone(DateTimeZone.UTC).build();
private final Rounding timeUnitRoundingUtcMonthOfYearJava = Rounding.builder(MONTH_OF_YEAR)
.timeZone(ZoneOffset.UTC).build();

@Benchmark
public long timeUnitRoundingUtcMonthOfYearJava() {
return timeUnitRoundingUtcMonthOfYearJava.round(timestamp);
}

@Benchmark
public long timeUnitRoundingUtcMonthOfYearJoda() {
return timeUnitRoundingUtcMonthOfYearJoda.round(timestamp);
}


private final org.elasticsearch.common.rounding.Rounding timeUnitRoundingUtcYearOfCenturyJoda =
org.elasticsearch.common.rounding.Rounding.builder(DateTimeUnit.YEAR_OF_CENTURY).timeZone(DateTimeZone.UTC).build();
private final Rounding timeUnitRoundingUtcYearOfCenturyJava = Rounding.builder(YEAR_OF_CENTURY)
.timeZone(ZoneOffset.UTC).build();

@Benchmark
public long timeUnitRoundingUtcYearOfCenturyJava() {
return timeUnitRoundingUtcYearOfCenturyJava.round(timestamp);
}

@Benchmark
public long timeUnitRoundingUtcYearOfCenturyJoda() {
return timeUnitRoundingUtcYearOfCenturyJoda.round(timestamp);
}
}
@@ -54,19 +54,51 @@
*/
public abstract class Rounding implements Writeable {

public static String format(long epochMillis) {
return Instant.ofEpochMilli(epochMillis) + "/" + epochMillis;
}

public enum DateTimeUnit {
WEEK_OF_WEEKYEAR( (byte) 1, IsoFields.WEEK_OF_WEEK_BASED_YEAR),
YEAR_OF_CENTURY( (byte) 2, ChronoField.YEAR_OF_ERA),
QUARTER_OF_YEAR( (byte) 3, IsoFields.QUARTER_OF_YEAR),
MONTH_OF_YEAR( (byte) 4, ChronoField.MONTH_OF_YEAR),
DAY_OF_MONTH( (byte) 5, ChronoField.DAY_OF_MONTH),
HOUR_OF_DAY( (byte) 6, ChronoField.HOUR_OF_DAY),
MINUTES_OF_HOUR( (byte) 7, ChronoField.MINUTE_OF_HOUR),
SECOND_OF_MINUTE( (byte) 8, ChronoField.SECOND_OF_MINUTE);
WEEK_OF_WEEKYEAR((byte) 1, IsoFields.WEEK_OF_WEEK_BASED_YEAR) {
long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis + 3 * 86400 * 1000L, 604800000) - 3 * 86400 * 1000L;
}
},
YEAR_OF_CENTURY((byte) 2, ChronoField.YEAR_OF_ERA) {
long roundFloor(long utcMillis) {
return DateUtils.getFirstDayOfYearMillis(utcMillis);
}
},
QUARTER_OF_YEAR((byte) 3, IsoFields.QUARTER_OF_YEAR) {
long roundFloor(long utcMillis) {
return DateUtils.roundQuarterOfYear(utcMillis);
}
},
MONTH_OF_YEAR((byte) 4, ChronoField.MONTH_OF_YEAR) {
long roundFloor(long utcMillis) {
return DateUtils.roundMonthOfYear(utcMillis);
}
},
DAY_OF_MONTH((byte) 5, ChronoField.DAY_OF_MONTH) {
final long unitMillis = ChronoField.DAY_OF_MONTH.getBaseUnit().getDuration().toMillis();
long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, unitMillis);
}
},
HOUR_OF_DAY((byte) 6, ChronoField.HOUR_OF_DAY) {
final long unitMillis = ChronoField.HOUR_OF_DAY.getBaseUnit().getDuration().toMillis();
long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, unitMillis);
}
},
MINUTES_OF_HOUR((byte) 7, ChronoField.MINUTE_OF_HOUR) {
final long unitMillis = ChronoField.MINUTE_OF_HOUR.getBaseUnit().getDuration().toMillis();
long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, unitMillis);
}
},
SECOND_OF_MINUTE((byte) 8, ChronoField.SECOND_OF_MINUTE) {
final long unitMillis = ChronoField.SECOND_OF_MINUTE.getBaseUnit().getDuration().toMillis();
long roundFloor(long utcMillis) {
return DateUtils.roundFloor(utcMillis, unitMillis);
}
};

private final byte id;
private final TemporalField field;
@@ -76,6 +108,15 @@ public static String format(long epochMillis) {
this.field = field;
}

/**
* This rounds down the supplied milliseconds since the epoch down to the next unit. In order to retain performance this method
* should be as fast as possiblee and not try to convert dates to java-time objects if possible

This comment has been minimized.

Copy link
@danielmitterdorfer

danielmitterdorfer Feb 4, 2019

Member

Nit: typo possiblee -> possible.

*
* @param utcMillis the milliseconds since the epoch
* @return the rounded down milliseconds since the epoch
*/
abstract long roundFloor(long utcMillis);

public byte getId() {
return id;
}
@@ -182,12 +223,13 @@ public Rounding build() {
private final DateTimeUnit unit;
private final ZoneId timeZone;
private final boolean unitRoundsToMidnight;

private final boolean isUtcTimeZone;

TimeUnitRounding(DateTimeUnit unit, ZoneId timeZone) {
this.unit = unit;
this.timeZone = timeZone;
this.unitRoundsToMidnight = this.unit.field.getBaseUnit().getDuration().toMillis() > 3600000L;
this.isUtcTimeZone = timeZone.normalized().equals(ZoneOffset.UTC);
}

TimeUnitRounding(StreamInput in) throws IOException {
@@ -223,9 +265,7 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) {
return LocalDateTime.of(localDateTime.getYear(), localDateTime.getMonthValue(), 1, 0, 0);

case QUARTER_OF_YEAR:
int quarter = (int) IsoFields.QUARTER_OF_YEAR.getFrom(localDateTime);
int month = ((quarter - 1) * 3) + 1;
return LocalDateTime.of(localDateTime.getYear(), month, 1, 0, 0);
return LocalDateTime.of(localDateTime.getYear(), localDateTime.getMonth().firstMonthOfQuarter(), 1, 0, 0);

case YEAR_OF_CENTURY:
return LocalDateTime.of(LocalDate.of(localDateTime.getYear(), 1, 1), LocalTime.MIDNIGHT);
@@ -236,7 +276,14 @@ private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) {
}

@Override
public long round(final long utcMillis) {
public long round(long utcMillis) {
// this works as long as the 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 are unnecessary in the common case
// of working in UTC.
if (isUtcTimeZone) {
return unit.roundFloor(utcMillis);
}

Instant instant = Instant.ofEpochMilli(utcMillis);
if (unitRoundsToMidnight) {
final LocalDateTime localDateTime = LocalDateTime.ofInstant(instant, timeZone);
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.