Skip to content

Commit

Permalink
Make ScopedCPUQuery async
Browse files Browse the repository at this point in the history 
To be closer to the final API, makes ScopedCPUQuery::QueryOnce()
return results asynchronously through a callback. Adapts
PageTimelineCPUMonitor and PageTimelineMonitor to make CPU queries
asynchronously.

Also adds more RunInGraph test helpers that pass
run_loop.QuitCallback() for tests to call manually after asynchronous
operations complete.

Bug: 1471683
Change-Id: I591dd45af741c2da57aa9914d4a890911180a98f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4950104
Reviewed-by: Alison Gale <agale@chromium.org>
Commit-Queue: Joe Mason <joenotcharles@google.com>
Cr-Commit-Position: refs/heads/main@{#1212575}
  • Loading branch information
@JoeNotCharlesGoogle
JoeNotCharlesGoogle authored and Chromium LUCI CQ committed Oct 20, 2023
1 parent 11f7df0 commit d04f887
Show file tree
Hide file tree
Showing 13 changed files with 311 additions and 108 deletions.
35 changes: 20 additions & 15 deletions chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/process/process_handle.h"
#include "base/process/process_metrics.h"
#include "base/sequence_checker.h"
#include "base/task/sequenced_task_runner.h"
#include "base/time/time.h"
#include "build/build_config.h"
#include "components/performance_manager/public/features.h"
Expand Down Expand Up @@ -138,25 +139,29 @@ void PageTimelineCPUMonitor::StopMonitoring(Graph* graph) {
}
}

PageTimelineCPUMonitor::CPUUsageMap
PageTimelineCPUMonitor::UpdateCPUMeasurements() {
void PageTimelineCPUMonitor::UpdateCPUMeasurements(
base::OnceCallback<void(const CPUUsageMap&)> callback) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
// Update CPU metrics, attributing the cumulative CPU of each process to its
// frames and workers.
CHECK(!last_measurement_time_.is_null());
CPUUsageMap cpu_usage_map;
const base::TimeTicks now = base::TimeTicks::Now();
if (features::kUseResourceAttributionCPUMonitor.Get()) {
cpu_usage_map =
UpdateResourceAttributionCPUMeasurements(now - last_measurement_time_);
cpu_query_->QueryOnce(base::BindOnce(
&PageTimelineCPUMonitor::UpdateResourceAttributionCPUMeasurements,
weak_factory_.GetWeakPtr(), std::move(callback),
now - last_measurement_time_));
} else {
CPUUsageMap cpu_usage_map;
for (auto& [process_node, cpu_measurement] : cpu_measurement_map_) {
cpu_measurement.MeasureAndDistributeCPUUsage(
process_node, last_measurement_time_, now, cpu_usage_map);
}
base::SequencedTaskRunner::GetCurrentDefault()->PostTask(
FROM_HERE,
base::BindOnce(std::move(callback), std::move(cpu_usage_map)));
}
last_measurement_time_ = now;
return cpu_usage_map;
}

// static
Expand Down Expand Up @@ -237,23 +242,23 @@ void PageTimelineCPUMonitor::MonitorCPUUsage(const ProcessNode* process_node) {
CHECK(was_inserted);
}

PageTimelineCPUMonitor::CPUUsageMap
PageTimelineCPUMonitor::UpdateResourceAttributionCPUMeasurements(
base::TimeDelta measurement_interval) {
void PageTimelineCPUMonitor::UpdateResourceAttributionCPUMeasurements(
base::OnceCallback<void(const CPUUsageMap&)> callback,
base::TimeDelta measurement_interval,
const resource_attribution::QueryResultMap& results) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
CHECK(features::kUseResourceAttributionCPUMonitor.Get());
if (measurement_interval.is_zero()) {
// No time passed to measure.
return CPUUsageMap();
// No time passed to measure. Ignore the results to avoid division by zero.
std::move(callback).Run({});
return;
}
CHECK(measurement_interval.is_positive());

// 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_query_->QueryOnce());
previous_measurements = std::exchange(cached_cpu_measurements_, results);

CPUUsageMap cpu_usage_map;
for (const auto& [context, result] : cached_cpu_measurements_) {
Expand Down Expand Up @@ -335,7 +340,7 @@ PageTimelineCPUMonitor::UpdateResourceAttributionCPUMeasurements(
CHECK(!current_cpu.is_negative());
cpu_usage_map.emplace(context, current_cpu / measurement_interval);
}
return cpu_usage_map;
std::move(callback).Run(std::move(cpu_usage_map));
}

PageTimelineCPUMonitor::CPUMeasurement::CPUMeasurement(
Expand Down
25 changes: 16 additions & 9 deletions chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
#include <map>
#include <memory>

#include "base/functional/callback_forward.h"
#include "base/memory/weak_ptr.h"
#include "base/sequence_checker.h"
#include "base/time/time.h"
#include "components/performance_manager/public/graph/process_node.h"
Expand Down Expand Up @@ -64,10 +66,11 @@ class PageTimelineCPUMonitor : public ProcessNode::ObserverDefaultImpl {
// Stops monitoring ProcessNode's in `graph`.
void StopMonitoring(Graph* graph);

// Updates the CPU measurements for each ProcessNode being tracked and returns
// the estimated CPU usage of each frame and worker in those processes since
// the last time UpdateCPUMeasurements() was called .
CPUUsageMap UpdateCPUMeasurements();
// Updates the CPU measurements for each ProcessNode being tracked and invokes
// `callback` with the estimated CPU usage of each frame and worker in those
// processes since the last time UpdateCPUMeasurements() was called .
void UpdateCPUMeasurements(
base::OnceCallback<void(const CPUUsageMap&)> callback);

// Helper to estimate the CPU usage of a PageNode given the estimates for all
// frames and workers. If the kUseResourceAttributionCPUMonitor feature
Expand Down Expand Up @@ -122,11 +125,13 @@ class PageTimelineCPUMonitor : public ProcessNode::ObserverDefaultImpl {
// `cpu_measurement_map_`.
void MonitorCPUUsage(const ProcessNode* process_node);

// Uses `cpu_measurement_monitor_` to update CPU measurements. Called from
// UpdateCPUMeasurements() if the kUseResourceAttributionCPUMonitor feature
// parameter is enabled.
CPUUsageMap UpdateResourceAttributionCPUMeasurements(
base::TimeDelta measurement_interval);
// Uses results from `cpu_measurement_monitor_` to update CPU measurements.
// Called from UpdateCPUMeasurements() if the
// kUseResourceAttributionCPUMonitor feature parameter is enabled.
void UpdateResourceAttributionCPUMeasurements(
base::OnceCallback<void(const CPUUsageMap&)> callback,
base::TimeDelta measurement_interval,
const resource_attribution::QueryResultMap& results);

SEQUENCE_CHECKER(sequence_checker_);

Expand Down Expand Up @@ -155,6 +160,8 @@ class PageTimelineCPUMonitor : public ProcessNode::ObserverDefaultImpl {
std::map<resource_attribution::ResourceContext,
resource_attribution::CPUTimeResult>
cached_cpu_measurements_ GUARDED_BY_CONTEXT(sequence_checker_);

base::WeakPtrFactory<PageTimelineCPUMonitor> weak_factory_{this};
};

} // namespace metrics
Expand Down
108 changes: 72 additions & 36 deletions chrome/browser/performance_manager/metrics/page_timeline_cpu_monitor_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,13 @@
#include <utility>

#include "base/check.h"
#include "base/functional/callback.h"
#include "base/functional/function_ref.h"
#include "base/memory/weak_ptr.h"
#include "base/process/kill.h"
#include "base/process/process.h"
#include "base/process/process_handle.h"
#include "base/run_loop.h"
#include "base/test/bind.h"
#include "base/test/scoped_feature_list.h"
#include "base/test/task_environment.h"
Expand Down Expand Up @@ -189,6 +192,19 @@ class PageTimelineCPUMonitorTest : public GraphTestHarness,
delegate_factory_.GetDelegate(process_node).SetError(usage_error);
}

PageTimelineCPUMonitor::CPUUsageMap WaitForCPUMeasurements() {
PageTimelineCPUMonitor::CPUUsageMap cpu_usage_map;
base::RunLoop run_loop;
cpu_monitor_.UpdateCPUMeasurements(
base::BindLambdaForTesting(
[&](const PageTimelineCPUMonitor::CPUUsageMap& results) {
cpu_usage_map = results;
})
.Then(run_loop.QuitClosure()));
run_loop.Run();
return cpu_usage_map;
}

base::test::ScopedFeatureList scoped_feature_list_;

// Factory to return CPUMeasurementDelegates for `cpu_monitor_`. This must be
Expand Down Expand Up @@ -257,7 +273,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurement) {
// usage. (As an optimization the monitor may not include it in the results.)
// `renderer5` is not measured yet.
{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements,
early_exit_renderer.resource_context),
Eq(absl::nullopt));
Expand All @@ -280,7 +296,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurement) {

// All nodes existed for entire measurement interval.
{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements, renderer1.resource_context),
Optional(DoubleEq(1.0)));
EXPECT_THAT(GetMeasurementResult(measurements, renderer2.resource_context),
Expand Down Expand Up @@ -313,7 +329,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurement) {
SetProcessCPUUsage(renderer4.process_node.get(), 0);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements, renderer1.resource_context),
Optional(DoubleEq(0.5)));
EXPECT_THAT(GetMeasurementResult(measurements, renderer2.resource_context),
Expand Down Expand Up @@ -342,7 +358,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurement) {
// TODO(crbug.com/1410503): Capture the final CPU usage correctly, and test
// that the renderers that have exited return their CPU usage for the time
// they were alive and 0% for the rest of the measurement interval.
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements, renderer1.resource_context),
ExpectedErrorResult());
EXPECT_THAT(GetMeasurementResult(measurements, renderer2.resource_context),
Expand Down Expand Up @@ -375,7 +391,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUDistribution) {

{
// No measurements if no time has passed.
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(measurements, IsEmpty());
}

Expand All @@ -389,7 +405,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUDistribution) {
task_env().FastForwardBy(kTimeBetweenMeasurements);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();

// `process` splits its 60% CPU usage evenly between `frame`, `other_frame`
// and `worker`. `other_process` splits its 50% CPU usage evenly between
Expand Down Expand Up @@ -427,7 +443,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUDistribution) {
task_env().FastForwardBy(kTimeBetweenMeasurements);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();

// `process` splits its 30% CPU usage evenly between `frame`, `other_frame`
// and `worker`. `other_process` splits its 80% CPU usage evenly between
Expand Down Expand Up @@ -463,7 +479,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUDistribution) {
task_env().FastForwardBy(kTimeBetweenMeasurements / 3);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();

// `process` splits its 30% CPU usage evenly between `frame`, `other_frame`
// and `worker`. `other_process` splits its 0% CPU usage evenly between
Expand Down Expand Up @@ -508,7 +524,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurementError) {
task_env().FastForwardBy(kTimeBetweenMeasurements);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements, renderer1.resource_context),
Optional(DoubleEq(1.0)));
EXPECT_THAT(GetMeasurementResult(measurements, renderer2.resource_context),
Expand All @@ -529,7 +545,7 @@ TEST_P(PageTimelineCPUMonitorTest, CPUMeasurementError) {
task_env().FastForwardBy(kTimeBetweenMeasurements);

{
auto measurements = cpu_monitor_.UpdateCPUMeasurements();
auto measurements = WaitForCPUMeasurements();
EXPECT_THAT(GetMeasurementResult(measurements, renderer1.resource_context),
ExpectedErrorResult());
EXPECT_THAT(GetMeasurementResult(measurements, renderer2.resource_context),
Expand Down Expand Up @@ -576,10 +592,32 @@ class PageTimelineCPUMonitorTimingTest
}

// Ensure some time passes to measure.
void LetTimePass() {
static void LetTimePass() {
base::TestWaitableEvent().TimedWait(TestTimeouts::tiny_timeout());
}

// Gets the measurement for the page containing `frame_node` from
// `measurements`, and passes it to `matcher_callback`. This is invoked on the
// PM sequence from UpdateCPUMeasurements().
static void TestPageMeasurement(
base::WeakPtr<FrameNode> frame_node,
base::FunctionRef<void(absl::optional<double>)> matcher_callback,
const PageTimelineCPUMonitor::CPUUsageMap& measurements) {
absl::optional<double> measurement_result;
if (features::kUseResourceAttributionCPUMonitor.Get()) {
// Resource Attribution stores page estimates directly in
// CPUUsageMap.
if (frame_node && frame_node->GetPageNode()) {
measurement_result = GetMeasurementResult(
measurements, frame_node->GetPageNode()->GetResourceContext());
}
} else if (frame_node) {
measurement_result =
GetMeasurementResult(measurements, frame_node->GetResourceContext());
}
matcher_callback(measurement_result);
}

base::test::ScopedFeatureList scoped_feature_list_;
PerformanceManagerTestHarnessHelper pm_helper_;
std::unique_ptr<PageTimelineCPUMonitor> cpu_monitor_;
Expand All @@ -599,61 +637,59 @@ TEST_P(PageTimelineCPUMonitorTimingTest, ProcessLifetime) {
base::WeakPtr<ProcessNode> process_node =
PerformanceManager::GetProcessNodeForRenderProcessHost(process());

auto get_measurement_result = [this](base::WeakPtr<FrameNode> frame_node) {
CHECK(frame_node);
if (features::kUseResourceAttributionCPUMonitor.Get()) {
// Resource Attribution stores page estimates directly in CPUUsageMap.
CHECK(frame_node->GetPageNode());
return GetMeasurementResult(
cpu_monitor_->UpdateCPUMeasurements(),
frame_node->GetPageNode()->GetResourceContext());
}
return GetMeasurementResult(cpu_monitor_->UpdateCPUMeasurements(),
frame_node->GetResourceContext());
};

// Since process() returns a MockRenderProcessHost, ProcessNode is created
// but has no pid. (Equivalent to the time between OnProcessNodeAdded and
// OnProcessLifetimeChange.)
LetTimePass();
RunInGraph([&] {
RunInGraph([&](base::OnceClosure quit_closure) {
ASSERT_TRUE(process_node);
EXPECT_EQ(process_node->GetProcessId(), base::kNullProcessId);

// Process can't be measured yet.
EXPECT_THAT(get_measurement_result(frame_node), Eq(absl::nullopt));
cpu_monitor_->UpdateCPUMeasurements(
base::BindOnce(&TestPageMeasurement, frame_node,
[](absl::optional<double> measurement) {
EXPECT_THAT(measurement, Eq(absl::nullopt));
})
.Then(std::move(quit_closure)));
});

// Assign a real process to the ProcessNode. (Will call
// OnProcessLifetimeChange.)
LetTimePass();
RunInGraph([&] {
RunInGraph([&](base::OnceClosure quit_closure) {
ASSERT_TRUE(process_node);
ProcessNodeImpl::FromNode(process_node.get())
->SetProcess(base::Process::Current(), base::TimeTicks::Now());
EXPECT_NE(process_node->GetProcessId(), base::kNullProcessId);

// Process can be measured now.
ASSERT_TRUE(frame_node);
EXPECT_THAT(get_measurement_result(frame_node), Optional(_));
cpu_monitor_->UpdateCPUMeasurements(
base::BindOnce(&TestPageMeasurement, frame_node,
[](absl::optional<double> measurement) {
EXPECT_THAT(measurement, Optional(_));
})
.Then(std::move(quit_closure)));
});

// Simulate that the process died.
LetTimePass();
process()->SimulateRenderProcessExit(
base::TERMINATION_STATUS_NORMAL_TERMINATION, 0);
RunInGraph([&] {
RunInGraph([&](base::OnceClosure quit_closure) {
// Process is no longer running, so can't be measured.
// TODO(crbug.com/1410503): Capture the final CPU usage correctly.
ASSERT_TRUE(process_node);
EXPECT_FALSE(process_node->GetProcess().IsValid());
// Depending on the order that observers fire, `frame_node` may or may not
// have been deleted already. If it's gone just check that the measurement
// doesn't crash.
const auto measurements = cpu_monitor_->UpdateCPUMeasurements();
if (frame_node) {
EXPECT_THAT(get_measurement_result(frame_node), Eq(absl::nullopt));
}
// have been deleted already. Either way, TestPageMeasurementResult will get
// nullopt.
cpu_monitor_->UpdateCPUMeasurements(
base::BindOnce(&TestPageMeasurement, frame_node,
[](absl::optional<double> measurement) {
EXPECT_THAT(measurement, Eq(absl::nullopt));
})
.Then(std::move(quit_closure)));
});
}

Expand Down

0 comments on commit d04f887

Please sign in to comment.