Clarify histogram logic.
Document constructor assumptions. More explicitly check them.
Make it easier to see the correctness argument for GrowBuckets().
Avoid using CHECK_ALIGNMENT for something that isn't an address.
Test: TreeHugger
Change-Id: I2256bd400607044a817c25674ab9ade1d3a1f451
diff --git a/libartbase/base/histogram-inl.h b/libartbase/base/histogram-inl.h
index 9832f03..ba2b75a 100644
--- a/libartbase/base/histogram-inl.h
+++ b/libartbase/base/histogram-inl.h
@@ -58,10 +58,12 @@
inline Histogram<Value>::Histogram(const char* name, Value initial_bucket_width,
size_t max_buckets)
: kAdjust(1000),
- kInitialBucketCount(8),
+ kInitialBucketCount(kMinBuckets),
name_(name),
max_buckets_(max_buckets),
bucket_width_(initial_bucket_width) {
+ CHECK_GE(max_buckets, kInitialBucketCount);
+ CHECK_EQ(max_buckets_ % 2, 0u);
Reset();
}
@@ -69,8 +71,9 @@
inline void Histogram<Value>::GrowBuckets(Value new_max) {
while (max_ < new_max) {
// If we have reached the maximum number of buckets, merge buckets together.
- if (frequency_.size() >= max_buckets_) {
- CHECK_ALIGNED(frequency_.size(), 2);
+ DCHECK_LE(frequency_.size(), max_buckets_);
+ if (frequency_.size() == max_buckets_) {
+ DCHECK_EQ(frequency_.size() % 2, 0u);
// We double the width of each bucket to reduce the number of buckets by a factor of 2.
bucket_width_ *= 2;
const size_t limit = frequency_.size() / 2;
diff --git a/libartbase/base/histogram.h b/libartbase/base/histogram.h
index 39a1866..2993fbe 100644
--- a/libartbase/base/histogram.h
+++ b/libartbase/base/histogram.h
@@ -39,10 +39,13 @@
std::vector<double> perc_;
};
+ // Minimum and initial number of allocated buckets in histogram.
+ static constexpr size_t kMinBuckets = 8;
// Used by the cumulative timing logger to search the histogram set using for an existing split
// with the same name using CumulativeLogger::HistogramComparator.
explicit Histogram(const char* name);
- // This is the expected constructor when creating new Histograms.
+ // This is the expected constructor when creating new Histograms. Max_buckets must be even.
+ // Max_buckets, if specified, must be at least kMinBuckets.
Histogram(const char* name, Value initial_bucket_width, size_t max_buckets = 100);
void AddValue(Value);
void AdjustAndAddValue(Value); // Add a value after dividing it by kAdjust.
