Skip to content

Commit

Permalink
Don't peek messages in the MessagePumpForUI class when we receive our…
Browse files Browse the repository at this point in the history
… kMsgHaveWork message.

Relanding this patch with fixes for content_unittests failures. We need to wait for the UI worker thread to start.

Currently the MessagePumpForUI class peeks Windows messages when we receive the kMsgHaveWork message in
our main message loop and in nested modal loops. This is because the posted message starves the message loop
of input and other lower priority messages. While this is ok for the main message loop our peeking and dispatching
messages in nested loops is wrong and violates the silent contract where in the nested loop should be the one peeking
and dispatching messages.

To fix this the approach we are taking is to create a worker thread which uses a waitable timer of 3 ms which posts
the kMsgHaveWork message to the main loop when the timer fires. As a result we can safely delete all the code
in the MessagePumpForUI::ScheduleWork function and simplify the ProcessPumpReplacementMessage function.

The MessagePumpForUI::ScheduleWork function now checks the delay for the task at the head of the queue
If it is 0 then we post the message right away as it is a regular task. Added functions (GetNewlyAddedTaskDelay) to the MessagePump::Delegate class and the IncomingTaskQueue for this.

The other change here is to change the GPU process to use the IO message pump by default and use the UI pump only
if we are using opengl. Reason being this patch causes a delay in processing tasks due to the worker thread which
causes tests like webgl_conformance to fail. We will continue working on addressing this over the coming days.

BUG=492837
TBR=cpu, scottmg

Review URL: https://codereview.chromium.org/1173193002

Cr-Commit-Position: refs/heads/master@{#333831}
  • Loading branch information
ananta authored and Commit bot committed Jun 10, 2015
1 parent f28bcd9 commit 29a2065
Show file tree
Hide file tree
Showing 8 changed files with 145 additions and 56 deletions.
5 changes: 5 additions & 0 deletions base/message_loop/incoming_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,11 @@ void IncomingTaskQueue::StartScheduling() {
ScheduleWork();
}

TimeTicks IncomingTaskQueue::GetNewlyAddedTaskDelay() {
return !incoming_queue_.empty() ? incoming_queue_.front().delayed_run_time :
TimeTicks();
}

IncomingTaskQueue::~IncomingTaskQueue() {
// Verify that WillDestroyCurrentMessageLoop() has been called.
DCHECK(!message_loop_);
Expand Down
3 changes: 3 additions & 0 deletions base/message_loop/incoming_task_queue.h
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ class BASE_EXPORT IncomingTaskQueue
// scheduling work.
void StartScheduling();

// Returns the delay for the most recently added task.
TimeTicks GetNewlyAddedTaskDelay();

private:
friend class RefCountedThreadSafe<IncomingTaskQueue>;
virtual ~IncomingTaskQueue();
Expand Down
4 changes: 4 additions & 0 deletions base/message_loop/message_loop.cc
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,10 @@ bool MessageLoop::DoIdleWork() {
return false;
}

TimeTicks MessageLoop::GetNewlyAddedTaskDelay() {
return incoming_task_queue_->GetNewlyAddedTaskDelay();
}

void MessageLoop::DeleteSoonInternal(const tracked_objects::Location& from_here,
void(*deleter)(const void*),
const void* object) {
Expand Down
1 change: 1 addition & 0 deletions base/message_loop/message_loop.h
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ class BASE_EXPORT MessageLoop : public MessagePump::Delegate {
bool DoWork() override;
bool DoDelayedWork(TimeTicks* next_delayed_work_time) override;
bool DoIdleWork() override;
TimeTicks GetNewlyAddedTaskDelay() override;

const Type type_;

Expand Down
3 changes: 3 additions & 0 deletions base/message_loop/message_pump.h
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ class BASE_EXPORT MessagePump : public NonThreadSafe {
// Returns true to indicate that idle work was done. Returning false means
// the pump will now wait.
virtual bool DoIdleWork() = 0;

// Returns the delay for the newly added task.
virtual TimeTicks GetNewlyAddedTaskDelay() = 0;
};

MessagePump();
Expand Down
139 changes: 84 additions & 55 deletions base/message_loop/message_pump_win.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "base/process/memory.h"
#include "base/profiler/scoped_tracker.h"
#include "base/strings/stringprintf.h"
#include "base/threading/thread.h"
#include "base/trace_event/trace_event.h"
#include "base/win/wrapped_window_proc.h"

Expand All @@ -34,6 +35,10 @@ static const wchar_t kWndClassFormat[] = L"Chrome_MessagePumpWindow_%p";
// task (a series of such messages creates a continuous task pump).
static const int kMsgHaveWork = WM_USER + 1;

// The default delay for the waitable timer used to wake up the UI worker
// thread.
static const int64 kDefaultUIWorkerThreadWakeupTimerMs = 3;

//-----------------------------------------------------------------------------
// MessagePumpWin public:

Expand Down Expand Up @@ -90,35 +95,39 @@ int MessagePumpWin::GetCurrentDelay() const {
MessagePumpForUI::MessagePumpForUI()
: atom_(0) {
InitMessageWnd();

ui_worker_thread_timer_.Set(::CreateWaitableTimer(NULL, FALSE, NULL));
ui_worker_thread_.reset(new base::Thread("UI Pump Worker thread"));
ui_worker_thread_->Start();
ui_worker_thread_->WaitUntilThreadStarted();
ui_worker_thread_->task_runner()->PostTask(
FROM_HERE,
base::Bind(&MessagePumpForUI::DoWorkerThreadRunLoop,
base::Unretained(this)));
}

MessagePumpForUI::~MessagePumpForUI() {
DestroyWindow(message_hwnd_);
UnregisterClass(MAKEINTATOM(atom_),
GetModuleFromAddress(&WndProcThunk));

::QueueUserAPC(
reinterpret_cast<PAPCFUNC>(&MessagePumpForUI::ShutdownWorkerThread),
ui_worker_thread_->thread_handle().platform_handle(), NULL);
ui_worker_thread_->Stop();
}

void MessagePumpForUI::ScheduleWork() {
if (InterlockedExchange(&have_work_, 1))
return; // Someone else continued the pumping.

// Make sure the MessagePump does some work for us.
BOOL ret = PostMessage(message_hwnd_, kMsgHaveWork,
reinterpret_cast<WPARAM>(this), 0);
if (ret)
return; // There was room in the Window Message queue.

// We have failed to insert a have-work message, so there is a chance that we
// will starve tasks/timers while sitting in a nested message loop. Nested
// loops only look at Windows Message queues, and don't look at *our* task
// queues, etc., so we might not get a time slice in such. :-(
// We could abort here, but the fear is that this failure mode is plausibly
// 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.
UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem", MESSAGE_POST_ERROR,
MESSAGE_LOOP_PROBLEM_MAX);
// If we have a regular posted task at the head of queue then we need to
// process it quickly.
if (state_ && state_->delegate->GetNewlyAddedTaskDelay().is_null()) {
// Make sure the MessagePump does some work for us.
PostWorkMessage();
return;
}
// Set a one shot timer to fire after 3 milliseconds. The actual resolution
// of the timer is dependent on timeBeginPeriod being called.
SetWakeupTimer(kDefaultUIWorkerThreadWakeupTimerMs);
}

void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
Expand Down Expand Up @@ -409,45 +418,65 @@ bool MessagePumpForUI::ProcessMessageHelper(const MSG& msg) {
}

bool MessagePumpForUI::ProcessPumpReplacementMessage() {
// When we encounter a kMsgHaveWork message, this method is called to peek
// and process a replacement message, such as a WM_PAINT or WM_TIMER. The
// goal is to make the kMsgHaveWork as non-intrusive as possible, even though
// a continuous stream of such messages are posted. This method carefully
// peeks a message while there is no chance for a kMsgHaveWork to be pending,
// then resets the have_work_ flag (allowing a replacement kMsgHaveWork to
// possibly be posted), and finally dispatches that peeked replacement. Note
// that the re-post of kMsgHaveWork may be asynchronous to this thread!!

bool have_message = false;
MSG msg;
// We should not process all window messages if we are in the context of an
// OS modal loop, i.e. in the context of a windows API call like MessageBox.
// This is to ensure that these messages are peeked out by the OS modal loop.
if (MessageLoop::current()->os_modal_loop()) {
// We only peek out WM_PAINT and WM_TIMER here for reasons mentioned above.
have_message = PeekMessage(&msg, NULL, WM_PAINT, WM_PAINT, PM_REMOVE) ||
PeekMessage(&msg, NULL, WM_TIMER, WM_TIMER, PM_REMOVE);
} else {
have_message = PeekMessage(&msg, NULL, 0, 0, PM_REMOVE) != FALSE;
}
// Since we discarded a kMsgHaveWork message, we must update the flag.
InterlockedExchange(&have_work_, 0);
return true;
}

DCHECK(!have_message || kMsgHaveWork != msg.message ||
msg.hwnd != message_hwnd_);
void MessagePumpForUI::DoWorkerThreadRunLoop() {
DCHECK(ui_worker_thread_timer_.Get());
while (TRUE) {
DWORD ret = WaitForSingleObjectEx(
ui_worker_thread_timer_.Get(), INFINITE, TRUE);
// The only APC this thread could receive is the Shutdown APC.
if (ret == WAIT_IO_COMPLETION)
return;

// Since we discarded a kMsgHaveWork message, we must update the flag.
int old_have_work = InterlockedExchange(&have_work_, 0);
DCHECK(old_have_work);
// Make sure the MessagePump does some work for us.
PostWorkMessage();

// We don't need a special time slice if we didn't have_message to process.
if (!have_message)
return false;
// Set a one shot timer to process pending delayed tasks if any in the
// queue. The actual resolution of the timer is dependent on the
// timeBeginPeriod API being called.
SetWakeupTimer(kDefaultUIWorkerThreadWakeupTimerMs);
}
}

// static
void CALLBACK MessagePumpForUI::ShutdownWorkerThread(ULONG_PTR param) {
// This function is empty because we only use the fact that an APC was posted
// to the worker thread to shut it down.
return;
}

void MessagePumpForUI::PostWorkMessage() {
BOOL posted = PostMessage(message_hwnd_, kMsgHaveWork,
reinterpret_cast<WPARAM>(this),
0);
if (!posted) {
// We have failed to insert a have-work message, so there is a chance
// that we will starve tasks/timers while sitting in a nested message
// loop. Nested loops only look at Windows Message queues, and don't
// look at *our* task queues, etc., so we might not get a time slice in
// such. :-(
// We could abort here, but the fear is that this failure mode is
// plausibly 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.
UMA_HISTOGRAM_ENUMERATION("Chrome.MessageLoopProblem",
MESSAGE_POST_ERROR,
MESSAGE_LOOP_PROBLEM_MAX);
}
}

// Guarantee we'll get another time slice in the case where we go into native
// windows code. This ScheduleWork() may hurt performance a tiny bit when
// tasks appear very infrequently, but when the event queue is busy, the
// kMsgHaveWork events get (percentage wise) rarer and rarer.
ScheduleWork();
return ProcessMessageHelper(msg);
void MessagePumpForUI::SetWakeupTimer(int64 delay_ms) {
// Set the timer for the delay passed in. The actual resolution of the
// timer is dependent on whether timeBeginPeriod was called.
LARGE_INTEGER due_time = {0};
due_time.QuadPart = -delay_ms * 10000;
BOOL timer_set = ::SetWaitableTimer(ui_worker_thread_timer_.Get(),
&due_time, 0, NULL, NULL, FALSE);
CHECK(timer_set);
}

//-----------------------------------------------------------------------------
Expand Down
31 changes: 31 additions & 0 deletions base/message_loop/message_pump_win.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

#include "base/base_export.h"
#include "base/basictypes.h"
#include "base/memory/scoped_ptr.h"
#include "base/message_loop/message_pump.h"
#include "base/message_loop/message_pump_dispatcher.h"
#include "base/observer_list.h"
Expand All @@ -19,6 +20,8 @@

namespace base {

class Thread;

// MessagePumpWin serves as the base for specialized versions of the MessagePump
// for Windows. It provides basic functionality like handling of observers and
// controlling the lifetime of the message pump.
Expand Down Expand Up @@ -135,11 +138,39 @@ class BASE_EXPORT MessagePumpForUI : public MessagePumpWin {
bool ProcessMessageHelper(const MSG& msg);
bool ProcessPumpReplacementMessage();

// We spawn a worker thread to periodically post (3 ms) the kMsgHaveWork
// message to the UI message pump. This is to ensure that the main thread
// gets to process tasks and delayed tasks when there is no activity in the
// Windows message pump or when there is a nested modal loop (sizing/moving/
// drag drop/message boxes) etc.
void DoWorkerThreadRunLoop();

// This function is posted as part of a user mode APC to shutdown the worker
// thread when the main message pump is shutting down.
static void CALLBACK ShutdownWorkerThread(ULONG_PTR param);

// Helper function for posting the kMsgHaveWork message to wake up the pump
// for processing tasks.
void PostWorkMessage();

// Helper function to set the waitable timer used to wake up the UI worker
// thread for processing delayed tasks.
// |delay_ms| : The delay in milliseconds.
void SetWakeupTimer(int64 delay_ms);

// Atom representing the registered window class.
ATOM atom_;

// A hidden message-only window.
HWND message_hwnd_;

// This thread is used to periodically wake up the main thread to process
// tasks.
scoped_ptr<base::Thread> ui_worker_thread_;

// The UI worker thread waits on this timer indefinitely. When the main
// thread has tasks ready to be processed it sets the timer.
base::win::ScopedHandle ui_worker_thread_timer_;
};

//-----------------------------------------------------------------------------
Expand Down
15 changes: 14 additions & 1 deletion content/gpu/gpu_main.cc
Original file line number Diff line number Diff line change
Expand Up @@ -160,9 +160,22 @@ int GpuMain(const MainFunctionParams& parameters) {
bool dead_on_arrival = false;

#if defined(OS_WIN)
base::MessageLoop::Type loop_type = base::MessageLoop::TYPE_IO;
// Use a UI message loop because ANGLE and the desktop GL platform can
// create child windows to render to.
base::MessageLoop main_message_loop(base::MessageLoop::TYPE_UI);
// TODO(ananta) : Recent changes to the UI message pump class on Windows
// will cause delays in tasks getting processed in the GPU process which
// will show up on the bots in webgl conformance tests, perf tests etc.
// It will be great if we can work around the issues with desktop GL and
// avoid creating a child window in the GPU process which requires a UI
// message pump.
if ((command_line.HasSwitch(switches::kUseGL) &&
command_line.GetSwitchValueASCII(switches::kUseGL) == "desktop") ||
(command_line.HasSwitch(switches::kUseANGLE) &&
command_line.GetSwitchValueASCII(switches::kUseANGLE) == "gl")) {
loop_type = base::MessageLoop::TYPE_UI;
}
base::MessageLoop main_message_loop(loop_type);
#elif defined(OS_LINUX) && defined(USE_X11)
// We need a UI loop so that we can grab the Expose events. See GLSurfaceGLX
// and https://crbug.com/326995.
Expand Down

0 comments on commit 29a2065

Please sign in to comment.