summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorjar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-10-17 06:05:31 +0000
committerjar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2009-10-17 06:05:31 +0000
commit705bd12a48fd8608219cebf41f1fa0099b7d0c08 (patch)
tree68da221372282ae627d14c1080a79af8eadfd02e
parent3f55e8712f88d8477d9e58f68958e83c92664389 (diff)
downloadchromium_src-705bd12a48fd8608219cebf41f1fa0099b7d0c08.zip
chromium_src-705bd12a48fd8608219cebf41f1fa0099b7d0c08.tar.gz
chromium_src-705bd12a48fd8608219cebf41f1fa0099b7d0c08.tar.bz2
Remove special code fro calculating bucket index in linear histograms
IMO, the original code was a premature optimization, and worse yet, it had a subtle off by 1 bug due to floating point rounding (which was identified by ERoman.... Thanks!!). This CL removes the special code from linear histograms, and relies on the "standard" binary search approach to locate the correct bucket when a new sample is processed. r=eroman BUG=24160 Review URL: http://codereview.chromium.org/273065 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@29371 0039d316-1c4b-4281-b951-d872f2087c98
-rw-r--r--base/histogram.cc27
-rw-r--r--base/histogram.h2
2 files changed, 8 insertions, 21 deletions
diff --git a/base/histogram.cc b/base/histogram.cc
index f50571d..3d2cd9e 100644
--- a/base/histogram.cc
+++ b/base/histogram.cc
@@ -401,11 +401,10 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
declared_max,
bucket_count);
} else if (histogram_type == LINEAR) {
- render_histogram = reinterpret_cast<Histogram*>
- (new LinearHistogram(histogram_name.c_str(),
- declared_min,
- declared_max,
- bucket_count));
+ render_histogram = new LinearHistogram(histogram_name.c_str(),
+ declared_min,
+ declared_max,
+ bucket_count);
} else {
LOG(ERROR) << "Error Deserializing Histogram Unknown histogram_type: " <<
histogram_type;
@@ -584,17 +583,6 @@ void LinearHistogram::InitializeBucketRange() {
}
}
-// Find bucket to increment for sample value.
-size_t LinearHistogram::BucketIndex(Sample value) const {
- if (value < declared_min()) return 0;
- if (value >= declared_max()) return bucket_count() - 1;
- size_t index;
- index = static_cast<size_t>(((value - declared_min()) * (bucket_count() - 2))
- / (declared_max() - declared_min()) + 1);
- DCHECK(1 <= index && bucket_count() > index);
- return index;
-}
-
double LinearHistogram::GetBucketSize(Count current, size_t i) const {
DCHECK(ranges(i + 1) > ranges(i));
// Adjacent buckets with different widths would have "surprisingly" many (few)
@@ -673,9 +661,10 @@ bool StatisticsRecorder::Register(Histogram* histogram) {
const std::string name = histogram->histogram_name();
AutoLock auto_lock(*lock_);
- DCHECK(histograms_->end() == histograms_->find(name)) << name << " is already"
- "registered as a histogram. Check for duplicate use of the name, or a "
- "race where a static initializer could be run by several threads.";
+ if (histograms_->end() != histograms_->find(name)) {
+ // Check to be sure it is compatible.... and if not, then do a CHECK()
+ return false; // This name is already registered.
+ }
(*histograms_)[name] = histogram;
return true;
}
diff --git a/base/histogram.h b/base/histogram.h
index 1a2b443..4d40c1b 100644
--- a/base/histogram.h
+++ b/base/histogram.h
@@ -466,8 +466,6 @@ class LinearHistogram : public Histogram {
protected:
// Initialize ranges_ mapping.
virtual void InitializeBucketRange();
- // Find bucket to increment for sample value.
- virtual size_t BucketIndex(Sample value) const;
virtual double GetBucketSize(Count current, size_t i) const;
// If we have a description for a bucket, then return that. Otherwise