Support saving browser metrics to disk and reading them during next run.

base/metrics/persistent_histogram_allocator.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_histogram_allocator.h"
 
 #include <memory>
 
+#include "base/files/important_file_writer.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram.h"
 #include "base/metrics/histogram_base.h"
 #include "base/metrics/histogram_samples.h"
 #include "base/metrics/persistent_sample_map.h"
 #include "base/metrics/sparse_histogram.h"
 #include "base/metrics/statistics_recorder.h"
 #include "base/synchronization/lock.h"
@@ skipping 20 matching lines @@
   kTypeIdCountsArray = 0x53215530 + 1,  // SHA1(CountsArray) v1
 };
 
 // The current globally-active persistent allocator for all new histograms.
 // The object held here will obviously not be destructed at process exit
 // but that's best since PersistentMemoryAllocator objects (that underlie
 // GlobalHistogramAllocator objects) are explicitly forbidden from doing
 // anything essential at exit anyway due to the fact that they depend on data
 // managed elsewhere and which could be destructed first.
 GlobalHistogramAllocator* g_allocator;
+bool g_allocator_enabled;

[Ilya Sherman, 2016/04/27 20:31:33]

Sorry, I'm going to go back to my previous question about this: Why wouldn't it
make sense to track internally to the GlobalHistogramAllocator object?  You
mentioned that you'd need to plumb it through to the base class, but I don't
actually follow why that would be necessary.

[bcwhite, 2016/05/02 14:46:47]

It would have to be tested by CreateHistogram() which is in the base class and
have it fail if not enabled.  Or the method would have to be overridden here,
which would also mean it should probably be virtual.

Code calling this, however, doesn't expect CreateHistogram to fail unless there
is a problem, such as corrupted or full.  Calling code does, however, expect
Get() to return null if it's not enabled so by testing g_enabled in Get(), all
the operational semantics remain the same.

[Ilya Sherman, 2016/05/05 22:22:52]

g_allocator_enabled is only accessed internally to this file, where every access
could be replaced by an access to g_allocator->enabled_.  What am I missing? 
(And, sorry, I thought I'd replied to this previously, but I guess I missed it.)

[bcwhite, 2016/05/06 16:59:27]

That would work just fine.  The difference would be that the static
Enable/Disable methods would now have to go inside the global g_allocator
singleton to adjust the flag.  The separation has been that static methods
operate on the existence of the singleton but don't go inside it.

Or, those methods would have to become non-static and manipulating them would
then become GetEvenIfDisabled()->Enable()...  which just looks bad to me.

 
 // Take an array of range boundaries and create a proper BucketRanges object
 // which is returned to the caller. A return of nullptr indicates that the
 // passed boundaries are invalid.
 std::unique_ptr<BucketRanges> CreateRangesFromData(
     HistogramBase::Sample* ranges_data,
     uint32_t ranges_checksum,
     size_t count) {
   // To avoid racy destruction at shutdown, the following may be leaked.
   std::unique_ptr<BucketRanges> ranges(new BucketRanges(count));
@@ skipping 585 matching lines @@
     NOTREACHED();
     return;
   }
 
   Set(WrapUnique(new GlobalHistogramAllocator(
       WrapUnique(new SharedPersistentMemoryAllocator(
           std::move(shm), 0, StringPiece(), /*readonly=*/false)))));
 }
 
 // static
+void GlobalHistogramAllocator::Enable() {
+  DCHECK(g_allocator);
+  g_allocator_enabled = true;
+}
+
+// static
+void GlobalHistogramAllocator::Disable() {
+  DCHECK(g_allocator);
+  g_allocator_enabled = false;
+}
+
+// static
 void GlobalHistogramAllocator::Set(
     std::unique_ptr<GlobalHistogramAllocator> allocator) {
   // Releasing or changing an allocator is extremely dangerous because it
   // likely has histograms stored within it. If the backing memory is also
   // also released, future accesses to those histograms will seg-fault.
   CHECK(!g_allocator);
   g_allocator = allocator.release();
+  g_allocator_enabled = true;
   size_t existing = StatisticsRecorder::GetHistogramCount();
 
   DLOG_IF(WARNING, existing)
       << existing << " histograms were created before persistence was enabled.";
 }
 
 // static
 GlobalHistogramAllocator* GlobalHistogramAllocator::Get() {
+  return g_allocator_enabled ? g_allocator : nullptr;
+}
+
+// static
+GlobalHistogramAllocator* GlobalHistogramAllocator::GetEvenIfDisabled() {
   return g_allocator;
 }
 
 // static
 std::unique_ptr<GlobalHistogramAllocator>
 GlobalHistogramAllocator::ReleaseForTesting() {
   GlobalHistogramAllocator* histogram_allocator = g_allocator;
   if (!histogram_allocator)
     return nullptr;
   PersistentMemoryAllocator* memory_allocator =
@@ skipping 18 matching lines @@
     // the method GetCreateHistogramResultHistogram() *before* setting
     // the (temporary) memory allocator via SetGlobalAllocator() so that
     // histogram is instead allocated from the process heap.
     DCHECK_NE(kResultHistogram, histogram_data->name);
   }
 
   g_allocator = nullptr;
   return WrapUnique(histogram_allocator);
 };
 
+void GlobalHistogramAllocator::SetPersistentLocation(const FilePath& location) {
+  persistent_location_ = location;
+}
+
+bool GlobalHistogramAllocator::WriteToPersistentLocation() {
+#if defined(OS_NACL)
+  // NACL doesn't support file operations, including ImportantFileWriter.
+  NOTREACHED();
+  return false;
+#else

[Ilya Sherman, 2016/04/27 20:31:33]

Please DCHECK(g_allocator_enabled) in this code.

[bcwhite, 2016/05/02 14:46:47]

Done.

+  // Stop if no destination is set, perhaps because it has not been enabled.
+  if (persistent_location_.empty()) {
+    NOTREACHED() << "Could not write \"" << Name() << "\" persistent histograms"
+                 << " to file because no location was set.";
+    return false;
+  }
+
+  StringPiece contents(static_cast<const char*>(data()), used());
+  if (!ImportantFileWriter::WriteFileAtomically(persistent_location_,
+                                                contents)) {
+    LOG(ERROR) << "Could not write \"" << Name() << "\" persistent histograms"
+               << " to file: " << persistent_location_.value();
+    return false;
+  }
+
+  return true;
+#endif
+}
+
 GlobalHistogramAllocator::GlobalHistogramAllocator(
     std::unique_ptr<PersistentMemoryAllocator> memory)
     : PersistentHistogramAllocator(std::move(memory)),
       import_iterator_(this) {}
 
 void GlobalHistogramAllocator::ImportHistogramsToStatisticsRecorder() {
   // Skip the import if it's the histogram that was last created. Should a
   // race condition cause the "last created" to be overwritten before it
   // is recognized here then the histogram will be created and be ignored
   // when it is detected as a duplicate by the statistics-recorder. This
   // simple check reduces the time of creating persistent histograms by
   // about 40%.
   Reference record_to_ignore = last_created();
 
   // There is no lock on this because the iterator is lock-free while still
   // guaranteed to only return each entry only once. The StatisticsRecorder
   // has its own lock so the Register operation is safe.
   while (true) {
     std::unique_ptr<HistogramBase> histogram =
         import_iterator_.GetNextWithIgnore(record_to_ignore);
     if (!histogram)
       break;
     StatisticsRecorder::RegisterOrDeleteDuplicate(histogram.release());
   }
 }
 
 }  // namespace base