Report reasons for a negative histogram sample count.

base/metrics/persistent_sample_map.cc

 // Copyright 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "base/metrics/persistent_sample_map.h"
 
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/metrics/persistent_histogram_allocator.h"
 #include "base/stl_util.h"
 
 namespace base {
 
 typedef HistogramBase::Count Count;
 typedef HistogramBase::Sample Sample;
 
 namespace {
 
+enum NegativeSampleReason {
+  PERSISTENT_SPARSE_HAVE_LOGGED_BUT_NOT_SAMPLE,
+  PERSISTENT_SPARSE_SAMPLE_LESS_THAN_LOGGED,
+  MAX_NEGATIVE_SAMPLE_REASONS
+};
+
 // An iterator for going through a PersistentSampleMap. The logic here is
 // identical to that of SampleMapIterator but with different data structures.
 // Changes here likely need to be duplicated there.
 class PersistentSampleMapIterator : public SampleCountIterator {
  public:
   typedef std::map<HistogramBase::Sample, HistogramBase::Count*>
       SampleToCountMap;
 
   explicit PersistentSampleMapIterator(const SampleToCountMap& sample_counts);
   ~PersistentSampleMapIterator() override;
@@ skipping 145 matching lines @@
   return ref;
 }
 
 bool PersistentSampleMap::AddSubtractImpl(SampleCountIterator* iter,
                                           Operator op) {
   Sample min;
   Sample max;
   Count count;
   for (; !iter->Done(); iter->Next()) {
     iter->Get(&min, &max, &count);
+    if (count == 0)
+      continue;
     if (min + 1 != max)
       return false;  // SparseHistogram only supports bucket with size 1.
 
+#if 0  // TODO(bcwhite) Re-enable efficient version after crbug.com/682680.
     *GetOrCreateSampleCountStorage(min) +=
         (op == HistogramSamples::ADD) ? count : -count;
+#else
+    if (op == HistogramSamples::ADD) {
+      *GetOrCreateSampleCountStorage(min) += count;
+    } else {
+      // Subtract is used only for determining deltas when reporting which
+      // means that it's in the "logged" iterator. It should have an active
+      // sample record and thus there is no need to try to create one.
+      NegativeSampleReason reason = MAX_NEGATIVE_SAMPLE_REASONS;
+      Count* bucket = GetSampleCountStorage(min);
+      if (bucket == nullptr) {
+        reason = PERSISTENT_SPARSE_HAVE_LOGGED_BUT_NOT_SAMPLE;
+      } else {
+        if (*bucket < count) {
+          reason = PERSISTENT_SPARSE_SAMPLE_LESS_THAN_LOGGED;
+          *bucket = 0;
+        } else {
+          *bucket -= count;
+        }
+      }
+      if (reason != MAX_NEGATIVE_SAMPLE_REASONS) {
+        UMA_HISTOGRAM_ENUMERATION("UMA.NegativeSamples.Reason", reason,
+                                  MAX_NEGATIVE_SAMPLE_REASONS);
+      }
+    }
+#endif
   }
   return true;
 }
 
 Count* PersistentSampleMap::GetSampleCountStorage(Sample value) {
   // If |value| is already in the map, just return that.
   auto it = sample_counts_.find(value);
   if (it != sample_counts_.end())
     return it->second;
 
@@ skipping 78 matching lines @@
         found_count = &record->count;
       if (!import_everything)
         break;
     }
   }
 
   return found_count;
 }
 
 }  // namespace base