Index: base/metrics/histogram_samples.cc |
diff --git a/base/metrics/histogram_samples.cc b/base/metrics/histogram_samples.cc |
index 3475cd59f71df439ebd7cd124d4614ccd588b98a..c3b20896ac01a446d6c21ea671a24b8e2d208354 100644 |
--- a/base/metrics/histogram_samples.cc |
+++ b/base/metrics/histogram_samples.cc |
@@ -4,13 +4,26 @@ |
#include "base/metrics/histogram_samples.h" |
+#include <limits> |
+ |
#include "base/compiler_specific.h" |
+#include "base/numerics/safe_math.h" |
#include "base/pickle.h" |
namespace base { |
namespace { |
+// A shorthand constant for the max value of size_t. |
+constexpr size_t kSizeMax = std::numeric_limits<size_t>::max(); |
+ |
+// A constant stored in an AtomicSingleSample (as_atomic) to indicate that the |
+// sample is "disabled" and no further accumulation should be done with it. The |
+// value is chosen such that it will be MAX_UINT16 for both |bucket| & |count|, |
+// and thus less likely to conflict with real use. Conflicts are explicitly |
+// handled in the code but it's worth making them as unlikely as possible. |
+constexpr int32_t kDisabledSingleSample = -1; |
+ |
class SampleCountPickleIterator : public SampleCountIterator { |
public: |
explicit SampleCountPickleIterator(PickleIterator* iter); |
@@ -59,6 +72,93 @@ void SampleCountPickleIterator::Get(HistogramBase::Sample* min, |
} // namespace |
+static_assert(sizeof(HistogramSamples::AtomicSingleSample) == |
+ sizeof(subtle::Atomic32), |
+ "AtomicSingleSample isn't 32 bits"); |
+ |
+HistogramSamples::SingleSample HistogramSamples::AtomicSingleSample::Load() |
+ const { |
+ AtomicSingleSample single_sample = subtle::Acquire_Load(&as_atomic); |
+ |
+ // If the sample was extracted/disabled, it's still zero to the outside. |
+ if (single_sample.as_atomic == kDisabledSingleSample) |
+ single_sample.as_atomic = 0; |
+ |
+ return single_sample.as_parts; |
+} |
+ |
+HistogramSamples::SingleSample HistogramSamples::AtomicSingleSample::Extract( |
+ bool disable) { |
+ AtomicSingleSample single_sample = subtle::NoBarrier_AtomicExchange( |
+ &as_atomic, disable ? kDisabledSingleSample : 0); |
+ if (single_sample.as_atomic == kDisabledSingleSample) |
+ single_sample.as_atomic = 0; |
+ return single_sample.as_parts; |
+} |
+ |
+bool HistogramSamples::AtomicSingleSample::Accumulate( |
+ size_t bucket, |
+ HistogramBase::Count count) { |
+ if (count == 0) |
+ return true; |
+ |
+ // Convert the parameters to 16-bit variables because it's all 16-bit below. |
+ if (count < std::numeric_limits<uint16_t>::min() || |
+ count > std::numeric_limits<uint16_t>::max() || |
+ bucket > std::numeric_limits<uint16_t>::max()) { |
+ return false; |
+ } |
+ uint16_t bucket16 = static_cast<uint16_t>(bucket); |
+ uint16_t count16 = static_cast<uint16_t>(count); |
+ |
+ // A local, unshared copy of the single-sample is necessary so the parts |
+ // can be manipulated without worrying about atomicity. |
+ AtomicSingleSample single_sample; |
+ |
+ subtle::Atomic32 existing; |
+ subtle::Atomic32 found; |
Alexei Svitkine (slow)
2017/04/20 19:56:10
Nit: How about renaming found to updated_value - s
bcwhite
2017/04/20 21:18:07
I think "updated_value" could be construed as the
Alexei Svitkine (slow)
2017/04/20 22:11:34
True, updated_value isn't a great name. How about
bcwhite
2017/04/21 13:25:31
Done. I've also changed the names from existing/f
|
+ do { |
+ existing = subtle::Acquire_Load(&as_atomic); |
+ if (existing == kDisabledSingleSample) |
+ return false; |
+ single_sample.as_atomic = existing; |
+ if (single_sample.as_atomic != 0) { |
+ // Only the same bucket (parameter and stored) can be counted multiple |
+ // times. |
+ if (single_sample.as_parts.bucket != bucket16) |
+ return false; |
+ } else { |
+ // The single sample was zero so becomes the |bucket| parameter, the |
Alexei Svitkine (slow)
2017/04/20 19:56:10
Nit: "single sample" -> "|single_sample|" to make
bcwhite
2017/04/20 21:18:07
Done.
|
+ // contents of which where checked above to fit in 16 bits. |
Alexei Svitkine (slow)
2017/04/20 19:56:10
Nit: where -> were
bcwhite
2017/04/20 21:18:07
Done.
|
+ single_sample.as_parts.bucket = bucket16; |
+ } |
+ |
+ // Update count, making sure that it doesn't overflow. |
+ CheckedNumeric<uint16_t> new_count(single_sample.as_parts.count); |
+ new_count += count16; |
+ if (!new_count.AssignIfValid(&single_sample.as_parts.count)) |
+ return false; |
+ |
+ // Store the updated single-sample back into memory. |
+ if (single_sample.as_atomic == kDisabledSingleSample) |
+ return false; |
+ found = subtle::Release_CompareAndSwap(&as_atomic, existing, |
+ single_sample.as_atomic); |
+ } while (found != existing); |
+ |
+ return true; |
+} |
+ |
+bool HistogramSamples::AtomicSingleSample::IsDisabled() const { |
+ return subtle::Acquire_Load(&as_atomic) == kDisabledSingleSample; |
+} |
+ |
+HistogramSamples::LocalMetadata::LocalMetadata() { |
+ // This is the same way it's done for persistent metadata since no ctor |
+ // is called for the data members in that case. |
+ memset(this, 0, sizeof(*this)); |
+} |
+ |
// Don't try to delegate behavior to the constructor below that accepts a |
// Matadata pointer by passing &local_meta_. Such cannot be reliably passed |
// because it has not yet been constructed -- no member variables have; the |
@@ -83,9 +183,7 @@ HistogramSamples::HistogramSamples(uint64_t id, Metadata* meta) |
HistogramSamples::~HistogramSamples() {} |
void HistogramSamples::Add(const HistogramSamples& other) { |
- IncreaseSum(other.sum()); |
- subtle::NoBarrier_AtomicIncrement(&meta_->redundant_count, |
- other.redundant_count()); |
+ IncreaseSumAndCount(other.sum(), other.redundant_count()); |
bool success = AddSubtractImpl(other.Iterator().get(), ADD); |
DCHECK(success); |
} |
@@ -97,18 +195,14 @@ bool HistogramSamples::AddFromPickle(PickleIterator* iter) { |
if (!iter->ReadInt64(&sum) || !iter->ReadInt(&redundant_count)) |
return false; |
- IncreaseSum(sum); |
- subtle::NoBarrier_AtomicIncrement(&meta_->redundant_count, |
- redundant_count); |
+ IncreaseSumAndCount(sum, redundant_count); |
SampleCountPickleIterator pickle_iter(iter); |
return AddSubtractImpl(&pickle_iter, ADD); |
} |
void HistogramSamples::Subtract(const HistogramSamples& other) { |
- IncreaseSum(-other.sum()); |
- subtle::NoBarrier_AtomicIncrement(&meta_->redundant_count, |
- -other.redundant_count()); |
+ IncreaseSumAndCount(-other.sum(), -other.redundant_count()); |
bool success = AddSubtractImpl(other.Iterator().get(), SUBTRACT); |
DCHECK(success); |
} |
@@ -133,16 +227,25 @@ bool HistogramSamples::Serialize(Pickle* pickle) const { |
return true; |
} |
-void HistogramSamples::IncreaseSum(int64_t diff) { |
+bool HistogramSamples::AccumulateSingleSample(HistogramBase::Sample value, |
+ HistogramBase::Count count, |
+ size_t bucket) { |
+ if (single_sample().Accumulate(bucket, count)) { |
+ // Success. Update the (separate) sum and redundant-count. |
+ IncreaseSumAndCount(static_cast<int64_t>(value) * count, count); |
+ return true; |
+ } |
+ return false; |
+} |
+ |
+void HistogramSamples::IncreaseSumAndCount(int64_t sum, |
+ HistogramBase::Count count) { |
#ifdef ARCH_CPU_64_BITS |
- subtle::NoBarrier_AtomicIncrement(&meta_->sum, diff); |
+ subtle::NoBarrier_AtomicIncrement(&meta_->sum, sum); |
#else |
- meta_->sum += diff; |
+ meta_->sum += sum; |
#endif |
-} |
- |
-void HistogramSamples::IncreaseRedundantCount(HistogramBase::Count diff) { |
- subtle::NoBarrier_AtomicIncrement(&meta_->redundant_count, diff); |
+ subtle::NoBarrier_AtomicIncrement(&meta_->redundant_count, count); |
} |
SampleCountIterator::~SampleCountIterator() {} |
@@ -152,4 +255,46 @@ bool SampleCountIterator::GetBucketIndex(size_t* index) const { |
return false; |
} |
+SingleSampleIterator::SingleSampleIterator(HistogramBase::Sample min, |
+ HistogramBase::Sample max, |
+ HistogramBase::Count count) |
+ : SingleSampleIterator(min, max, count, kSizeMax) {} |
+ |
+SingleSampleIterator::SingleSampleIterator(HistogramBase::Sample min, |
+ HistogramBase::Sample max, |
+ HistogramBase::Count count, |
+ size_t bucket_index) |
+ : min_(min), max_(max), bucket_index_(bucket_index), count_(count) {} |
+ |
+SingleSampleIterator::~SingleSampleIterator() {} |
+ |
+bool SingleSampleIterator::Done() const { |
+ return count_ == 0; |
+} |
+ |
+void SingleSampleIterator::Next() { |
+ DCHECK(!Done()); |
+ count_ = 0; |
+} |
+ |
+void SingleSampleIterator::Get(HistogramBase::Sample* min, |
+ HistogramBase::Sample* max, |
+ HistogramBase::Count* count) const { |
+ DCHECK(!Done()); |
+ if (min != nullptr) |
+ *min = min_; |
+ if (max != nullptr) |
+ *max = max_; |
+ if (count != nullptr) |
+ *count = count_; |
+} |
+ |
+bool SingleSampleIterator::GetBucketIndex(size_t* index) const { |
+ DCHECK(!Done()); |
+ if (bucket_index_ == kSizeMax) |
+ return false; |
+ *index = bucket_index_; |
+ return true; |
+} |
+ |
} // namespace base |