Version of MessagePumpForUI optimized for GPU process

base/message_loop/message_pump_win.cc

Index: base/message_loop/message_pump_win.cc
diff --git a/base/message_loop/message_pump_win.cc b/base/message_loop/message_pump_win.cc
index 803bb5d4f82e1602cd821f898b36bf734dc8228e..f238c3c3a132643f89277e245b0b122779527da4 100644
--- a/base/message_loop/message_pump_win.cc
+++ b/base/message_loop/message_pump_win.cc
@@ -80,21 +80,21 @@ int MessagePumpWin::GetCurrentDelay() const {
 }
 
 //-----------------------------------------------------------------------------
-// MessagePumpForUI public:
+// MessagePumpForUIBase public:
 
-MessagePumpForUI::MessagePumpForUI()
+MessagePumpForUIBase::MessagePumpForUIBase()
     : atom_(0) {
   InitMessageWnd();

[jbauman, 2016/03/18 01:00:50]

We could essentially get rid of MessagePumpForUIBase as a separate class and
avoid complicating MessagePumpForUI even more by either using an event object to
schedule MsgWaitForMultipleObjectsEx (this may even help power usage), or by
using PostThreadMessage so we don't have to bother with the message window, etc.

[stanisc, 2016/03/18 18:16:46]

Thanks for the feedback! I've tried a separate class with an event. It worked
fine on Windows 7 but would hang on Windows 10. I guess a Windows message loop
is still needed. One issue with SetEvent is that it temporarily boosts receiving
thread priority and might result in preempting the producer thread on a machine
with a small number of CPU cores. I think in case of GPU process that would just
result in an increasing the number of context switches and doing more work, not
less. I'll go ahead and rewrite this to use a completely separate class and
PostThreadMessage.

 }
 
-MessagePumpForUI::~MessagePumpForUI() {
+MessagePumpForUIBase::~MessagePumpForUIBase() {
   DestroyWindow(message_hwnd_);
   UnregisterClass(MAKEINTATOM(atom_),
                   GetModuleFromAddress(&WndProcThunk));
 }
 
-void MessagePumpForUI::ScheduleWork() {
-  if (InterlockedExchange(&have_work_, 1))
+void MessagePumpForUIBase::ScheduleWork() {
+  if (READY != InterlockedExchange(&work_state_, HAVE_WORK))
     return;  // Someone else continued the pumping.
 
   // Make sure the MessagePump does some work for us.
@@ -111,33 +111,112 @@ void MessagePumpForUI::ScheduleWork() {
   // common (queue is full, of about 2000 messages), so we'll do a near-graceful
   // recovery.  Nested loops are pretty transient (we think), so this will
   // probably be recoverable.
-  InterlockedExchange(&have_work_, 0);  // Clarify that we didn't really insert.
+
+  // Clarify that we didn't really insert.
+  InterlockedExchange(&work_state_, READY);
   UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", MESSAGE_POST_ERROR,
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
 
-void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
-  delayed_work_time_ = delayed_work_time;
-  RescheduleTimer();
+//-----------------------------------------------------------------------------
+// MessagePumpForUIBase protected:
+
+void MessagePumpForUIBase::SetTimer(int delay_msec) {
+  // We would *like* to provide high resolution timers.  Windows timers using
+  // SetTimer() have a 10ms granularity.  We have to use WM_TIMER as a wakeup
+  // mechanism because the application can enter modal windows loops where it
+  // is not running our MessageLoop; the only way to have our timers fire in
+  // these cases is to post messages there.
+  //
+  // To provide sub-10ms timers, we process timers directly from our run loop.
+  // For the common case, timers will be processed there as the run loop does
+  // its normal work.  However, we *also* set the system timer so that WM_TIMER
+  // events fire.  This mops up the case of timers not being able to work in
+  // modal message loops.  It is possible for the SetTimer to pop and have no
+  // pending timers, because they could have already been processed by the
+  // run loop itself.
+  //
+  // We use a single SetTimer corresponding to the timer that will expire
+  // soonest.  As new timers are created and destroyed, we update SetTimer.
+  // Getting a spurious SetTimer event firing is benign, as we'll just be
+  // processing an empty timer queue.
+  if (delay_msec < USER_TIMER_MINIMUM)
+    delay_msec = USER_TIMER_MINIMUM;
+
+  // Create a WM_TIMER event that will wake us up to check for any pending
+  // timers (in case we are running within a nested, external sub-pump).
+  BOOL ret = ::SetTimer(message_hwnd_, reinterpret_cast<UINT_PTR>(this),
+                      delay_msec, NULL);
+  if (ret)
+    return;
+  // If we can't set timers, we are in big trouble... but cross our fingers
+  // for now.
+  // TODO(jar): If we don't see this error, use a CHECK() here instead.
+  UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", SET_TIMER_ERROR,
+                            MESSAGE_LOOP_PROBLEM_MAX);
+}
+
+void MessagePumpForUIBase::KillTimer() {
+  ::KillTimer(message_hwnd_, reinterpret_cast<UINT_PTR>(this));
+}
+
+bool MessagePumpForUIBase::IsWorkMessage(const MSG& msg) const {
+  return msg.message == kMsgHaveWork && msg.hwnd == message_hwnd_;
 }
 
 //-----------------------------------------------------------------------------
-// MessagePumpForUI private:
+// MessagePumpForUIBase private:
 
 // static
-LRESULT CALLBACK MessagePumpForUI::WndProcThunk(
+LRESULT CALLBACK MessagePumpForUIBase::WndProcThunk(
     HWND hwnd, UINT message, WPARAM wparam, LPARAM lparam) {
   switch (message) {
     case kMsgHaveWork:
-      reinterpret_cast<MessagePumpForUI*>(wparam)->HandleWorkMessage();
+      reinterpret_cast<MessagePumpForUIBase*>(wparam)->HandleWorkMessage();
       break;
     case WM_TIMER:
-      reinterpret_cast<MessagePumpForUI*>(wparam)->HandleTimerMessage();
+      reinterpret_cast<MessagePumpForUIBase*>(wparam)->HandleTimerMessage();
       break;
   }
   return DefWindowProc(hwnd, message, wparam, lparam);
 }
 
+void MessagePumpForUIBase::InitMessageWnd() {
+  // Generate a unique window class name.
+  string16 class_name = StringPrintf(kWndClassFormat, this);
+
+  HINSTANCE instance = GetModuleFromAddress(&WndProcThunk);
+  WNDCLASSEX wc = {0};
+  wc.cbSize = sizeof(wc);
+  wc.lpfnWndProc = base::win::WrappedWindowProc<WndProcThunk>;
+  wc.hInstance = instance;
+  wc.lpszClassName = class_name.c_str();
+  atom_ = RegisterClassEx(&wc);
+  DCHECK(atom_);
+
+  message_hwnd_ = CreateWindow(MAKEINTATOM(atom_), 0, 0, 0, 0, 0, 0,
+                               HWND_MESSAGE, 0, instance, 0);
+  DCHECK(message_hwnd_);
+}
+
+//-----------------------------------------------------------------------------
+// MessagePumpForUI public:
+
+MessagePumpForUI::MessagePumpForUI() {
+}
+
+MessagePumpForUI::~MessagePumpForUI() {
+}
+
+//-----------------------------------------------------------------------------
+// MessagePumpForUI private:
+
+void MessagePumpForUI::ScheduleDelayedWork(
+    const TimeTicks& delayed_work_time) {
+  delayed_work_time_ = delayed_work_time;
+  RescheduleTimer();
+}
+
 void MessagePumpForUI::DoRunLoop() {
   // IF this was just a simple PeekMessage() loop (servicing all possible work
   // queues), then Windows would try to achieve the following order according
@@ -176,7 +255,7 @@ void MessagePumpForUI::DoRunLoop() {
     // don't want to disturb that timer if it is already in flight.  However,
     // if we did do all remaining delayed work, then lets kill the WM_TIMER.
     if (more_work_is_plausible && delayed_work_time_.is_null())
-      KillTimer(message_hwnd_, reinterpret_cast<UINT_PTR>(this));
+      KillTimer();
     if (state_->should_quit)
       break;
 
@@ -194,24 +273,6 @@ void MessagePumpForUI::DoRunLoop() {
   }
 }
 
-void MessagePumpForUI::InitMessageWnd() {
-  // Generate a unique window class name.
-  string16 class_name = StringPrintf(kWndClassFormat, this);
-
-  HINSTANCE instance = GetModuleFromAddress(&WndProcThunk);
-  WNDCLASSEX wc = {0};
-  wc.cbSize = sizeof(wc);
-  wc.lpfnWndProc = base::win::WrappedWindowProc<WndProcThunk>;
-  wc.hInstance = instance;
-  wc.lpszClassName = class_name.c_str();
-  atom_ = RegisterClassEx(&wc);
-  DCHECK(atom_);
-
-  message_hwnd_ = CreateWindow(MAKEINTATOM(atom_), 0, 0, 0, 0, 0, 0,
-                               HWND_MESSAGE, 0, instance, 0);
-  DCHECK(message_hwnd_);
-}
-
 void MessagePumpForUI::WaitForWork() {
   // Wait until a message is available, up to the time needed by the timer
   // manager to fire the next set of timers.
@@ -253,7 +314,7 @@ void MessagePumpForUI::HandleWorkMessage() {
   // sort.
   if (!state_) {
     // Since we handled a kMsgHaveWork message, we must still update this flag.
-    InterlockedExchange(&have_work_, 0);
+    InterlockedExchange(&work_state_, READY);
     return;
   }
 
@@ -271,7 +332,7 @@ void MessagePumpForUI::HandleWorkMessage() {
 }
 
 void MessagePumpForUI::HandleTimerMessage() {
-  KillTimer(message_hwnd_, reinterpret_cast<UINT_PTR>(this));
+  KillTimer();
 
   // If we are being called outside of the context of Run, then don't do
   // anything.  This could correspond to a MessageBox call or something of
@@ -286,45 +347,13 @@ void MessagePumpForUI::HandleTimerMessage() {
 void MessagePumpForUI::RescheduleTimer() {
   if (delayed_work_time_.is_null())
     return;
-  //
-  // We would *like* to provide high resolution timers.  Windows timers using
-  // SetTimer() have a 10ms granularity.  We have to use WM_TIMER as a wakeup
-  // mechanism because the application can enter modal windows loops where it
-  // is not running our MessageLoop; the only way to have our timers fire in
-  // these cases is to post messages there.
-  //
-  // To provide sub-10ms timers, we process timers directly from our run loop.
-  // For the common case, timers will be processed there as the run loop does
-  // its normal work.  However, we *also* set the system timer so that WM_TIMER
-  // events fire.  This mops up the case of timers not being able to work in
-  // modal message loops.  It is possible for the SetTimer to pop and have no
-  // pending timers, because they could have already been processed by the
-  // run loop itself.
-  //
-  // We use a single SetTimer corresponding to the timer that will expire
-  // soonest.  As new timers are created and destroyed, we update SetTimer.
-  // Getting a spurious SetTimer event firing is benign, as we'll just be
-  // processing an empty timer queue.
-  //
+
   int delay_msec = GetCurrentDelay();
   DCHECK_GE(delay_msec, 0);
   if (delay_msec == 0) {
     ScheduleWork();
   } else {
-    if (delay_msec < USER_TIMER_MINIMUM)
-      delay_msec = USER_TIMER_MINIMUM;
-
-    // Create a WM_TIMER event that will wake us up to check for any pending
-    // timers (in case we are running within a nested, external sub-pump).
-    BOOL ret = SetTimer(message_hwnd_, reinterpret_cast<UINT_PTR>(this),
-                        delay_msec, NULL);
-    if (ret)
-      return;
-    // If we can't set timers, we are in big trouble... but cross our fingers
-    // for now.
-    // TODO(jar): If we don't see this error, use a CHECK() here instead.
-    UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", SET_TIMER_ERROR,
-                              MESSAGE_LOOP_PROBLEM_MAX);
+    SetTimer(delay_msec);
   }
 }
 
@@ -346,7 +375,7 @@ bool MessagePumpForUI::ProcessNextWindowsMessage() {
 }
 
 bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) {
-  TRACE_EVENT1("base", "MessagePumpForUI::ProcessMessageHelper",
+  TRACE_EVENT1("base", "MessagePumpForUIBase::ProcessMessageHelper",
                "message", msg.message);
   if (WM_QUIT == msg.message) {
     // Repost the QUIT message so that it will be retrieved by the primary
@@ -357,7 +386,7 @@ bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) {
   }
 
   // While running our main message pump, we discard kMsgHaveWork messages.
-  if (msg.message == kMsgHaveWork && msg.hwnd == message_hwnd_)
+  if (IsWorkMessage(msg))
     return ProcessPumpReplacementMessage();
 
   if (CallMsgFilter(const_cast<MSG*>(&msg), kMessageFilterCode))
@@ -392,12 +421,11 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
     have_message = PeekMessage(&msg, NULL, 0, 0, PM_REMOVE) != FALSE;
   }
 
-  DCHECK(!have_message || kMsgHaveWork != msg.message ||
-         msg.hwnd != message_hwnd_);
+  DCHECK(!have_message || !IsWorkMessage(msg));
 
   // Since we discarded a kMsgHaveWork message, we must update the flag.
-  int old_have_work = InterlockedExchange(&have_work_, 0);
-  DCHECK(old_have_work);
+  int old_work_state_ = InterlockedExchange(&work_state_, READY);
+  DCHECK_EQ(HAVE_WORK, old_work_state_);
 
   // We don't need a special time slice if we didn't have_message to process.
   if (!have_message)
@@ -412,6 +440,125 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
 }
 
 //-----------------------------------------------------------------------------
+// MessagePumpForGpu public:
+
+MessagePumpForGpu::MessagePumpForGpu() {
+}
+
+MessagePumpForGpu::~MessagePumpForGpu() {
+}
+
+//-----------------------------------------------------------------------------
+// MessagePumpForGpu private:
+
+void MessagePumpForGpu::ScheduleDelayedWork(
+    const TimeTicks& delayed_work_time) {
+  // We know that we can't be blocked right now since this method can only be
+  // called on the same thread as Run, so we only need to update our record of
+  // how long to sleep when we do sleep.
+  delayed_work_time_ = delayed_work_time;
+}
+
+void MessagePumpForGpu::DoRunLoop() {
+  while (!state_->should_quit) {
+    // Indicate that the loop is handling the work.
+    // If there is a race condition between switching to WORKING state here and
+    // the producer thread setting the HAVE_WORK state after exiting the wait,
+    // the event might remain in the signalled state. That might be less than
+    // optimal but wouldn't result in failing to handle the work.
+    InterlockedExchange(&work_state_, WORKING);
+
+    bool more_work_is_plausible = state_->delegate->DoWork();
+    if (state_->should_quit)
+      break;
+
+    more_work_is_plausible |=
+        state_->delegate->DoDelayedWork(&delayed_work_time_);
+    if (state_->should_quit)
+      break;
+
+    if (more_work_is_plausible)
+      continue;
+
+    more_work_is_plausible = state_->delegate->DoIdleWork();
+    if (state_->should_quit)
+      break;
+
+    if (more_work_is_plausible)
+      continue;
+
+    // Switch that working state to READY to indicate that the loop is
+    // waiting for accepting new work if it is still in WORKING state and hasn't
+    // been signalled. Otherwise if it is in HAVE_WORK state skip the wait
+    // and proceed to handing the work.
+    if (HAVE_WORK == InterlockedCompareExchange(&work_state_, READY, WORKING))
+      continue; // Skip wait, more work was requested.
+
+    WaitForWork();  // Wait (sleep) until we have work to do again.
+  }
+}
+
+void MessagePumpForGpu::WaitForWork() {
+  // Wait until a message is available, up to the time needed by the timer
+  // manager to fire the next set of timers.
+  int delay;
+
+  // The while loop handles the situation where on Windows 7 and later versions
+  // MsgWaitForMultipleObjectsEx might time out slightly earlier (less than one
+  // ms) than the specified |delay|. In that situation it is more optimal to
+  // just wait again rather than waste a DoRunLoop cycle.
+  while((delay = GetCurrentDelay()) != 0) {
+    if (delay < 0)  // Negative value means no timers waiting.
+      delay = INFINITE;
+
+    DWORD result = MsgWaitForMultipleObjectsEx(0, NULL, delay, QS_ALLINPUT,
+                                               MWMO_INPUTAVAILABLE);
+    if (WAIT_OBJECT_0 == result) {
+      // A WM_* message is available.
+      ProcessWindowsMessages();
+      return;
+    }
+
+    DCHECK_NE(WAIT_FAILED, result) << GetLastError();
+  }
+}
+
+void MessagePumpForGpu::ProcessWindowsMessages() {
+  MSG msg;
+  while (PeekMessage(&msg, NULL, 0, 0, PM_REMOVE) != FALSE) {
+    if (WM_QUIT == msg.message) {
+      // Repost the QUIT message so that it will be retrieved by the primary
+      // GetMessage() loop.
+      state_->should_quit = true;
+      PostQuitMessage(static_cast<int>(msg.wParam));
+      return;
+    }
+
+    if (!IsWorkMessage(msg)) {
+      TranslateMessage(&msg);
+      DispatchMessage(&msg);
+    }
+  }
+}
+
+void MessagePumpForGpu::HandleWorkMessage() {
+  NOTREACHED();
+}
+
+void MessagePumpForGpu::HandleTimerMessage() {
+  NOTREACHED();
+}
+
+void MessagePumpForGpu::InitFactory() {
+  MessageLoop::InitMessagePumpForUIFactory(
+      &MessagePumpForGpu::CreateMessagePumpForGpu);
+}
+
+scoped_ptr<MessagePump> MessagePumpForGpu::CreateMessagePumpForGpu() {
+  return scoped_ptr<MessagePump>(new MessagePumpForGpu);
+}
+
+//-----------------------------------------------------------------------------
 // MessagePumpForIO public:
 
 MessagePumpForIO::MessagePumpForIO() {
@@ -423,7 +570,7 @@ MessagePumpForIO::~MessagePumpForIO() {
 }
 
 void MessagePumpForIO::ScheduleWork() {
-  if (InterlockedExchange(&have_work_, 1))
+  if (READY != InterlockedExchange(&work_state_, HAVE_WORK))
     return;  // Someone else continued the pumping.
 
   // Make sure the MessagePump does some work for us.
@@ -434,7 +581,7 @@ void MessagePumpForIO::ScheduleWork() {
     return;  // Post worked perfectly.
 
   // See comment in MessagePumpForUI::ScheduleWork() for this error recovery.
-  InterlockedExchange(&have_work_, 0);  // Clarify that we didn't succeed.
+  InterlockedExchange(&work_state_, READY);  // Clarify that we didn't succeed.
   UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", COMPLETION_POST_ERROR,
                             MESSAGE_LOOP_PROBLEM_MAX);
 }
@@ -579,7 +726,7 @@ bool MessagePumpForIO::ProcessInternalIOItem(const IOItem& item) {
       reinterpret_cast<void*>(this) == reinterpret_cast<void*>(item.handler)) {
     // This is our internal completion.
     DCHECK(!item.bytes_transfered);
-    InterlockedExchange(&have_work_, 0);
+    InterlockedExchange(&work_state_, READY);
     return true;
   }
   return false;