Skip to content

Commit

Permalink
Change GPU watchdog timeout init factor from 2 to 1 on Windows
Browse files Browse the repository at this point in the history
When we first set the GPU watchdog timeout length for the gpu
initialization on Windows, we doubled the timeout length due to some
slow library loads on Windows. Now we pause the watchdog before
loading libraries. We should be able to switch back to the normal
timeout length.

The current histogram shows we have far less watchdog
kills in GPU initialization than in normal work flow. That's a
hint we can make the timeout shorter for the GPU initialization.

Add inlining constexpr variable definitions in headers outside of
classes to avoid unnecessary copies of the constant. See
https://abseil.io/tips/168 for more details.

Change-Id: I3a8dccebeec5dfe828906682637e8b55beb87f45
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4956709
Reviewed-by: Zhenyao Mo <zmo@chromium.org>
Auto-Submit: Maggie Chen <magchen@chromium.org>
Commit-Queue: Maggie Chen <magchen@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1213096}
  • Loading branch information
Maggie Chen authored and Chromium LUCI CQ committed Oct 21, 2023
1 parent d9e12eb commit 27c2c18
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 33 deletions.
12 changes: 2 additions & 10 deletions gpu/ipc/common/gpu_watchdog_timeout.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,10 @@ constexpr base::TimeDelta kGpuWatchdogTimeout = base::Seconds(15);
// It usually takes longer to finish a GPU task when the system just resumes
// from power suspension or when the Android app switches from the background to
// the foreground. This is the factor the original timeout will be multiplied.
constexpr int kRestartFactor = 2;

// It takes longer to initialize GPU process in Windows. See
// https://crbug.com/949839 for details.
#if BUILDFLAG(IS_WIN)
constexpr int kInitFactor = 2;
#else
constexpr int kInitFactor = 1;
#endif
inline constexpr int kRestartFactor = 2;

// Software rasterizer runs slower than hardware accelerated.
constexpr int kSoftwareRenderingFactor = 2;
inline constexpr int kSoftwareRenderingFactor = 2;

} // namespace gpu

Expand Down
17 changes: 6 additions & 11 deletions gpu/ipc/service/gpu_watchdog_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -66,13 +66,11 @@ base::TimeDelta GetGpuWatchdogTimeout(bool software_rendering) {
}

GpuWatchdogThread::GpuWatchdogThread(base::TimeDelta timeout,
int init_factor,
int restart_factor,
bool is_test_mode,
const std::string& thread_name)
: base::Thread(thread_name),
watchdog_timeout_(timeout),
watchdog_init_factor_(init_factor),
watchdog_restart_factor_(restart_factor),
is_test_mode_(is_test_mode) {
base::CurrentThread::Get()->AddTaskObserver(this);
Expand Down Expand Up @@ -141,12 +139,11 @@ GpuWatchdogThread::~GpuWatchdogThread() {
std::unique_ptr<GpuWatchdogThread> GpuWatchdogThread::Create(
bool start_backgrounded,
base::TimeDelta timeout,
int init_factor,
int restart_factor,
bool is_test_mode,
const std::string& thread_name) {
auto watchdog_thread = base::WrapUnique(new GpuWatchdogThread(
timeout, init_factor, restart_factor, is_test_mode, thread_name));
timeout, restart_factor, is_test_mode, thread_name));
watchdog_thread->Start();
if (start_backgrounded)
watchdog_thread->OnBackgrounded();
Expand All @@ -159,7 +156,7 @@ std::unique_ptr<GpuWatchdogThread> GpuWatchdogThread::Create(
bool software_rendering,
const std::string& thread_name) {
return Create(start_backgrounded, GetGpuWatchdogTimeout(software_rendering),
kInitFactor, kRestartFactor, /*test_mode=*/false, thread_name);
kRestartFactor, /*test_mode=*/false, thread_name);
}

// static
Expand All @@ -169,7 +166,6 @@ std::unique_ptr<GpuWatchdogThread> GpuWatchdogThread::Create(
const std::string& thread_name) {
DCHECK(existing_watchdog);
return Create(start_backgrounded, existing_watchdog->watchdog_timeout_,
existing_watchdog->watchdog_init_factor_,
existing_watchdog->watchdog_restart_factor_,
/*test_mode=*/false, thread_name);
}
Expand Down Expand Up @@ -288,24 +284,23 @@ void GpuWatchdogThread::DisableReportOnlyMode() {
void GpuWatchdogThread::Init() {
// Get and Invalidate weak_ptr should be done on the watchdog thread only.
weak_ptr_ = weak_factory_.GetWeakPtr();
base::TimeDelta timeout = watchdog_timeout_ * kInitFactor;
task_runner()->PostDelayedTask(
FROM_HERE,
base::BindOnce(&GpuWatchdogThread::OnWatchdogTimeout, weak_ptr_),
timeout);
watchdog_timeout_);

last_arm_disarm_counter_ = ReadArmDisarmCounter();
watchdog_start_timeticks_ = base::TimeTicks::Now();
last_on_watchdog_timeout_timeticks_ = watchdog_start_timeticks_;
next_on_watchdog_timeout_time_ = base::Time::Now() + timeout;
next_on_watchdog_timeout_time_ = base::Time::Now() + watchdog_timeout_;
in_gpu_initialization_ = true;

#if BUILDFLAG(IS_WIN)
if (watched_thread_handle_) {
if (base::ThreadTicks::IsSupported())
base::ThreadTicks::WaitUntilInitialized();
last_on_watchdog_timeout_thread_ticks_ = GetWatchedThreadTime();
remaining_watched_thread_ticks_ = timeout;
remaining_watched_thread_ticks_ = watchdog_timeout_;
}
#endif
}
Expand Down Expand Up @@ -398,7 +393,7 @@ void GpuWatchdogThread::RestartWatchdogTimeoutTask(
if (!is_paused_)
return;
is_paused_ = false;
timeout = watchdog_timeout_ * watchdog_init_factor_;
timeout = watchdog_timeout_;
watchdog_resume_timeticks_ = base::TimeTicks::Now();
break;
}
Expand Down
5 changes: 0 additions & 5 deletions gpu/ipc/service/gpu_watchdog_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,6 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThread
static std::unique_ptr<GpuWatchdogThread> Create(
bool start_backgrounded,
base::TimeDelta timeout,
int init_factor,
int restart_factor,
bool test_mode,
const std::string& thread_name);
Expand Down Expand Up @@ -177,7 +176,6 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThread
};

GpuWatchdogThread(base::TimeDelta timeout,
int init_factor,
int restart_factor,
bool test_mode,
const std::string& thread_name);
Expand Down Expand Up @@ -235,9 +233,6 @@ class GPU_IPC_SERVICE_EXPORT GpuWatchdogThread
// Timeout on the watchdog thread to check if gpu hangs.
base::TimeDelta watchdog_timeout_;

// The one-time watchdog timeout multiplier in the gpu initialization.
const int watchdog_init_factor_;

// The one-time watchdog timeout multiplier after the watchdog pauses and
// restarts.
const int watchdog_restart_factor_;
Expand Down
10 changes: 3 additions & 7 deletions gpu/ipc/service/gpu_watchdog_thread_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,6 @@ void GpuWatchdogTest::SetUp() {
watchdog_thread_ = gpu::GpuWatchdogThread::Create(
/*start_backgrounded=*/false,
/*timeout=*/timeout_,
/*init_factor=*/kInitFactor,
/*restart_factor=*/kRestartFactor,
/*test_mode=*/true, /*thread_name=*/"GpuWatchdog");
}
Expand Down Expand Up @@ -251,8 +250,7 @@ TEST_F(GpuWatchdogTest, GpuInitializationComplete) {

// GPU Hang In Initialization.
TEST_F(GpuWatchdogTest, GpuInitializationHang) {
auto allowed_time =
timeout_ * (kInitFactor + 1) + full_thread_time_on_windows_;
auto allowed_time = timeout_ * 2 + full_thread_time_on_windows_;

// GPU init takes longer than timeout.
SimpleTask(allowed_time, /*extra_time=*/extra_gpu_job_time_);
Expand All @@ -265,8 +263,7 @@ TEST_F(GpuWatchdogTest, GpuInitializationHang) {

// GPU Hang In Initialization.
TEST_F(GpuWatchdogTest, GpuInitializationHangWithReportOnly) {
auto allowed_time =
timeout_ * (kInitFactor + 1) + full_thread_time_on_windows_;
auto allowed_time = timeout_ * 2 + full_thread_time_on_windows_;

watchdog_thread_->EnableReportOnlyMode();

Expand Down Expand Up @@ -410,8 +407,7 @@ TEST_F(GpuWatchdogTest, GpuInitializationPause) {
watchdog_thread_->ResumeWatchdog();

// The Gpu init continues for longer than allowed init time.
auto allowed_time =
timeout_ * (kInitFactor + 1) + full_thread_time_on_windows_;
auto allowed_time = timeout_ * 2 + full_thread_time_on_windows_;

SimpleTask(allowed_time, /*extra_time=*/extra_gpu_job_time_);

Expand Down

0 comments on commit 27c2c18

Please sign in to comment.