Add QueryScheduler and make CPUMeasurementMonitor private

The QueryScheduler class is now the sole owner of
CPUMeasurementMonitor, allowing it to multiplex multiple queries to
the same monitor. To replace the public uses of
CPUMeasurementMonitor, adds a temporary ScopedCPUQuery class that will
be replaced by the full resource attribution QueryBuilder when it's
ready.

Bug: 1471683
Change-Id: I915b5f94822d978956559b3e021ee9f939bd40ce
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4950877
Commit-Queue: Joe Mason <joenotcharles@google.com>
Auto-Submit: Joe Mason <joenotcharles@google.com>
Reviewed-by: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1212573}

chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor.cc

@@ -82,12 +82,12 @@ PageTimelineCPUMonitor::PageTimelineCPUMonitor()
 PageTimelineCPUMonitor::~PageTimelineCPUMonitor() = default;
 
 void PageTimelineCPUMonitor::SetCPUMeasurementDelegateFactoryForTesting(
+    Graph* graph,
     CPUMeasurementDelegate::FactoryCallback factory) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (features::kUseResourceAttributionCPUMonitor.Get()) {
-    cpu_measurement_monitor_
-        .SetCPUMeasurementDelegateFactoryForTesting(  // IN_TEST
-            std::move(factory));
+    CPUMeasurementDelegate::SetDelegateFactoryForTesting(  // IN_TEST
+        graph, std::move(factory));
     return;
   }
   // Ensure that all CPU measurements use the same delegate.
@@ -108,7 +108,7 @@ void PageTimelineCPUMonitor::StartMonitoring(Graph* graph) {
 
   if (features::kUseResourceAttributionCPUMonitor.Get()) {
     CHECK(cached_cpu_measurements_.empty());
-    cpu_measurement_monitor_.StartMonitoring(graph);
+    cpu_query_ = std::make_unique<resource_attribution::ScopedCPUQuery>(graph);
     return;
   }
 
@@ -130,7 +130,7 @@ void PageTimelineCPUMonitor::StopMonitoring(Graph* graph) {
   last_measurement_time_ = base::TimeTicks();
 
   if (features::kUseResourceAttributionCPUMonitor.Get()) {
-    cpu_measurement_monitor_.StopMonitoring();
+    cpu_query_.reset();
     cached_cpu_measurements_.clear();
   } else {
     cpu_measurement_map_.clear();
@@ -250,10 +250,10 @@ PageTimelineCPUMonitor::UpdateResourceAttributionCPUMeasurements(
 
   // Swap a new measurement into `cached_cpu_measurements_`, storing the
   // previous contents in `previous_measurements`.
+  CHECK(cpu_query_);
   std::map<ResourceContext, resource_attribution::CPUTimeResult>
       previous_measurements =
-          std::exchange(cached_cpu_measurements_,
-                        cpu_measurement_monitor_.UpdateAndGetCPUMeasurements());
+          std::exchange(cached_cpu_measurements_, cpu_query_->QueryOnce());
 
   CPUUsageMap cpu_usage_map;
   for (const auto& [context, result] : cached_cpu_measurements_) {

chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor.h

@@ -12,9 +12,9 @@
 #include "base/time/time.h"
 #include "components/performance_manager/public/graph/process_node.h"
 #include "components/performance_manager/public/resource_attribution/cpu_measurement_delegate.h"
-#include "components/performance_manager/public/resource_attribution/cpu_measurement_monitor.h"
 #include "components/performance_manager/public/resource_attribution/query_results.h"
 #include "components/performance_manager/public/resource_attribution/resource_contexts.h"
+#include "components/performance_manager/public/resource_attribution/scoped_cpu_query.h"
 
 namespace performance_manager {
 
@@ -53,8 +53,9 @@ class PageTimelineCPUMonitor : public ProcessNode::ObserverDefaultImpl {
   PageTimelineCPUMonitor& operator=(const PageTimelineCPUMonitor&) = delete;
 
   // The given `factory_callback` will be called to create a
-  // CPUMeasurementDelegate for each ProcessNode to be measured.
+  // CPUMeasurementDelegate for each ProcessNode in `graph` to be measured.
   void SetCPUMeasurementDelegateFactoryForTesting(
+      Graph* graph,
       CPUMeasurementDelegate::FactoryCallback factory_callback);
 
   // Starts monitoring CPU usage for all renderer ProcessNode's in `graph`.
@@ -145,7 +146,7 @@ class PageTimelineCPUMonitor : public ProcessNode::ObserverDefaultImpl {
   // If the kUseResourceAttributionCPUMonitor feature parameter is enabled,
   // PageTimelineCPUMonitor will get CPU measurements from this, otherwise it
   // will perform its own measurements.
-  resource_attribution::CPUMeasurementMonitor cpu_measurement_monitor_
+  std::unique_ptr<resource_attribution::ScopedCPUQuery> cpu_query_
       GUARDED_BY_CONTEXT(sequence_checker_);
 
   // If the kUseResourceAttributionCPUMonitor feature parameter is enabled, this

chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor_unittest.cc

@@ -8,7 +8,6 @@
 #include <utility>
 
 #include "base/check.h"
-
 #include "base/memory/weak_ptr.h"
 #include "base/process/kill.h"
 #include "base/process/process.h"
@@ -20,6 +19,7 @@
 #include "base/test/test_waitable_event.h"
 #include "base/time/time.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
+#include "components/performance_manager/embedder/graph_features.h"
 #include "components/performance_manager/graph/frame_node_impl.h"
 #include "components/performance_manager/graph/page_node_impl.h"
 #include "components/performance_manager/graph/process_node_impl.h"
@@ -124,6 +124,9 @@ class PageTimelineCPUMonitorTest : public GraphTestHarness,
   }
 
   void SetUp() override {
+    if (features::kUseResourceAttributionCPUMonitor.Get()) {
+      GetGraphFeatures().EnableResourceAttributionScheduler();
+    }
     Super::SetUp();
 
     mock_graph_ =
@@ -135,7 +138,7 @@ class PageTimelineCPUMonitorTest : public GraphTestHarness,
                                       /*launch_time=*/base::TimeTicks::Now());
 
     cpu_monitor_.SetCPUMeasurementDelegateFactoryForTesting(
-        delegate_factory_.GetFactoryCallback());
+        graph(), delegate_factory_.GetFactoryCallback());
   }
 
   // Creates a renderer process containing a single page and frame, for simple
@@ -553,6 +556,9 @@ class PageTimelineCPUMonitorTimingTest
 
   void SetUp() override {
     Super::SetUp();
+    if (features::kUseResourceAttributionCPUMonitor.Get()) {
+      pm_helper_.GetGraphFeatures().EnableResourceAttributionScheduler();
+    }
     pm_helper_.SetUp();
     RunInGraph([&](Graph* graph) {
       cpu_monitor_ = std::make_unique<PageTimelineCPUMonitor>();

chrome/browser/performance_manager/metrics/page_timeline_monitor_unittest.cc

@@ -17,6 +17,7 @@
 #include "base/test/task_environment.h"
 #include "build/build_config.h"
 #include "chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor.h"
+#include "components/performance_manager/embedder/graph_features.h"
 #include "components/performance_manager/public/decorators/page_live_state_decorator.h"
 #include "components/performance_manager/public/decorators/tab_page_decorator.h"
 #include "components/performance_manager/public/features.h"
@@ -70,7 +71,7 @@ class PageTimelineMonitorUnitTest : public GraphTestHarness {
     monitor_->SetShouldCollectSliceCallbackForTesting(
         base::BindRepeating([]() { return true; }));
     monitor_->cpu_monitor_.SetCPUMeasurementDelegateFactoryForTesting(
-        cpu_delegate_factory_.GetFactoryCallback());
+        graph(), cpu_delegate_factory_.GetFactoryCallback());
     graph()->PassToGraph(std::move(monitor));
     ResetUkmRecorder();
   }
@@ -147,6 +148,13 @@ class PageTimelineMonitorWithFeatureTest
         {});
   }
 
+  void SetUp() override {
+    if (features::kUseResourceAttributionCPUMonitor.Get()) {
+      GetGraphFeatures().EnableResourceAttributionScheduler();
+    }
+    PageTimelineMonitorUnitTest::SetUp();
+  }
+
   void TearDown() override {
     // Destroy `monitor_` before `scoped_feature_list_` so that the feature flag
     // doesn't change during its destructor.

components/performance_manager/BUILD.gn

@@ -164,13 +164,12 @@ static_library("performance_manager") {
     "public/render_process_host_proxy.h",
     "public/resource_attribution/attribution_helpers.h",
     "public/resource_attribution/cpu_measurement_delegate.h",
-    "public/resource_attribution/cpu_measurement_monitor.h",
     "public/resource_attribution/frame_context.h",
-    "public/resource_attribution/graph_change.h",
     "public/resource_attribution/page_context.h",
     "public/resource_attribution/process_context.h",
     "public/resource_attribution/query_results.h",
     "public/resource_attribution/resource_contexts.h",
+    "public/resource_attribution/scoped_cpu_query.h",
     "public/resource_attribution/type_helpers.h",
     "public/resource_attribution/worker_context.h",
     "public/user_tuning/prefs.h",
@@ -186,10 +185,16 @@ static_library("performance_manager") {
     "render_process_user_data.cc",
     "render_process_user_data.h",
     "resource_attribution/attribution_impl_helpers.h",
+    "resource_attribution/cpu_measurement_delegate.cc",
     "resource_attribution/cpu_measurement_monitor.cc",
+    "resource_attribution/cpu_measurement_monitor.h",
     "resource_attribution/frame_context.cc",
+    "resource_attribution/graph_change.h",
     "resource_attribution/page_context.cc",
     "resource_attribution/process_context.cc",
+    "resource_attribution/query_scheduler.cc",
+    "resource_attribution/query_scheduler.h",
+    "resource_attribution/scoped_cpu_query.cc",
     "resource_attribution/worker_context.cc",
     "service_worker_client.cc",
     "service_worker_client.h",
@@ -345,6 +350,7 @@ source_set("unit_tests") {
     "resource_attribution/frame_context_unittest.cc",
     "resource_attribution/page_context_unittest.cc",
     "resource_attribution/process_context_unittest.cc",
+    "resource_attribution/query_scheduler_unittest.cc",
     "resource_attribution/resource_contexts_unittest.cc",
     "resource_attribution/worker_context_unittest.cc",
     "test_support/mock_graphs_unittest.cc",

components/performance_manager/embedder/graph_features.h

@@ -48,6 +48,7 @@ class GraphFeatures {
       bool node_impl_describers : 1;
       bool page_load_tracker_decorator : 1;
       bool process_hosted_content_types_aggregator : 1;
+      bool resource_attribution_scheduler : 1;
       bool site_data_recorder : 1;
       bool tab_connectedness_decorator : 1;
       bool tab_page_decorator : 1;
@@ -102,6 +103,11 @@ class GraphFeatures {
     return *this;
   }
 
+  constexpr GraphFeatures& EnableResourceAttributionScheduler() {
+    flags_.resource_attribution_scheduler = true;
+    return *this;
+  }
+
   // This is a nop on the Android platform, as the feature isn't available
   // there.
   constexpr GraphFeatures& EnableSiteDataRecorder() {
@@ -149,6 +155,7 @@ class GraphFeatures {
     EnableNodeImplDescribers();
     EnablePageLoadTrackerDecorator();
     EnableProcessHostedContentTypesAggregator();
+    EnableResourceAttributionScheduler();
     EnableSiteDataRecorder();
     EnableTabPropertiesDecorator();
     EnableV8ContextTracker();

components/performance_manager/graph_features.cc

@@ -21,6 +21,7 @@
 #include "components/performance_manager/public/decorators/tab_page_decorator.h"
 #include "components/performance_manager/public/graph/graph.h"
 #include "components/performance_manager/public/metrics/metrics_collector.h"
+#include "components/performance_manager/resource_attribution/query_scheduler.h"
 #include "components/performance_manager/v8_memory/v8_context_tracker.h"
 
 #if !BUILDFLAG(IS_ANDROID)
@@ -57,6 +58,9 @@ void GraphFeatures::ConfigureGraph(Graph* graph) const {
     Install<PageLoadTrackerDecorator>(graph);
   if (flags_.process_hosted_content_types_aggregator)
     Install<ProcessHostedContentTypesAggregator>(graph);
+  if (flags_.resource_attribution_scheduler) {
+    Install<resource_attribution::QueryScheduler>(graph);
+  }
 
 #if !BUILDFLAG(IS_ANDROID)
   if (flags_.site_data_recorder)

components/performance_manager/graph_features_unittest.cc

@@ -60,7 +60,7 @@ TEST_F(GraphFeaturesTest, EnableDefault) {
       execution_context::ExecutionContextRegistry::GetFromGraph(&graph));
   EXPECT_FALSE(v8_memory::V8ContextTracker::GetFromGraph(&graph));
 
-  size_t graph_owned_count = 14;
+  size_t graph_owned_count = 15;
 #if !BUILDFLAG(IS_ANDROID)
   // The SiteDataRecorder is not available on Android.
   graph_owned_count++;
@@ -70,7 +70,7 @@ TEST_F(GraphFeaturesTest, EnableDefault) {
   features.EnableDefault();
   features.ConfigureGraph(&graph);
   EXPECT_EQ(graph_owned_count, graph.GraphOwnedCountForTesting());
-  EXPECT_EQ(5u, graph.GraphRegisteredCountForTesting());
+  EXPECT_EQ(6u, graph.GraphRegisteredCountForTesting());
   EXPECT_EQ(7u, graph.NodeDataDescriberCountForTesting());
   // Ensure the GraphRegistered objects can be queried directly.
   EXPECT_TRUE(

components/performance_manager/public/resource_attribution/cpu_measurement_delegate.h

@@ -11,8 +11,9 @@
 #include "base/time/time.h"
 
 namespace performance_manager {
+class Graph;
 class ProcessNode;
-}
+}  // namespace performance_manager
 
 namespace performance_manager::resource_attribution {
 
@@ -26,6 +27,11 @@ class CPUMeasurementDelegate {
       base::RepeatingCallback<std::unique_ptr<CPUMeasurementDelegate>(
           const ProcessNode*)>;
 
+  // The given `factory_callback` will be called to create a
+  // CPUMeasurementDelegate for each ProcessNode in `graph` to be measured.
+  static void SetDelegateFactoryForTesting(Graph* graph,
+                                           FactoryCallback factory_callback);
+
   CPUMeasurementDelegate() = default;
   virtual ~CPUMeasurementDelegate() = default;
 

components/performance_manager/public/resource_attribution/query_results.h

@@ -5,7 +5,10 @@
 #ifndef COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RESOURCE_ATTRIBUTION_QUERY_RESULTS_H_
 #define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RESOURCE_ATTRIBUTION_QUERY_RESULTS_H_
 
+#include <map>
+
 #include "base/time/time.h"
+#include "components/performance_manager/public/resource_attribution/resource_contexts.h"
 
 namespace performance_manager::resource_attribution {
 
@@ -38,6 +41,8 @@ struct CPUTimeResult {
   base::TimeDelta cumulative_cpu;
 };
 
+using QueryResultMap = std::map<ResourceContext, CPUTimeResult>;
+
 }  // namespace performance_manager::resource_attribution
 
 #endif  // COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RESOURCE_ATTRIBUTION_QUERY_RESULTS_H_

components/performance_manager/public/resource_attribution/scoped_cpu_query.h

@@ -0,0 +1,42 @@
+// Copyright 2023 The Chromium Authors
+// 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_RESOURCE_ATTRIBUTION_SCOPED_CPU_QUERY_H_
+#define COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RESOURCE_ATTRIBUTION_SCOPED_CPU_QUERY_H_
+
+#include "base/memory/weak_ptr.h"
+#include "components/performance_manager/public/resource_attribution/query_results.h"
+
+namespace performance_manager {
+class Graph;
+}
+
+namespace performance_manager::resource_attribution {
+
+class QueryScheduler;
+
+// A temporary public interface to request CPU measurements. As soon as a
+// ScopedCPUQuery instance is created, CPUMeasurementMonitor will begin
+// monitoring CPU usage. When no more instances exist, it will stop.
+//
+// TODO(crbug.com/1471683): Replace this with the full Resource Attribution
+// query API described in bit.ly/resource-attribution-api.
+class ScopedCPUQuery {
+ public:
+  explicit ScopedCPUQuery(Graph* graph);
+  ~ScopedCPUQuery();
+
+  ScopedCPUQuery(const ScopedCPUQuery&) = delete;
+  ScopedCPUQuery& operator=(const ScopedCPUQuery&) = delete;
+
+  // Requests the current CPU measurements.
+  QueryResultMap QueryOnce();
+
+ private:
+  base::WeakPtr<QueryScheduler> scheduler_;
+};
+
+}  // namespace performance_manager::resource_attribution
+
+#endif  // COMPONENTS_PERFORMANCE_MANAGER_PUBLIC_RESOURCE_ATTRIBUTION_SCOPED_CPU_QUERY_H_

components/performance_manager/resource_attribution/cpu_measurement_delegate.cc

@@ -0,0 +1,26 @@
+// Copyright 2023 The Chromium Authors
+// 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/resource_attribution/cpu_measurement_delegate.h"
+
+#include <utility>
+
+#include "base/check.h"
+#include "components/performance_manager/resource_attribution/cpu_measurement_monitor.h"
+#include "components/performance_manager/resource_attribution/query_scheduler.h"
+
+namespace performance_manager::resource_attribution {
+
+void CPUMeasurementDelegate::SetDelegateFactoryForTesting(
+    Graph* graph,
+    FactoryCallback factory_callback) {
+  auto* scheduler = QueryScheduler::GetFromGraph(graph);
+  CHECK(scheduler);
+  scheduler
+      ->GetCPUMonitorForTesting()                   // IN-TEST
+      .SetCPUMeasurementDelegateFactoryForTesting(  // IN-TEST
+          std::move(factory_callback));
+}
+
+}  // namespace performance_manager::resource_attribution