Refactor: Wrap hdr_histogram in its own class

owend74 · owend74 · commit 09771ed1ab8a · 2018-05-02T10:02:13.000Z
Wraps the hdr_histogram data structure in its own class (HdrHistogram) thereby making it possible to use (for example to hold statistics) without needing to refer directly to the underlying C data structure. Change-Id: I4d4475cbbc41ff45250ba4940fe7b4e1cf2dd177 Reviewed-on: http://review.couchbase.org/93536 Reviewed-by: Dave Rigby <daver@couchbase.com> Tested-by: Build Bot <build@couchbase.com>
diff --git a/engines/ep/CMakeLists.txt b/engines/ep/CMakeLists.txt
@@ -205,6 +205,7 @@ ADD_LIBRARY(ep_objs OBJECT
             src/flusher.cc
             src/globaltask.cc
             src/hash_table.cc
+            src/hdrhistogram.cc
             src/hlc.cc
             src/htresizer.cc
             src/item.cc
diff --git a/engines/ep/src/hdrhistogram.cc b/engines/ep/src/hdrhistogram.cc
@@ -0,0 +1,89 @@
+/* -*- Mode: C++; tab-width: 4; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ *     Copyright 2018 Couchbase, Inc
+ *
+ *   Licensed 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.
+ */
+
+#include "hdrhistogram.h"
+
+#include <cstdlib> // Required due to the use of free
+
+// Custom deleter for the hdr_histogram struct.
+void HdrHistogram::HdrDeleter::operator()(struct hdr_histogram* val) {
+    free(val);
+}
+
+HdrHistogram::HdrHistogram(uint64_t lowestTrackableValue,
+                           uint64_t highestTrackableValue,
+                           int significantFigures) {
+    struct hdr_histogram* hist;
+    // We add a bias of +1 to the lowest and highest trackable value
+    // because we add +1 to all values we store in the histogram (as this
+    // allows us to record the value 0).
+    hdr_init(lowestTrackableValue + 1,
+             highestTrackableValue + 1, // Add one because all values
+             significantFigures,
+             &hist); // Pointer to initialise
+    histogram.reset(hist);
+}
+
+void HdrHistogram::addValue(uint64_t v) {
+    // A hdr_histogram cannot store 0, therefore we add a bias of +1.
+    int64_t vBiased = v + 1;
+    hdr_record_value(histogram.get(), vBiased);
+}
+
+void HdrHistogram::addValueAndCount(uint64_t v, uint64_t count) {
+    // A hdr_histogram cannot store 0, therefore we add a bias of +1.
+    int64_t vBiased = v + 1;
+    hdr_record_values(histogram.get(), vBiased, count);
+}
+
+uint64_t HdrHistogram::getValueCount() const {
+    return histogram->total_count;
+}
+
+void HdrHistogram::reset() {
+    hdr_reset(histogram.get());
+}
+
+uint64_t HdrHistogram::getValueAtPercentile(double percentage) const {
+    // We added the bias of +1 to the input value (see
+    // addValueToFreqHistogram).  Therefore need to minus the bias
+    // before returning the value from the histogram.
+    uint64_t value = hdr_value_at_percentile(histogram.get(), percentage);
+    return (value - 1);
+}
+
+HdrHistogram::Iterator HdrHistogram::makeLinearIterator(
+        int64_t valueUnitsPerBucket) const {
+    HdrHistogram::Iterator iter;
+    hdr_iter_linear_init(&iter, histogram.get(), valueUnitsPerBucket);
+    return iter;
+}
+
+boost::optional<std::pair<uint64_t, uint64_t>>
+HdrHistogram::getNextValueAndCount(Iterator& iter) const {
+    boost::optional<std::pair<uint64_t, uint64_t>> valueAndCount;
+    if (hdr_iter_next(&iter)) {
+        uint64_t value = iter.value;
+        uint64_t count = hdr_count_at_value(histogram.get(), value);
+        // We added the bias of +1 to the input value (see
+        // addValueToFreqHistogram).  Therefore need to minus the bias
+        // before returning value.
+        return valueAndCount = std::make_pair(value - 1, count);
+    } else {
+        return valueAndCount;
+    }
+}
diff --git a/engines/ep/src/hdrhistogram.h b/engines/ep/src/hdrhistogram.h
@@ -0,0 +1,116 @@
+/* -*- Mode: C++; tab-width: 4; c-basic-offset: 4; indent-tabs-mode: nil -*- */
+/*
+ *     Copyright 2018 Couchbase, Inc
+ *
+ *   Licensed 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.
+ */
+
+#pragma once
+
+#include <boost/optional/optional.hpp>
+#include <memory>
+#include <utility>
+
+#include <hdr_histogram.h>
+
+/**
+ * A container for the c hdr_histogram data structure.
+ *
+ */
+class HdrHistogram {
+    // Custom deleter for the hdr_histogram struct.
+    struct HdrDeleter {
+        void operator()(struct hdr_histogram* val);
+    };
+
+    using UniquePtr = std::unique_ptr<struct hdr_histogram, HdrDeleter>;
+
+public:
+#ifdef WIN32
+    /**
+     * The following typedef is required because of an issue msvc2015 where
+     * use of HdrHistogram::Iterator in statwriter.h causes the following
+     * error:
+     * C2079: 'iter' uses undefined struct 'HdrHistogram::hdr_iter'
+     */
+    typedef struct hdr_iter HdrIter;
+    using Iterator = HdrIter;
+#else
+    using Iterator = struct hdr_iter;
+#endif
+
+    /**
+     * Constructor for the histogram.
+     * @param lowestTrackableValue  smallest value that can be held in the
+     *        histogram.  Note the underlying hdr_histogram cannot store 0 and
+     *        therefore the value must be greater than or equal to 1.
+     * @param highestTrackableValue  the largest values that can be held in
+     *        the histogram
+     * @param sigificantFigures  the level of precision for the histogram.
+     *        Note the underlying hdr_histogram requires the value to be
+     *        between 1 and 5 (inclusive).
+     */
+    HdrHistogram(uint64_t lowestTrackableValue,
+                 uint64_t highestTrackableValue,
+                 int significantFigures);
+
+    /**
+     * Adds a value to the histogram.
+     */
+    void addValue(uint64_t v);
+
+    /**
+     * Adds a value and associated count to the histogram.
+     */
+    void addValueAndCount(uint64_t v, uint64_t count);
+
+    /**
+     * Returns the number of values added to the histogram.
+     */
+    uint64_t getValueCount() const;
+
+    /**
+     * Clears the histogram.
+     */
+    void reset();
+
+    /**
+     * Returns the value held in the histogram at the percentile defined by
+     * the input parameter percentage.
+     */
+    uint64_t getValueAtPercentile(double percentage) const;
+
+    /**
+     * Returns a linear iterator for the histogram
+     * @param valueUnitsPerBucket  the number of values to be grouped into
+     *        a single bucket
+     */
+    Iterator makeLinearIterator(int64_t valueUnitsPerBucket) const;
+
+    /**
+     * Gets the next value and corresponding count from the histogram
+     * Returns an optional pair, comprising of:
+     * 1) value
+     * 2) count associated with the value
+     * The pair is optional because iterating past the last value in the
+     * histogram will return no result.
+     */
+    boost::optional<std::pair<uint64_t, uint64_t>> getNextValueAndCount(
+            Iterator& iter) const;
+
+private:
+    /**
+     * unique_ptr to a hdr_histogram structure
+     */
+    UniquePtr histogram;
+};
diff --git a/engines/ep/src/item_eviction.cc b/engines/ep/src/item_eviction.cc
@@ -19,45 +19,32 @@
 #include "item.h"
 
 #include <gsl/gsl>
-#include <limits>
 
-#include <hdr_histogram.h>
-
-ItemEviction::ItemEviction() : requiredToUpdateInterval(learningPopulation) {
-    struct hdr_histogram* hist;
-    hdr_init(minFreqValue,
-             maxFreqValue,
-             3, // Number of significant figures
-             &hist); // Pointer to initialise
-    freqHistogram.reset(hist);
+ItemEviction::ItemEviction() {
 }
 
 void ItemEviction::addValueToFreqHistogram(uint8_t v) {
-    //  A hdr_histogram cannot store 0.  Therefore we add one so the
-    // range moves from 0 -> 255 to 1 -> 256.
-    int64_t vBiased = v + 1;
-    hdr_record_value(freqHistogram.get(), vBiased);
+    freqHistogram.addValue(v);
 }
 
 uint64_t ItemEviction::getFreqHistogramValueCount() const {
-    return freqHistogram->total_count;
+    return freqHistogram.getValueCount();
 }
 
 void ItemEviction::reset() {
-    hdr_reset(freqHistogram.get());
+    freqHistogram.reset();
+    requiredToUpdateInterval = 1;
 }
 
 uint16_t ItemEviction::getFreqThreshold(double percentage) const {
-    // We added one to the input value (see addValueToFreqHistogram).
-    // Therefore need to minus one before returning value from the histogram.
     uint16_t freq = gsl::narrow<uint16_t>(
-            hdr_value_at_percentile(freqHistogram.get(), percentage));
-    return (freq - 1);
+            freqHistogram.getValueAtPercentile(percentage));
+    return freq;
 }
 
-uint8_t ItemEviction::convertFreqCountToNRUValue(uint8_t statCounter) {
+uint8_t ItemEviction::convertFreqCountToNRUValue(uint8_t probCounter) {
     /*
-     * The statstical counter has a range form 0 to 255, however the
+     * The probabilistic counter has a range form 0 to 255, however the
      * increments are not linear - it gets more difficult to increment the
      * counter as its increases value.  Therefore incrementing from 0 to 1 is
      * much easier than incrementing from 254 to 255.
@@ -67,11 +54,11 @@ uint8_t ItemEviction::convertFreqCountToNRUValue(uint8_t statCounter) {
      * in the 4 NRU states.  Therefore we map as follows:
      * 0-3 => 3 (coldest), 4-31 => 2, 32->63 => 1, 64->255 => 0 (hottest),
      */
-    if (statCounter >= 64) {
+    if (probCounter >= 64) {
         return MIN_NRU_VALUE; /* 0 - the hottest */
-    } else if (statCounter >= 32) {
+    } else if (probCounter >= 32) {
         return 1;
-    } else if (statCounter >= 4) {
+    } else if (probCounter >= 4) {
         return INITIAL_NRU_VALUE; /* 2 */
     }
     return MAX_NRU_VALUE; /* 3 - the coldest */
diff --git a/engines/ep/src/item_eviction.h b/engines/ep/src/item_eviction.h
@@ -17,12 +17,10 @@
 
 #pragma once
 
-#include <atomic>
+#include "hdrhistogram.h"
+
 #include <cstdlib> // Required due to the use of free
 #include <limits>
-#include <memory>
-
-#include <hdr_histogram.h>
 
 /**
  * A container for data structures that are used in the algorithm for
@@ -73,7 +71,8 @@ class ItemEviction {
     // Returns the number of values added to the frequency histogram.
     uint64_t getFreqHistogramValueCount() const;
 
-    // Clears the frequency histogram.
+    // Clears the frequency histogram and sets the requiredToUpdateInterval
+    // back to 1.
     void reset();
 
     // StatCounter: Returns the value held in the frequency histogram at the
@@ -111,22 +110,26 @@ class ItemEviction {
     static const uint64_t learningPopulation = 100;
 
 private:
-    // unique_ptr to a hdr_histogram structure, used to record a
-    // histogram of key reference frequencies.  For example, if two keys
-    // are referenced 10 times, whilst three keys are referenced 20 times,
-    // the histogram would contain 2 at the 10 entry and 3 at the 20
-    // entry.
-    HdrHistogramUniquePtr freqHistogram;
-
     //  The minimum value that can be added to the frequency histogram
-    const int64_t minFreqValue = 1; // hdr_histogram cannot take 0
+    static const uint64_t minFreqValue = 0;
+
     // The maximum value that can be added to the frequency histogram
-    // Because we cannot store 0 we have to offset by 1 so we have a maximum
-    // of 256, instead of 255.
-    const int64_t maxFreqValue = std::numeric_limits<uint8_t>::max() + 1;
+    static const uint64_t maxFreqValue = std::numeric_limits<uint8_t>::max();
+
+    // The level of precision for the histogram.  The value must be between 1
+    // and 5 (inclusive).
+    static const int significantFigures = 3;
+
+    // The value units per bucket that we use when creating the iterator
+    // that traverses over the frequency histogram in the copyToHistogram
+    // method.
+    static const int valueUnitsPerBucket = 1;
+
+    // The execution frequency histogram
+    HdrHistogram freqHistogram{minFreqValue, maxFreqValue, significantFigures};
 
     // StatCounter: The number of frequencies that need to be added to the
     // frequency histogram before it is necessary to update the frequency
     // threshold.
-    uint64_t requiredToUpdateInterval;
+    uint64_t requiredToUpdateInterval{1};
 };
diff --git a/engines/ep/tests/module_tests/hdrhistogram_test.cc b/engines/ep/tests/module_tests/hdrhistogram_test.cc
