Index: base/metrics/histogram_samples.h |
diff --git a/base/metrics/histogram_samples.h b/base/metrics/histogram_samples.h |
index 93f6d21c8aabd34eed61dd9c20d2e030cabada8f..9c9716f08c53ef212bfab7c7bdb5a18c4b586bff 100644 |
--- a/base/metrics/histogram_samples.h |
+++ b/base/metrics/histogram_samples.h |
@@ -24,8 +24,55 @@ class SampleCountIterator; |
// elements must be of a fixed width to ensure 32/64-bit interoperability. |
// If this structure changes, bump the version number for kTypeIdHistogram |
// in persistent_histogram_allocator.cc. |
+// |
+// Note that though these samples are individually consistent (through the use |
+// of atomic operations on the counts), there is only "eventual consistency" |
+// overall when multiple threads are accessing this data. That means that the |
+// sum, redundant-count, etc. could be momentarily out-of-sync with the stored |
+// counts but will settle to a consistent "steady state" once all threads have |
+// exited this code. |
class BASE_EXPORT HistogramSamples { |
public: |
+ // A single value and count. To fit within a single atomic on 32-bit build |
+ // architectures, both |value| and |count| are reduced in size to 16 bits. |
+ // This limits the functionality somewhat but if an entry can't fit then |
+ // the full array of samples can be allocated and used. |
+ struct SingleSample { |
+ int16_t value; |
Alexei Svitkine (slow)
2017/04/18 20:52:20
uint16_t for both?
Or is there a valid case for t
bcwhite
2017/04/19 18:13:02
It was significantly simpler this way because the
bcwhite
2017/04/19 20:10:22
Another option is to change it from "value" to "bu
|
+ int16_t count; |
+ }; |
+ |
+ // A structure for managing an atomic single sample. Because this is generally |
+ // used in association with other atomic values, the defined methods use |
+ // acquire/release operations to guarantee ordering with outside values. |
+ union AtomicSingleSample { |
+ AtomicSingleSample() : as_atomic(0) {} |
+ AtomicSingleSample(subtle::Atomic32 rhs) : as_atomic(rhs) {} |
+ |
+ // Returns the single value in an atomic manner. This in an "acquire" |
+ // load. The returned value isn't shared and thus its parts can be safely |
+ // accessed. |
+ SingleSample Load() const; |
+ |
+ // Extracts the single value in an atomic manner. If |disable| is true then |
+ // the single-sample will be set so it can never accumulate another value. |
+ // This is "no barrier" so doesn't enforce ordering with other atomic ops. |
+ SingleSample Extract(bool disable); |
+ |
+ // Adds a given count to the held value. If not possible, it returns false |
+ // and leaves the internal value unchanged. This in an "acquire/release" |
+ // operation. Once extracted/disabled, this always returns false. |
+ bool Accumulate(HistogramBase::Sample value, HistogramBase::Count count); |
+ |
+ private: |
+ // The actual sample value and count. |
+ SingleSample as_parts; |
+ |
+ // The sample as an atomic value. Atomic64 would provide more flexibility |
+ // but isn't available on all builds. |
Alexei Svitkine (slow)
2017/04/18 20:52:20
Please expand these field comments to mention that
bcwhite
2017/04/19 18:13:02
Done.
|
+ subtle::Atomic32 as_atomic; |
+ }; |
+ |
struct Metadata { |
// Expected size for 32/64-bit check. |
static constexpr size_t kExpectedInstanceSize = 24; |
@@ -58,21 +105,17 @@ class BASE_EXPORT HistogramSamples { |
// might mismatch even when no memory corruption has happened. |
HistogramBase::AtomicCount redundant_count; |
- // 4 bytes of padding to explicitly extend this structure to a multiple of |
- // 64-bits. This is required to ensure the structure is the same size on |
- // both 32-bit and 64-bit builds. |
- char padding[4]; |
+ // A single histogram value and associated count. This allows histograms |
+ // that typically report only a single value to not require full storage |
+ // to be allocated. |
+ AtomicSingleSample single_sample; // 32 bits |
}; |
// Because sturctures held in persistent memory must be POD, there can be no |
Alexei Svitkine (slow)
2017/04/18 20:52:20
Nit: structures is misspelled
bcwhite
2017/04/19 18:13:02
Done.
|
// default constructor to clear the fields. This derived class exists just |
// to clear them when being allocated on the heap. |
- struct LocalMetadata : Metadata { |
- LocalMetadata() { |
- id = 0; |
- sum = 0; |
- redundant_count = 0; |
- } |
+ struct BASE_EXPORT LocalMetadata : Metadata { |
+ LocalMetadata(); |
}; |
explicit HistogramSamples(uint64_t id); |
@@ -112,8 +155,15 @@ class BASE_EXPORT HistogramSamples { |
enum Operator { ADD, SUBTRACT }; |
virtual bool AddSubtractImpl(SampleCountIterator* iter, Operator op) = 0; |
- void IncreaseSum(int64_t diff); |
- void IncreaseRedundantCount(HistogramBase::Count diff); |
+ bool AccumulateSingleSample(HistogramBase::Sample value, |
+ HistogramBase::Count count); |
+ |
+ void IncreaseSumAndCount(int64_t sum, HistogramBase::Count count); |
+ |
+ AtomicSingleSample& single_sample() { return meta_->single_sample; } |
+ const AtomicSingleSample& single_sample() const { |
+ return meta_->single_sample; |
+ } |
private: |
// In order to support histograms shared through an external memory segment, |
@@ -145,6 +195,35 @@ class BASE_EXPORT SampleCountIterator { |
virtual bool GetBucketIndex(size_t* index) const; |
}; |
+class BASE_EXPORT SingleSampleIterator : public SampleCountIterator { |
+ public: |
+ SingleSampleIterator(HistogramBase::Sample min, |
+ HistogramBase::Sample max, |
+ HistogramBase::Count count); |
+ SingleSampleIterator(HistogramBase::Sample min, |
+ HistogramBase::Sample max, |
+ HistogramBase::Count count, |
+ size_t bucket_index); |
+ ~SingleSampleIterator() override; |
+ |
+ // SampleCountIterator implementation: |
Alexei Svitkine (slow)
2017/04/18 20:52:20
Nit: Just // SampleCounterIterator:
(For new code
bcwhite
2017/04/19 18:13:02
Done.
|
+ bool Done() const override; |
+ void Next() override; |
+ void Get(HistogramBase::Sample* min, |
+ HistogramBase::Sample* max, |
+ HistogramBase::Count* count) const override; |
+ |
+ // SampleVector uses predefined buckets, so iterator can return bucket index. |
+ bool GetBucketIndex(size_t* index) const override; |
+ |
+ private: |
+ // Information about the single value to return. |
+ const HistogramBase::Sample min_; |
+ const HistogramBase::Sample max_; |
+ const size_t bucket_index_; |
+ HistogramBase::Count count_; |
+}; |
+ |
} // namespace base |
#endif // BASE_METRICS_HISTOGRAM_SAMPLES_H_ |