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;
+GlobalHistogramAllocator* g_allocator_disabled;
 
 // 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() {
+  if (g_allocator) {
+    // Already enabled.
+    DCHECK(!g_allocator_disabled);
+    return;
+  }
+
+  DCHECK(g_allocator_disabled);
+  std::swap(g_allocator, g_allocator_disabled);
+}
+
+// static
+void GlobalHistogramAllocator::Disable() {
+  if (g_allocator_disabled) {
+    // Already disabled.
+    DCHECK(!g_allocator);
+    return;
+  }
+
+  DCHECK(g_allocator);
+  std::swap(g_allocator, g_allocator_disabled);
+}
+
+// static
+bool GlobalHistogramAllocator::IsEnabled() {
+  return g_allocator != nullptr;
+}
+
+// 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);
+  CHECK(!g_allocator && !g_allocator_disabled);
   g_allocator = allocator.release();
   size_t existing = StatisticsRecorder::GetHistogramCount();
 
   DLOG_IF(WARNING, existing)
       << existing << " histograms were created before persistence was enabled.";
 }
 
 // static
 GlobalHistogramAllocator* GlobalHistogramAllocator::Get() {
   return g_allocator;
 }
 
 // static
+GlobalHistogramAllocator* GlobalHistogramAllocator::GetEvenIfDisabled() {
+  return g_allocator ? g_allocator : g_allocator_disabled;
+}
+
+// static
 std::unique_ptr<GlobalHistogramAllocator>
 GlobalHistogramAllocator::ReleaseForTesting() {
-  GlobalHistogramAllocator* histogram_allocator = g_allocator;
+  GlobalHistogramAllocator* histogram_allocator = GetEvenIfDisabled();
   if (!histogram_allocator)
     return nullptr;
   PersistentMemoryAllocator* memory_allocator =
       histogram_allocator->memory_allocator();
 
   // Before releasing the memory, it's necessary to have the Statistics-
   // Recorder forget about the histograms contained therein; otherwise,
   // some operations will try to access them and the released memory.
   PersistentMemoryAllocator::Iterator iter(memory_allocator);
   PersistentMemoryAllocator::Reference ref;
   while ((ref = iter.GetNextOfType(kTypeIdHistogram)) != 0) {
     PersistentHistogramData* histogram_data =
         memory_allocator->GetAsObject<PersistentHistogramData>(
             ref, kTypeIdHistogram);
     DCHECK(histogram_data);
     StatisticsRecorder::ForgetHistogramForTesting(histogram_data->name);
 
     // If a test breaks here then a memory region containing a histogram
     // actively used by this code is being released back to the test.
     // If that memory segment were to be deleted, future calls to create
     // persistent histograms would crash. To avoid this, have the test call
     // 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);
   }
 
+  DCHECK_NE(g_allocator != nullptr, g_allocator_disabled != nullptr);
   g_allocator = nullptr;
+  g_allocator_disabled = 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
+  // Stop if no destination is set, perhaps because it has not been enabled.

[Ilya Sherman, 2016/04/25 19:48:40]

I was kind of expecting that you'd check the enabled state explicitly, and
return early (sans warning) in just that case.

[bcwhite, 2016/04/25 20:37:17]

"Enabled" has nothing to do with being able to save the state.  It could have
been enabled and then disabled.  If it was never enabled then the amount of data
to save will be very small.

[Ilya Sherman, 2016/04/25 21:12:11]

Why would we enable and then subsequently disable it?  The only reason we even
have enable/disable state both as viable possibilities is that we're not
shipping to 100% of users yet.  But that's a one-time decision, and should only
be able to flip in a single direction.

[bcwhite, 2016/04/26 13:32:30]

The PHA is also used in setup.exe and in subprocesses.  In those cases, they
always start enabled and remain that way.  The many tests that use the PHA also
work that way.  To start disabled would to go against the standard use-case.

Supporting enabled/disabled is a hack to deal with the limitations of not being
able to determine "should I" or "shouldn't I" at the moment when the decision
needs to be made.

When the experiment is removed, I expect enable/disable will also be removed. 
Perhaps Activate/Deactivate would have been a better choice in names.  It's
always operational; it's just not always hooked in.

[Ilya Sherman, 2016/04/26 20:01:24]

Okay, I see your point about this code being used both by Chrome and setup.exe. 
Thanks for explaining that to me.

+  if (persistent_location_.empty()) {
+    LOG(WARNING) << "Could not write \"" << Name() << "\" persistent histograms"
+                 << " to file because no location was set.";

[Ilya Sherman, 2016/04/25 19:48:40]

I still think this should be an error rather than a warning.  What is a code
path that could result in WriteToPersistentLocation() being called without a
persistent location being set, and why is it desirable for that code path to
exist?

[bcwhite, 2016/04/25 20:37:17]

Unlike below, where there is a specific local failure, there is no "error" here.
 This condition is dealing, safely, with how an external caller is using this
module.  This module shouldn't be judging if something outside is wrong.

[Ilya Sherman, 2016/04/25 21:12:11]

If I'm understanding you correctly, then I disagree: IMO this module absolutely
should document its invariants, and fail ungracefully if those invariants are
not satisfied.  I would much rather find broken invariants due to failing
DCHECKs and crash reports rather than by eventually noticing that we seem to
sometimes be dropping data on the floor.

[bcwhite, 2016/04/26 13:32:30]

I can't say I agree but I understand your point.  Done.

+    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