Fixes for timing logger and histogram.
Reorder when the index is checked in Percentile to avoid reading from a
negative index.
Add missing copyright to histogram unit test.
Switch histogram test to use UniquePtr rather than new/delete (MeanTest had
missed the delete).
Make timing/cumulative logger fields that can be const const. Document SetName
for cumulative logger. Place the cumulative logger's std::string lock name in
the cumulative logger so its lifespan is clearly the same.
Change-Id: I4056c6b6ee8efdb73f7b10f690fc9d959fd4a569
diff --git a/src/base/histogram-inl.h b/src/base/histogram-inl.h
index 9e3de9f..9514209 100644
--- a/src/base/histogram-inl.h
+++ b/src/base/histogram-inl.h
@@ -218,8 +218,8 @@
break;
}
- if (per >= cumulative_perc_[idx] &&
- cumulative_perc_[idx] != cumulative_perc_[idx - 1] && idx != 0) {
+ if (per >= cumulative_perc_[idx] && idx != 0 &&
+ cumulative_perc_[idx] != cumulative_perc_[idx - 1]) {
lower_idx = idx;
}
}
diff --git a/src/base/histogram_test.cc b/src/base/histogram_test.cc
index 7a6c235..ea3e35f 100644
--- a/src/base/histogram_test.cc
+++ b/src/base/histogram_test.cc
@@ -1,6 +1,25 @@
+/*
+ * Copyright (C) 2013 The Android Open Source Project
+ *
+ * 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 "gtest/gtest.h"
#include "histogram-inl.h"
+#include "UniquePtr.h"
+
#include <sstream>
+
using namespace art;
//Simple usage:
@@ -15,8 +34,8 @@
// PerValue = hist->PercentileVal(0.50); finds the 50th percentile(median).
TEST(Histtest, MeanTest) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("MeanTest"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("MeanTest");
double mean;
for (size_t Idx = 0; Idx < 90; Idx++) {
hist->AddValue(static_cast<uint64_t>(50));
@@ -33,8 +52,8 @@
}
TEST(Histtest, VarianceTest) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("VarianceTest"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("VarianceTest");
double variance;
hist->AddValue(9);
hist->AddValue(17);
@@ -43,12 +62,11 @@
hist->CreateHistogram();
variance = hist->Variance();
EXPECT_EQ(64.25, variance);
- delete hist;
}
TEST(Histtest, Percentile) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("Percentile"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("Percentile");
double PerValue;
hist->AddValue(20);
@@ -70,13 +88,11 @@
hist->CreateHistogram();
PerValue = hist->Percentile(0.50);
EXPECT_EQ(875, static_cast<int>(PerValue * 10));
-
- delete hist;
}
TEST(Histtest, UpdateRange) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("UpdateRange"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("UpdateRange");
double PerValue;
hist->AddValue(15);
@@ -112,14 +128,12 @@
EXPECT_EQ(expected, stream.str());
EXPECT_GE(PerValue, 132);
EXPECT_LE(PerValue, 145);
-
- delete hist;
}
;
TEST(Histtest, Reset) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("Reset"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("Reset");
double PerValue;
hist->AddValue(0);
hist->AddValue(189);
@@ -158,14 +172,12 @@
EXPECT_EQ(expected, stream.str());
EXPECT_GE(PerValue, 132);
EXPECT_LE(PerValue, 145);
-
- delete hist;
}
;
TEST(Histtest, MultipleCreateHist) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("MultipleCreateHist"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("MultipleCreateHist");
double PerValue;
hist->AddValue(15);
hist->AddValue(17);
@@ -200,27 +212,24 @@
EXPECT_EQ(expected, stream.str());
EXPECT_GE(PerValue, 132);
EXPECT_LE(PerValue, 145);
-
- delete hist;
}
TEST(Histtest, SingleValue) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("SingleValue"));
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("SingleValue");
hist->AddValue(1);
hist->CreateHistogram();
std::stringstream stream;
std::string expected = "SingleValue:\t99% C.I. 1us-1us Avg: 1us Max: 1us\n";
hist->PrintConfidenceIntervals(stream, 0.99);
EXPECT_EQ(expected, stream.str());
- delete hist;
}
TEST(Histtest, CappingPercentiles) {
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("CappingPercentiles"));
double per_995;
double per_005;
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("CappingPercentiles");
// All values are similar.
for (uint64_t idx = 0ull; idx < 150ull; idx++) {
hist->AddValue(0);
@@ -239,12 +248,10 @@
per_995 = hist->Percentile(0.995);
EXPECT_EQ(1, per_005);
EXPECT_EQ(4, per_995);
- delete hist;
}
TEST(Histtest, SpikyValues) {
-
- Histogram<uint64_t> *hist = new Histogram<uint64_t>("SpikyValues");
+ UniquePtr<Histogram<uint64_t> > hist(new Histogram<uint64_t>("SpikyValues"));
for (uint64_t idx = 0ull; idx < 30ull; idx++) {
for (uint64_t idx_inner = 0ull; idx_inner < 5ull; idx_inner++) {
@@ -258,6 +265,4 @@
"SpikyValues:\t99% C.I. 0.089us-2541.825us Avg: 95.033us Max: 10000us\n";
hist->PrintConfidenceIntervals(stream, 0.99);
EXPECT_EQ(expected, stream.str());
-
- delete hist;
}
diff --git a/src/base/timing_logger.cc b/src/base/timing_logger.cc
index d2c7eef..6d5586c 100644
--- a/src/base/timing_logger.cc
+++ b/src/base/timing_logger.cc
@@ -70,15 +70,20 @@
os << name_ << ": end, " << NsToMs(GetTotalNs()) << " ms\n";
}
-CumulativeLogger::CumulativeLogger(const std::string &name)
+CumulativeLogger::CumulativeLogger(const std::string& name)
: name_(name),
- lock_(("CumulativeLoggerLock" + name).c_str(), kDefaultMutexLevel, true) {
+ lock_name_("CumulativeLoggerLock" + name),
+ lock_(lock_name_.c_str(), kDefaultMutexLevel, true) {
Reset();
}
-CumulativeLogger::~CumulativeLogger() { STLDeleteElements(&histograms_); }
+CumulativeLogger::~CumulativeLogger() {
+ STLDeleteElements(&histograms_);
+}
-void CumulativeLogger::SetName(const std::string &name) { name_ = name; }
+void CumulativeLogger::SetName(const std::string& name) {
+ name_ = name;
+}
void CumulativeLogger::Start() {
MutexLock mu(Thread::Current(), lock_);
diff --git a/src/base/timing_logger.h b/src/base/timing_logger.h
index d3f7ef4..bbcc286 100644
--- a/src/base/timing_logger.h
+++ b/src/base/timing_logger.h
@@ -30,15 +30,15 @@
class TimingLogger {
public:
- explicit TimingLogger(const std::string &name, bool precise);
- void AddSplit(const std::string &label);
- void Dump(std::ostream &os) const;
+ explicit TimingLogger(const std::string& name, bool precise);
+ void AddSplit(const std::string& label);
+ void Dump(std::ostream& os) const;
void Reset();
uint64_t GetTotalNs() const;
protected:
- std::string name_;
- bool precise_;
+ const std::string name_;
+ const bool precise_;
std::vector<uint64_t> times_;
std::vector<std::string> labels_;
@@ -49,16 +49,18 @@
public:
- explicit CumulativeLogger(const std::string &name);
+ explicit CumulativeLogger(const std::string& name);
void prepare_stats();
~CumulativeLogger();
void Start();
void End();
void Reset();
- void Dump(std::ostream &os) LOCKS_EXCLUDED(lock_);
+ void Dump(std::ostream& os) LOCKS_EXCLUDED(lock_);
uint64_t GetTotalNs() const;
- void SetName(const std::string &name);
- void AddLogger(const TimingLogger &logger) LOCKS_EXCLUDED(lock_);
+ // Allow the name to be modified, particularly when the cumulative logger is a field within a
+ // parent class that is unable to determine the "name" of a sub-class.
+ void SetName(const std::string& name);
+ void AddLogger(const TimingLogger& logger) LOCKS_EXCLUDED(lock_);
private:
@@ -69,6 +71,7 @@
static const uint64_t kAdjust = 1000;
std::vector<Histogram<uint64_t> *> histograms_ GUARDED_BY(lock_);
std::string name_;
+ const std::string lock_name_;
mutable Mutex lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
size_t index_ GUARDED_BY(lock_);
size_t iterations_ GUARDED_BY(lock_);