Skip to content

Commit

Permalink
[ComputePressure] Remove CPU frequency information.
Browse files Browse the repository at this point in the history
CPU frequency statistics are not a good metric. A CPU can
have multiple boost modes depending on which core is running
and depending on whether on AC or DC power. We cannot deduct
that a CPU is stressed just because it is running at a high
frequency. Also it might be running at a lower frequency due
to power settings or being on DC power and then the system
can easily be under pressure at this lower frequency. (GitHub
issue: w3c/compute-pressure#24,
Spec change: w3c/compute-pressure@e3da844).

We remove CPU frequency information from the implementation
in this patch and use only CPU utilization temporarily. We
will switch to PressureState in the future according to the
newest spec (https://wicg.github.io/compute-pressure/#pressure-states)
and we are working on a robust algorithm to calculate
PressureState.

Bug: 1339205
Change-Id: I15a17fd1eefeeefaddc5f3df5e2a98b04cac4368
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3725753
Reviewed-by: Matthew Denton <[email protected]>
Commit-Queue: Wei4 Wang <[email protected]>
Reviewed-by: Raphael Kubo Da Costa <[email protected]>
Reviewed-by: Reilly Grant <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1025486}
NOKEYCHECK=True
GitOrigin-RevId: 55b9c5aa2a3c05b4ac31cd19fdc6bb6132e69be9
  • Loading branch information
wangw-1991 authored and copybara-github committed Jul 19, 2022
1 parent 9feac80 commit 7901c13
Show file tree
Hide file tree
Showing 18 changed files with 41 additions and 99 deletions.
4 changes: 0 additions & 4 deletions blink/public/mojom/compute_pressure/compute_pressure.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,11 @@ import "services/device/public/mojom/compute_pressure_state.mojom";
// interval is 0.65, the average of 0.5 and 0.8.
struct ComputePressureQuantization {
array<double> cpu_utilization_thresholds;
array<double> cpu_speed_thresholds;
};

// The maximum size of ComputePressureQuantization.cpu_utilization_thresholds.
const int32 kMaxComputePressureCpuUtilizationThresholds = 3;

// The maximum size of ComputePressureQuantization.cpu_speed_thresholds.
const int32 kMaxComputePressureCpuSpeedThresholds = 1;

// Implemented by renderers to receive compute pressure info from the browser.
//
// The values in the ComputePressureState reported to the renderer are always
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,19 +88,6 @@ bool NormalizeObserverOptions(ComputePressureObserverOptions& options,
}
options.setCpuUtilizationThresholds(cpu_utilization_thresholds);

Vector<double> cpu_speed_thresholds = options.cpuSpeedThresholds();
if (cpu_speed_thresholds.size() >
mojom::blink::kMaxComputePressureCpuSpeedThresholds) {
cpu_speed_thresholds.resize(
mojom::blink::kMaxComputePressureCpuSpeedThresholds);
}
std::sort(cpu_speed_thresholds.begin(), cpu_speed_thresholds.end());
if (!ValidateThresholds(cpu_speed_thresholds, exception_state)) {
DCHECK(exception_state.HadException());
return false;
}
options.setCpuSpeedThresholds(cpu_speed_thresholds);

return true;
}

Expand Down Expand Up @@ -152,8 +139,7 @@ ScriptPromise ComputePressureObserver::observe(
->GetTaskRunner(TaskType::kMiscPlatformAPI);

auto mojo_options = mojom::blink::ComputePressureQuantization::New(
normalized_options_->cpuUtilizationThresholds(),
normalized_options_->cpuSpeedThresholds());
normalized_options_->cpuUtilizationThresholds());

compute_pressure_service_->AddObserver(
receiver_.BindNewPipeAndPassRemote(std::move(task_runner)),
Expand Down Expand Up @@ -206,7 +192,6 @@ void ComputePressureObserver::OnUpdate(
device::mojom::blink::ComputePressureStatePtr state) {
auto* record = ComputePressureRecord::Create();
record->setCpuUtilization(state->cpu_utilization);
record->setCpuSpeed(state->cpu_speed);

// This should happen infrequently since `records_` is supposed
// to be emptied at every callback invoking or takeRecords().
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,4 @@
SecureContext
] dictionary ComputePressureObserverOptions {
sequence<double> cpuUtilizationThresholds = [];
sequence<double> cpuSpeedThresholds = [];
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
Exposed=Window,
SecureContext
] dictionary ComputePressureRecord {
// TODO(crbug.com/1308316): Remove cpuSpeed and cpuUtilization.
// when switching to new API.
double cpuSpeed;
// TODO(crbug.com/1308316): Remove cpuUtilization when switching
// to new API.
double cpuUtilization;
};
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
'use strict';

for (const property of ['cpuUtilizationThresholds', 'cpuSpeedThresholds']) {
for (const property of ['cpuUtilizationThresholds']) {
for (const out_of_range_value of [-1.0, 0.0, 1.0, 2.0]) {
test(t => {
const callback = () => {};

const options = {
cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5] };
cpuUtilizationThresholds: [0.5] };
options[property] = [out_of_range_value];

assert_throws_js(TypeError, () => {
Expand All @@ -21,7 +21,7 @@ for (const property of ['cpuUtilizationThresholds', 'cpuSpeedThresholds']) {
const callback = () => {};

const options = {
cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5] };
cpuUtilizationThresholds: [0.5] };
options[property] = [valid_value];

const observer = new ComputePressureObserver(callback, options);
Expand All @@ -38,7 +38,7 @@ test(t => {
assert_throws_js(TypeError, () => {
new ComputePressureObserver(
callback,
{ cpuUtilizationThresholds: [0.5, 0.5], cpuSpeedThresholds: [0.5] });
{ cpuUtilizationThresholds: [0.5, 0.5] });
});
}, 'ComputePressureObserver constructor throws when cpuUtilizationThresholds ' +
'has duplicates');
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ promise_test(async t => {

const update = await new Promise((resolve, reject) => {
const observer = new ComputePressureObserver(
resolve, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
resolve, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer.disconnect());
observer.observe('cpu').catch(reject);
});
Expand All @@ -16,9 +16,4 @@ promise_test(async t => {
assert_less_than_equal(update.cpuUtilization, 1.0, 'cpuUtilization range');
assert_in_array(update.cpuUtilization, [0.25, 0.75],
'cpuUtilization quantization');

assert_equals(typeof update.cpuSpeed, 'number');
assert_greater_than_equal(update.cpuSpeed, 0.0, 'cpuSpeed range');
assert_less_than_equal(update.cpuSpeed, 1.0, 'cpuUSpeed range');
assert_in_array(update.cpuSpeed, [0.25, 0.75], 'cpuSpeed quantization');
}, 'An active ComputePressureObserver calls its callback at least once');
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

const observer = new frame_window.ComputePressureObserver(
() => {},
{cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
{cpuUtilizationThresholds: [0.5]});
const iframe_DOMException = frame_window.DOMException;

iframe.remove();
Expand All @@ -40,7 +40,7 @@

const observer = new frame_window.ComputePressureObserver(
() => {},
{cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
{cpuUtilizationThresholds: [0.5]});

await observer.observe('cpu');

Expand All @@ -57,7 +57,7 @@

const observer = new frame_window.ComputePressureObserver(
() => {},
{cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
{cpuUtilizationThresholds: [0.5]});
const iframe_DOMException = frame_window.DOMException;

// await is intentionally not used here. We want to remove the iframe while
Expand All @@ -70,13 +70,12 @@
// call in the removed iframe's ComputePressureObserver.
const update = await new Promise((resolve, reject) => {
const observer = new ComputePressureObserver(
resolve, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
resolve, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer.disconnect());
observer.observe('cpu').catch(reject);
});
assert_in_array(update.cpuUtilization, [0.25, 0.75],
'cpuUtilization quantization');
assert_in_array(update.cpuSpeed, [0.25, 0.75], 'cpuSpeed quantization')
}, 'Detaching frame while ComputePressureObserver.observe() settles');

</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ promise_test(async t => {
const observer1_updates = [];
const observer1 = new ComputePressureObserver(
update => { observer1_updates.push(update); },
{cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
{cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer1.disconnect());
// Ensure that observer1's quantization scheme gets registered as the frame's
// scheme before observer2 starts.
Expand All @@ -17,7 +17,7 @@ promise_test(async t => {
observer2_updates.push(update);
resolve();
},
{cpuUtilizationThresholds: [0.25], cpuSpeedThresholds: [0.75]});
{cpuUtilizationThresholds: [0.25]});
t.add_cleanup(() => observer2.disconnect());
observer2.observe('cpu').catch(reject);
});
Expand All @@ -27,7 +27,7 @@ promise_test(async t => {
//
// The check below assumes that observer2.observe() completes before the
// browser dispatches any update for observer1. This assumption is highly
// likely to be true, because there shold be a 1-second delay between
// likely to be true, because there should be a 1-second delay between
// observer1.observe() and the first update that observer1 would receive.
assert_equals(
observer1_updates.length, 0,
Expand All @@ -37,8 +37,6 @@ promise_test(async t => {
assert_equals(observer2_updates.length, 1);
assert_in_array(observer2_updates[0].cpuUtilization, [0.125, 0.625],
'cpuUtilization quantization');
assert_in_array(observer2_updates[0].cpuSpeed, [0.375, 0.875],
'cpuSpeed quantization');

// Go through one more update cycle so any (incorrect) update for observer1
// makes it through the IPC queues.
Expand All @@ -52,7 +50,7 @@ promise_test(async t => {
observer3_updates.push(update);
resolve();
},
{cpuUtilizationThresholds: [0.75], cpuSpeedThresholds: [0.25]});
{cpuUtilizationThresholds: [0.75]});
t.add_cleanup(() => observer3.disconnect());
observer3.observe('cpu').catch(reject);
});
Expand All @@ -72,8 +70,5 @@ promise_test(async t => {
assert_equals(observer3_updates.length, 1);
assert_in_array(observer3_updates[0].cpuUtilization, [0.375, 0.875],
'cpuUtilization quantization');
assert_in_array(observer3_updates[0].cpuSpeed, [0.125, 0.625],
'cpuSpeed quantization');

}, 'ComputePressureObserver with a new quantization schema stops all ' +
'other active observers in the same frame');
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ promise_test(async t => {
const observer1_updates = [];
const observer1 = new ComputePressureObserver(
update => { observer1_updates.push(update); },
{cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
{cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer1.disconnect());
// Ensure that observer1's quantization scheme gets registered as the origin's
// scheme before observer2 starts.
Expand All @@ -22,7 +22,7 @@ promise_test(async t => {
observer2_updates.push(update);
resolve();
},
{cpuUtilizationThresholds: [0.25], cpuSpeedThresholds: [0.75]});
{cpuUtilizationThresholds: [0.25]});
t.add_cleanup(() => observer2.disconnect());
observer2.observe('cpu').catch(reject);
});
Expand All @@ -32,7 +32,7 @@ promise_test(async t => {
//
// The check below assumes that observer2.observe() completes before the
// browser dispatches any update for observer1. This assumption is highly
// likely to be true, because there shold be a 1-second delay between
// likely to be true, because there should be a 1-second delay between
// observer1.observe() and the first update that observer1 would receive.
assert_equals(
observer1_updates.length, 0,
Expand All @@ -42,8 +42,6 @@ promise_test(async t => {
assert_equals(observer2_updates.length, 1);
assert_in_array(observer2_updates[0].cpuUtilization, [0.125, 0.625],
'cpuUtilization quantization');
assert_in_array(observer2_updates[0].cpuSpeed, [0.375, 0.875],
'cpuSpeed quantization');

// Go through one more update cycle so any (incorrect) update for observer1
// makes it through the IPC queues.
Expand All @@ -60,7 +58,7 @@ promise_test(async t => {
observer3_updates.push(update);
resolve();
},
{cpuUtilizationThresholds: [0.75], cpuSpeedThresholds: [0.25]});
{cpuUtilizationThresholds: [0.75]});
t.add_cleanup(() => observer3.disconnect());
observer3.observe('cpu').catch(reject);
});
Expand All @@ -80,8 +78,5 @@ promise_test(async t => {
assert_equals(observer3_updates.length, 1);
assert_in_array(observer3_updates[0].cpuUtilization, [0.375, 0.875],
'cpuUtilization quantization');
assert_in_array(observer3_updates[0].cpuSpeed, [0.125, 0.625],
'cpuSpeed quantization');

}, 'ComputePressureObserver with a new quantization schema stops all ' +
'other active observers');
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ promise_test(async t => {
const observer1_updates = [];
const observer1 = new ComputePressureObserver(update => {
observer1_updates.push(update);
}, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
}, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer1.disconnect());
// Ensure that observer1's schema gets registered before observer2 starts.
await observer1.observe('cpu');
Expand All @@ -15,7 +15,7 @@ promise_test(async t => {
const observer2 = new ComputePressureObserver(update => {
observer2_updates.push(update);
resolve();
}, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
}, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer2.disconnect());
observer2.observe('cpu').catch(reject);
});
Expand All @@ -28,8 +28,6 @@ promise_test(async t => {
assert_in_array(
observer2_updates[0].cpuUtilization, [0.25, 0.75],
'cpuUtilization quantization');
assert_in_array(
observer2_updates[0].cpuSpeed, [0.25, 0.75], 'cpuSpeed quantization');

// Go through one more update cycle so any (incorrect) update for observer1
// makes it through the IPC queues.
Expand All @@ -39,7 +37,7 @@ promise_test(async t => {
const observer3 = new ComputePressureObserver(update => {
observer3_updates.push(update);
resolve();
}, {cpuUtilizationThresholds: [0.75], cpuSpeedThresholds: [0.25]});
}, {cpuUtilizationThresholds: [0.75]});
t.add_cleanup(() => observer3.disconnect());
observer3.observe('cpu').catch(reject);
});
Expand All @@ -52,6 +50,4 @@ promise_test(async t => {
assert_in_array(
observer3_updates[0].cpuUtilization, [0.375, 0.875],
'cpuUtilization quantization');
assert_in_array(
observer3_updates[0].cpuSpeed, [0.125, 0.625], 'cpuSpeed quantization');
}, 'Stopped ComputePressureObservers do not receive updates');
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ promise_test(async t => {
const observer1_updates = [];
const observer1 = new ComputePressureObserver(update => {
observer1_updates.push(update);
}, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
}, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer1.disconnect());
// Ensure that observer1's schema gets registered before observer2 starts.
observer1.observe('cpu');
Expand All @@ -15,7 +15,7 @@ promise_test(async t => {
const observer2 = new ComputePressureObserver(update => {
observer2_updates.push(update);
resolve();
}, {cpuUtilizationThresholds: [0.5], cpuSpeedThresholds: [0.5]});
}, {cpuUtilizationThresholds: [0.5]});
t.add_cleanup(() => observer2.disconnect());
observer2.observe('cpu').catch(reject);
});
Expand All @@ -28,8 +28,6 @@ promise_test(async t => {
assert_in_array(
observer2_updates[0].cpuUtilization, [0.25, 0.75],
'cpuUtilization quantization');
assert_in_array(
observer2_updates[0].cpuSpeed, [0.25, 0.75], 'cpuSpeed quantization');

// Go through one more update cycle so any (incorrect) update for observer1
// makes it through the IPC queues.
Expand All @@ -39,7 +37,7 @@ promise_test(async t => {
const observer3 = new ComputePressureObserver(update => {
observer3_updates.push(update);
resolve();
}, {cpuUtilizationThresholds: [0.75], cpuSpeedThresholds: [0.25]});
}, {cpuUtilizationThresholds: [0.75]});
t.add_cleanup(() => observer3.disconnect());
observer3.observe('cpu').catch(reject);
});
Expand All @@ -52,6 +50,4 @@ promise_test(async t => {
assert_in_array(
observer3_updates[0].cpuUtilization, [0.375, 0.875],
'cpuUtilization quantization');
assert_in_array(
observer3_updates[0].cpuSpeed, [0.125, 0.625], 'cpuSpeed quantization');
}, 'Stopped ComputePressureObservers do not receive updates');
Loading

0 comments on commit 7901c13

Please sign in to comment.