Reorganize histograms for persistence.

base/metrics/histogram_samples.h

Index: base/metrics/histogram_samples.h
diff --git a/base/metrics/histogram_samples.h b/base/metrics/histogram_samples.h
index 54185cf383ba02d25e5e586b2d69a9f822275db2..bbd0e6d24e2e58f05d300f5c0661825f34605b77 100644
--- a/base/metrics/histogram_samples.h
+++ b/base/metrics/histogram_samples.h
@@ -5,6 +5,7 @@
 #ifndef BASE_METRICS_HISTOGRAM_SAMPLES_H_
 #define BASE_METRICS_HISTOGRAM_SAMPLES_H_
 
+#include "base/atomicops.h"
 #include "base/basictypes.h"
 #include "base/metrics/histogram_base.h"
 #include "base/memory/scoped_ptr.h"
@@ -18,7 +19,38 @@ class SampleCountIterator;
 // HistogramSamples is a container storing all samples of a histogram.
 class BASE_EXPORT HistogramSamples {
  public:
-  HistogramSamples();
+  struct Metadata {
+    // Initialized when the sample-set is first created with a value provided
+    // by the caller. It is generally used to identify the sample-set across
+    // threads and processes, though not necessarily uniquely as it is possible
+    // to have multiple sample-sets representing subsets of the data.
+    const uint64 id;
+
+    // The sum of all the entries, effectivly the sum(sample * count) for
+    // all samples. Despite being atomic, no guarantees are made on the
+    // accuracy of this value; there may be races during histogram
+    // accumulation and snapshotting that we choose to accept. It should
+    // be treated as approximate.
+#ifdef ARCH_CPU_64_BITS
+    subtle::Atomic64 sum;
+#else  // No Atomic64 on 32-bit platforms.
+    int64 sum;
+#endif
+
+    // A "redundant" count helps identify memory corruption. It redundantly
+    // stores the total number of samples accumulated in the histogram. We
+    // can compare this count to the sum of the counts (TotalCount() function),
+    // and detect problems. Note, depending on the implementation of different
+    // histogram types, there might be races during histogram accumulation
+    // and snapshotting that we choose to accept. In this case, the tallies
+    // might mismatch even when no memory corruption has happened.
+    HistogramBase::AtomicCount redundant_count;
+
+    Metadata() : id(0), sum(0), redundant_count(0) {}
+  };
+
+  explicit HistogramSamples(uint64 id);

[Alexei Svitkine (slow), 2015/11/25 18:05:27]

Why does HistogramSamples need an id, as opposed to the Histogram object?

The Histogram object is providing backed memory for the samples right? Can it
simply allocate the backed memory, put an id in there and pass in the remainder
of the buffer?

[bcwhite, 2015/11/25 21:54:17]

The id is shared.  Histograms use shared sample-sets but are not themselves
shared.  All sharing happens within the sample-set.
It could, but that would mean the Histogram would have to know about the
internal metadata of the sample set which seems a bad idea.  Better to
explicitly pass in the required value during construction.

+  HistogramSamples(uint64 id, Metadata* meta);
   virtual ~HistogramSamples();
 
   virtual void Accumulate(HistogramBase::Sample value,
@@ -37,9 +69,10 @@ class BASE_EXPORT HistogramSamples {
   virtual bool Serialize(Pickle* pickle) const;
 
   // Accessor fuctions.
-  int64 sum() const { return sum_; }
+  uint64 id() const { return meta_->id; }
+  int64 sum() const { return meta_->sum; }
   HistogramBase::Count redundant_count() const {
-    return subtle::NoBarrier_Load(&redundant_count_);
+    return subtle::NoBarrier_Load(&meta_->redundant_count);
   }
 
  protected:
@@ -51,16 +84,13 @@ class BASE_EXPORT HistogramSamples {
   void IncreaseRedundantCount(HistogramBase::Count diff);
 
  private:
-  int64 sum_;
-
-  // |redundant_count_| helps identify memory corruption. It redundantly stores
-  // the total number of samples accumulated in the histogram. We can compare
-  // this count to the sum of the counts (TotalCount() function), and detect
-  // problems. Note, depending on the implementation of different histogram
-  // types, there might be races during histogram accumulation and snapshotting
-  // that we choose to accept. In this case, the tallies might mismatch even
-  // when no memory corruption has happened.
-  HistogramBase::AtomicCount redundant_count_;
+  void SetMetadataId(uint64 id);
+
+  // In order to support histograms shared through an external memory segment,
+  // meta values may be the local storage or external storage depending on the
+  // wishes of the derived class.

[Alexei Svitkine (slow), 2015/11/25 18:05:27]

HistogramSamples is meant to just be an interface. Can this logic live in
subclasses?

[bcwhite, 2015/11/25 21:54:17]

Yes, but then it has to be repeated.  It's never been just an interface because
metadata like "redundant_count" and "sum" are present here and partially managed
here.

+  Metadata local_meta_;
+  Metadata* meta_;
 };
 
 class BASE_EXPORT SampleCountIterator {