diff options
author | jar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-17 06:05:31 +0000 |
---|---|---|
committer | jar@chromium.org <jar@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2009-10-17 06:05:31 +0000 |
commit | 705bd12a48fd8608219cebf41f1fa0099b7d0c08 (patch) | |
tree | 68da221372282ae627d14c1080a79af8eadfd02e /base | |
parent | 3f55e8712f88d8477d9e58f68958e83c92664389 (diff) | |
download | chromium_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
Diffstat (limited to 'base')
-rw-r--r-- | base/histogram.cc | 27 | ||||
-rw-r--r-- | base/histogram.h | 2 |
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 |