Commit 2a27d6c6 authored by bcwhite's avatar bcwhite Committed by Commit bot

Support experiment to store persistent metrics in memory-mapped file.

Metrics have to be persisted to disk and one possibility
is to simply use file-backed memory which leaves the
responsibility to the OS. It is simple and reliable but
there are concerns about what impact this could have on
performance.

Enabling this as an experiment will allow direct
collection of real data as to how Chrome is affected
by it and thus make possible an informed decision about
whether to use it or not.

BUG=546019

Review-Url: https://codereview.chromium.org/2010173005
Cr-Commit-Position: refs/heads/master@{#403368}
parent 6a675c85
...@@ -812,6 +812,10 @@ void GlobalHistogramAllocator::SetPersistentLocation(const FilePath& location) { ...@@ -812,6 +812,10 @@ void GlobalHistogramAllocator::SetPersistentLocation(const FilePath& location) {
persistent_location_ = location; persistent_location_ = location;
} }
const FilePath& GlobalHistogramAllocator::GetPersistentLocation() const {
return persistent_location_;
}
bool GlobalHistogramAllocator::WriteToPersistentLocation() { bool GlobalHistogramAllocator::WriteToPersistentLocation() {
#if defined(OS_NACL) #if defined(OS_NACL)
// NACL doesn't support file operations, including ImportantFileWriter. // NACL doesn't support file operations, including ImportantFileWriter.
......
...@@ -439,6 +439,10 @@ class BASE_EXPORT GlobalHistogramAllocator ...@@ -439,6 +439,10 @@ class BASE_EXPORT GlobalHistogramAllocator
// in order to persist the data for a later use. // in order to persist the data for a later use.
void SetPersistentLocation(const FilePath& location); void SetPersistentLocation(const FilePath& location);
// Retrieves a previously set pathname to which the contents of this allocator
// are to be saved.
const FilePath& GetPersistentLocation() const;
// Writes the internal data to a previously set location. This is generally // Writes the internal data to a previously set location. This is generally
// called when a process is exiting from a section of code that may not know // called when a process is exiting from a section of code that may not know
// the filesystem. The data is written in an atomic manner. The return value // the filesystem. The data is written in an atomic manner. The return value
......
...@@ -8,16 +8,20 @@ ...@@ -8,16 +8,20 @@
#include "base/command_line.h" #include "base/command_line.h"
#include "base/feature_list.h" #include "base/feature_list.h"
#include "base/files/file_util.h"
#include "base/metrics/field_trial.h" #include "base/metrics/field_trial.h"
#include "base/metrics/histogram_base.h" #include "base/metrics/histogram_base.h"
#include "base/metrics/persistent_histogram_allocator.h" #include "base/metrics/persistent_histogram_allocator.h"
#include "base/path_service.h"
#include "base/strings/string_util.h" #include "base/strings/string_util.h"
#include "base/time/time.h" #include "base/time/time.h"
#include "build/build_config.h" #include "build/build_config.h"
#include "chrome/browser/metrics/chrome_metrics_service_client.h" #include "chrome/browser/metrics/chrome_metrics_service_client.h"
#include "chrome/browser/tracing/background_tracing_field_trial.h" #include "chrome/browser/tracing/background_tracing_field_trial.h"
#include "chrome/common/chrome_paths.h"
#include "chrome/common/chrome_switches.h" #include "chrome/common/chrome_switches.h"
#include "components/metrics/metrics_pref_names.h" #include "components/metrics/metrics_pref_names.h"
#include "components/variations/variations_associated_data.h"
#if defined(OS_ANDROID) #if defined(OS_ANDROID)
#include "chrome/browser/chrome_browser_field_trials_mobile.h" #include "chrome/browser/chrome_browser_field_trials_mobile.h"
...@@ -29,21 +33,58 @@ namespace { ...@@ -29,21 +33,58 @@ namespace {
// Check for feature enabling the use of persistent histogram storage and // Check for feature enabling the use of persistent histogram storage and
// enable the global allocator if so. // enable the global allocator if so.
// TODO(bcwhite): Move this and CreateInstallerFileMetricsProvider into a new
// file and make kBrowserMetricsName local to that file.
void InstantiatePersistentHistograms() { void InstantiatePersistentHistograms() {
if (base::FeatureList::IsEnabled(base::kPersistentHistogramsFeature)) { base::FilePath metrics_dir;
// Create persistent/shared memory and allow histograms to be stored in if (!base::PathService::Get(chrome::DIR_USER_DATA, &metrics_dir))
// it. Memory that is not actualy used won't be physically mapped by the return;
// system. BrowserMetrics usage, as reported in UMA, peaked around 1.9MiB
// as of 2016-02-20. base::FilePath metrics_file =
base::GlobalHistogramAllocator::CreateWithLocalMemory( metrics_dir
3 << 20, // 3 MiB .AppendASCII(ChromeMetricsServiceClient::kBrowserMetricsName)
0x935DDD43, // SHA1(BrowserMetrics) .AddExtension(base::PersistentMemoryAllocator::kFileExtension);
ChromeMetricsServiceClient::kBrowserMetricsName); base::FilePath active_file =
base::GlobalHistogramAllocator* allocator = metrics_dir
base::GlobalHistogramAllocator::Get(); .AppendASCII(
allocator->CreateTrackingHistograms( std::string(ChromeMetricsServiceClient::kBrowserMetricsName) +
"-active")
.AddExtension(base::PersistentMemoryAllocator::kFileExtension);
// Move any existing "active" file to the final name from which it will be
// read when reporting initial stability metrics. If there is no file to
// move, remove any old, existing file from before the previous session.
if (!base::ReplaceFile(active_file, metrics_file, nullptr))
base::DeleteFile(metrics_file, /*recursive=*/false);
// Create persistent/shared memory and allow histograms to be stored in
// it. Memory that is not actualy used won't be physically mapped by the
// system. BrowserMetrics usage, as reported in UMA, peaked around 1.9MiB
// as of 2016-02-20.
const size_t kAllocSize = 3 << 20; // 3 MiB
const uint32_t kAllocId = 0x935DDD43; // SHA1(BrowserMetrics)
std::string storage = variations::GetVariationParamValueByFeature(
base::kPersistentHistogramsFeature, "storage");
if (storage == "MappedFile") {
// Create global allocator with the "active" file.
base::GlobalHistogramAllocator::CreateWithFile(
active_file, kAllocSize, kAllocId,
ChromeMetricsServiceClient::kBrowserMetricsName); ChromeMetricsServiceClient::kBrowserMetricsName);
} else if (storage == "LocalMemory") {
// Use local memory for storage even though it will not persist across
// an unclean shutdown.
base::GlobalHistogramAllocator::CreateWithLocalMemory(
kAllocSize, kAllocId, ChromeMetricsServiceClient::kBrowserMetricsName);
} else {
// Persistent metric storage is disabled.
return;
} }
base::GlobalHistogramAllocator* allocator =
base::GlobalHistogramAllocator::Get();
allocator->CreateTrackingHistograms(
ChromeMetricsServiceClient::kBrowserMetricsName);
allocator->SetPersistentLocation(active_file);
} }
} // namespace } // namespace
......
...@@ -182,6 +182,29 @@ CreateInstallerFileMetricsProvider(bool metrics_reporting_enabled) { ...@@ -182,6 +182,29 @@ CreateInstallerFileMetricsProvider(bool metrics_reporting_enabled) {
return file_metrics_provider; return file_metrics_provider;
} }
// If there is a global metrics file being updated on disk, mark it to be
// deleted when the process exits. A normal shutdown is almost complete
// so there is no benefit in keeping a file with no new data to be processed
// during the next startup sequence. Deleting the file during shutdown adds
// an extra disk-access or two to shutdown but eliminates the unnecessary
// processing of the contents during startup only to find nothing.
void CleanUpGlobalPersistentHistogramStorage() {
base::GlobalHistogramAllocator* allocator =
base::GlobalHistogramAllocator::Get();
if (!allocator)
return;
const base::FilePath& path = allocator->GetPersistentLocation();
if (path.empty())
return;
// Open (with delete) and then immediately close the file by going out of
// scope. This is the only cross-platform safe way to delete a file that may
// be open elsewhere.
base::File file(path, base::File::FLAG_OPEN | base::File::FLAG_READ |
base::File::FLAG_DELETE_ON_CLOSE);
}
} // namespace } // namespace
...@@ -213,6 +236,7 @@ ChromeMetricsServiceClient::ChromeMetricsServiceClient( ...@@ -213,6 +236,7 @@ ChromeMetricsServiceClient::ChromeMetricsServiceClient(
ChromeMetricsServiceClient::~ChromeMetricsServiceClient() { ChromeMetricsServiceClient::~ChromeMetricsServiceClient() {
DCHECK(thread_checker_.CalledOnValidThread()); DCHECK(thread_checker_.CalledOnValidThread());
CleanUpGlobalPersistentHistogramStorage();
} }
// static // static
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment