Skip to content
Snippets Groups Projects
Commit dd4664af authored by jar@chromium.org's avatar jar@chromium.org
Browse files

Revert 64687 - Try to detect internal corruption of histogram instances.

Corruptions can include changes in bucket boundaries,
large changes in sample counts (in a bucket), etc.

We now detect problems, and don't forward the corrupt
data any further. This means it won't exit the renderer
and go to the browser if corrupt, and it won't exit
the browser and be sent up via UMA if corrupt.

IF the would-be corruption is caused by a race to
snapshot the data, then a later snapshot should get
the clean copy, and all data (across the precluded
period) will be sent onward.

BUG=61281
r=mbelshe
Review URL: http://codereview.chromium.org/4174002

TBR=jar@chromium.org
Review URL: http://codereview.chromium.org/4349002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64846 0039d316-1c4b-4281-b951-d872f2087c98
parent 97310d6e
No related branches found
No related tags found
No related merge requests found
...@@ -61,7 +61,6 @@ Histogram::Histogram(const std::string& name, Sample minimum, ...@@ -61,7 +61,6 @@ Histogram::Histogram(const std::string& name, Sample minimum,
bucket_count_(bucket_count), bucket_count_(bucket_count),
flags_(kNoFlags), flags_(kNoFlags),
ranges_(bucket_count + 1, 0), ranges_(bucket_count + 1, 0),
range_checksum_(0),
sample_() { sample_() {
Initialize(); Initialize();
} }
...@@ -74,7 +73,6 @@ Histogram::Histogram(const std::string& name, TimeDelta minimum, ...@@ -74,7 +73,6 @@ Histogram::Histogram(const std::string& name, TimeDelta minimum,
bucket_count_(bucket_count), bucket_count_(bucket_count),
flags_(kNoFlags), flags_(kNoFlags),
ranges_(bucket_count + 1, 0), ranges_(bucket_count + 1, 0),
range_checksum_(0),
sample_() { sample_() {
Initialize(); Initialize();
} }
...@@ -88,7 +86,6 @@ Histogram::~Histogram() { ...@@ -88,7 +86,6 @@ Histogram::~Histogram() {
// Just to make sure most derived class did this properly... // Just to make sure most derived class did this properly...
DCHECK(ValidateBucketRanges()); DCHECK(ValidateBucketRanges());
DCHECK(HasValidRangeChecksum());
} }
bool Histogram::PrintEmptyBucket(size_t index) const { bool Histogram::PrintEmptyBucket(size_t index) const {
...@@ -221,7 +218,7 @@ void Histogram::Initialize() { ...@@ -221,7 +218,7 @@ void Histogram::Initialize() {
// We have to be careful that we don't pick a ratio between starting points in // We have to be careful that we don't pick a ratio between starting points in
// consecutive buckets that is sooo small, that the integer bounds are the same // consecutive buckets that is sooo small, that the integer bounds are the same
// (effectively making one bucket get no values). We need to avoid: // (effectively making one bucket get no values). We need to avoid:
// ranges_[i] == ranges_[i + 1] // (ranges_[i] == ranges_[i + 1]
// To avoid that, we just do a fine-grained bucket width as far as we need to // To avoid that, we just do a fine-grained bucket width as far as we need to
// until we get a ratio that moves us along at least 2 units at a time. From // until we get a ratio that moves us along at least 2 units at a time. From
// that bucket onward we do use the exponential growth of buckets. // that bucket onward we do use the exponential growth of buckets.
...@@ -247,7 +244,6 @@ void Histogram::InitializeBucketRange() { ...@@ -247,7 +244,6 @@ void Histogram::InitializeBucketRange() {
++current; // Just do a narrow bucket, and keep trying. ++current; // Just do a narrow bucket, and keep trying.
SetBucketRange(bucket_index, current); SetBucketRange(bucket_index, current);
} }
ResetRangeChecksum();
DCHECK_EQ(bucket_count(), bucket_index); DCHECK_EQ(bucket_count(), bucket_index);
} }
...@@ -291,23 +287,6 @@ double Histogram::GetBucketSize(Count current, size_t i) const { ...@@ -291,23 +287,6 @@ double Histogram::GetBucketSize(Count current, size_t i) const {
return current/denominator; return current/denominator;
} }
void Histogram::ResetRangeChecksum() {
range_checksum_ = CalculateRangeChecksum();
}
bool Histogram::HasValidRangeChecksum() const {
return CalculateRangeChecksum() == range_checksum_;
}
Histogram::Sample Histogram::CalculateRangeChecksum() const {
DCHECK_EQ(ranges_.size(), bucket_count() + 1);
Sample checksum = 0;
for (size_t index = 0; index < bucket_count(); ++index) {
checksum += ranges(index);
}
return checksum;
}
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// The following two methods can be overridden to provide a thread safe // The following two methods can be overridden to provide a thread safe
// version of this class. The cost of locking is low... but an error in each // version of this class. The cost of locking is low... but an error in each
...@@ -438,7 +417,6 @@ std::string Histogram::SerializeHistogramInfo(const Histogram& histogram, ...@@ -438,7 +417,6 @@ std::string Histogram::SerializeHistogramInfo(const Histogram& histogram,
pickle.WriteInt(histogram.declared_min()); pickle.WriteInt(histogram.declared_min());
pickle.WriteInt(histogram.declared_max()); pickle.WriteInt(histogram.declared_max());
pickle.WriteSize(histogram.bucket_count()); pickle.WriteSize(histogram.bucket_count());
pickle.WriteInt(histogram.range_checksum());
pickle.WriteInt(histogram.histogram_type()); pickle.WriteInt(histogram.histogram_type());
pickle.WriteInt(histogram.flags()); pickle.WriteInt(histogram.flags());
...@@ -454,21 +432,19 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) { ...@@ -454,21 +432,19 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
Pickle pickle(histogram_info.data(), Pickle pickle(histogram_info.data(),
static_cast<int>(histogram_info.size())); static_cast<int>(histogram_info.size()));
std::string histogram_name; void* iter = NULL;
size_t bucket_count;
int declared_min; int declared_min;
int declared_max; int declared_max;
size_t bucket_count;
int range_checksum;
int histogram_type; int histogram_type;
int pickle_flags; int pickle_flags;
std::string histogram_name;
SampleSet sample; SampleSet sample;
void* iter = NULL;
if (!pickle.ReadString(&iter, &histogram_name) || if (!pickle.ReadString(&iter, &histogram_name) ||
!pickle.ReadInt(&iter, &declared_min) || !pickle.ReadInt(&iter, &declared_min) ||
!pickle.ReadInt(&iter, &declared_max) || !pickle.ReadInt(&iter, &declared_max) ||
!pickle.ReadSize(&iter, &bucket_count) || !pickle.ReadSize(&iter, &bucket_count) ||
!pickle.ReadInt(&iter, &range_checksum) ||
!pickle.ReadInt(&iter, &histogram_type) || !pickle.ReadInt(&iter, &histogram_type) ||
!pickle.ReadInt(&iter, &pickle_flags) || !pickle.ReadInt(&iter, &pickle_flags) ||
!sample.Histogram::SampleSet::Deserialize(&iter, pickle)) { !sample.Histogram::SampleSet::Deserialize(&iter, pickle)) {
...@@ -507,7 +483,6 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) { ...@@ -507,7 +483,6 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
DCHECK_EQ(render_histogram->declared_min(), declared_min); DCHECK_EQ(render_histogram->declared_min(), declared_min);
DCHECK_EQ(render_histogram->declared_max(), declared_max); DCHECK_EQ(render_histogram->declared_max(), declared_max);
DCHECK_EQ(render_histogram->bucket_count(), bucket_count); DCHECK_EQ(render_histogram->bucket_count(), bucket_count);
DCHECK_EQ(render_histogram->range_checksum(), range_checksum);
DCHECK_EQ(render_histogram->histogram_type(), histogram_type); DCHECK_EQ(render_histogram->histogram_type(), histogram_type);
if (render_histogram->flags() & kIPCSerializationSourceFlag) { if (render_histogram->flags() & kIPCSerializationSourceFlag) {
...@@ -521,56 +496,6 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) { ...@@ -521,56 +496,6 @@ bool Histogram::DeserializeHistogramInfo(const std::string& histogram_info) {
return true; return true;
} }
//------------------------------------------------------------------------------
// Methods for the validating a sample and a related histogram.
//------------------------------------------------------------------------------
Histogram::Inconsistencies Histogram::FindCorruption(
const SampleSet& snapshot) const {
int inconsistencies = NO_INCONSISTENCIES;
Sample previous_range = -1; // Bottom range is always 0.
Sample checksum = 0;
int64 count = 0;
for (size_t index = 0; index < bucket_count(); ++index) {
count += snapshot.counts(index);
int new_range = ranges(index);
checksum += new_range;
if (previous_range >= new_range)
inconsistencies |= BUCKET_ORDER_ERROR;
previous_range = new_range;
}
if (checksum != range_checksum_)
inconsistencies |= RANGE_CHECKSUM_ERROR;
int64 delta64 = snapshot.redundant_count() - count;
if (delta64 != 0) {
int delta = static_cast<int>(delta64);
if (delta != delta64)
delta = INT_MAX; // Flag all giant errors as INT_MAX.
// Since snapshots of histograms are taken asynchronously relative to
// sampling (and snapped from different threads), it is pretty likely that
// we'll catch a redundant count that doesn't match the sample count. We
// allow for a certain amount of slop before flagging this as an
// inconsistency. Even with an inconsistency, we'll snapshot it again (for
// UMA in about a half hour, so we'll eventually get the data, if it was
// not the result of a corruption. If histograms show that 1 is "too tight"
// then we may try to use 2 or 3 for this slop value.
const int kCommonRaceBasedCountMismatch = 1;
if (delta > 0) {
UMA_HISTOGRAM_COUNTS("Histogram.InconsistentCountHigh", delta);
if (delta > kCommonRaceBasedCountMismatch)
inconsistencies |= COUNT_HIGH_ERROR;
} else {
DCHECK_GT(0, delta);
UMA_HISTOGRAM_COUNTS("Histogram.InconsistentCountLow", -delta);
if (-delta > kCommonRaceBasedCountMismatch)
inconsistencies |= COUNT_LOW_ERROR;
}
}
return static_cast<Inconsistencies>(inconsistencies);
}
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
// Methods for the Histogram::SampleSet class // Methods for the Histogram::SampleSet class
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
...@@ -578,8 +503,7 @@ Histogram::Inconsistencies Histogram::FindCorruption( ...@@ -578,8 +503,7 @@ Histogram::Inconsistencies Histogram::FindCorruption(
Histogram::SampleSet::SampleSet() Histogram::SampleSet::SampleSet()
: counts_(), : counts_(),
sum_(0), sum_(0),
square_sum_(0), square_sum_(0) {
redundant_count_(0) {
} }
Histogram::SampleSet::~SampleSet() { Histogram::SampleSet::~SampleSet() {
...@@ -600,11 +524,9 @@ void Histogram::SampleSet::Accumulate(Sample value, Count count, ...@@ -600,11 +524,9 @@ void Histogram::SampleSet::Accumulate(Sample value, Count count,
counts_[index] += count; counts_[index] += count;
sum_ += count * value; sum_ += count * value;
square_sum_ += (count * value) * static_cast<int64>(value); square_sum_ += (count * value) * static_cast<int64>(value);
redundant_count_ += count;
DCHECK_GE(counts_[index], 0); DCHECK_GE(counts_[index], 0);
DCHECK_GE(sum_, 0); DCHECK_GE(sum_, 0);
DCHECK_GE(square_sum_, 0); DCHECK_GE(square_sum_, 0);
DCHECK_GE(redundant_count_, 0);
} }
Count Histogram::SampleSet::TotalCount() const { Count Histogram::SampleSet::TotalCount() const {
...@@ -614,7 +536,6 @@ Count Histogram::SampleSet::TotalCount() const { ...@@ -614,7 +536,6 @@ Count Histogram::SampleSet::TotalCount() const {
++it) { ++it) {
total += *it; total += *it;
} }
DCHECK_EQ(total, redundant_count_);
return total; return total;
} }
...@@ -622,7 +543,6 @@ void Histogram::SampleSet::Add(const SampleSet& other) { ...@@ -622,7 +543,6 @@ void Histogram::SampleSet::Add(const SampleSet& other) {
DCHECK_EQ(counts_.size(), other.counts_.size()); DCHECK_EQ(counts_.size(), other.counts_.size());
sum_ += other.sum_; sum_ += other.sum_;
square_sum_ += other.square_sum_; square_sum_ += other.square_sum_;
redundant_count_ += other.redundant_count_;
for (size_t index = 0; index < counts_.size(); ++index) for (size_t index = 0; index < counts_.size(); ++index)
counts_[index] += other.counts_[index]; counts_[index] += other.counts_[index];
} }
...@@ -634,7 +554,6 @@ void Histogram::SampleSet::Subtract(const SampleSet& other) { ...@@ -634,7 +554,6 @@ void Histogram::SampleSet::Subtract(const SampleSet& other) {
// calculated). As a result, we don't currently CHCEK() for positive values. // calculated). As a result, we don't currently CHCEK() for positive values.
sum_ -= other.sum_; sum_ -= other.sum_;
square_sum_ -= other.square_sum_; square_sum_ -= other.square_sum_;
redundant_count_ -= other.redundant_count_;
for (size_t index = 0; index < counts_.size(); ++index) { for (size_t index = 0; index < counts_.size(); ++index) {
counts_[index] -= other.counts_[index]; counts_[index] -= other.counts_[index];
DCHECK_GE(counts_[index], 0); DCHECK_GE(counts_[index], 0);
...@@ -644,7 +563,6 @@ void Histogram::SampleSet::Subtract(const SampleSet& other) { ...@@ -644,7 +563,6 @@ void Histogram::SampleSet::Subtract(const SampleSet& other) {
bool Histogram::SampleSet::Serialize(Pickle* pickle) const { bool Histogram::SampleSet::Serialize(Pickle* pickle) const {
pickle->WriteInt64(sum_); pickle->WriteInt64(sum_);
pickle->WriteInt64(square_sum_); pickle->WriteInt64(square_sum_);
pickle->WriteInt64(redundant_count_);
pickle->WriteSize(counts_.size()); pickle->WriteSize(counts_.size());
for (size_t index = 0; index < counts_.size(); ++index) { for (size_t index = 0; index < counts_.size(); ++index) {
...@@ -658,13 +576,11 @@ bool Histogram::SampleSet::Deserialize(void** iter, const Pickle& pickle) { ...@@ -658,13 +576,11 @@ bool Histogram::SampleSet::Deserialize(void** iter, const Pickle& pickle) {
DCHECK_EQ(counts_.size(), 0u); DCHECK_EQ(counts_.size(), 0u);
DCHECK_EQ(sum_, 0); DCHECK_EQ(sum_, 0);
DCHECK_EQ(square_sum_, 0); DCHECK_EQ(square_sum_, 0);
DCHECK_EQ(redundant_count_, 0);
size_t counts_size; size_t counts_size;
if (!pickle.ReadInt64(iter, &sum_) || if (!pickle.ReadInt64(iter, &sum_) ||
!pickle.ReadInt64(iter, &square_sum_) || !pickle.ReadInt64(iter, &square_sum_) ||
!pickle.ReadInt64(iter, &redundant_count_) ||
!pickle.ReadSize(iter, &counts_size)) { !pickle.ReadSize(iter, &counts_size)) {
return false; return false;
} }
...@@ -672,16 +588,14 @@ bool Histogram::SampleSet::Deserialize(void** iter, const Pickle& pickle) { ...@@ -672,16 +588,14 @@ bool Histogram::SampleSet::Deserialize(void** iter, const Pickle& pickle) {
if (counts_size == 0) if (counts_size == 0)
return false; return false;
int count = 0;
for (size_t index = 0; index < counts_size; ++index) { for (size_t index = 0; index < counts_size; ++index) {
int i; int i;
if (!pickle.ReadInt(iter, &i)) if (!pickle.ReadInt(iter, &i))
return false; return false;
counts_.push_back(i); counts_.push_back(i);
count += i;
} }
DCHECK_EQ(count, redundant_count_);
return count == redundant_count_; return true;
} }
//------------------------------------------------------------------------------ //------------------------------------------------------------------------------
...@@ -780,7 +694,6 @@ void LinearHistogram::InitializeBucketRange() { ...@@ -780,7 +694,6 @@ void LinearHistogram::InitializeBucketRange() {
(bucket_count() - 2); (bucket_count() - 2);
SetBucketRange(i, static_cast<int> (linear_range + 0.5)); SetBucketRange(i, static_cast<int> (linear_range + 0.5));
} }
ResetRangeChecksum();
} }
double LinearHistogram::GetBucketSize(Count current, size_t i) const { double LinearHistogram::GetBucketSize(Count current, size_t i) const {
...@@ -827,7 +740,7 @@ BooleanHistogram::BooleanHistogram(const std::string& name) ...@@ -827,7 +740,7 @@ BooleanHistogram::BooleanHistogram(const std::string& name)
scoped_refptr<Histogram> CustomHistogram::FactoryGet( scoped_refptr<Histogram> CustomHistogram::FactoryGet(
const std::string& name, const std::string& name,
const std::vector<Sample>& custom_ranges, const std::vector<int>& custom_ranges,
Flags flags) { Flags flags) {
scoped_refptr<Histogram> histogram(NULL); scoped_refptr<Histogram> histogram(NULL);
...@@ -861,7 +774,7 @@ Histogram::ClassType CustomHistogram::histogram_type() const { ...@@ -861,7 +774,7 @@ Histogram::ClassType CustomHistogram::histogram_type() const {
} }
CustomHistogram::CustomHistogram(const std::string& name, CustomHistogram::CustomHistogram(const std::string& name,
const std::vector<Sample>& custom_ranges) const std::vector<int>& custom_ranges)
: Histogram(name, custom_ranges[1], custom_ranges.back(), : Histogram(name, custom_ranges[1], custom_ranges.back(),
custom_ranges.size()) { custom_ranges.size()) {
DCHECK_GT(custom_ranges.size(), 1u); DCHECK_GT(custom_ranges.size(), 1u);
...@@ -876,7 +789,6 @@ void CustomHistogram::InitializeBucketRange() { ...@@ -876,7 +789,6 @@ void CustomHistogram::InitializeBucketRange() {
DCHECK_LE(ranges_vector_->size(), bucket_count()); DCHECK_LE(ranges_vector_->size(), bucket_count());
for (size_t index = 0; index < ranges_vector_->size(); ++index) for (size_t index = 0; index < ranges_vector_->size(); ++index)
SetBucketRange(index, (*ranges_vector_)[index]); SetBucketRange(index, (*ranges_vector_)[index]);
ResetRangeChecksum();
} }
double CustomHistogram::GetBucketSize(Count current, size_t i) const { double CustomHistogram::GetBucketSize(Count current, size_t i) const {
......
...@@ -36,7 +36,6 @@ ...@@ -36,7 +36,6 @@
#include <string> #include <string>
#include <vector> #include <vector>
#include "base/gtest_prod_util.h"
#include "base/ref_counted.h" #include "base/ref_counted.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/time.h" #include "base/time.h"
...@@ -244,8 +243,8 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -244,8 +243,8 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
typedef std::vector<Count> Counts; typedef std::vector<Count> Counts;
typedef std::vector<Sample> Ranges; typedef std::vector<Sample> Ranges;
// These enums are used to facilitate deserialization of renderer histograms /* These enums are meant to facilitate deserialization of renderer histograms
// into the browser. into the browser. */
enum ClassType { enum ClassType {
HISTOGRAM, HISTOGRAM,
LINEAR_HISTOGRAM, LINEAR_HISTOGRAM,
...@@ -274,16 +273,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -274,16 +273,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
kHexRangePrintingFlag = 0x8000, // Fancy bucket-naming supported. kHexRangePrintingFlag = 0x8000, // Fancy bucket-naming supported.
}; };
enum Inconsistencies {
NO_INCONSISTENCIES = 0x0,
RANGE_CHECKSUM_ERROR = 0x1,
BUCKET_ORDER_ERROR = 0x2,
COUNT_HIGH_ERROR = 0x4,
COUNT_LOW_ERROR = 0x8,
NEVER_EXCEEDED_VALUE = 0x10
};
struct DescriptionPair { struct DescriptionPair {
Sample sample; Sample sample;
const char* description; // Null means end of a list of pairs. const char* description; // Null means end of a list of pairs.
...@@ -309,7 +298,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -309,7 +298,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
Count TotalCount() const; Count TotalCount() const;
int64 sum() const { return sum_; } int64 sum() const { return sum_; }
int64 square_sum() const { return square_sum_; } int64 square_sum() const { return square_sum_; }
int64 redundant_count() const { return redundant_count_; }
// Arithmetic manipulation of corresponding elements of the set. // Arithmetic manipulation of corresponding elements of the set.
void Add(const SampleSet& other); void Add(const SampleSet& other);
...@@ -327,21 +315,7 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -327,21 +315,7 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
// without shared memory at some point. // without shared memory at some point.
int64 sum_; // sum of samples. int64 sum_; // sum of samples.
int64 square_sum_; // sum of squares of samples. int64 square_sum_; // sum of squares of samples.
private:
// Allow tests to corrupt our innards for testing purposes.
FRIEND_TEST(HistogramTest, CorruptSampleCounts);
// To help identify memory corruption, we reduntantly save the number of
// samples we've accumulated into all of our buckets. We can compare this
// count to the sum of the counts in all buckets, and detect problems. Note
// that due to races in histogram accumulation (if a histogram is indeed
// updated on several threads simultaneously), the tallies might mismatch,
// and also the snapshotting code may asynchronously get a mismatch (though
// generally either race based mismatch cause is VERY rare).
int64 redundant_count_;
}; };
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
// minimum should start from 1. 0 is invalid as a minimum. 0 is an implicit // minimum should start from 1. 0 is invalid as a minimum. 0 is an implicit
// default underflow bucket. // default underflow bucket.
...@@ -393,13 +367,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -393,13 +367,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
// browser process. // browser process.
static bool DeserializeHistogramInfo(const std::string& histogram_info); static bool DeserializeHistogramInfo(const std::string& histogram_info);
// Check to see if bucket ranges, counts and tallies in the snapshot are
// consistent with the bucket ranges and checksums in our histogram. This can
// produce a false-alarm if a race occurred in the reading of the data during
// a SnapShot process, but should otherwise be false at all times (unless we
// have memory over-writes, or DRAM failures).
Inconsistencies FindCorruption(const SampleSet& snapshot) const;
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
// Accessors for factory constuction, serialization and testing. // Accessors for factory constuction, serialization and testing.
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
...@@ -408,7 +375,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -408,7 +375,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
Sample declared_min() const { return declared_min_; } Sample declared_min() const { return declared_min_; }
Sample declared_max() const { return declared_max_; } Sample declared_max() const { return declared_max_; }
virtual Sample ranges(size_t i) const { return ranges_[i];} virtual Sample ranges(size_t i) const { return ranges_[i];}
Sample range_checksum() const { return range_checksum_; }
virtual size_t bucket_count() const { return bucket_count_; } virtual size_t bucket_count() const { return bucket_count_; }
// Snapshot the current complete set of sample data. // Snapshot the current complete set of sample data.
// Override with atomic/locked snapshot if needed. // Override with atomic/locked snapshot if needed.
...@@ -443,9 +409,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -443,9 +409,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
// Get normalized size, relative to the ranges_[i]. // Get normalized size, relative to the ranges_[i].
virtual double GetBucketSize(Count current, size_t i) const; virtual double GetBucketSize(Count current, size_t i) const;
// Recalculate range_checksum_.
void ResetRangeChecksum();
// Return a string description of what goes in a given bucket. // Return a string description of what goes in a given bucket.
// Most commonly this is the numeric value, but in derived classes it may // Most commonly this is the numeric value, but in derived classes it may
// be a name (or string description) given to the bucket. // be a name (or string description) given to the bucket.
...@@ -467,18 +430,9 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -467,18 +430,9 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
bool ValidateBucketRanges() const; bool ValidateBucketRanges() const;
private: private:
// Allow tests to corrupt our innards for testing purposes.
FRIEND_TEST(HistogramTest, CorruptBucketBounds);
FRIEND_TEST(HistogramTest, CorruptSampleCounts);
// Post constructor initialization. // Post constructor initialization.
void Initialize(); void Initialize();
// Return true iff the range_checksum_ matches current ranges_ vector.
bool HasValidRangeChecksum() const;
Sample CalculateRangeChecksum() const;
//---------------------------------------------------------------------------- //----------------------------------------------------------------------------
// Helpers for emitting Ascii graphic. Each method appends data to output. // Helpers for emitting Ascii graphic. Each method appends data to output.
...@@ -523,11 +477,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> { ...@@ -523,11 +477,6 @@ class Histogram : public base::RefCountedThreadSafe<Histogram> {
// The dimension of ranges_ is bucket_count + 1. // The dimension of ranges_ is bucket_count + 1.
Ranges ranges_; Ranges ranges_;
// For redundancy, we store the sum of all the sample ranges when ranges are
// generated. If ever there is ever a difference, then the histogram must
// have been corrupted.
Sample range_checksum_;
// Finally, provide the state that changes with the addition of each new // Finally, provide the state that changes with the addition of each new
// sample. // sample.
SampleSet sample_; SampleSet sample_;
...@@ -612,11 +561,11 @@ class CustomHistogram : public Histogram { ...@@ -612,11 +561,11 @@ class CustomHistogram : public Histogram {
virtual ClassType histogram_type() const; virtual ClassType histogram_type() const;
static scoped_refptr<Histogram> FactoryGet(const std::string& name, static scoped_refptr<Histogram> FactoryGet(const std::string& name,
const std::vector<Sample>& custom_ranges, Flags flags); const std::vector<int>& custom_ranges, Flags flags);
protected: protected:
CustomHistogram(const std::string& name, CustomHistogram(const std::string& name,
const std::vector<Sample>& custom_ranges); const std::vector<int>& custom_ranges);
// Initialize ranges_ mapping. // Initialize ranges_ mapping.
virtual void InitializeBucketRange(); virtual void InitializeBucketRange();
...@@ -624,7 +573,7 @@ class CustomHistogram : public Histogram { ...@@ -624,7 +573,7 @@ class CustomHistogram : public Histogram {
private: private:
// Temporary pointer used during construction/initialization, and then NULLed. // Temporary pointer used during construction/initialization, and then NULLed.
const std::vector<Sample>* ranges_vector_; const std::vector<int>* ranges_vector_;
DISALLOW_COPY_AND_ASSIGN(CustomHistogram); DISALLOW_COPY_AND_ASSIGN(CustomHistogram);
}; };
......
...@@ -308,57 +308,4 @@ TEST(HistogramTest, BucketPlacementTest) { ...@@ -308,57 +308,4 @@ TEST(HistogramTest, BucketPlacementTest) {
} }
} // namespace } // namespace
//------------------------------------------------------------------------------
// We can't be an an anonymous namespace while being friends, so we pop back
// out to the base namespace here. We need to be friends to corrupt the
// internals of the histogram and/or sampleset.
TEST(HistogramTest, CorruptSampleCounts) {
scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
"Histogram", 1, 64, 8, Histogram::kNoFlags); // As per header file.
EXPECT_EQ(0, histogram->sample_.redundant_count());
histogram->Add(20); // Add some samples.
histogram->Add(40);
EXPECT_EQ(2, histogram->sample_.redundant_count());
Histogram::SampleSet snapshot;
histogram->SnapshotSample(&snapshot);
EXPECT_EQ(Histogram::NO_INCONSISTENCIES, 0);
EXPECT_EQ(0, histogram->FindCorruption(snapshot)); // No default corruption.
EXPECT_EQ(2, snapshot.redundant_count());
snapshot.counts_[3] += 100; // Sample count won't match redundant count.
EXPECT_EQ(Histogram::COUNT_LOW_ERROR, histogram->FindCorruption(snapshot));
snapshot.counts_[2] -= 200;
EXPECT_EQ(Histogram::COUNT_HIGH_ERROR, histogram->FindCorruption(snapshot));
// But we can't spot a corruption if it is compensated for.
snapshot.counts_[1] += 100;
EXPECT_EQ(0, histogram->FindCorruption(snapshot));
}
TEST(HistogramTest, CorruptBucketBounds) {
scoped_refptr<Histogram> histogram = Histogram::FactoryGet(
"Histogram", 1, 64, 8, Histogram::kNoFlags); // As per header file.
Histogram::SampleSet snapshot;
histogram->SnapshotSample(&snapshot);
EXPECT_EQ(Histogram::NO_INCONSISTENCIES, 0);
EXPECT_EQ(0, histogram->FindCorruption(snapshot)); // No default corruption.
std::swap(histogram->ranges_[1], histogram->ranges_[2]);
EXPECT_EQ(Histogram::BUCKET_ORDER_ERROR, histogram->FindCorruption(snapshot));
std::swap(histogram->ranges_[1], histogram->ranges_[2]);
EXPECT_EQ(0, histogram->FindCorruption(snapshot));
++histogram->ranges_[3];
EXPECT_EQ(Histogram::RANGE_CHECKSUM_ERROR,
histogram->FindCorruption(snapshot));
// Repair histogram so that destructor won't DCHECK().
--histogram->ranges_[3];
}
} // namespace base } // namespace base
...@@ -484,24 +484,8 @@ void MetricsServiceBase::RecordHistogram(const Histogram& histogram) { ...@@ -484,24 +484,8 @@ void MetricsServiceBase::RecordHistogram(const Histogram& histogram) {
// Get up-to-date snapshot of sample stats. // Get up-to-date snapshot of sample stats.
Histogram::SampleSet snapshot; Histogram::SampleSet snapshot;
histogram.SnapshotSample(&snapshot); histogram.SnapshotSample(&snapshot);
const std::string& histogram_name = histogram.histogram_name();
int corruption = histogram.FindCorruption(snapshot); const std::string& histogram_name = histogram.histogram_name();
if (corruption) {
NOTREACHED();
// Don't send corrupt data to metrics survices.
UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowser",
corruption, Histogram::NEVER_EXCEEDED_VALUE);
typedef std::map<std::string, int> ProblemMap;
static ProblemMap* inconsistencies = new ProblemMap;
int old_corruption = (*inconsistencies)[histogram_name];
if (old_corruption == (corruption | old_corruption))
return; // We've already seen this corruption for this histogram.
(*inconsistencies)[histogram_name] |= corruption;
UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesBrowserUnique",
corruption, Histogram::NEVER_EXCEEDED_VALUE);
return;
}
// Find the already sent stats, or create an empty set. // Find the already sent stats, or create an empty set.
LoggedSampleMap::iterator it = logged_samples_.find(histogram_name); LoggedSampleMap::iterator it = logged_samples_.find(histogram_name);
......
...@@ -52,33 +52,17 @@ void RendererHistogramSnapshots::UploadAllHistrograms(int sequence_number) { ...@@ -52,33 +52,17 @@ void RendererHistogramSnapshots::UploadAllHistrograms(int sequence_number) {
sequence_number, pickled_histograms)); sequence_number, pickled_histograms));
} }
// Extract snapshot data, remember what we've seen so far, and then send off the // Extract snapshot data and then send it off the the Browser process
// delta to the browser. // to save it.
void RendererHistogramSnapshots::UploadHistrogram( void RendererHistogramSnapshots::UploadHistrogram(
const Histogram& histogram, const Histogram& histogram,
HistogramPickledList* pickled_histograms) { HistogramPickledList* pickled_histograms) {
// Get up-to-date snapshot of sample stats. // Get up-to-date snapshot of sample stats.
Histogram::SampleSet snapshot; Histogram::SampleSet snapshot;
histogram.SnapshotSample(&snapshot); histogram.SnapshotSample(&snapshot);
const std::string& histogram_name = histogram.histogram_name(); const std::string& histogram_name = histogram.histogram_name();
int corruption = histogram.FindCorruption(snapshot);
if (corruption) {
NOTREACHED();
// Don't send corrupt data to the browser.
UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRenderer",
corruption, Histogram::NEVER_EXCEEDED_VALUE);
typedef std::map<std::string, int> ProblemMap;
static ProblemMap* inconsistencies = new ProblemMap;
int old_corruption = (*inconsistencies)[histogram_name];
if (old_corruption == (corruption | old_corruption))
return; // We've already seen this corruption for this histogram.
(*inconsistencies)[histogram_name] |= corruption;
UMA_HISTOGRAM_ENUMERATION("Histogram.InconsistenciesRendererUnique",
corruption, Histogram::NEVER_EXCEEDED_VALUE);
return;
}
// Find the already sent stats, or create an empty set. // Find the already sent stats, or create an empty set.
LoggedSampleMap::iterator it = logged_samples_.find(histogram_name); LoggedSampleMap::iterator it = logged_samples_.find(histogram_name);
Histogram::SampleSet* already_logged; Histogram::SampleSet* already_logged;
...@@ -105,6 +89,7 @@ void RendererHistogramSnapshots::UploadHistogramDelta( ...@@ -105,6 +89,7 @@ void RendererHistogramSnapshots::UploadHistogramDelta(
const Histogram& histogram, const Histogram& histogram,
const Histogram::SampleSet& snapshot, const Histogram::SampleSet& snapshot,
HistogramPickledList* pickled_histograms) { HistogramPickledList* pickled_histograms) {
DCHECK(0 != snapshot.TotalCount()); DCHECK(0 != snapshot.TotalCount());
snapshot.CheckSize(histogram); snapshot.CheckSize(histogram);
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment