Merge multiple histogram snapshots into single one for reporting.

base/metrics/histogram_snapshot_manager.cc

Index: base/metrics/histogram_snapshot_manager.cc
diff --git a/base/metrics/histogram_snapshot_manager.cc b/base/metrics/histogram_snapshot_manager.cc
index ef1ab26284b4e20852acfe7a278446af7943fdea..ccfdc105802936f5cb4fce2e41587bc3e5d8aa7d 100644
--- a/base/metrics/histogram_snapshot_manager.cc
+++ b/base/metrics/histogram_snapshot_manager.cc
@@ -10,6 +10,10 @@
 #include "base/metrics/statistics_recorder.h"
 #include "base/stl_util.h"
 
+namespace {
+const int kNewInconsistency = (int)0x80000000;
+}  // namespace
+
 namespace base {
 
 HistogramSnapshotManager::HistogramSnapshotManager(
@@ -19,22 +23,41 @@ HistogramSnapshotManager::HistogramSnapshotManager(
 }
 
 HistogramSnapshotManager::~HistogramSnapshotManager() {
-  STLDeleteValues(&logged_samples_);
 }
 
-void HistogramSnapshotManager::PrepareDelta(const HistogramBase& histogram) {
+void HistogramSnapshotManager::StartDeltas() {
+#ifdef DEBUG
+  for (HashInfoMap::iterator iter = known_histograms_.begin();
+       iter != known_histograms_.end();
+       ++iter) {
+    CHECK(!iter->second.histogram);
+    CHECK(!iter->second.accumulated);
+    CHECK(!(iter->second.inconsistencies & kNewInconsistency));
+  }
+#endif
+}
+
+void HistogramSnapshotManager::PrepareDelta(const HistogramBase* histogram) {
   DCHECK(histogram_flattener_);
 
   // Get up-to-date snapshot of sample stats.
-  scoped_ptr<HistogramSamples> snapshot(histogram.SnapshotSamples());
-  const std::string& histogram_name = histogram.histogram_name();
-  const uint64_t histogram_hash = histogram.name_hash();
-
-  int corruption = histogram.FindCorruption(*snapshot);
+  scoped_ptr<HistogramSamples> snapshot(histogram->SnapshotSamples());
+
+  // Get information known about this histogram.
+  const std::string& histogram_name = histogram->histogram_name();
+  SampleInfo& sample_info = known_histograms_[histogram->name_hash()];

[Alexei Svitkine (slow), 2015/12/04 18:30:11]

Nit: Make this a pointer, since non-const refs are discouraged.

[bcwhite, 2015/12/08 17:32:18]

Done.

+  if (sample_info.histogram) {
+    DCHECK_EQ(sample_info.histogram->histogram_name(), histogram_name)
+        << "hash collision";
+  } else {
+    // First time this histogram has been seen; datafill.
+    sample_info.histogram = histogram;
+  }
 
   // Crash if we detect that our histograms have been overwritten.  This may be
   // a fair distance from the memory smasher, but we hope to correlate these
   // crashes with other events, such as plugins, or usage patterns, etc.
+  int corruption = histogram->FindCorruption(*snapshot);
   if (HistogramBase::BUCKET_ORDER_ERROR & corruption) {
     // The checksum should have caught this, so crash separately if it didn't.
     CHECK_NE(0, HistogramBase::RANGE_CHECKSUM_ERROR & corruption);
@@ -47,38 +70,75 @@ void HistogramSnapshotManager::PrepareDelta(const HistogramBase& histogram) {
   // COUNT_LOW_ERROR and they never arise together, so we don't need to extract
   // bits from corruption.
   if (corruption) {
-    DLOG(ERROR) << "Histogram: " << histogram_name
-                << " has data corruption: " << corruption;
+    DLOG(ERROR) << "Histogram \"" << histogram_name
+                << "\" has data corruption: " << corruption;
     histogram_flattener_->InconsistencyDetected(
         static_cast<HistogramBase::Inconsistency>(corruption));
     // Don't record corrupt data to metrics services.
-    int old_corruption = inconsistencies_[histogram_hash];
+    const int old_corruption = sample_info.inconsistencies;
     if (old_corruption == (corruption | old_corruption))
       return;  // We've already seen this corruption for this histogram.
-    inconsistencies_[histogram_hash] |= corruption;
-    histogram_flattener_->UniqueInconsistencyDetected(
-        static_cast<HistogramBase::Inconsistency>(corruption));
+    sample_info.inconsistencies |= corruption | kNewInconsistency;
+    // TODO(bcwhite): Can we clear the inconsistency for future collection?
     return;
   }
 
-  HistogramSamples* to_log;
-  std::map<uint64_t, HistogramSamples*>::iterator it =
-      logged_samples_.find(histogram_hash);
-  if (it == logged_samples_.end()) {
-    to_log = snapshot.release();
-
-    // This histogram has not been logged before, add a new entry.
-    logged_samples_[histogram_hash] = to_log;
+  if (!sample_info.accumulated) {
+    // This histogram has not been seen before; add a new entry.
+    sample_info.accumulated = snapshot.release();
   } else {
-    HistogramSamples* already_logged = it->second;
-    InspectLoggedSamplesInconsistency(*snapshot, already_logged);
-    snapshot->Subtract(*already_logged);
-    already_logged->Add(*snapshot);
-    to_log = snapshot.get();
+    // There are previous values from this histogram; add them together.
+    sample_info.accumulated->Add(*snapshot);
   }
+}
 
-  if (to_log->TotalCount() > 0)
-    histogram_flattener_->RecordDelta(histogram, *to_log);
+void HistogramSnapshotManager::FinishDeltas() {
+  // Iterate over all known histograms to see what should be recorded.
+  for (HashInfoMap::iterator iter = known_histograms_.begin();
+       iter != known_histograms_.end();
+       ++iter) {

[Alexei Svitkine (slow), 2015/12/04 18:30:11]

Nit: Can this use C++11 for syntax?

[bcwhite, 2015/12/08 17:32:18]

Done.

+    SampleInfo& sample_info = iter->second;
+
+    // First, record any histograms in which corruption was detected.
+    if (sample_info.inconsistencies & kNewInconsistency) {
+      sample_info.inconsistencies &= ~kNewInconsistency;
+      histogram_flattener_->UniqueInconsistencyDetected(
+          static_cast<HistogramBase::Inconsistency>(
+              sample_info.inconsistencies));
+    }
+
+    // Second, record actual sample data.
+    if (sample_info.accumulated) {
+      if (sample_info.logged) {
+        // Subtract the previous report from this one and report that. Since
+        // the existing sum will be required for the next reporting cycle,
+        // perform subtraction in-place with the previous values (to be
+        // released later).  LOGGED = CURRENT - LOGGED = -LOGGED + CURRENT
+        InspectLoggedSamplesInconsistency(*sample_info.accumulated,
+                                          sample_info.logged.get());
+        sample_info.logged->Negate();
+        sample_info.logged->Add(*sample_info.accumulated);
+      } else {
+        // This histogram has no previous report; log as-is.
+        sample_info.logged.reset(sample_info.accumulated);
+        sample_info.accumulated = nullptr;
+      }
+
+      if (sample_info.logged->TotalCount() > 0) {
+        histogram_flattener_->RecordDelta(*sample_info.histogram,
+                                          *sample_info.logged.get());
+      }
+
+      if (sample_info.accumulated) {
+        sample_info.logged.reset(sample_info.accumulated);
+        sample_info.accumulated = nullptr;
+      }
+    } else if (sample_info.logged) {
+      DLOG(ERROR) << "Histogram \"" << sample_info.histogram->histogram_name()
+                  << "\" was lost since previous report";
+      sample_info.logged.reset();
+    }
+  }
 }
 
 void HistogramSnapshotManager::InspectLoggedSamplesInconsistency(