Skip to content

Commit

Permalink
Scheduler: Filter out continuous input for kPrioritizeCompositingAfte…
Browse files Browse the repository at this point in the history 
…rInput

Continuous input, when rAF-algined, hops through the input task queue
before ultimately running in rAF. This means we need to do more
filtering in the scheduler to only prioritize rendering after discrete
inputs. This CL adds that filtering.

This also moves the kPrioritizeCompositingAfterInput from
OnTaskComplete() to
MaybeUpdateCompositorTaskQueuePriorityOnTaskCompleted(), consolidating
the logic to update compositor priority after a task runs.

(cherry picked from commit c528d65)

Bug: 853771, 1068426
Change-Id: Ifd3d00440688b015015d7d20320de330a70db376
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3641430
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Reviewed-by: Minoru Chikamune <chikamune@chromium.org>
Commit-Queue: Scott Haseley <shaseley@chromium.org>
Reviewed-by: Dave Tapuska <dtapuska@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1002987}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3648579
Auto-Submit: Scott Haseley <shaseley@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Cr-Commit-Position: refs/branch-heads/5060@{#23}
Cr-Branched-From: b83393d-refs/heads/main@{#1002911}
  • Loading branch information
@shaseley
shaseley authored and Chromium LUCI CQ committed May 16, 2022
1 parent 8480ea6 commit d52adbc
Show file tree
Hide file tree
Showing 3 changed files with 127 additions and 44 deletions.
48 changes: 23 additions & 25 deletions third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,14 @@
#include "services/metrics/public/cpp/ukm_builders.h"
#include "third_party/abseil-cpp/absl/types/optional.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/common/input/web_input_event.h"
#include "third_party/blink/public/common/input/web_input_event_attribution.h"
#include "third_party/blink/public/common/input/web_mouse_wheel_event.h"
#include "third_party/blink/public/common/input/web_touch_event.h"
#include "third_party/blink/public/common/page/launching_process_state.h"
#include "third_party/blink/public/platform/scheduler/web_agent_group_scheduler.h"
#include "third_party/blink/public/platform/scheduler/web_renderer_process_type.h"
#include "third_party/blink/public/platform/web_input_event_result.h"
#include "third_party/blink/renderer/platform/instrumentation/resource_coordinator/renderer_resource_coordinator.h"
#include "third_party/blink/renderer/platform/scheduler/common/features.h"
#include "third_party/blink/renderer/platform/scheduler/common/process_state.h"
Expand All @@ -48,6 +50,7 @@
#include "third_party/blink/renderer/platform/scheduler/main_thread/frame_scheduler_impl.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/main_thread.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/page_scheduler_impl.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/pending_user_input.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/task_type_names.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/widget_scheduler.h"
#include "third_party/blink/renderer/platform/scheduler/public/event_loop.h"
Expand Down Expand Up @@ -870,9 +873,6 @@ void MainThreadSchedulerImpl::RemoveTaskObserver(
}

void MainThreadSchedulerImpl::WillBeginFrame(const viz::BeginFrameArgs& args) {
// TODO(crbug/1068426): Figure out when and if |UpdatePolicy| should be called
// and, if needed, remove the call to it from
// |SetPrioritizeCompositingAfterInput|.
TRACE_EVENT1(TRACE_DISABLED_BY_DEFAULT("renderer.scheduler"),
"MainThreadSchedulerImpl::WillBeginFrame", "args",
args.AsValue());
Expand All @@ -888,7 +888,6 @@ void MainThreadSchedulerImpl::WillBeginFrame(const viz::BeginFrameArgs& args) {
base::AutoLock lock(any_thread_lock_);
any_thread().begin_main_frame_on_critical_path = args.on_critical_path;
}
SetPrioritizeCompositingAfterInput(false);
main_thread_only().have_seen_a_frame = true;
}

Expand Down Expand Up @@ -1338,6 +1337,11 @@ void MainThreadSchedulerImpl::DidHandleInputEventOnMainThread(
UpdatePolicyLocked(UpdateType::kMayEarlyOutIfPolicyUnchanged);
}
}
if (result != WebInputEventResult::kNotHandled &&
result != WebInputEventResult::kHandledSuppressed &&
!PendingUserInput::IsContinuousEventType(web_input_event.GetType())) {
main_thread_only().did_handle_discrete_input_event = true;
}
}

bool MainThreadSchedulerImpl::IsHighPriorityWorkAnticipated() {
Expand Down Expand Up @@ -2527,14 +2531,6 @@ void MainThreadSchedulerImpl::OnTaskCompleted(

RecordTaskUkm(queue.get(), task, *task_timing);

// Assume this input will result in a frame, which we want to show ASAP.
if (queue &&
queue->GetPrioritisationType() ==
MainThreadTaskQueue::QueueTraits::PrioritisationType::kInput) {
SetPrioritizeCompositingAfterInput(
scheduling_settings().prioritize_compositing_after_input);
}

MaybeUpdateCompositorTaskQueuePriorityOnTaskCompleted(queue.get(),
*task_timing);

Expand Down Expand Up @@ -2729,17 +2725,6 @@ MainThreadSchedulerImpl::scheduling_settings() const {
return scheduling_settings_;
}

void MainThreadSchedulerImpl::SetPrioritizeCompositingAfterInput(
bool prioritize_compositing_after_input) {
if (main_thread_only().prioritize_compositing_after_input ==
prioritize_compositing_after_input) {
return;
}
main_thread_only().prioritize_compositing_after_input =
prioritize_compositing_after_input;
UpdateCompositorTaskQueuePriority();
}

TaskQueue::QueuePriority MainThreadSchedulerImpl::ComputeCompositorPriority()
const {
if (main_thread_only().prioritize_compositing_after_input) {
Expand Down Expand Up @@ -2783,7 +2768,9 @@ void MainThreadSchedulerImpl::
MaybeUpdateCompositorTaskQueuePriorityOnTaskCompleted(
MainThreadTaskQueue* queue,
const base::sequence_manager::TaskQueue::TaskTiming& task_timing) {
bool current_should_prioritize_compositor_task_queue =
bool current_prioritize_compositer_after_input =
main_thread_only().prioritize_compositing_after_input;
bool current_prioritize_compositor_after_delay =
main_thread_only().should_prioritize_compositor_task_queue_after_delay;
if (queue &&
queue->queue_type() == MainThreadTaskQueue::QueueType::kCompositor &&
Expand All @@ -2792,14 +2779,25 @@ void MainThreadSchedulerImpl::
main_thread_only().have_seen_a_frame = false;
main_thread_only().should_prioritize_compositor_task_queue_after_delay =
false;
main_thread_only().prioritize_compositing_after_input = false;
} else if (scheduling_settings().prioritize_compositing_after_input &&
queue &&
queue->queue_type() == MainThreadTaskQueue::QueueType::kInput &&
main_thread_only().did_handle_discrete_input_event) {
// Assume this input will result in a frame, which we want to show ASAP.
main_thread_only().prioritize_compositing_after_input = true;
} else if (task_timing.end_time() - main_thread_only().last_frame_time >=
kPrioritizeCompositingAfterDelay) {
main_thread_only().should_prioritize_compositor_task_queue_after_delay =
true;
}

main_thread_only().did_handle_discrete_input_event = false;

if (main_thread_only().should_prioritize_compositor_task_queue_after_delay !=
current_should_prioritize_compositor_task_queue) {
current_prioritize_compositor_after_delay ||
main_thread_only().prioritize_compositing_after_input !=
current_prioritize_compositer_after_input) {
UpdateCompositorTaskQueuePriority();
}
}
Expand Down
8 changes: 5 additions & 3 deletions third_party/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,9 +397,6 @@ class PLATFORM_EXPORT MainThreadSchedulerImpl

const SchedulingSettings& scheduling_settings() const;

void SetPrioritizeCompositingAfterInput(
bool prioritize_compositing_after_input);

base::WeakPtr<MainThreadSchedulerImpl> GetWeakPtr();

base::sequence_manager::TaskQueue::QueuePriority compositor_priority() const {
Expand Down Expand Up @@ -916,6 +913,11 @@ class PLATFORM_EXPORT MainThreadSchedulerImpl
bool should_prioritize_compositor_task_queue_after_delay;
bool have_seen_a_frame;

// Set when a discrete input event is handled on the main thread. This is
// used by the kPrioritizeCompositingAfterInput experiment to determine if
// the next frame should be prioritized.
bool did_handle_discrete_input_event = false;

WTF::Vector<AgentGroupSchedulerScope> agent_group_scheduler_scope_stack;

std::unique_ptr<power_scheduler::PowerModeVoter> audible_power_mode_voter;
Expand Down
115 changes: 99 additions & 16 deletions ...arty/blink/renderer/platform/scheduler/main_thread/main_thread_scheduler_impl_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "base/memory/ptr_util.h"
#include "base/metrics/field_trial_params.h"
#include "base/run_loop.h"
#include "base/strings/string_util.h"
#include "base/task/common/task_annotator.h"
#include "base/task/sequence_manager/test/fake_task.h"
#include "base/task/sequence_manager/test/sequence_manager_for_test.h"
Expand All @@ -35,6 +36,7 @@
#include "third_party/blink/public/common/input/web_touch_event.h"
#include "third_party/blink/public/common/page/launching_process_state.h"
#include "third_party/blink/public/platform/scheduler/web_widget_scheduler.h"
#include "third_party/blink/public/platform/web_input_event_result.h"
#include "third_party/blink/renderer/platform/scheduler/common/features.h"
#include "third_party/blink/renderer/platform/scheduler/common/throttling/budget_pool.h"
#include "third_party/blink/renderer/platform/scheduler/main_thread/auto_advancing_virtual_time_domain.h"
Expand Down Expand Up @@ -811,12 +813,40 @@ class MainThreadSchedulerImplTest : public testing::Test {
base::RunLoop().RunUntilIdle();
}

void AppendToVectorBeginMainFrameTask(Vector<String>* vector, String value) {
DoMainFrame();
AppendToVectorTestTask(vector, value);
}

void AppendToVectorBeginMainFrameTaskWithInput(Vector<String>* vector,
String value) {
scheduler_->DidHandleInputEventOnMainThread(
FakeInputEvent(WebInputEvent::Type::kMouseMove),
WebInputEventResult::kHandledApplication);
AppendToVectorBeginMainFrameTask(vector, value);
}

void AppendToVectorInputEventTask(WebInputEvent::Type event_type,
Vector<String>* vector,
String value) {
scheduler_->DidHandleInputEventOnMainThread(
FakeInputEvent(event_type), WebInputEventResult::kHandledApplication);
AppendToVectorTestTask(vector, value);
}

// Helper for posting several tasks of specific types. |task_descriptor| is a
// string with space delimited task identifiers. The first letter of each
// task identifier specifies the task type:
// task identifier specifies the task type. For 'C' and 'P' types, the second
// letter specifies that type of task to simulate.
// - 'D': Default task
// - 'C': Compositor task
// - "CM": Compositor task that simulates running a main frame
// - "CI": Compositor task that simulates running a main frame with
// rAF-algined input
// - 'P': Input task
// - "PC": Input task that simulates dispatching a continuous input event
// - "PD": Input task that simulates dispatching a discrete input event
// - 'E': Input task that dispatches input events
// - 'L': Loading task
// - 'M': Loading Control task
// - 'I': Idle task
Expand All @@ -836,14 +866,46 @@ class MainThreadSchedulerImplTest : public testing::Test {
String::FromUTF8(task)));
break;
case 'C':
compositor_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AppendToVectorTestTask, run_order,
String::FromUTF8(task)));
if (base::StartsWith(task, "CM")) {
compositor_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&MainThreadSchedulerImplTest::
AppendToVectorBeginMainFrameTask,
base::Unretained(this), run_order,
String::FromUTF8(task)));
} else if (base::StartsWith(task, "CI")) {
compositor_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&MainThreadSchedulerImplTest::
AppendToVectorBeginMainFrameTaskWithInput,
base::Unretained(this), run_order,
String::FromUTF8(task)));
} else {
compositor_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AppendToVectorTestTask, run_order,
String::FromUTF8(task)));
}
break;
case 'P':
input_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AppendToVectorTestTask, run_order,
String::FromUTF8(task)));
if (base::StartsWith(task, "PC")) {
input_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
&MainThreadSchedulerImplTest::AppendToVectorInputEventTask,
base::Unretained(this), WebInputEvent::Type::kMouseMove,
run_order, String::FromUTF8(task)));

} else if (base::StartsWith(task, "PD")) {
input_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(
&MainThreadSchedulerImplTest::AppendToVectorInputEventTask,
base::Unretained(this), WebInputEvent::Type::kMouseUp,
run_order, String::FromUTF8(task)));
} else {
input_task_runner_->PostTask(
FROM_HERE, base::BindOnce(&AppendToVectorTestTask, run_order,
String::FromUTF8(task)));
}
break;
case 'L':
loading_task_queue()->GetTaskRunnerWithDefaultTaskType()->PostTask(
Expand Down Expand Up @@ -3410,25 +3472,46 @@ class MainThreadSchedulerImplWithCompositingAfterInputPrioritizationTest
TEST_F(MainThreadSchedulerImplWithCompositingAfterInputPrioritizationTest,
CompositingAfterInput) {
Vector<String> run_order;
// Input tasks cause compositor tasks to be prioritized until a BeginMainFrame
// runs.

// Input tasks don't cause compositor tasks to be prioritized unless an input
// event was handled.
PostTestTasks(&run_order, "P1 T1 C1 C2");
base::RunLoop().RunUntilIdle();
EXPECT_THAT(run_order, testing::ElementsAre("P1", "C1", "C2", "T1"));
EXPECT_THAT(run_order, testing::ElementsAre("P1", "T1", "C1", "C2"));
run_order.clear();

scheduler_->WillBeginFrame(viz::BeginFrameArgs());

PostTestTasks(&run_order, "T2 C3 C4");
// Tasks with input events cause compositor tasks to be prioritized until a
// BeginMainFrame runs.
PostTestTasks(&run_order, "T1 P1 PD1 C1 C2 CM1 C2 T2 CM2");
base::RunLoop().RunUntilIdle();
EXPECT_THAT(run_order, testing::ElementsAre("T2", "C3", "C4"));
EXPECT_THAT(run_order, testing::ElementsAre("P1", "PD1", "C1", "C2", "CM1",
"T1", "C2", "T2", "CM2"));
run_order.clear();

// Input tasks and compositor tasks will be interleaved because they have the
// same priority.
PostTestTasks(&run_order, "T3 P2 C5 P3 C6");
PostTestTasks(&run_order, "T1 PD1 C1 PD2 C2");
base::RunLoop().RunUntilIdle();
EXPECT_THAT(run_order, testing::ElementsAre("PD1", "C1", "PD2", "C2", "T1"));
run_order.clear();
}

TEST_F(MainThreadSchedulerImplWithCompositingAfterInputPrioritizationTest,
CompositorNotPrioritizedAfterContinuousInput) {
Vector<String> run_order;

// rAF-aligned input should not cause the next frame to be prioritized.
PostTestTasks(&run_order, "P1 T1 CI1 T2 CI2 T3 CM1");
base::RunLoop().RunUntilIdle();
EXPECT_THAT(run_order, testing::ElementsAre("P1", "T1", "CI1", "T2", "CI2",
"T3", "CM1"));
run_order.clear();

// Continuous input that runs outside of rAF should not cause the next frame
// to be prioritized.
PostTestTasks(&run_order, "PC1 T1 CM1 T2 CM2");
base::RunLoop().RunUntilIdle();
EXPECT_THAT(run_order, testing::ElementsAre("P2", "C5", "P3", "C6", "T3"));
EXPECT_THAT(run_order, testing::ElementsAre("PC1", "T1", "CM1", "T2", "CM2"));
run_order.clear();
}

Expand Down

0 comments on commit d52adbc

Please sign in to comment.