De-duplicate ranges information in persistent memory.

base/metrics/persistent_histogram_allocator.cc

Index: base/metrics/persistent_histogram_allocator.cc
diff --git a/base/metrics/persistent_histogram_allocator.cc b/base/metrics/persistent_histogram_allocator.cc
index 5f44b67311c57e9f9d08d8c4674a2860be8985bc..40c66becf59e46a2d0af3225d1da6956e91159ea 100644
--- a/base/metrics/persistent_histogram_allocator.cc
+++ b/base/metrics/persistent_histogram_allocator.cc
@@ -340,24 +340,49 @@ std::unique_ptr<HistogramBase> PersistentHistogramAllocator::AllocateHistogram(
       return nullptr;
     }
 
-    size_t ranges_count = bucket_count + 1;
-    size_t ranges_bytes = ranges_count * sizeof(HistogramBase::Sample);
+    // Since the StasticsRecorder keeps a global collection of BucketRanges
+    // objects for re-use, it would be dangerous for one to hold a reference
+    // from a persistent allocator that is not the global one (which is
+    // permanent once set). If this stops being the case, this check can
+    // become an "if" condition beside "ranges_ref == 0" below and before
+    // set_persistent_reference() farther down.
+    DCHECK_EQ(this, GlobalHistogramAllocator::Get());
+
+    // Re-use an existing BucketRanges persistent allocation if one is known;
+    // otherwise, create one.
+    PersistentMemoryAllocator::Reference ranges_ref =
+        bucket_ranges->persistent_reference();
+    if (ranges_ref == 0) {
+      size_t ranges_count = bucket_count + 1;
+      size_t ranges_bytes = ranges_count * sizeof(HistogramBase::Sample);
+      ranges_ref =
+          memory_allocator_->Allocate(ranges_bytes, kTypeIdRangesArray);
+      if (ranges_ref) {
+        HistogramBase::Sample* ranges_data =
+            memory_allocator_->GetAsArray<HistogramBase::Sample>(
+                ranges_ref, kTypeIdRangesArray, ranges_count);
+        if (ranges_data) {
+          for (size_t i = 0; i < bucket_ranges->size(); ++i)
+            ranges_data[i] = bucket_ranges->range(i);
+          bucket_ranges->set_persistent_reference(ranges_ref);
+        } else {
+          // This should never happen but be tolerant if it does.
+          NOTREACHED();
+          ranges_ref = PersistentMemoryAllocator::kReferenceNull;

[Alexei Svitkine (slow), 2017/04/05 20:57:51]

Nit: = 0

Since the rest of your code uses 0 / checking in an if.

[bcwhite, 2017/04/06 16:15:34]

I think it should be the other.  References are generally treated as opaque
values so "if (ref)" or "if (!ref)" is the norm.  I've probably cheated in a few
places and used zero; I'll have to keep a lookout for that.

+        }
+      }
+    } else {
+      DCHECK_EQ(kTypeIdRangesArray, memory_allocator_->GetType(ranges_ref));
+    }
+
     PersistentMemoryAllocator::Reference counts_ref =
         memory_allocator_->Allocate(counts_bytes, kTypeIdCountsArray);
-    PersistentMemoryAllocator::Reference ranges_ref =
-        memory_allocator_->Allocate(ranges_bytes, kTypeIdRangesArray);
-    HistogramBase::Sample* ranges_data =
-        memory_allocator_->GetAsArray<HistogramBase::Sample>(
-            ranges_ref, kTypeIdRangesArray, ranges_count);
 
     // Only continue here if all allocations were successful. If they weren't,
     // there is no way to free the space but that's not really a problem since
     // the allocations only fail because the space is full or corrupt and so
     // any future attempts will also fail.
-    if (counts_ref && ranges_data && histogram_data) {
-      for (size_t i = 0; i < bucket_ranges->size(); ++i)
-        ranges_data[i] = bucket_ranges->range(i);
-
+    if (counts_ref && ranges_ref && histogram_data) {
       histogram_data->minimum = minimum;
       histogram_data->maximum = maximum;
       // |bucket_count| must fit within 32-bits or the allocation of the counts