Skip to content

Commit 003c7ff

Browse files
nik9000nik9000
authored
Deprecte Rounding#round (#57845)
This deprecates `Rounding#round` and `Rounding#nextRoundingValue` in favor of calling ``` Rounding.Prepared prepared = rounding.prepare(min, max); ... prepared.round(val) ``` because it is always going to be faster to prepare once. There are going to be some cases where we won't know what to prepare *for* and in those cases you can call `prepareForUnknown` and stil be faster than calling the deprecated method over and over and over again. Ultimately, this is important because it doesn't look like there is an easy way to cache `Rounding.Prepared` or any of its precursors like `LocalTimeOffset.Lookup`. Instead, we can just build it at most once per request. Relates to #56124
1 parent 9524daf commit 003c7ff

File tree

7 files changed

+48
-38
lines changed

7 files changed

+48
-38
lines changed

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -240,9 +240,9 @@ public interface Prepared {
240240

241241
/**
242242
* Rounds the given value.
243-
* <p>
244-
* Prefer {@link #prepare(long, long)} if rounding many values.
243+
* @deprecated Prefer {@link #prepare} and then {@link Prepared#round(long)}
245244
*/
245+
@Deprecated
246246
public final long round(long utcMillis) {
247247
return prepare(utcMillis, utcMillis).round(utcMillis);
248248
}
@@ -252,9 +252,9 @@ public final long round(long utcMillis) {
252252
* {@link #round(long)}, returns the next rounding value. For
253253
* example, with interval based rounding, if the interval is
254254
* 3, {@code nextRoundValue(6) = 9}.
255-
* <p>
256-
* Prefer {@link #prepare(long, long)} if rounding many values.
255+
* @deprecated Prefer {@link #prepare} and then {@link Prepared#nextRoundingValue(long)}
257256
*/
257+
@Deprecated
258258
public final long nextRoundingValue(long utcMillis) {
259259
return prepare(utcMillis, utcMillis).nextRoundingValue(utcMillis);
260260
}

‎server/src/test/java/org/elasticsearch/common/rounding/RoundingDuelTests.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,16 +35,16 @@ public class RoundingDuelTests extends ESTestCase {
3535
public void testDuellingImplementations() {
3636
org.elasticsearch.common.Rounding.DateTimeUnit randomDateTimeUnit =
3737
randomFrom(org.elasticsearch.common.Rounding.DateTimeUnit.values());
38-
org.elasticsearch.common.Rounding rounding;
38+
org.elasticsearch.common.Rounding.Prepared rounding;
3939
Rounding roundingJoda;
4040

4141
if (randomBoolean()) {
42-
rounding = org.elasticsearch.common.Rounding.builder(randomDateTimeUnit).timeZone(ZoneOffset.UTC).build();
42+
rounding = org.elasticsearch.common.Rounding.builder(randomDateTimeUnit).timeZone(ZoneOffset.UTC).build().prepareForUnknown();
4343
DateTimeUnit dateTimeUnit = DateTimeUnit.resolve(randomDateTimeUnit.getId());
4444
roundingJoda = Rounding.builder(dateTimeUnit).timeZone(DateTimeZone.UTC).build();
4545
} else {
4646
TimeValue interval = timeValue();
47-
rounding = org.elasticsearch.common.Rounding.builder(interval).timeZone(ZoneOffset.UTC).build();
47+
rounding = org.elasticsearch.common.Rounding.builder(interval).timeZone(ZoneOffset.UTC).build().prepareForUnknown();
4848
roundingJoda = Rounding.builder(interval).timeZone(DateTimeZone.UTC).build();
4949
}
5050

‎server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalAutoDateHistogramTests.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
157157

158158
int roundingIndex = reduced.getBucketInfo().roundingIdx;
159159
RoundingInfo roundingInfo = AutoDateHistogramAggregationBuilder.buildRoundings(null, null)[roundingIndex];
160+
Rounding.Prepared prepared = roundingInfo.rounding.prepare(lowest, highest);
160161

161162
long normalizedDuration = (highest - lowest) / roundingInfo.getRoughEstimateDurationMillis();
162163
int innerIntervalIndex = 0;
@@ -185,7 +186,7 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
185186
} else {
186187
do {
187188
innerIntervalToUse = roundingInfo.innerIntervals[innerIntervalIndex];
188-
int bucketCountAtInterval = getBucketCount(lowest, highest, roundingInfo.rounding, innerIntervalToUse);
189+
int bucketCountAtInterval = getBucketCount(lowest, highest, prepared, innerIntervalToUse);
189190
if (bucketCountAtInterval == reduced.getBuckets().size()) {
190191
break;
191192
}
@@ -199,11 +200,11 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
199200
assertThat(reduced.getInterval().toString(), equalTo(innerIntervalToUse + roundingInfo.unitAbbreviation));
200201
Map<Instant, Long> expectedCounts = new TreeMap<>();
201202
if (totalBucketConut > 0) {
202-
long keyForBucket = roundingInfo.rounding.round(lowest);
203-
while (keyForBucket <= roundingInfo.rounding.round(highest)) {
203+
long keyForBucket = prepared.round(lowest);
204+
while (keyForBucket <= prepared.round(highest)) {
204205
long nextKey = keyForBucket;
205206
for (int i = 0; i < innerIntervalToUse; i++) {
206-
nextKey = roundingInfo.rounding.nextRoundingValue(nextKey);
207+
nextKey = prepared.nextRoundingValue(nextKey);
207208
}
208209
Instant key = Instant.ofEpochMilli(keyForBucket);
209210
expectedCounts.put(key, 0L);
@@ -214,7 +215,7 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
214215

215216
for (InternalAutoDateHistogram histogram : inputs) {
216217
for (Histogram.Bucket bucket : histogram.getBuckets()) {
217-
long roundedBucketKey = roundingInfo.rounding.round(((ZonedDateTime) bucket.getKey()).toInstant().toEpochMilli());
218+
long roundedBucketKey = prepared.round(((ZonedDateTime) bucket.getKey()).toInstant().toEpochMilli());
218219
long docCount = bucket.getDocCount();
219220
if (roundedBucketKey >= keyForBucket && roundedBucketKey < nextKey) {
220221
expectedCounts.compute(key,
@@ -227,8 +228,8 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
227228

228229
// If there is only a single bucket, and we haven't added it above, add a bucket with no documents.
229230
// this step is necessary because of the roundedBucketKey < keyForBucket + intervalInMillis above.
230-
if (roundingInfo.rounding.round(lowest) == roundingInfo.rounding.round(highest) && expectedCounts.isEmpty()) {
231-
expectedCounts.put(Instant.ofEpochMilli(roundingInfo.rounding.round(lowest)), 0L);
231+
if (prepared.round(lowest) == prepared.round(highest) && expectedCounts.isEmpty()) {
232+
expectedCounts.put(Instant.ofEpochMilli(prepared.round(lowest)), 0L);
232233
}
233234
}
234235

@@ -249,12 +250,12 @@ protected void assertReduced(InternalAutoDateHistogram reduced, List<InternalAut
249250
assertThat(reduced.getInterval(), equalTo(expectedInterval));
250251
}
251252

252-
private int getBucketCount(long min, long max, Rounding rounding, int interval) {
253+
private int getBucketCount(long min, long max, Rounding.Prepared prepared, int interval) {
253254
int bucketCount = 0;
254-
long key = rounding.round(min);
255+
long key = prepared.round(min);
255256
while (key < max) {
256257
for (int i = 0; i < interval; i++) {
257-
key = rounding.nextRoundingValue(key);
258+
key = prepared.nextRoundingValue(key);
258259
}
259260
bucketCount++;
260261
}

‎server/src/test/java/org/elasticsearch/search/aggregations/bucket/histogram/InternalDateHistogramTests.java‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,8 @@ public void setUp() throws Exception {
5858
long interval = randomIntBetween(1, 3);
5959
intervalMillis = randomFrom(timeValueSeconds(interval), timeValueMinutes(interval), timeValueHours(interval)).getMillis();
6060
Rounding rounding = Rounding.builder(TimeValue.timeValueMillis(intervalMillis)).build();
61-
baseMillis = rounding.round(System.currentTimeMillis());
61+
long now = System.currentTimeMillis();
62+
baseMillis = rounding.prepare(now, now).round(now);
6263
if (randomBoolean()) {
6364
minDocCount = randomIntBetween(1, 10);
6465
emptyBucketInfo = null;
@@ -108,8 +109,12 @@ protected void assertReduced(InternalDateHistogram reduced, List<InternalDateHis
108109
long minBound = -1;
109110
long maxBound = -1;
110111
if (emptyBucketInfo.bounds != null) {
111-
minBound = emptyBucketInfo.rounding.round(emptyBucketInfo.bounds.getMin());
112-
maxBound = emptyBucketInfo.rounding.round(emptyBucketInfo.bounds.getMax());
112+
Rounding.Prepared prepared = emptyBucketInfo.rounding.prepare(
113+
emptyBucketInfo.bounds.getMin(),
114+
emptyBucketInfo.bounds.getMax()
115+
);
116+
minBound = prepared.round(emptyBucketInfo.bounds.getMin());
117+
maxBound = prepared.round(emptyBucketInfo.bounds.getMax());
113118
if (expectedCounts.isEmpty() && minBound <= maxBound) {
114119
expectedCounts.put(minBound, 0L);
115120
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/rollup/job/DateHistogramGroupConfig.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ public String getTimeZone() {
304304
/**
305305
* Create the rounding for this date histogram
306306
*/
307-
public Rounding createRounding() {
307+
public Rounding.Prepared createRounding() {
308308
return createRounding(interval.toString(), timeZone);
309309
}
310310

@@ -363,7 +363,7 @@ public static DateHistogramGroupConfig fromXContent(final XContentParser parser)
363363
return PARSER.parse(parser, null);
364364
}
365365

366-
private static Rounding createRounding(final String expr, final String timeZone) {
366+
private static Rounding.Prepared createRounding(final String expr, final String timeZone) {
367367
Rounding.DateTimeUnit timeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(expr);
368368
final Rounding.Builder rounding;
369369
if (timeUnit != null) {
@@ -372,6 +372,6 @@ private static Rounding createRounding(final String expr, final String timeZone)
372372
rounding = new Rounding.Builder(TimeValue.parseTimeValue(expr, "createRounding"));
373373
}
374374
rounding.timeZone(ZoneId.of(timeZone, ZoneId.SHORT_IDS));
375-
return rounding.build();
375+
return rounding.build().prepareForUnknown();
376376
}
377377
}

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/transform/transforms/pivot/DateHistogramGroupSource.java‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,27 @@ private void writeInterval(Interval interval, StreamOutput out) throws IOExcepti
209209

210210
private final Interval interval;
211211
private final ZoneId timeZone;
212-
private Rounding rounding;
212+
private final Rounding.Prepared rounding;
213213

214214
public DateHistogramGroupSource(String field, ScriptConfig scriptConfig, Interval interval, ZoneId timeZone) {
215215
super(field, scriptConfig);
216216
this.interval = interval;
217217
this.timeZone = timeZone;
218+
rounding = buildRounding();
219+
}
220+
221+
public DateHistogramGroupSource(StreamInput in) throws IOException {
222+
super(in);
223+
this.interval = readInterval(in);
224+
this.timeZone = in.readOptionalZoneId();
225+
// Format was optional in 7.2.x, removed in 7.3+
226+
if (in.getVersion().before(Version.V_7_3_0)) {
227+
in.readOptionalString();
228+
}
229+
rounding = buildRounding();
230+
}
218231

232+
private Rounding.Prepared buildRounding() {
219233
Rounding.DateTimeUnit timeUnit = DateHistogramAggregationBuilder.DATE_FIELD_UNITS.get(interval.toString());
220234
final Rounding.Builder roundingBuilder;
221235
if (timeUnit != null) {
@@ -227,17 +241,7 @@ public DateHistogramGroupSource(String field, ScriptConfig scriptConfig, Interva
227241
if (timeZone != null) {
228242
roundingBuilder.timeZone(timeZone);
229243
}
230-
this.rounding = roundingBuilder.build();
231-
}
232-
233-
public DateHistogramGroupSource(StreamInput in) throws IOException {
234-
super(in);
235-
this.interval = readInterval(in);
236-
this.timeZone = in.readOptionalZoneId();
237-
// Format was optional in 7.2.x, removed in 7.3+
238-
if (in.getVersion().before(Version.V_7_3_0)) {
239-
in.readOptionalString();
240-
}
244+
return roundingBuilder.build().prepareForUnknown();
241245
}
242246

243247
private static ConstructingObjectParser<DateHistogramGroupSource, Void> createParser(boolean lenient) {
@@ -296,7 +300,7 @@ public ZoneId getTimeZone() {
296300
return timeZone;
297301
}
298302

299-
public Rounding getRounding() {
303+
Rounding.Prepared getRounding() {
300304
return rounding;
301305
}
302306

‎x-pack/plugin/rollup/src/test/java/org/elasticsearch/xpack/rollup/job/RollupIndexerIndexingTests.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,7 @@ public void testSimpleDateHistoWithDelay() throws Exception {
274274
asMap("the_histo", now)
275275
)
276276
);
277-
final Rounding rounding = dateHistoConfig.createRounding();
277+
final Rounding.Prepared rounding = dateHistoConfig.createRounding();
278278
executeTestCase(dataset, job, now, (resp) -> {
279279
assertThat(resp.size(), equalTo(3));
280280
IndexRequest request = resp.get(0);
@@ -338,7 +338,7 @@ public void testSimpleDateHistoWithOverlappingDelay() throws Exception {
338338
asMap("the_histo", now)
339339
)
340340
);
341-
final Rounding rounding = dateHistoConfig.createRounding();
341+
final Rounding.Prepared rounding = dateHistoConfig.createRounding();
342342
executeTestCase(dataset, job, now, (resp) -> {
343343
assertThat(resp.size(), equalTo(2));
344344
IndexRequest request = resp.get(0);

0 commit comments

Comments
 (0)