Updated negative-counts histogram for changes in logged/unlogged accounting.

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/numerics/safe_conversions.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,
   PERSISTENT_SPARSE_ADDED_NEGATIVE_COUNT,
   PERSISTENT_SPARSE_ADD_WENT_NEGATIVE,
   PERSISTENT_SPARSE_ADD_OVERFLOW,
+  PERSISTENT_SPARSE_ACCUMULATE_NEGATIVE_COUNT,
+  PERSISTENT_SPARSE_ACCUMULATE_WENT_NEGATIVE,
+  PERSISTENT_SPARSE_ACCUMULATE_OVERFLOW,
   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;
@@ skipping 74 matching lines @@
     PersistentHistogramAllocator* allocator,
     Metadata* meta)
     : HistogramSamples(id, meta), allocator_(allocator) {}
 
 PersistentSampleMap::~PersistentSampleMap() {
   if (records_)
     records_->Release(this);
 }
 
 void PersistentSampleMap::Accumulate(Sample value, Count count) {
+#if 0  // TODO(bcwhite) Re-enable efficient version after crbug.com/682680.
   *GetOrCreateSampleCountStorage(value) += count;
+#else
+  NegativeSampleReason reason = MAX_NEGATIVE_SAMPLE_REASONS;
+  Count* local_count_ptr = GetOrCreateSampleCountStorage(value);
+  if (count < 0) {
+    reason = PERSISTENT_SPARSE_ACCUMULATE_NEGATIVE_COUNT;
+    if (*local_count_ptr < -count)
+      reason = PERSISTENT_SPARSE_ACCUMULATE_WENT_NEGATIVE;
+    *local_count_ptr += count;
+  } else {
+    *local_count_ptr += count;
+    if (*local_count_ptr < 0)
+      reason = PERSISTENT_SPARSE_ACCUMULATE_OVERFLOW;
+  }
+  if (reason != MAX_NEGATIVE_SAMPLE_REASONS) {
+    UMA_HISTOGRAM_ENUMERATION("UMA.NegativeSamples.Reason", reason,
+                              MAX_NEGATIVE_SAMPLE_REASONS);
+  }
+#endif
   IncreaseSumAndCount(strict_cast<int64_t>(count) * value, count);
 }
 
 Count PersistentSampleMap::GetCount(Sample value) const {
   // Have to override "const" to make sure all samples have been loaded before
   // being able to know what value to return.
   Count* count_pointer =
       const_cast<PersistentSampleMap*>(this)->GetSampleCountStorage(value);
   return count_pointer ? *count_pointer : 0;
 }
@@ skipping 56 matching lines @@
                                           Operator op) {
   Sample min;
   int64_t max;
   Count count;
   for (; !iter->Done(); iter->Next()) {
     iter->Get(&min, &max, &count);
     if (count == 0)
       continue;
     if (strict_cast<int64_t>(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
-    NegativeSampleReason reason = MAX_NEGATIVE_SAMPLE_REASONS;
-    if (op == HistogramSamples::ADD) {
-      // Add should generally be adding only positive values.
-      Count* local_count_ptr = GetOrCreateSampleCountStorage(min);
-      if (count < 0) {
-        reason = PERSISTENT_SPARSE_ADDED_NEGATIVE_COUNT;
-        if (*local_count_ptr < -count) {
-          reason = PERSISTENT_SPARSE_ADD_WENT_NEGATIVE;
-          *local_count_ptr = 0;
-        }
-      } else {
-        *local_count_ptr += count;
-        if (*local_count_ptr < 0)
-          reason = PERSISTENT_SPARSE_ADD_OVERFLOW;
-      }
-    } 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.
-      Count* local_count_ptr = GetSampleCountStorage(min);
-      if (local_count_ptr == nullptr) {
-        reason = PERSISTENT_SPARSE_HAVE_LOGGED_BUT_NOT_SAMPLE;
-      } else {
-        if (*local_count_ptr < count) {
-          reason = PERSISTENT_SPARSE_SAMPLE_LESS_THAN_LOGGED;
-          *local_count_ptr = 0;
-        } else {
-          *local_count_ptr -= count;
-        }
-      }
-    }
-    if (reason != MAX_NEGATIVE_SAMPLE_REASONS) {
-      NOTREACHED();
-      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