Skip to content

Commit

Permalink
Add a metrics provider for performance manager-related UMA filters
Browse files Browse the repository at this point in the history
This MetricsProvider subclass logs the current Efficiency Mode into a
histogram on each UMA upload, which will subsequently allow filtering
other histograms based on this value.

Bug: 1308741
Change-Id: I19738100edb60ebbda56004f700a80cf465ece3b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3573013
Reviewed-by: Weilun Shi <sweilun@chromium.org>
Commit-Queue: Anthony Vallée-Dubois <anthonyvd@chromium.org>
Reviewed-by: Francois Pierre Doray <fdoray@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001471}
  • Loading branch information
Anthony Vallee-Dubois authored and Chromium LUCI CQ committed May 10, 2022
1 parent c1d9294 commit aef516e
Show file tree
Hide file tree
Showing 12 changed files with 325 additions and 0 deletions.
4 changes: 4 additions & 0 deletions chrome/browser/metrics/chrome_metrics_service_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@
#include "components/metrics/android_metrics_provider.h"
#else
#include "chrome/browser/metrics/browser_activity_watcher.h"
#include "components/performance_manager/public/metrics/metrics_provider.h"
#endif

#if BUILDFLAG(IS_POSIX)
Expand Down Expand Up @@ -772,6 +773,9 @@ void ChromeMetricsServiceClient::RegisterMetricsServiceProviders() {
std::make_unique<PageLoadMetricsProvider>());
metrics_service_->RegisterMetricsProvider(
std::make_unique<FamilyLinkUserMetricsProvider>());
#else
metrics_service_->RegisterMetricsProvider(
std::make_unique<performance_manager::MetricsProvider>(local_state));
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_WIN)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,9 @@ TEST_F(ChromeMetricsServiceClientTest, TestRegisterMetricsServiceProviders) {
// AndroidMetricsProvider, ChromeAndroidMetricsProvider,
// FamilyLinkUserMetricsProvider, and PageLoadMetricsProvider.
expected_providers += 4;
#else
// performance_manager::MetricsProvider
expected_providers += 1;
#endif // BUILDFLAG(IS_ANDROID)

#if BUILDFLAG(IS_WIN)
Expand Down
1 change: 1 addition & 0 deletions components/metrics/generate_expired_histograms_array.gni
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ template("generate_expired_histograms_array") {
"//tools/metrics/histograms/metadata/payment/histograms.xml",
"//tools/metrics/histograms/metadata/pcscan/histograms.xml",
"//tools/metrics/histograms/metadata/pdf/histograms.xml",
"//tools/metrics/histograms/metadata/performance_manager/histograms.xml",
"//tools/metrics/histograms/metadata/permissions/histograms.xml",
"//tools/metrics/histograms/metadata/phonehub/histograms.xml",
"//tools/metrics/histograms/metadata/platform/histograms.xml",
Expand Down
5 changes: 5 additions & 0 deletions components/performance_manager/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ static_library("performance_manager") {
"graph/worker_node_impl_describer.h",
"graph_features.cc",
"metrics/metrics_collector.cc",
"metrics/metrics_provider.cc",
"owned_objects.h",
"performance_manager.cc",
"performance_manager_feature_observer_client.cc",
Expand Down Expand Up @@ -140,6 +141,7 @@ static_library("performance_manager") {
"public/graph/worker_node.h",
"public/metrics/background_metrics_reporter.h",
"public/metrics/metrics_collector.h",
"public/metrics/metrics_provider.h",
"public/performance_manager.h",
"public/performance_manager_main_thread_mechanism.h",
"public/performance_manager_main_thread_observer.h",
Expand Down Expand Up @@ -191,6 +193,7 @@ static_library("performance_manager") {

deps = [
"//build:chromeos_buildflags",
"//components/metrics",
"//components/pref_registry:pref_registry",
"//components/prefs:prefs",
"//third_party/blink/public/common:headers",
Expand Down Expand Up @@ -285,6 +288,7 @@ source_set("unit_tests") {
"graph/worker_node_impl_unittest.cc",
"graph_features_unittest.cc",
"metrics/metrics_collector_unittest.cc",
"metrics/metrics_provider_unittest.cc",
"owned_objects_unittest.cc",
"performance_manager_impl_unittest.cc",
"performance_manager_registry_impl_unittest.cc",
Expand All @@ -311,6 +315,7 @@ source_set("unit_tests") {
"test_support:test_support_common",
"//base/test:test_support",
"//components/memory_pressure:test_support",
"//components/prefs:test_support",
"//components/services/storage/public/cpp",
"//components/ukm:test_support",
"//content/test:test_support",
Expand Down
1 change: 1 addition & 0 deletions components/performance_manager/DEPS
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
include_rules = [
"+components/memory_pressure",
"+components/metrics",
"+components/pref_registry",
"+components/prefs",
"+components/services/storage/public/cpp",
Expand Down
72 changes: 72 additions & 0 deletions components/performance_manager/metrics/metrics_provider.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
// Copyright 2022 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 "components/performance_manager/public/metrics/metrics_provider.h"

#include "base/metrics/histogram_functions.h"
#include "components/performance_manager/public/user_tuning/prefs.h"
#include "components/prefs/pref_service.h"

namespace performance_manager {

MetricsProvider::MetricsProvider(PrefService* local_state)
: local_state_(local_state) {
pref_change_registrar_.Init(local_state_);
pref_change_registrar_.Add(
performance_manager::user_tuning::prefs::kHighEfficiencyModeEnabled,
base::BindRepeating(&MetricsProvider::OnEfficiencyModeChanged,
base::Unretained(this)));
pref_change_registrar_.Add(
performance_manager::user_tuning::prefs::kBatterySaverModeEnabled,
base::BindRepeating(&MetricsProvider::OnEfficiencyModeChanged,
base::Unretained(this)));

current_mode_ = ComputeCurrentMode();
}

MetricsProvider::~MetricsProvider() = default;

void MetricsProvider::ProvideCurrentSessionData(
metrics::ChromeUserMetricsExtension* uma_proto) {
base::UmaHistogramEnumeration("PerformanceManager.UserTuning.EfficiencyMode",
current_mode_);

// Set `current_mode_` to represent the state of the modes as they are now, so
// that this mode is what is adequately reported at the next report, unless it
// changes in the meantime.
current_mode_ = ComputeCurrentMode();
}

void MetricsProvider::OnEfficiencyModeChanged() {
EfficiencyMode new_mode = ComputeCurrentMode();

// If the mode changes between UMA reports, mark it as Mixed for this
// interval.
if (current_mode_ != new_mode) {
current_mode_ = EfficiencyMode::kMixed;
}
}

MetricsProvider::EfficiencyMode MetricsProvider::ComputeCurrentMode() const {
bool high_efficiency_enabled = local_state_->GetBoolean(
performance_manager::user_tuning::prefs::kHighEfficiencyModeEnabled);
bool battery_saver_enabled = local_state_->GetBoolean(
performance_manager::user_tuning::prefs::kBatterySaverModeEnabled);

if (high_efficiency_enabled && battery_saver_enabled) {
return EfficiencyMode::kBoth;
}

if (high_efficiency_enabled) {
return EfficiencyMode::kHighEfficiency;
}

if (battery_saver_enabled) {
return EfficiencyMode::kBatterySaver;
}

return EfficiencyMode::kNormal;
}

} // namespace performance_manager
137 changes: 137 additions & 0 deletions components/performance_manager/metrics/metrics_provider_unittest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
// Copyright 2022 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 "components/performance_manager/public/metrics/metrics_provider.h"

#include "base/test/metrics/histogram_tester.h"
#include "components/performance_manager/public/user_tuning/prefs.h"
#include "components/prefs/testing_pref_service.h"
#include "testing/gtest/include/gtest/gtest.h"

class PerformanceManagerMetricsProviderTest : public testing::Test {
protected:
PrefService* local_state() { return &local_state_; }

void SetHighEfficiencyEnabled(bool enabled) {
local_state()->SetBoolean(
performance_manager::user_tuning::prefs::kHighEfficiencyModeEnabled,
enabled);
}

void SetBatterySaverEnabled(bool enabled) {
local_state()->SetBoolean(
performance_manager::user_tuning::prefs::kBatterySaverModeEnabled,
enabled);
}

void ExpectSingleUniqueSample(
const base::HistogramTester& tester,
performance_manager::MetricsProvider::EfficiencyMode sample) {
tester.ExpectUniqueSample("PerformanceManager.UserTuning.EfficiencyMode",
sample, 1);
}

private:
void SetUp() override {
performance_manager::user_tuning::prefs::RegisterLocalStatePrefs(
local_state_.registry());
}

TestingPrefServiceSimple local_state_;
};

TEST_F(PerformanceManagerMetricsProviderTest, TestNormalMode) {
base::HistogramTester tester;

performance_manager::MetricsProvider provider(local_state());
provider.ProvideCurrentSessionData(nullptr);

ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kNormal);
}

TEST_F(PerformanceManagerMetricsProviderTest, TestMixedMode) {
performance_manager::MetricsProvider provider(local_state());

{
base::HistogramTester tester;
// Start in normal mode
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kNormal);
}

{
base::HistogramTester tester;
// Enabled High-Efficiency Mode, the next reported value should be "mixed"
// because we transitioned from normal to High-Efficiency during the
// interval.
SetHighEfficiencyEnabled(true);
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kMixed);
}

{
base::HistogramTester tester;
// If another UMA upload happens without mode changes, this one will report
// High-Efficiency Mode.
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester,
performance_manager::MetricsProvider::EfficiencyMode::kHighEfficiency);
}
}

TEST_F(PerformanceManagerMetricsProviderTest, TestBothModes) {
SetHighEfficiencyEnabled(true);
SetBatterySaverEnabled(true);

performance_manager::MetricsProvider provider(local_state());

{
base::HistogramTester tester;
// Start with both modes enabled (such as a Chrome startup after having
// enabled both modes in a previous session).
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kBoth);
}

{
base::HistogramTester tester;
// Disabling High-Efficiency Mode will cause the next report to be "mixed".
SetHighEfficiencyEnabled(false);
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kMixed);
}

{
base::HistogramTester tester;
// No changes until the following report, "Battery saver" is reported
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester,
performance_manager::MetricsProvider::EfficiencyMode::kBatterySaver);
}

{
base::HistogramTester tester;
// Re-enabling High-Efficiency Mode will cause the next report to indicate
// "mixed".
SetHighEfficiencyEnabled(true);
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kMixed);
}

{
base::HistogramTester tester;
// One more report with no changes, this one reports "both" again.
provider.ProvideCurrentSessionData(nullptr);
ExpectSingleUniqueSample(
tester, performance_manager::MetricsProvider::EfficiencyMode::kBoth);
}
}
54 changes: 54 additions & 0 deletions components/performance_manager/public/metrics/metrics_provider.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright 2022 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.

#ifndef COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_
#define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_

#include "components/metrics/metrics_provider.h"

#include "components/prefs/pref_change_registrar.h"

class PrefService;

namespace performance_manager {

// A metrics provider to add some performance manager related metrics to the UMA
// protos on each upload.
class MetricsProvider : public metrics::MetricsProvider {
public:
enum class EfficiencyMode {
// No efficiency mode for the entire upload window
kNormal = 0,
// In high efficiency mode for the entire upload window
kHighEfficiency = 1,
// In battery saver mode for the entire upload window
kBatterySaver = 2,
// Both modes enabled for the entire upload window
kBoth = 3,
// The modes were changed during the upload window
kMixed = 4,
// Max value, used in UMA histograms macros
kMaxValue = kMixed
};

explicit MetricsProvider(PrefService* local_state);
~MetricsProvider() override;

// metrics::MetricsProvider:
// This is only called from UMA code but is public for testing.
void ProvideCurrentSessionData(
metrics::ChromeUserMetricsExtension* uma_proto) override;

private:
void OnEfficiencyModeChanged();
EfficiencyMode ComputeCurrentMode() const;

PrefChangeRegistrar pref_change_registrar_;
PrefService* const local_state_;
EfficiencyMode current_mode_ = EfficiencyMode::kNormal;
};

} // namespace performance_manager

#endif // COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_METRICS_METRICS_PROVIDER_H_
8 changes: 8 additions & 0 deletions tools/metrics/histograms/enums.xml
Original file line number Diff line number Diff line change
Expand Up @@ -28670,6 +28670,14 @@ Called by update_document_policy_enum.py.-->
<int value="8294400" label="3840 x 2160"/>
</enum>

<enum name="EfficiencyMode">
<int value="0" label="Normal"/>
<int value="1" label="High Efficiency"/>
<int value="2" label="Battery Saver"/>
<int value="3" label="Both"/>
<int value="4" label="Mixed"/>
</enum>

<enum name="EGLDisplayType">
<int value="0" label="Default"/>
<int value="1" label="SwiftShader"/>
Expand Down
1 change: 1 addition & 0 deletions tools/metrics/histograms/histograms_index.txt
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ tools/metrics/histograms/metadata/password/histograms.xml
tools/metrics/histograms/metadata/payment/histograms.xml
tools/metrics/histograms/metadata/pcscan/histograms.xml
tools/metrics/histograms/metadata/pdf/histograms.xml
tools/metrics/histograms/metadata/performance_manager/histograms.xml
tools/metrics/histograms/metadata/permissions/histograms.xml
tools/metrics/histograms/metadata/phonehub/histograms.xml
tools/metrics/histograms/metadata/platform/histograms.xml
Expand Down
6 changes: 6 additions & 0 deletions tools/metrics/histograms/metadata/performance_manager/OWNERS
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
per-file OWNERS=file://tools/metrics/histograms/metadata/METRIC_REVIEWER_OWNERS

# Prefer sending CLs to the owners listed below.
# Use chromium-metrics-reviews@google.com as a backup.
chrisha@chromium.org
fdoray@chromium.org

0 comments on commit aef516e

Please sign in to comment.