Skip to content

Commit

Permalink
Disable GPU watchdog while backgrounded
Browse files Browse the repository at this point in the history
Android OS may deschedule Chrome more aggressively when the app is
backgrounded. This leads to longer task execution times and likely causes
spurious watchdog timeouts.

This change forwards visibility messages from the Browser process to the
GPU process, disabling the GPU watchdog when we are backgrounded.

Bug: 836986
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ic209d463c6433e6ffcc8bff06c811272513e66c8
Reviewed-on: https://chromium-review.googlesource.com/1042531
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Eric Karl <ericrk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#559112}
  • Loading branch information
Eric Karl authored and Commit Bot committed May 16, 2018
1 parent bdba608 commit 033cd99
Show file tree
Hide file tree
Showing 9 changed files with 332 additions and 25 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,12 @@ class TestGpuService : public mojom::GpuService {

void DestroyAllChannels() override {}

void OnBackgroundCleanup() override {}

void OnBackgrounded() override {}

void OnForegrounded() override {}

void Crash() override {}

void Hang() override {}
Expand Down
15 changes: 13 additions & 2 deletions components/viz/service/gl/gpu_service_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -720,12 +720,13 @@ void GpuServiceImpl::DestroyAllChannels() {
gpu_channel_manager_->DestroyAllChannels();
}

void GpuServiceImpl::OnBackgrounded() {
void GpuServiceImpl::OnBackgroundCleanup() {
// Currently only called on Android.
#if defined(OS_ANDROID)
if (io_runner_->BelongsToCurrentThread()) {
main_runner_->PostTask(
FROM_HERE, base::BindOnce(&GpuServiceImpl::OnBackgrounded, weak_ptr_));
FROM_HERE,
base::BindOnce(&GpuServiceImpl::OnBackgroundCleanup, weak_ptr_));
return;
}
DVLOG(1) << "GPU: Performing background cleanup";
Expand All @@ -735,6 +736,16 @@ void GpuServiceImpl::OnBackgrounded() {
#endif
}

void GpuServiceImpl::OnBackgrounded() {
if (watchdog_thread_)
watchdog_thread_->OnBackgrounded();
}

void GpuServiceImpl::OnForegrounded() {
if (watchdog_thread_)
watchdog_thread_->OnForegrounded();
}

void GpuServiceImpl::Crash() {
DCHECK(io_runner_->BelongsToCurrentThread());
DVLOG(1) << "GPU: Simulating GPU crash";
Expand Down
2 changes: 2 additions & 0 deletions components/viz/service/gl/gpu_service_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,9 @@ class VIZ_SERVICE_EXPORT GpuServiceImpl : public gpu::GpuChannelManagerDelegate,
void WakeUpGpu() override;
void GpuSwitched() override;
void DestroyAllChannels() override;
void OnBackgroundCleanup() override;
void OnBackgrounded() override;
void OnForegrounded() override;
void Crash() override;
void Hang() override;
void ThrowJavaException() override;
Expand Down
33 changes: 30 additions & 3 deletions content/browser/renderer_host/compositor_impl_android.cc
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,28 @@ class VulkanOutputSurface : public viz::OutputSurface {
};
#endif

void SendOnBackgroundedToGpuService() {
content::GpuProcessHost::CallOnIO(
content::GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
false /* force_create */,
base::BindRepeating([](content::GpuProcessHost* host) {
if (host) {
host->gpu_service()->OnBackgrounded();
}
}));
}

void SendOnForegroundedToGpuService() {
content::GpuProcessHost::CallOnIO(
content::GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
false /* force_create */,
base::BindRepeating([](content::GpuProcessHost* host) {
if (host) {
host->gpu_service()->OnForegrounded();
}
}));
}

static bool g_initialized = false;

} // anonymous namespace
Expand Down Expand Up @@ -660,12 +682,14 @@ void CompositorImpl::SetVisible(bool visible) {
root_window_->GetBeginFrameSource());
}
display_.reset();
SendOnBackgroundedToGpuService();
EnqueueLowEndBackgroundCleanup();
} else {
host_->SetVisible(true);
has_submitted_frame_since_became_visible_ = false;
if (layer_tree_frame_sink_request_pending_)
HandlePendingLayerTreeFrameSinkRequest();
SendOnForegroundedToGpuService();
low_end_background_cleanup_task_.Cancel();
}
}
Expand Down Expand Up @@ -784,9 +808,12 @@ void CompositorImpl::OnGpuChannelEstablished(
return;
}

// We don't need the context anymore if we are invisible.
if (!host_->IsVisible())
// We don't need the context anymore if we are invisible. Additionally, we
// should notify the channel that we are invisible.
if (!host_->IsVisible()) {
SendOnBackgroundedToGpuService();
return;
}

DCHECK(window_);
DCHECK_NE(surface_handle_, gpu::kNullSurfaceHandle);
Expand Down Expand Up @@ -1041,7 +1068,7 @@ void CompositorImpl::DoLowEndBackgroundCleanup() {
false /* force_create */,
base::BindRepeating([](content::GpuProcessHost* host) {
if (host) {
host->gpu_service()->OnBackgrounded();
host->gpu_service()->OnBackgroundCleanup();
}
}));
}
Expand Down
1 change: 1 addition & 0 deletions gpu/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,7 @@ test("gpu_unittests") {
"ipc/service/gpu_channel_test_common.cc",
"ipc/service/gpu_channel_test_common.h",
"ipc/service/gpu_channel_unittest.cc",
"ipc/service/gpu_watchdog_thread_unittest.cc",
]

if (is_mac) {
Expand Down
131 changes: 115 additions & 16 deletions gpu/ipc/service/gpu_watchdog_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ GpuWatchdogThread::GpuWatchdogThread()
watched_thread_handle_(0),
arm_cpu_time_(),
#endif
suspended_(false),
suspension_counter_(this),
#if defined(USE_X11)
display_(NULL),
window_(0),
Expand Down Expand Up @@ -120,6 +120,24 @@ void GpuWatchdogThread::ReportProgress() {
CheckArmed();
}

void GpuWatchdogThread::OnBackgrounded() {
// As we stop the task runner before destroying this class, the unretained
// reference will always outlive the task.
task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&GpuWatchdogThread::OnBackgroundedOnWatchdogThread,
base::Unretained(this)));
}

void GpuWatchdogThread::OnForegrounded() {
// As we stop the task runner before destroying this class, the unretained
// reference will always outlive the task.
task_runner()->PostTask(
FROM_HERE,
base::BindOnce(&GpuWatchdogThread::OnForegroundedOnWatchdogThread,
base::Unretained(this)));
}

void GpuWatchdogThread::Init() {
// Schedule the first check.
OnCheck(false);
Expand All @@ -144,8 +162,62 @@ void GpuWatchdogThread::GpuWatchdogTaskObserver::WillProcessTask(
void GpuWatchdogThread::GpuWatchdogTaskObserver::DidProcessTask(
const base::PendingTask& pending_task) {}

GpuWatchdogThread::SuspensionCounter::SuspensionCounterRef::
SuspensionCounterRef(SuspensionCounter* counter)
: counter_(counter) {
counter_->OnAddRef();
}

GpuWatchdogThread::SuspensionCounter::SuspensionCounterRef::
~SuspensionCounterRef() {
counter_->OnReleaseRef();
}

GpuWatchdogThread::SuspensionCounter::SuspensionCounter(
GpuWatchdogThread* watchdog_thread)
: watchdog_thread_(watchdog_thread) {
// This class will only be used on the watchdog thread, but is constructed on
// the main thread. Detach.
DETACH_FROM_SEQUENCE(watchdog_thread_sequence_checker_);
}

std::unique_ptr<GpuWatchdogThread::SuspensionCounter::SuspensionCounterRef>
GpuWatchdogThread::SuspensionCounter::Take() {
DCHECK_CALLED_ON_VALID_SEQUENCE(watchdog_thread_sequence_checker_);
return std::make_unique<SuspensionCounterRef>(this);
}

bool GpuWatchdogThread::SuspensionCounter::HasRefs() const {
DCHECK_CALLED_ON_VALID_SEQUENCE(watchdog_thread_sequence_checker_);
return suspend_count_ > 0;
}

void GpuWatchdogThread::SuspensionCounter::OnWatchdogThreadStopped() {
DETACH_FROM_SEQUENCE(watchdog_thread_sequence_checker_);

// Null the |watchdog_thread_| ptr at shutdown to avoid trying to suspend or
// resume after the thread is stopped.
watchdog_thread_ = nullptr;
}

void GpuWatchdogThread::SuspensionCounter::OnAddRef() {
DCHECK_CALLED_ON_VALID_SEQUENCE(watchdog_thread_sequence_checker_);
suspend_count_++;
if (watchdog_thread_ && suspend_count_ == 1)
watchdog_thread_->SuspendStateChanged();
}

void GpuWatchdogThread::SuspensionCounter::OnReleaseRef() {
DCHECK_CALLED_ON_VALID_SEQUENCE(watchdog_thread_sequence_checker_);
DCHECK_GT(suspend_count_, 0u);
suspend_count_--;
if (watchdog_thread_ && suspend_count_ == 0)
watchdog_thread_->SuspendStateChanged();
}

GpuWatchdogThread::~GpuWatchdogThread() {
Stop();
suspension_counter_.OnWatchdogThreadStopped();

#if defined(OS_WIN)
CloseHandle(watched_thread_handle_);
Expand Down Expand Up @@ -182,7 +254,7 @@ void GpuWatchdogThread::OnAcknowledge() {
weak_factory_.InvalidateWeakPtrs();
armed_ = false;

if (suspended_) {
if (suspension_counter_.HasRefs()) {
responsive_acknowledge_count_ = 0;
return;
}
Expand Down Expand Up @@ -219,7 +291,7 @@ void GpuWatchdogThread::OnCheck(bool after_suspend) {

// Do not create any new termination tasks if one has already been created
// or the system is suspended.
if (armed_ || suspended_)
if (armed_ || suspension_counter_.HasRefs())
return;

armed_ = true;
Expand Down Expand Up @@ -258,7 +330,7 @@ void GpuWatchdogThread::OnCheck(bool after_suspend) {

void GpuWatchdogThread::OnCheckTimeout() {
// Should not get here while the system is suspended.
DCHECK(!suspended_);
DCHECK(!suspension_counter_.HasRefs());

// If the watchdog woke up significantly behind schedule, disarm and reset
// the watchdog check. This is to prevent the watchdog thread from terminating
Expand Down Expand Up @@ -296,7 +368,12 @@ void GpuWatchdogThread::OnCheckTimeout() {
// Use the --disable-gpu-watchdog command line switch to disable this.
void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
// Should not get here while the system is suspended.
DCHECK(!suspended_);
DCHECK(!suspension_counter_.HasRefs());

if (alternative_terminate_for_testing_) {
alternative_terminate_for_testing_.Run();
return;
}

#if defined(OS_WIN)
// Defer termination until a certain amount of CPU time has elapsed on the
Expand Down Expand Up @@ -469,21 +546,34 @@ void GpuWatchdogThread::OnAddPowerObserver() {
}

void GpuWatchdogThread::OnSuspend() {
suspended_ = true;
suspend_time_ = base::Time::Now();

// When suspending force an acknowledgement to cancel any pending termination
// tasks.
OnAcknowledge();
power_suspend_ref_ = suspension_counter_.Take();
}

void GpuWatchdogThread::OnResume() {
suspended_ = false;
resume_time_ = base::Time::Now();
power_suspend_ref_.reset();
}

// After resuming jump-start the watchdog again.
armed_ = false;
OnCheck(true);
void GpuWatchdogThread::OnBackgroundedOnWatchdogThread() {
background_suspend_ref_ = suspension_counter_.Take();
}

void GpuWatchdogThread::OnForegroundedOnWatchdogThread() {
background_suspend_ref_.reset();
}

void GpuWatchdogThread::SuspendStateChanged() {
if (suspension_counter_.HasRefs()) {
suspend_time_ = base::Time::Now();
// When suspending force an acknowledgement to cancel any pending
// termination tasks.
OnAcknowledge();
} else {
resume_time_ = base::Time::Now();

// After resuming jump-start the watchdog again.
armed_ = false;
OnCheck(true);
}
}

#if defined(OS_WIN)
Expand Down Expand Up @@ -538,4 +628,13 @@ int GpuWatchdogThread::GetActiveTTY() const {
}
#endif

void GpuWatchdogThread::SetAlternativeTerminateFunctionForTesting(
base::RepeatingClosure on_terminate) {
alternative_terminate_for_testing_ = std::move(on_terminate);
}

void GpuWatchdogThread::SetTimeoutForTesting(base::TimeDelta timeout) {
timeout_ = timeout;
}

} // namespace gpu
Loading

0 comments on commit 033cd99

Please sign in to comment.