zentol commented on a change in pull request #8888: [FLINK-12983][metrics]
replace descriptive histogram's storage back-end
URL: https://github.com/apache/flink/pull/8888#discussion_r314416005
##########
File path:
flink-runtime/src/main/java/org/apache/flink/runtime/metrics/DescriptiveStatisticsHistogram.java
##########
@@ -27,27 +27,63 @@
*/
public class DescriptiveStatisticsHistogram implements
org.apache.flink.metrics.Histogram {
- private final DescriptiveStatistics descriptiveStatistics;
-
- private long elementsSeen = 0L;
+ private final CircularDoubleArray descriptiveStatistics;
public DescriptiveStatisticsHistogram(int windowSize) {
- this.descriptiveStatistics = new
DescriptiveStatistics(windowSize);
+ this.descriptiveStatistics = new
CircularDoubleArray(windowSize);
}
@Override
public void update(long value) {
- elementsSeen += 1L;
this.descriptiveStatistics.addValue(value);
}
@Override
public long getCount() {
- return this.elementsSeen;
+ return this.descriptiveStatistics.getElementsSeen();
}
@Override
public HistogramStatistics getStatistics() {
return new
DescriptiveStatisticsHistogramStatistics(this.descriptiveStatistics);
}
+
+ /**
+ * Fixed-size array that wraps around at the end and has a dynamic
start position.
+ */
+ static class CircularDoubleArray {
+ private final double[] backingArray;
+ private int nextPos = 0;
+ private boolean fullSize = false;
+ private long elementsSeen = 0;
+
+ CircularDoubleArray(int windowSize) {
+ this.backingArray = new double[windowSize];
+ }
+
+ synchronized void addValue(double value) {
+ backingArray[nextPos] = value;
+ ++elementsSeen;
+ ++nextPos;
+ if (nextPos == backingArray.length) {
+ nextPos = 0;
+ fullSize = true;
+ }
+ }
+
+ synchronized double[] toUnsortedArray() {
+ final int size = getSize();
+ double[] result = new double[size];
+ System.arraycopy(backingArray, 0, result, 0,
result.length);
Review comment:
I'm wondering whether the order could be relevant to _someone_. How
expensive would it be to keep the insertion order (i.e. by adding a second
arraycopy call)?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
