Commit 70eceecb authored by skyostil's avatar skyostil Committed by Commit bot

scheduler: Don't block expensive loading tasks during main thread gestures

Previously we were using the presence of main thread input gestures
(e.g., a mouse drag) as a hint that expensive loading and timer tasks
are non-essential. This patch relaxes the policy to allow loading tasks
in this use case to avoid starving loading work for the entire duration
of the gesture, e.g., for loading new content in an infinite scrolling
scenario.

BUG=548606

Review URL: https://codereview.chromium.org/1420773003

Cr-Commit-Position: refs/heads/master@{#357122}
parent 9c1e60d7
...@@ -610,11 +610,13 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) { ...@@ -610,11 +610,13 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) {
} }
Policy new_policy; Policy new_policy;
bool block_expensive_tasks = false; bool block_expensive_loading_tasks = false;
bool block_expensive_timer_tasks = false;
switch (use_case) { switch (use_case) {
case UseCase::COMPOSITOR_GESTURE: case UseCase::COMPOSITOR_GESTURE:
if (touchstart_expected_soon) { if (touchstart_expected_soon) {
block_expensive_tasks = true; block_expensive_loading_tasks = true;
block_expensive_timer_tasks = true;
} else { } else {
// What we really want to do is priorize loading tasks, but that doesn't // What we really want to do is priorize loading tasks, but that doesn't
// seem to be safe. Instead we do that by proxy by deprioritizing // seem to be safe. Instead we do that by proxy by deprioritizing
...@@ -625,20 +627,29 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) { ...@@ -625,20 +627,29 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) {
break; break;
case UseCase::MAIN_THREAD_GESTURE: case UseCase::MAIN_THREAD_GESTURE:
// In main thread gestures we don't have perfect knowledge about which
// things we should be prioritizing. The following is best guess
// heuristic which lets us produce frames quickly but does not prevent
// loading of additional content.
new_policy.compositor_queue_priority = TaskQueue::HIGH_PRIORITY; new_policy.compositor_queue_priority = TaskQueue::HIGH_PRIORITY;
block_expensive_tasks = true; block_expensive_loading_tasks = false;
block_expensive_timer_tasks = true;
break; break;
case UseCase::TOUCHSTART: case UseCase::TOUCHSTART:
new_policy.compositor_queue_priority = TaskQueue::HIGH_PRIORITY; new_policy.compositor_queue_priority = TaskQueue::HIGH_PRIORITY;
new_policy.loading_queue_priority = TaskQueue::DISABLED_PRIORITY; new_policy.loading_queue_priority = TaskQueue::DISABLED_PRIORITY;
new_policy.timer_queue_priority = TaskQueue::DISABLED_PRIORITY; new_policy.timer_queue_priority = TaskQueue::DISABLED_PRIORITY;
block_expensive_tasks = true; // NOTE this is a nop due to the above. // NOTE these are nops due to the above.
block_expensive_loading_tasks = true;
block_expensive_timer_tasks = true;
break; break;
case UseCase::NONE: case UseCase::NONE:
if (touchstart_expected_soon) if (touchstart_expected_soon) {
block_expensive_tasks = true; block_expensive_loading_tasks = true;
block_expensive_timer_tasks = true;
}
break; break;
case UseCase::LOADING: case UseCase::LOADING:
...@@ -651,17 +662,21 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) { ...@@ -651,17 +662,21 @@ void RendererSchedulerImpl::UpdatePolicyLocked(UpdateType update_type) {
} }
// Don't block expensive tasks unless we have actually seen something. // Don't block expensive tasks unless we have actually seen something.
if (!MainThreadOnly().have_seen_a_begin_main_frame) if (!MainThreadOnly().have_seen_a_begin_main_frame) {
block_expensive_tasks = false; block_expensive_loading_tasks = false;
block_expensive_timer_tasks = false;
}
// Don't block expensive tasks if we are expecting a navigation. // Don't block expensive tasks if we are expecting a navigation.
if (MainThreadOnly().navigation_task_expected_count > 0) if (MainThreadOnly().navigation_task_expected_count > 0) {
block_expensive_tasks = false; block_expensive_loading_tasks = false;
block_expensive_timer_tasks = false;
}
if (block_expensive_tasks && loading_tasks_seem_expensive) if (block_expensive_loading_tasks && loading_tasks_seem_expensive)
new_policy.loading_queue_priority = TaskQueue::DISABLED_PRIORITY; new_policy.loading_queue_priority = TaskQueue::DISABLED_PRIORITY;
if ((block_expensive_tasks && timer_tasks_seem_expensive) || if ((block_expensive_timer_tasks && timer_tasks_seem_expensive) ||
MainThreadOnly().timer_queue_suspend_count != 0 || MainThreadOnly().timer_queue_suspend_count != 0 ||
MainThreadOnly().timer_queue_suspended_when_backgrounded) { MainThreadOnly().timer_queue_suspended_when_backgrounded) {
new_policy.timer_queue_priority = TaskQueue::DISABLED_PRIORITY; new_policy.timer_queue_priority = TaskQueue::DISABLED_PRIORITY;
......
...@@ -2150,6 +2150,27 @@ TEST_F( ...@@ -2150,6 +2150,27 @@ TEST_F(
EXPECT_THAT(run_order, testing::ElementsAre(std::string("D1"))); EXPECT_THAT(run_order, testing::ElementsAre(std::string("D1")));
} }
TEST_F(RendererSchedulerImplTest,
ExpensiveLoadingTasksNotBlockedDuringMainThreadGestures) {
std::vector<std::string> run_order;
SimulateExpensiveTasks(loading_task_runner_);
// Loading tasks should not be disabled during main thread user user
// interactions.
PostTestTasks(&run_order, "C1 L1");
// Trigger main_thread_gesture UseCase
WillBeginMainThreadGestureFrame();
RunUntilIdle();
EXPECT_EQ(RendererScheduler::UseCase::MAIN_THREAD_GESTURE, CurrentUseCase());
EXPECT_TRUE(LoadingTasksSeemExpensive());
EXPECT_FALSE(TimerTasksSeemExpensive());
EXPECT_THAT(run_order,
testing::ElementsAre(std::string("C1"), std::string("L1")));
}
TEST_F(RendererSchedulerImplTest, ModeratelyExpensiveTimer_NotBlocked) { TEST_F(RendererSchedulerImplTest, ModeratelyExpensiveTimer_NotBlocked) {
scheduler_->SetHasVisibleRenderWidgetWithTouchHandler(true); scheduler_->SetHasVisibleRenderWidgetWithTouchHandler(true);
for (int i = 0; i < 20; i++) { for (int i = 0; i < 20; i++) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment