1 files changed, 47 insertions, 15 deletions

diff --git a/base/metrics/persistent_sample_map.cc b/base/metrics/persistent_sample_map.cc
index 15f83cdb33..51cc0c709d 100644
--- a/base/metrics/persistent_sample_map.cc
+++ b/base/metrics/persistent_sample_map.cc
@@ -6,6 +6,7 @@
 
 #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"
 
@@ -16,6 +17,12 @@ 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.
@@ -82,14 +89,17 @@ void PersistentSampleMapIterator::SkipEmptyBuckets() {
 // memory allocator. The "id" must be unique across all maps held by an
 // allocator or they will get attached to the wrong sample map.
 struct SampleRecord {
+  // SHA1(SampleRecord): Increment this if structure changes!
+  static constexpr uint32_t kPersistentTypeId = 0x8FE6A69F + 1;
+
+  // Expected size for 32/64-bit check.
+  static constexpr size_t kExpectedInstanceSize = 16;
+
   uint64_t id;   // Unique identifier of owner.
   Sample value;  // The value for which this record holds a count.
   Count count;   // The count associated with the above value.
 };
 
-// The type-id used to identify sample records inside an allocator.
-const uint32_t kTypeIdSampleRecord = 0x8FE6A69F + 1;  // SHA1(SampleRecord) v1
-
 }  // namespace
 
 PersistentSampleMap::PersistentSampleMap(
@@ -141,15 +151,12 @@ PersistentMemoryAllocator::Reference
 PersistentSampleMap::GetNextPersistentRecord(
     PersistentMemoryAllocator::Iterator& iterator,
     uint64_t* sample_map_id) {
-  PersistentMemoryAllocator::Reference ref =
-      iterator.GetNextOfType(kTypeIdSampleRecord);
-  const SampleRecord* record =
-      iterator.GetAsObject<SampleRecord>(ref, kTypeIdSampleRecord);
+  const SampleRecord* record = iterator.GetNextOfObject<SampleRecord>();
   if (!record)
     return 0;
 
   *sample_map_id = record->id;
-  return ref;
+  return iterator.GetAsReference(record);
 }
 
 // static
@@ -158,11 +165,7 @@ PersistentSampleMap::CreatePersistentRecord(
     PersistentMemoryAllocator* allocator,
     uint64_t sample_map_id,
     Sample value) {
-  PersistentMemoryAllocator::Reference ref =
-      allocator->Allocate(sizeof(SampleRecord), kTypeIdSampleRecord);
-  SampleRecord* record =
-      allocator->GetAsObject<SampleRecord>(ref, kTypeIdSampleRecord);
-
+  SampleRecord* record = allocator->New<SampleRecord>();
   if (!record) {
     NOTREACHED() << "full=" << allocator->IsFull()
                  << ", corrupt=" << allocator->IsCorrupt();
@@ -172,6 +175,8 @@ PersistentSampleMap::CreatePersistentRecord(
   record->id = sample_map_id;
   record->value = value;
   record->count = 0;
+
+  PersistentMemoryAllocator::Reference ref = allocator->GetAsReference(record);
   allocator->MakeIterable(ref);
   return ref;
 }
@@ -183,11 +188,39 @@ bool PersistentSampleMap::AddSubtractImpl(SampleCountIterator* iter,
   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;
 }
@@ -253,8 +286,7 @@ Count* PersistentSampleMap::ImportSamples(Sample until_value,
   PersistentMemoryAllocator::Reference ref;
   PersistentSampleMapRecords* records = GetRecords();
   while ((ref = records->GetNext()) != 0) {
-    SampleRecord* record =
-        records->GetAsObject<SampleRecord>(ref, kTypeIdSampleRecord);
+    SampleRecord* record = records->GetAsObject<SampleRecord>(ref);
     if (!record)
       continue;