GPU Watchdog to check I/O before terminating GPU process

content/gpu/gpu_watchdog_thread.cc

Index: content/gpu/gpu_watchdog_thread.cc
diff --git a/content/gpu/gpu_watchdog_thread.cc b/content/gpu/gpu_watchdog_thread.cc
index 68ba816ed8c831850ecc29ca1a7a1b44ba6b327b..1f4045db24b4c97417ea8e5e031c8f110b863ee4 100644
--- a/content/gpu/gpu_watchdog_thread.cc
+++ b/content/gpu/gpu_watchdog_thread.cc
@@ -12,6 +12,7 @@
 #include "base/command_line.h"
 #include "base/compiler_specific.h"
 #include "base/debug/alias.h"
+#include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/macros.h"
@@ -19,7 +20,9 @@
 #include "base/process/process.h"
 #include "base/single_thread_task_runner.h"
 #include "base/threading/platform_thread.h"
+#include "base/timer/elapsed_timer.h"
 #include "build/build_config.h"
+#include "content/common/gpu_watchdog_utils.h"
 #include "content/public/common/content_switches.h"
 #include "content/public/common/result_codes.h"
 
@@ -77,6 +80,26 @@ GpuWatchdogThread::GpuWatchdogThread(int timeout)
   SetupXServer();
 #endif
   watched_message_loop_->AddTaskObserver(&task_observer_);
+
+#if defined(OS_WIN)
+  // Create a temp file for checking whether I/O is a bottleneck.
+  // This code runs on the main GPU thread before the sandbox is lowered.
+  base::FilePath temp_file_path;
+  if (GetGpuWatchdogTempFile(&temp_file_path)) {
+    // Please note that multiple instances of GPU process may reuse the same
+    // temporary file. That's OK because the file is written to only when
+    // the watchdog gets triggered and about to crash the process. The file
+    // should be deleted when the last handle is closed.
+    temp_file_for_io_checking_.Initialize(
+        temp_file_path, base::File::FLAG_OPEN_ALWAYS | base::File::FLAG_WRITE |
+                            base::File::FLAG_DELETE_ON_CLOSE |
+                            base::File::FLAG_SHARE_DELETE);

[ncarter (slow), 2016/06/09 16:14:43]

I imagine that running browsertests in parallel with
--enable-pixel-output-in-tests will give decent coverage of contention on this
file.

[stanisc, 2016/06/09 20:52:28]

I've tested contention on this file by running two instances of Chrome with
different profile directories. The file gets deleted when the last instance of
Chrome is closed. The file gets written to only when the hang is already
detected (15 sec on not getting acknowledge) and chrome is about to crash. If we
limit this solution to Windows only we could probably not create this file until
we are ready to write to it, but on other platforms I think it needs to be
created before the sandbox is activated (which is here, in constructor). 

+    if (!temp_file_for_io_checking_.IsValid()) {
+      LOG(ERROR) << "Couldn't create " << temp_file_path.value().c_str()
+                 << ", error: " << temp_file_for_io_checking_.error_details();
+    }
+  }
+#endif
 }
 
 void GpuWatchdogThread::PostAcknowledge() {
@@ -224,14 +247,12 @@ void GpuWatchdogThread::OnCheck(bool after_suspend) {
   // Post a task to the watchdog thread to exit if the monitored thread does
   // not respond in time.
   task_runner()->PostDelayedTask(
-      FROM_HERE,
-      base::Bind(&GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang,
-                 weak_factory_.GetWeakPtr()),
+      FROM_HERE, base::Bind(&GpuWatchdogThread::BeginTerminating,
+                            weak_factory_.GetWeakPtr()),
       timeout);
 }
 
-// Use the --disable-gpu-watchdog command line switch to disable this.
-void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
+void GpuWatchdogThread::BeginTerminating() {
   // Should not get here while the system is suspended.
   DCHECK(!suspended_);
 
@@ -242,15 +263,38 @@ void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
   base::TimeDelta time_since_arm = current_cpu_time - arm_cpu_time_;
   if (use_thread_cpu_time_ && (time_since_arm < timeout_)) {
     message_loop()->PostDelayedTask(
-        FROM_HERE,
-        base::Bind(
-            &GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang,
-            weak_factory_.GetWeakPtr()),
+        FROM_HERE, base::Bind(&GpuWatchdogThread::BeginTerminating,
+                              weak_factory_.GetWeakPtr()),
         timeout_ - time_since_arm);
     return;
   }
+
+  // If the machine is busy with heavy I/O activity this should defer
+  // termination until the I/O queue clears (on the drive that contains
+  // the temp directory).
+  if (temp_file_for_io_checking_.IsValid()) {
+    // Write a few bytes and wait for the write to flush.
+    const char temp_data[32] = {0};

[brucedawson, 2016/06/09 01:01:13]

VC++ still generates sub-optimal code for = {0}. You should prefer = {} instead.

I just tested and the compiler still generates *crazy* code that writes 1 byte,
then 8, 8, 8, 4, 2, 1. I only reported the bug six years ago - give them time.
On a 64-bit (release) test build this wastes 13 bytes of code.

[stanisc, 2016/06/09 20:52:28]

Done.

+    base::ElapsedTimer timer;
+    temp_file_for_io_checking_.Write(0, temp_data, sizeof(temp_data));
+    temp_file_for_io_checking_.Flush();
+    io_check_duration_ = timer.Elapsed();

[ncarter (slow), 2016/06/09 16:14:43]

Did you intend to use this for anything?

[brucedawson, 2016/06/09 17:29:05]

The io_check_duration_ member will end up in the crash dump (because 'this' is
in a register when we crash) so this value may give us some hints about how big
a factor I/O load is.

Stan - add a comment to that effect?

[manzagop (departed), 2016/06/09 19:41:54]

Drive-by question! Is memory pulled in around registers? I thought it was only
around stack addresses? (Also, is the answer for MiniDumpWriteDump or for
CrashPad?)

[ncarter (slow), 2016/06/09 20:11:20]

The memory around |*this|, for all stack frames, are captured pretty reliably in
minidumps on Windows nowadays. It regressed for about a month and a half during
the breakpad/crashpad switchover late last year, but has been decent since. No
idea about other platforms. One can use crashkeys if you want the values to be
visible on the crash/ web UI.

[stanisc, 2016/06/09 20:52:28]

I've added a comment suggested by Bruce.

Using crash key is a good idea for something like the CPU time delta for the
watched thread. I think as long as io_check_duration_ is visible when debugging
the crash dump that should be good enough.

+  }
 #endif
 
+  // Post a new task to actually terminate unless an acknowledge from the
+  // watched thread arrives in between.
+  message_loop()->PostTask(
+      FROM_HERE,
+      base::Bind(&GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang,
+                 weak_factory_.GetWeakPtr()));
+}
+
+// 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_);
+
   // If the watchdog woke up significantly behind schedule, disarm and reset
   // the watchdog check. This is to prevent the watchdog thread from terminating
   // when a machine wakes up from sleep or hibernation, which would otherwise
@@ -336,9 +380,12 @@ void GpuWatchdogThread::DeliberatelyTerminateToRecoverFromHang() {
   // This is the time since the watchdog was armed, in 100ns intervals,
   // ignoring time where the computer is suspended.
   ULONGLONG interrupt_delay = fire_interrupt_time - arm_interrupt_time_;
-
   base::debug::Alias(&interrupt_delay);
+
+  base::ThreadTicks current_cpu_time = GetWatchedThreadTime();
   base::debug::Alias(&current_cpu_time);
+
+  base::TimeDelta time_since_arm = current_cpu_time - arm_cpu_time_;
   base::debug::Alias(&time_since_arm);
 
   bool using_thread_ticks = base::ThreadTicks::IsSupported();