Commit 6971ec6d authored by ericrk's avatar ericrk Committed by Commit bot

Throttle GpuWatchdogThread task posting

The GpuWatchdogThread currently allows multiple tasks to disarm the
watchdog to be posted in a row, only stopping posting when the watchdog
is actually disarmed. This is fairly wasteful, as all but the first
task will just early out. This change adjusts the logic so that we only
post one disarm task, at a time, blocking future disarm tasks until
we have successfully disarmed / rearmed the watchdog.

This change also removes the current "volatile bool", and replaces this with a
base::subtle::Atomic32, which makes the expectations regarding synchronization
more explicit.

BUG=612219
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Review-Url: https://codereview.chromium.org/2389633003
Cr-Commit-Position: refs/heads/master@{#422549}
parent eefd038d
......@@ -66,6 +66,8 @@ GpuWatchdogThread::GpuWatchdogThread()
host_tty_(-1),
#endif
weak_factory_(this) {
base::subtle::NoBarrier_Store(&awaiting_acknowledge_, false);
#if defined(OS_WIN)
// GetCurrentThread returns a pseudo-handle that cannot be used by one thread
// to identify another. DuplicateHandle creates a "real" handle that can be
......@@ -100,11 +102,12 @@ void GpuWatchdogThread::PostAcknowledge() {
}
void GpuWatchdogThread::CheckArmed() {
// Acknowledge the watchdog if it has armed itself. The watchdog will not
// change its armed state until it is acknowledged.
if (armed()) {
// If the watchdog is |awaiting_acknowledge_|, reset this variable to false
// and post an acknowledge task now. No barrier is needed as
// |awaiting_acknowledge_| is only ever read from this thread.
if (base::subtle::NoBarrier_CompareAndSwap(&awaiting_acknowledge_, true,
false))
PostAcknowledge();
}
}
void GpuWatchdogThread::Init() {
......@@ -209,11 +212,14 @@ void GpuWatchdogThread::OnCheck(bool after_suspend) {
if (armed_ || suspended_)
return;
// Must set armed before posting the task. This task might be the only task
// that will activate the TaskObserver on the watched thread and it must not
// miss the false -> true transition.
armed_ = true;
// Must set |awaiting_acknowledge_| before posting the task. This task might
// be the only task that will activate the TaskObserver on the watched thread
// and it must not miss the false -> true transition. No barrier is needed
// here, as the PostTask which follows contains a barrier.
base::subtle::NoBarrier_Store(&awaiting_acknowledge_, true);
#if defined(OS_WIN)
arm_cpu_time_ = GetWatchedThreadTime();
......
......@@ -5,6 +5,7 @@
#ifndef GPU_IPC_SERVICE_GPU_WATCHDOG_THREAD_H_
#define GPU_IPC_SERVICE_GPU_WATCHDOG_THREAD_H_
#include "base/atomicops.h"
#include "base/macros.h"
#include "base/memory/ref_counted.h"
#include "base/memory/weak_ptr.h"
......@@ -37,10 +38,7 @@ class GPU_EXPORT GpuWatchdogThread
public:
static scoped_refptr<GpuWatchdogThread> Create();
// Accessible on watched thread but only modified by watchdog thread.
bool armed() const { return armed_; }
void PostAcknowledge();
void CheckArmed();
// Must be called after a PowerMonitor has been created. Can be called from
......@@ -97,9 +95,14 @@ class GPU_EXPORT GpuWatchdogThread
base::MessageLoop* watched_message_loop_;
base::TimeDelta timeout_;
volatile bool armed_;
bool armed_;
GpuWatchdogTaskObserver task_observer_;
// |awaiting_acknowledge_| is only ever read on the watched thread, but may
// be modified on either the watched or watchdog thread. Reads/writes should
// be careful to ensure that appropriate synchronization is used.
base::subtle::Atomic32 awaiting_acknowledge_;
// True if the watchdog should wait for a certain amount of CPU to be used
// before killing the process.
bool use_thread_cpu_time_;
......
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