diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/AbstractExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/AbstractExponentialHistogram.java new file mode 100644 index 0000000000000..81b24dfe209e6 --- /dev/null +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/AbstractExponentialHistogram.java @@ -0,0 +1,49 @@ +/* + * Copyright Elasticsearch B.V., and/or licensed to Elasticsearch B.V. + * under one or more license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + * + * This file is based on a modification of https://github.com/open-telemetry/opentelemetry-java which is licensed under the Apache 2.0 License. + */ + +package org.elasticsearch.exponentialhistogram; + +/** + * Basic implementation for {@link ExponentialHistogram} with common functionality. + */ +public abstract class AbstractExponentialHistogram implements ExponentialHistogram { + + @Override + public long valueCount() { + return zeroBucket().count() + negativeBuckets().valueCount() + positiveBuckets().valueCount(); + } + + @Override + public int hashCode() { + return ExponentialHistogram.hashCode(this); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj instanceof ExponentialHistogram) { + return ExponentialHistogram.equals(this, (ExponentialHistogram) obj); + } + return false; + } +} diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java index 3c57955fdf77a..a97e3498c2e5f 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/EmptyExponentialHistogram.java @@ -23,7 +23,7 @@ import java.util.OptionalLong; -class EmptyExponentialHistogram implements ReleasableExponentialHistogram { +class EmptyExponentialHistogram extends AbstractExponentialHistogram implements ReleasableExponentialHistogram { static final EmptyExponentialHistogram INSTANCE = new EmptyExponentialHistogram(); diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java index 40c5020f46436..8f0e6ea12464d 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialHistogram.java @@ -102,6 +102,14 @@ public interface ExponentialHistogram extends Accountable { */ double sum(); + /** + * Returns the number of values represented by this histogram. + * In other words, this is the sum of the counts of all buckets including the zero bucket. + * + * @return the value count, guaranteed to be zero for empty histograms + */ + long valueCount(); + /** * Returns minimum of all values represented by this histogram. * @@ -132,6 +140,60 @@ interface Buckets { } + /** + * Value-based equality for exponential histograms. + * @param a the first histogram (can be null) + * @param b the second histogram (can be null) + * @return true, if both histograms are equal + */ + static boolean equals(ExponentialHistogram a, ExponentialHistogram b) { + if (a == b) return true; + if (a == null) return false; + if (b == null) return false; + + return a.scale() == b.scale() + && a.sum() == b.sum() + && equalsIncludingNaN(a.min(), b.min()) + && a.zeroBucket().equals(b.zeroBucket()) + && bucketIteratorsEqual(a.negativeBuckets().iterator(), b.negativeBuckets().iterator()) + && bucketIteratorsEqual(a.positiveBuckets().iterator(), b.positiveBuckets().iterator()); + } + + private static boolean equalsIncludingNaN(double a, double b) { + return (a == b) || (Double.isNaN(a) && Double.isNaN(b)); + } + + private static boolean bucketIteratorsEqual(BucketIterator a, BucketIterator b) { + if (a.scale() != b.scale()) { + return false; + } + while (a.hasNext() && b.hasNext()) { + if (a.peekIndex() != b.peekIndex() || a.peekCount() != b.peekCount()) { + return false; + } + a.advance(); + b.advance(); + } + return a.hasNext() == b.hasNext(); + } + + /** + * Default hash code implementation to be used with {@link #equals(ExponentialHistogram, ExponentialHistogram)}. + * @param histogram the histogram to hash + * @return the hash code + */ + static int hashCode(ExponentialHistogram histogram) { + int hash = histogram.scale(); + hash = 31 * hash + Double.hashCode(histogram.sum()); + hash = 31 * hash + Long.hashCode(histogram.valueCount()); + hash = 31 * hash + Double.hashCode(histogram.min()); + hash = 31 * hash + histogram.zeroBucket().hashCode(); + // we intentionally don't include the hash of the buckets here, because that is likely expensive to compute + // instead, we assume that the value count and sum are a good enough approximation in most cases to minimize collisions + // the value count is typically available as a cached value and doesn't involve iterating over all buckets + return hash; + } + static ExponentialHistogram empty() { return EmptyExponentialHistogram.INSTANCE; } diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialScaleUtils.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialScaleUtils.java index a27fbeea89093..314f99f7c7087 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialScaleUtils.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ExponentialScaleUtils.java @@ -185,6 +185,16 @@ public static int getMaximumScaleIncrease(long index) { return Long.numberOfLeadingZeros(index) - (64 - MAX_INDEX_BITS); } + /** + * Returns a scale at to which the given index can be scaled down without changing the exponentially scaled number it represents. + * @param index the index of the number + * @param scale the current scale of the number + * @return the new scale + */ + static int normalizeScale(long index, int scale) { + return Math.max(MIN_SCALE, scale - Long.numberOfTrailingZeros(index)); + } + /** * Returns the upper boundary of the bucket with the given index and scale. * diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java index 67b478af22427..5db708302fc56 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/FixedCapacityExponentialHistogram.java @@ -31,7 +31,7 @@ * Consumers must ensure that if the histogram is mutated, all previously acquired {@link BucketIterator} * instances are no longer used. */ -final class FixedCapacityExponentialHistogram implements ReleasableExponentialHistogram { +final class FixedCapacityExponentialHistogram extends AbstractExponentialHistogram implements ReleasableExponentialHistogram { static final long BASE_SIZE = RamUsageEstimator.shallowSizeOfInstance(FixedCapacityExponentialHistogram.class) + ZeroBucket.SHALLOW_SIZE + 2 * Buckets.SHALLOW_SIZE; diff --git a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java index 6a9d24e87c0e1..86d95573769a3 100644 --- a/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java +++ b/libs/exponential-histogram/src/main/java/org/elasticsearch/exponentialhistogram/ZeroBucket.java @@ -27,9 +27,11 @@ import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_SCALE; import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_INDEX; import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_SCALE; +import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.adjustScale; import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.compareExponentiallyScaledValues; import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.computeIndex; import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.exponentiallyScaledToDoubleValue; +import static org.elasticsearch.exponentialhistogram.ExponentialScaleUtils.normalizeScale; /** * Represents the bucket for values around zero in an exponential histogram. @@ -62,13 +64,7 @@ public final class ZeroBucket { // A singleton for an empty zero bucket with the smallest possible threshold. private static final ZeroBucket MINIMAL_EMPTY = new ZeroBucket(MIN_INDEX, MIN_SCALE, 0); - /** - * Creates a new zero bucket with a specific threshold and count. - * - * @param zeroThreshold The threshold defining the bucket's range [-zeroThreshold, +zeroThreshold]. - * @param count The number of values in the bucket. - */ - public ZeroBucket(double zeroThreshold, long count) { + private ZeroBucket(double zeroThreshold, long count) { assert zeroThreshold >= 0.0 : "zeroThreshold must not be negative"; this.index = Long.MAX_VALUE; // compute lazily when needed this.scale = MAX_SCALE; @@ -85,11 +81,11 @@ private ZeroBucket(long index, int scale, long count) { this.count = count; } - private ZeroBucket(double realThreshold, long index, int scale, long count) { - this.realThreshold = realThreshold; - this.index = index; - this.scale = scale; - this.count = count; + private ZeroBucket(ZeroBucket toCopy, long newCount) { + this.realThreshold = toCopy.realThreshold; + this.index = toCopy.index; + this.scale = toCopy.scale; + this.count = newCount; } /** @@ -109,8 +105,37 @@ public static ZeroBucket minimalWithCount(long count) { if (count == 0) { return MINIMAL_EMPTY; } else { - return new ZeroBucket(MINIMAL_EMPTY.zeroThreshold(), MINIMAL_EMPTY.index(), MINIMAL_EMPTY.scale(), count); + return new ZeroBucket(MINIMAL_EMPTY, count); + } + } + + /** + * Creates a zero bucket from the given threshold represented as double. + * + * @param zeroThreshold the zero threshold defining the bucket range [-zeroThreshold, +zeroThreshold], must be non-negative + * @param count the number of values in the bucket + * @return the new {@link ZeroBucket} + */ + public static ZeroBucket create(double zeroThreshold, long count) { + if (zeroThreshold == 0) { + return minimalWithCount(count); } + return new ZeroBucket(zeroThreshold, count); + } + + /** + * Creates a zero bucket from the given threshold represented as exponentially scaled number. + * + * @param index the index of the exponentially scaled number defining the zero threshold + * @param scale the corresponding scale for the index + * @param count the number of values in the bucket + * @return the new {@link ZeroBucket} + */ + public static ZeroBucket create(long index, int scale, long count) { + if (index == MINIMAL_EMPTY.index && scale == MINIMAL_EMPTY.scale) { + return minimalWithCount(count); + } + return new ZeroBucket(index, scale, count); } /** @@ -158,9 +183,9 @@ public ZeroBucket merge(ZeroBucket other) { long totalCount = count + other.count; // Both are populated, so we need to use the higher zero-threshold. if (this.compareZeroThreshold(other) >= 0) { - return new ZeroBucket(realThreshold, index, scale, totalCount); + return new ZeroBucket(this, totalCount); } else { - return new ZeroBucket(other.realThreshold, other.index, other.scale, totalCount); + return new ZeroBucket(other, totalCount); } } } @@ -219,10 +244,33 @@ public ZeroBucket collapseOverlappingBuckets(BucketIterator buckets) { long collapsedUpperBoundIndex = highestCollapsedIndex + 1; if (compareExponentiallyScaledValues(index(), scale(), collapsedUpperBoundIndex, buckets.scale()) >= 0) { // Our current zero-threshold is larger than the upper boundary of the largest collapsed bucket, so we keep it. - return new ZeroBucket(realThreshold, index, scale, newZeroCount); + return new ZeroBucket(this, newZeroCount); } else { return new ZeroBucket(collapsedUpperBoundIndex, buckets.scale(), newZeroCount); } } } + + @Override + public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + ZeroBucket that = (ZeroBucket) o; + if (count() != that.count()) return false; + if (Double.compare(zeroThreshold(), that.zeroThreshold()) != 0) return false; + if (compareExponentiallyScaledValues(index(), scale(), that.index(), that.scale()) != 0) return false; + return true; + } + + @Override + public int hashCode() { + int normalizedScale = normalizeScale(index(), scale); + int scaleAdjustment = normalizedScale - scale; + long normalizedIndex = adjustScale(index(), scale, scaleAdjustment); + + int result = normalizedScale; + result = 31 * result + Long.hashCode(normalizedIndex); + result = 31 * result + Double.hashCode(zeroThreshold()); + result = 31 * result + Long.hashCode(count); + return result; + } } diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java new file mode 100644 index 0000000000000..fe2ec27cd65ca --- /dev/null +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramEqualityTests.java @@ -0,0 +1,178 @@ +/* + * Copyright Elasticsearch B.V., and/or licensed to Elasticsearch B.V. + * under one or more license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch B.V. 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. + * + * This file is based on a modification of https://github.com/open-telemetry/opentelemetry-java which is licensed under the Apache 2.0 License. + */ + +package org.elasticsearch.exponentialhistogram; + +import com.carrotsearch.randomizedtesting.annotations.ParametersFactory; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_INDEX; +import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MAX_SCALE; +import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_INDEX; +import static org.elasticsearch.exponentialhistogram.ExponentialHistogram.MIN_SCALE; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; + +public class ExponentialHistogramEqualityTests extends ExponentialHistogramTestCase { + + public enum Modification { + SCALE, + SUM, + MIN, + ZERO_THRESHOLD, + ZERO_COUNT, + POSITIVE_BUCKETS, + NEGATIVE_BUCKETS + } + + private final Modification modification; + + @ParametersFactory + public static Iterable parameters() { + return Arrays.stream(Modification.values()).map(modification -> new Object[] { modification }).toList(); + } + + public ExponentialHistogramEqualityTests(Modification modification) { + this.modification = modification; + } + + public void testEquality() { + ExponentialHistogram histo = randomHistogram(); + ExponentialHistogram copy = copyWithModification(histo, null); + ExponentialHistogram modifiedCopy = copyWithModification(histo, modification); + + assertThat(histo, equalTo(copy)); + assertThat(histo.hashCode(), equalTo(copy.hashCode())); + assertThat(histo, not(equalTo(modifiedCopy))); + } + + public void testHashQuality() { + ExponentialHistogram histo = randomHistogram(); + // of 10 tries, at least one should produce a different hash code + for (int i = 0; i < 10; i++) { + ExponentialHistogram modifiedCopy = copyWithModification(histo, modification); + if (histo.hashCode() != modifiedCopy.hashCode()) { + return; + } + } + fail("Could not produce different hash code after 10 tries"); + } + + private ExponentialHistogram randomHistogram() { + return createAutoReleasedHistogram(randomIntBetween(4, 20), randomDoubles(randomIntBetween(0, 200)).toArray()); + } + + private ExponentialHistogram copyWithModification(ExponentialHistogram toCopy, Modification modification) { + int totalBucketCount = getBucketCount(toCopy.positiveBuckets(), toCopy.negativeBuckets()); + FixedCapacityExponentialHistogram copy = createAutoReleasedHistogram(totalBucketCount + 2); + if (modification == Modification.SCALE) { + copy.resetBuckets((int) createRandomLongBetweenOtherThan(MIN_SCALE, MAX_SCALE, toCopy.scale())); + } else { + copy.resetBuckets(toCopy.scale()); + } + if (modification == Modification.SUM) { + copy.setSum(randomDouble()); + } else { + copy.setSum(toCopy.sum()); + } + if (modification == Modification.MIN) { + copy.setMin(randomDouble()); + } else { + copy.setMin(toCopy.min()); + } + long zeroCount = toCopy.zeroBucket().count(); + double zeroThreshold = toCopy.zeroBucket().zeroThreshold(); + if (modification == Modification.ZERO_COUNT) { + zeroCount = createRandomLongBetweenOtherThan(0, Long.MAX_VALUE, zeroCount); + } else if (modification == Modification.ZERO_THRESHOLD) { + zeroThreshold = randomDouble(); + } + copy.setZeroBucket(ZeroBucket.create(zeroThreshold, zeroCount)); + copyBuckets(copy, toCopy.negativeBuckets(), modification == Modification.NEGATIVE_BUCKETS, false); + copyBuckets(copy, toCopy.positiveBuckets(), modification == Modification.POSITIVE_BUCKETS, true); + + return copy; + } + + private void copyBuckets( + FixedCapacityExponentialHistogram into, + ExponentialHistogram.Buckets buckets, + boolean modify, + boolean isPositive + ) { + List indices = new ArrayList<>(); + List counts = new ArrayList<>(); + BucketIterator it = buckets.iterator(); + while (it.hasNext()) { + indices.add(it.peekIndex()); + counts.add(it.peekCount()); + it.advance(); + } + if (modify) { + if (counts.isEmpty() == false && randomBoolean()) { + int toModify = randomIntBetween(0, indices.size() - 1); + counts.set(toModify, createRandomLongBetweenOtherThan(1, Long.MAX_VALUE, counts.get(toModify))); + } else { + insertRandomBucket(indices, counts); + } + } + for (int i = 0; i < indices.size(); i++) { + into.tryAddBucket(indices.get(i), counts.get(i), isPositive); + } + } + + private void insertRandomBucket(List indices, List counts) { + long minIndex = indices.stream().mapToLong(Long::longValue).min().orElse(MIN_INDEX); + long maxIndex = indices.stream().mapToLong(Long::longValue).min().orElse(MAX_INDEX); + long newIndex; + do { + newIndex = randomLongBetween(Math.max(MIN_INDEX, minIndex - 10), Math.min(MAX_INDEX, maxIndex + 10)); + } while (indices.contains(newIndex)); + int position = -(Collections.binarySearch(indices, newIndex) + 1); + indices.add(position, newIndex); + counts.add(position, randomLongBetween(1, Long.MAX_VALUE)); + } + + private static long createRandomLongBetweenOtherThan(long min, long max, long notAllowedValue) { + assert min != max || notAllowedValue != min; + long result; + do { + result = randomLongBetween(min, max); + } while (result == notAllowedValue); + return result; + } + + private static int getBucketCount(ExponentialHistogram.Buckets... buckets) { + int count = 0; + for (ExponentialHistogram.Buckets val : buckets) { + BucketIterator it = val.iterator(); + while (it.hasNext()) { + count++; + it.advance(); + } + } + return count; + } +} diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java index 17f4f172ecace..9b57bf5129a7a 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramMergerTests.java @@ -44,7 +44,7 @@ public class ExponentialHistogramMergerTests extends ExponentialHistogramTestCas public void testZeroThresholdCollapsesOverlappingBuckets() { FixedCapacityExponentialHistogram first = createAutoReleasedHistogram(100); - first.setZeroBucket(new ZeroBucket(2.0001, 10)); + first.setZeroBucket(ZeroBucket.create(2.0001, 10)); FixedCapacityExponentialHistogram second = createAutoReleasedHistogram(100); first.resetBuckets(0); // scale 0 means base 2 @@ -77,7 +77,7 @@ public void testZeroThresholdCollapsesOverlappingBuckets() { // ensure buckets of the accumulated histogram are collapsed too if needed FixedCapacityExponentialHistogram third = createAutoReleasedHistogram(100); - third.setZeroBucket(new ZeroBucket(45.0, 1)); + third.setZeroBucket(ZeroBucket.create(45.0, 1)); mergeResult = mergeWithMinimumScale(100, 0, mergeResult, third); assertThat(mergeResult.zeroBucket().zeroThreshold(), closeTo(45.0, 0.000001)); @@ -88,12 +88,12 @@ public void testZeroThresholdCollapsesOverlappingBuckets() { public void testEmptyZeroBucketIgnored() { FixedCapacityExponentialHistogram first = createAutoReleasedHistogram(100); - first.setZeroBucket(new ZeroBucket(2.0, 10)); + first.setZeroBucket(ZeroBucket.create(2.0, 10)); first.resetBuckets(0); // scale 0 means base 2 first.tryAddBucket(2, 42L, true); // bucket (4, 8] FixedCapacityExponentialHistogram second = createAutoReleasedHistogram(100); - second.setZeroBucket(new ZeroBucket(100.0, 0)); + second.setZeroBucket(ZeroBucket.create(100.0, 0)); ExponentialHistogram mergeResult = mergeWithMinimumScale(100, 0, first, second); diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java index b9005c9e84083..87dfb3a12ad7b 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramUtilsTests.java @@ -113,7 +113,7 @@ public void testMinimumEstimation() { ExponentialHistogram histo = createAutoReleasedHistogram(bucketCount, values); OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin( - new ZeroBucket(zeroThreshold, zeroValueCount), + ZeroBucket.create(zeroThreshold, zeroValueCount), histo.negativeBuckets(), histo.positiveBuckets() ); diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java index 06c9e1ad4150f..c1040566d0238 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ExponentialHistogramXContentTests.java @@ -38,7 +38,7 @@ public void testEmptyHistogram() { public void testFullHistogram() { FixedCapacityExponentialHistogram histo = createAutoReleasedHistogram(100); - histo.setZeroBucket(new ZeroBucket(0.1234, 42)); + histo.setZeroBucket(ZeroBucket.create(0.1234, 42)); histo.resetBuckets(7); histo.setSum(1234.56); histo.setMin(-321.123); @@ -63,7 +63,7 @@ public void testFullHistogram() { public void testOnlyZeroThreshold() { FixedCapacityExponentialHistogram histo = createAutoReleasedHistogram(10); - histo.setZeroBucket(new ZeroBucket(5.0, 0)); + histo.setZeroBucket(ZeroBucket.create(5.0, 0)); histo.resetBuckets(3); histo.setSum(1.1); assertThat(toJson(histo), equalTo("{\"scale\":3,\"sum\":1.1,\"zero\":{\"threshold\":5.0}}")); @@ -71,7 +71,7 @@ public void testOnlyZeroThreshold() { public void testOnlyZeroCount() { FixedCapacityExponentialHistogram histo = createAutoReleasedHistogram(10); - histo.setZeroBucket(new ZeroBucket(0.0, 7)); + histo.setZeroBucket(ZeroBucket.create(0.0, 7)); histo.resetBuckets(2); histo.setSum(1.1); histo.setMin(0); diff --git a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ZeroBucketTests.java b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ZeroBucketTests.java index 1d2cbd57604ab..2ed25afb31107 100644 --- a/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ZeroBucketTests.java +++ b/libs/exponential-histogram/src/test/java/org/elasticsearch/exponentialhistogram/ZeroBucketTests.java @@ -22,6 +22,7 @@ package org.elasticsearch.exponentialhistogram; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.not; public class ZeroBucketTests extends ExponentialHistogramTestCase { @@ -30,13 +31,13 @@ public void testMinimalBucketHasZeroThreshold() { } public void testExactThresholdPreserved() { - ZeroBucket bucket = new ZeroBucket(3.0, 10); + ZeroBucket bucket = ZeroBucket.create(3.0, 10); assertThat(bucket.zeroThreshold(), equalTo(3.0)); } public void testMergingPreservesExactThreshold() { - ZeroBucket bucketA = new ZeroBucket(3.0, 10); - ZeroBucket bucketB = new ZeroBucket(3.5, 20); + ZeroBucket bucketA = ZeroBucket.create(3.0, 10); + ZeroBucket bucketB = ZeroBucket.create(3.5, 20); ZeroBucket merged = bucketA.merge(bucketB); assertThat(merged.zeroThreshold(), equalTo(3.5)); assertThat(merged.count(), equalTo(30L)); @@ -47,7 +48,7 @@ public void testBucketCollapsingPreservesExactThreshold() { histo.resetBuckets(0); histo.tryAddBucket(0, 42, true); // bucket [1,2] - ZeroBucket bucketA = new ZeroBucket(3.0, 10); + ZeroBucket bucketA = ZeroBucket.create(3.0, 10); CopyableBucketIterator iterator = histo.positiveBuckets().iterator(); ZeroBucket merged = bucketA.collapseOverlappingBuckets(iterator); @@ -57,4 +58,33 @@ public void testBucketCollapsingPreservesExactThreshold() { assertThat(merged.count(), equalTo(52L)); } + public void testHashCodeEquality() { + assertEqualsContract(ZeroBucket.minimalEmpty(), ZeroBucket.create(0.0, 0)); + assertThat(ZeroBucket.minimalEmpty(), not(equalTo(ZeroBucket.create(0.0, 1)))); + assertThat(ZeroBucket.minimalEmpty(), not(equalTo(ZeroBucket.create(0.1, 0)))); + + assertEqualsContract(ZeroBucket.minimalWithCount(42), ZeroBucket.create(0.0, 42)); + assertThat(ZeroBucket.minimalWithCount(42), not(equalTo(ZeroBucket.create(0.0, 12)))); + assertThat(ZeroBucket.minimalWithCount(42), not(equalTo(ZeroBucket.create(0.1, 42)))); + + ZeroBucket minimalEmpty = ZeroBucket.minimalEmpty(); + assertEqualsContract(ZeroBucket.minimalWithCount(42), ZeroBucket.create(minimalEmpty.index(), minimalEmpty.scale(), 42)); + assertThat(ZeroBucket.minimalWithCount(42), not(equalTo(ZeroBucket.create(minimalEmpty.index(), minimalEmpty.scale(), 41)))); + assertThat(ZeroBucket.minimalWithCount(42), not(equalTo(ZeroBucket.create(minimalEmpty.index() + 1, minimalEmpty.scale(), 42)))); + + assertEqualsContract(ZeroBucket.create(123.56, 123), ZeroBucket.create(123.56, 123)); + assertThat(ZeroBucket.create(123.56, 123), not(equalTo(ZeroBucket.create(123.57, 123)))); + assertThat(ZeroBucket.create(123.56, 123), not(equalTo(ZeroBucket.create(123.56, 12)))); + + // the exponentially scaled numbers (index=2, scale=0) and (index=1, scale=-1) both represent 4.0 + // therefore the zero buckets should be equal + assertEqualsContract(ZeroBucket.create(2, 0, 123), ZeroBucket.create(1, -1, 123)); + assertEqualsContract(ZeroBucket.create(4, 1, 123), ZeroBucket.create(1, -1, 123)); + } + + void assertEqualsContract(ZeroBucket a, ZeroBucket b) { + assertThat(a, equalTo(b)); + assertThat(a.hashCode(), equalTo(b.hashCode())); + } + } diff --git a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java index f076ca072f323..a516ca6820971 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/CompressedExponentialHistogram.java @@ -10,6 +10,7 @@ import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.RamUsageEstimator; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.exponentialhistogram.AbstractExponentialHistogram; import org.elasticsearch.exponentialhistogram.BucketIterator; import org.elasticsearch.exponentialhistogram.CopyableBucketIterator; import org.elasticsearch.exponentialhistogram.ExponentialHistogram; @@ -27,7 +28,7 @@ * While this implementation is optimized for a minimal memory footprint, it is still a fully compliant {@link ExponentialHistogram} * and can therefore be directly consumed for merging / quantile estimation without requiring any prior copying or decoding. */ -public class CompressedExponentialHistogram implements ExponentialHistogram { +public class CompressedExponentialHistogram extends AbstractExponentialHistogram { private static final long SHALLOW_SIZE = RamUsageEstimator.shallowSizeOfInstance(CompressedExponentialHistogram.class); @@ -50,7 +51,7 @@ public int scale() { public ZeroBucket zeroBucket() { if (lazyZeroBucket == null) { long zeroCount = valueCount - negativeBuckets.valueCount() - positiveBuckets.valueCount(); - lazyZeroBucket = new ZeroBucket(zeroThreshold, zeroCount); + lazyZeroBucket = ZeroBucket.create(zeroThreshold, zeroCount); } return lazyZeroBucket; } @@ -60,6 +61,11 @@ public double sum() { return sum; } + @Override + public long valueCount() { + return valueCount; + } + @Override public double min() { return min; diff --git a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java index 1d33e0c7eaa23..0d5b21b08b2aa 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/main/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapper.java @@ -373,7 +373,7 @@ public void parse(DocumentParserContext context) throws IOException { if (min == null) { OptionalDouble estimatedMin = ExponentialHistogramUtils.estimateMin( - new ZeroBucket(zeroBucket.threshold(), zeroBucket.count), + ZeroBucket.create(zeroBucket.threshold(), zeroBucket.count), IndexWithCount.asBuckets(scale, negativeBuckets), IndexWithCount.asBuckets(scale, positiveBuckets) ); diff --git a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java index 07a7ef241f76a..9067c4da5aa52 100644 --- a/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java +++ b/x-pack/plugin/mapper-exponential-histogram/src/test/java/org/elasticsearch/xpack/exponentialhistogram/ExponentialHistogramFieldMapperTests.java @@ -500,9 +500,9 @@ private ZeroBucket mapToZeroBucket(Map zeroBucket) { Number threshold = Types.forciblyCast(zeroBucket.get("threshold")); Number count = Types.forciblyCast(zeroBucket.get("count")); if (threshold != null && count != null) { - return new ZeroBucket(threshold.doubleValue(), count.longValue()); + return ZeroBucket.create(threshold.doubleValue(), count.longValue()); } else if (threshold != null) { - return new ZeroBucket(threshold.doubleValue(), 0); + return ZeroBucket.create(threshold.doubleValue(), 0); } else if (count != null) { return ZeroBucket.minimalWithCount(count.longValue()); } else {