From 2301bc4e5ac21f97aae7cd51ca7ccd3a5824a7e2 Mon Sep 17 00:00:00 2001 From: Alexander Reelsen Date: Wed, 30 Jan 2019 23:52:48 +0100 Subject: [PATCH] Reduce object creation in Rounding class While benchmarking the rounding code due to slower date histogram aggregations I noticed a lot of useless object creations due to the builder/fluent java time API. This reduces creations by properly creating the objects. Furthermore a few unneeded ZonedDateTime objects were created in order to create other objects out of them. This was changed as well. Running the benchmarks shows a much faster performance for all of the java time based Rounding classes, which means this was not yet the aggs culprit. --- .../benchmark/time/RoundingBenchmark.java | 97 ++++++++++++++ .../org/elasticsearch/common/Rounding.java | 126 ++++++++---------- 2 files changed, 150 insertions(+), 73 deletions(-) create mode 100644 benchmarks/src/main/java/org/elasticsearch/benchmark/time/RoundingBenchmark.java diff --git a/benchmarks/src/main/java/org/elasticsearch/benchmark/time/RoundingBenchmark.java b/benchmarks/src/main/java/org/elasticsearch/benchmark/time/RoundingBenchmark.java new file mode 100644 index 0000000000000..6da6d5290bfee --- /dev/null +++ b/benchmarks/src/main/java/org/elasticsearch/benchmark/time/RoundingBenchmark.java @@ -0,0 +1,97 @@ +/* + * 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.Rounding; +import org.elasticsearch.common.rounding.DateTimeUnit; +import org.elasticsearch.common.time.DateUtils; +import org.elasticsearch.common.unit.TimeValue; +import org.joda.time.DateTimeZone; +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.ZoneId; +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 RoundingBenchmark { + + private final ZoneId zoneId = ZoneId.of("Europe/Amsterdam"); + private final DateTimeZone timeZone = DateUtils.zoneIdToDateTimeZone(zoneId); + + private final org.elasticsearch.common.rounding.Rounding jodaRounding = + 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); + } + + @Benchmark + public long timeRoundingDateTimeUnitJava() { + return javaRounding.round(timestamp); + } + + @Benchmark + public long timeRoundingDateTimeUnitDayOfMonthJoda() { + return jodaDayOfMonthRounding.round(timestamp); + } + + @Benchmark + public long timeRoundingDateTimeUnitDayOfMonthJava() { + return javaDayOfMonthRounding.round(timestamp); + } + + @Benchmark + public long timeIntervalRoundingJava() { + return timeIntervalRoundingJava.round(timestamp); + } + + @Benchmark + public long timeIntervalRoundingJoda() { + return timeIntervalRoundingJoda.round(timestamp); + } +} diff --git a/server/src/main/java/org/elasticsearch/common/Rounding.java b/server/src/main/java/org/elasticsearch/common/Rounding.java index dab29c88634e9..6d11739133dda 100644 --- a/server/src/main/java/org/elasticsearch/common/Rounding.java +++ b/server/src/main/java/org/elasticsearch/common/Rounding.java @@ -27,8 +27,8 @@ import org.elasticsearch.common.unit.TimeValue; import java.io.IOException; -import java.time.DayOfWeek; import java.time.Instant; +import java.time.LocalDate; import java.time.LocalDateTime; import java.time.LocalTime; import java.time.OffsetDateTime; @@ -39,7 +39,9 @@ import java.time.temporal.ChronoUnit; import java.time.temporal.IsoFields; import java.time.temporal.TemporalField; +import java.time.temporal.TemporalQueries; import java.time.zone.ZoneOffsetTransition; +import java.time.zone.ZoneRules; import java.util.List; import java.util.Objects; @@ -185,13 +187,11 @@ static class TimeUnitRounding extends Rounding { TimeUnitRounding(DateTimeUnit unit, ZoneId timeZone) { this.unit = unit; this.timeZone = timeZone; - this.unitRoundsToMidnight = this.unit.field.getBaseUnit().getDuration().toMillis() > 60L * 60L * 1000L; + this.unitRoundsToMidnight = this.unit.field.getBaseUnit().getDuration().toMillis() > 3600000L; } TimeUnitRounding(StreamInput in) throws IOException { - unit = DateTimeUnit.resolve(in.readByte()); - timeZone = DateUtils.of(in.readString()); - unitRoundsToMidnight = unit.getField().getBaseUnit().getDuration().toMillis() > 60L * 60L * 1000L; + this(DateTimeUnit.resolve(in.readByte()), DateUtils.of(in.readString())); } @Override @@ -200,85 +200,67 @@ public byte id() { } private LocalDateTime truncateLocalDateTime(LocalDateTime localDateTime) { - localDateTime = localDateTime.withNano(0); - assert localDateTime.getNano() == 0; - if (unit.equals(DateTimeUnit.SECOND_OF_MINUTE)) { - return localDateTime; - } + switch (unit) { + case SECOND_OF_MINUTE: + return localDateTime.withNano(0); - localDateTime = localDateTime.withSecond(0); - assert localDateTime.getSecond() == 0; - if (unit.equals(DateTimeUnit.MINUTES_OF_HOUR)) { - return localDateTime; - } + case MINUTES_OF_HOUR: + return LocalDateTime.of(localDateTime.getYear(), localDateTime.getMonthValue(), localDateTime.getDayOfMonth(), + localDateTime.getHour(), localDateTime.getMinute(), 0, 0); - localDateTime = localDateTime.withMinute(0); - assert localDateTime.getMinute() == 0; - if (unit.equals(DateTimeUnit.HOUR_OF_DAY)) { - return localDateTime; - } + case HOUR_OF_DAY: + return LocalDateTime.of(localDateTime.getYear(), localDateTime.getMonth(), localDateTime.getDayOfMonth(), + localDateTime.getHour(), 0, 0); - localDateTime = localDateTime.withHour(0); - assert localDateTime.getHour() == 0; - if (unit.equals(DateTimeUnit.DAY_OF_MONTH)) { - return localDateTime; - } + case DAY_OF_MONTH: + LocalDate localDate = localDateTime.query(TemporalQueries.localDate()); + return localDate.atStartOfDay(); - if (unit.equals(DateTimeUnit.WEEK_OF_WEEKYEAR)) { - localDateTime = localDateTime.with(ChronoField.DAY_OF_WEEK, 1); - assert localDateTime.getDayOfWeek() == DayOfWeek.MONDAY; - return localDateTime; - } + case WEEK_OF_WEEKYEAR: + return LocalDateTime.of(localDateTime.toLocalDate(), LocalTime.MIDNIGHT).with(ChronoField.DAY_OF_WEEK, 1); - localDateTime = localDateTime.withDayOfMonth(1); - assert localDateTime.getDayOfMonth() == 1; - if (unit.equals(DateTimeUnit.MONTH_OF_YEAR)) { - return localDateTime; - } + case MONTH_OF_YEAR: + return LocalDateTime.of(localDateTime.getYear(), localDateTime.getMonthValue(), 1, 0, 0); - if (unit.equals(DateTimeUnit.QUARTER_OF_YEAR)) { - int quarter = (int) IsoFields.QUARTER_OF_YEAR.getFrom(localDateTime); - int month = ((quarter - 1) * 3) + 1; - localDateTime = localDateTime.withMonth(month); - assert localDateTime.getMonthValue() % 3 == 1; - return localDateTime; - } + 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); - if (unit.equals(DateTimeUnit.YEAR_OF_CENTURY)) { - localDateTime = localDateTime.withMonth(1); - assert localDateTime.getMonthValue() == 1; - return localDateTime; - } + case YEAR_OF_CENTURY: + return LocalDateTime.of(LocalDate.of(localDateTime.getYear(), 1, 1), LocalTime.MIDNIGHT); - throw new IllegalArgumentException("NOT YET IMPLEMENTED for unit " + unit); + default: + throw new IllegalArgumentException("NOT YET IMPLEMENTED for unit " + unit); + } } @Override - public long round(long utcMillis) { + public long round(final long utcMillis) { + Instant instant = Instant.ofEpochMilli(utcMillis); if (unitRoundsToMidnight) { - final ZonedDateTime zonedDateTime = Instant.ofEpochMilli(utcMillis).atZone(timeZone); - final LocalDateTime localDateTime = zonedDateTime.toLocalDateTime(); + final LocalDateTime localDateTime = LocalDateTime.ofInstant(instant, timeZone); final LocalDateTime localMidnight = truncateLocalDateTime(localDateTime); return firstTimeOnDay(localMidnight); } else { + final ZoneRules rules = timeZone.getRules(); while (true) { - final Instant truncatedTime = truncateAsLocalTime(utcMillis); - final ZoneOffsetTransition previousTransition = timeZone.getRules().previousTransition(Instant.ofEpochMilli(utcMillis)); + final Instant truncatedTime = truncateAsLocalTime(instant, rules); + final ZoneOffsetTransition previousTransition = rules.previousTransition(instant); if (previousTransition == null) { // truncateAsLocalTime cannot have failed if there were no previous transitions return truncatedTime.toEpochMilli(); } - final long previousTransitionMillis = previousTransition.getInstant().toEpochMilli(); - - if (truncatedTime != null && previousTransitionMillis <= truncatedTime.toEpochMilli()) { + Instant previousTransitionInstant = previousTransition.getInstant(); + if (truncatedTime != null && previousTransitionInstant.compareTo(truncatedTime) < 1) { return truncatedTime.toEpochMilli(); } // There was a transition in between the input time and the truncated time. Return to the transition time and // round that down instead. - utcMillis = previousTransitionMillis - 1; + instant = previousTransitionInstant.minusNanos(1_000_000); } } } @@ -289,7 +271,7 @@ private long firstTimeOnDay(LocalDateTime localMidnight) { // Now work out what localMidnight actually means final List currentOffsets = timeZone.getRules().getValidOffsets(localMidnight); - if (currentOffsets.size() >= 1) { + if (currentOffsets.isEmpty() == false) { // There is at least one midnight on this day, so choose the first final ZoneOffset firstOffset = currentOffsets.get(0); final OffsetDateTime offsetMidnight = localMidnight.atOffset(firstOffset); @@ -302,23 +284,23 @@ private long firstTimeOnDay(LocalDateTime localMidnight) { } } - private Instant truncateAsLocalTime(long utcMillis) { + private Instant truncateAsLocalTime(Instant instant, final ZoneRules rules) { assert unitRoundsToMidnight == false : "truncateAsLocalTime should not be called if unitRoundsToMidnight"; - final LocalDateTime truncatedLocalDateTime - = truncateLocalDateTime(Instant.ofEpochMilli(utcMillis).atZone(timeZone).toLocalDateTime()); - final List currentOffsets = timeZone.getRules().getValidOffsets(truncatedLocalDateTime); + LocalDateTime localDateTime = LocalDateTime.ofInstant(instant, timeZone); + final LocalDateTime truncatedLocalDateTime = truncateLocalDateTime(localDateTime); + final List currentOffsets = rules.getValidOffsets(truncatedLocalDateTime); - if (currentOffsets.size() >= 1) { + if (currentOffsets.isEmpty() == false) { // at least one possibilities - choose the latest one that's still no later than the input time for (int offsetIndex = currentOffsets.size() - 1; offsetIndex >= 0; offsetIndex--) { final Instant result = truncatedLocalDateTime.atOffset(currentOffsets.get(offsetIndex)).toInstant(); - if (result.toEpochMilli() <= utcMillis) { + if (result.isAfter(instant) == false) { return result; } } - assert false : "rounded time not found for " + utcMillis + " with " + this; + assert false : "rounded time not found for " + instant + " with " + this; return null; } else { // The chosen local time didn't happen. This means we were given a time in an hour (or a minute) whose start @@ -328,7 +310,7 @@ private Instant truncateAsLocalTime(long utcMillis) { } private LocalDateTime nextRelevantMidnight(LocalDateTime localMidnight) { - assert localMidnight.toLocalTime().equals(LocalTime.of(0, 0, 0)) : "nextRelevantMidnight should only be called at midnight"; + assert localMidnight.toLocalTime().equals(LocalTime.MIDNIGHT) : "nextRelevantMidnight should only be called at midnight"; assert unitRoundsToMidnight : "firstTimeOnDay should only be called if unitRoundsToMidnight"; switch (unit) { @@ -350,8 +332,7 @@ private LocalDateTime nextRelevantMidnight(LocalDateTime localMidnight) { @Override public long nextRoundingValue(long utcMillis) { if (unitRoundsToMidnight) { - final ZonedDateTime zonedDateTime = Instant.ofEpochMilli(utcMillis).atZone(timeZone); - final LocalDateTime localDateTime = zonedDateTime.toLocalDateTime(); + final LocalDateTime localDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(utcMillis), timeZone); final LocalDateTime earlierLocalMidnight = truncateLocalDateTime(localDateTime); final LocalDateTime localMidnight = nextRelevantMidnight(earlierLocalMidnight); return firstTimeOnDay(localMidnight); @@ -433,14 +414,14 @@ public byte id() { @Override public long round(final long utcMillis) { final Instant utcInstant = Instant.ofEpochMilli(utcMillis); - final LocalDateTime rawLocalDateTime = Instant.ofEpochMilli(utcMillis).atZone(timeZone).toLocalDateTime(); + 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 = Instant.ofEpochMilli(roundedMillis).atZone(ZoneOffset.UTC).toLocalDateTime(); + final LocalDateTime roundedLocalDateTime = LocalDateTime.ofInstant(Instant.ofEpochMilli(roundedMillis), ZoneOffset.UTC); // Now work out what roundedLocalDateTime actually means final List currentOffsets = timeZone.getRules().getValidOffsets(roundedLocalDateTime); @@ -485,9 +466,8 @@ private static long roundKey(long value, long interval) { @Override public long nextRoundingValue(long time) { int offsetSeconds = timeZone.getRules().getOffset(Instant.ofEpochMilli(time)).getTotalSeconds(); - return ZonedDateTime.ofInstant(Instant.ofEpochMilli(time), ZoneOffset.UTC) - .plusSeconds(offsetSeconds) - .plusNanos(interval * 1_000_000) + long millis = time + interval + offsetSeconds * 1000; + return ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneOffset.UTC) .withZoneSameLocal(timeZone) .toInstant().toEpochMilli(); }