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:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to