summaryrefslogtreecommitdiffstats
path: root/base/message_loop
diff options
context:
space:
mode:
authorananta <ananta@chromium.org>2015-06-21 23:10:42 -0700
committerCommit bot <commit-bot@chromium.org>2015-06-22 06:11:12 +0000
commit4d5496cde5b81436954e31a9fe82e35ccfe5de49 (patch)
tree2b793f863e11e18f216656f457d7aab4303fadc5 /base/message_loop
parent9592add2c95e6a1b8caee96b4987b69d1140d4f5 (diff)
downloadchromium_src-4d5496cde5b81436954e31a9fe82e35ccfe5de49.zip
chromium_src-4d5496cde5b81436954e31a9fe82e35ccfe5de49.tar.gz
chromium_src-4d5496cde5b81436954e31a9fe82e35ccfe5de49.tar.bz2
Don't peek messages in the MessagePumpForUI class when we receive our kMsgHaveWork message.
Relanding this patch with fixes for the performance issues reported with the worker thread hogging the CPU continuosly. (https://codereview.chromium.org/1181263008/) 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. To prevent the UI worker thread from continuously posting the timer we use a flag to control the same. The flag ensures that the worker thread posts the timer once for each iteration. This patch also contains a fix for the crash reported in bug 501602 which occurs due to the GetNewlyAddedTaskDelay function being called from the WndProc which does not have the incoming queue lock. Fix is to move some of the logic in the ScheduleWork function to the newly added ScheduleWorkHelper function. BUG=492837, 501602 TBR=cpu Review URL: https://codereview.chromium.org/1194673004 Cr-Commit-Position: refs/heads/master@{#335475}
Diffstat (limited to 'base/message_loop')
-rw-r--r--base/message_loop/incoming_task_queue.cc5
-rw-r--r--base/message_loop/incoming_task_queue.h3
-rw-r--r--base/message_loop/message_loop.cc4
-rw-r--r--base/message_loop/message_loop.h1
-rw-r--r--base/message_loop/message_pump.h3
-rw-r--r--base/message_loop/message_pump_win.cc158
-rw-r--r--base/message_loop/message_pump_win.h40
7 files changed, 158 insertions, 56 deletions
diff --git a/base/message_loop/incoming_task_queue.cc b/base/message_loop/incoming_task_queue.cc
index 5e9a461..642222e 100644
--- a/base/message_loop/incoming_task_queue.cc
+++ b/base/message_loop/incoming_task_queue.cc
@@ -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_);
diff --git a/base/message_loop/incoming_task_queue.h b/base/message_loop/incoming_task_queue.h
index 7dd1e82..544f3e9 100644
--- a/base/message_loop/incoming_task_queue.h
+++ b/base/message_loop/incoming_task_queue.h
@@ -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();
diff --git a/base/message_loop/message_loop.cc b/base/message_loop/message_loop.cc
index 0eb24cf..74bfdb6 100644
--- a/base/message_loop/message_loop.cc
+++ b/base/message_loop/message_loop.cc
@@ -638,6 +638,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) {
diff --git a/base/message_loop/message_loop.h b/base/message_loop/message_loop.h
index 9c635ee..b1b219a 100644
--- a/base/message_loop/message_loop.h
+++ b/base/message_loop/message_loop.h
@@ -465,6 +465,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_;
diff --git a/base/message_loop/message_pump.h b/base/message_loop/message_pump.h
index a2edb45..77a49ce 100644
--- a/base/message_loop/message_pump.h
+++ b/base/message_loop/message_pump.h
@@ -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();
diff --git a/base/message_loop/message_pump_win.cc b/base/message_loop/message_pump_win.cc
index cdbf0c2..187678a 100644
--- a/base/message_loop/message_pump_win.cc
+++ b/base/message_loop/message_pump_win.cc
@@ -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"
@@ -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:
@@ -88,37 +93,41 @@ int MessagePumpWin::GetCurrentDelay() const {
// MessagePumpForUI public:
MessagePumpForUI::MessagePumpForUI()
- : atom_(0) {
+ : atom_(0),
+ force_fallback_timer_for_tasks_(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.
+ // 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;
+ }
- // 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);
+ ScheduleWorkHelper();
}
void MessagePumpForUI::ScheduleDelayedWork(const TimeTicks& delayed_work_time) {
@@ -281,7 +290,7 @@ void MessagePumpForUI::HandleWorkMessage() {
// Now give the delegate a chance to do some work. He'll let us know if he
// needs to do more work.
if (state_->delegate->DoWork())
- ScheduleWork();
+ ScheduleWorkHelper();
state_->delegate->DoDelayedWork(&delayed_work_time_);
RescheduleTimer();
}
@@ -325,7 +334,7 @@ void MessagePumpForUI::RescheduleTimer() {
int delay_msec = GetCurrentDelay();
DCHECK_GE(delay_msec, 0);
if (delay_msec == 0) {
- ScheduleWork();
+ ScheduleWorkHelper();
} else {
if (delay_msec < USER_TIMER_MINIMUM)
delay_msec = USER_TIMER_MINIMUM;
@@ -409,45 +418,82 @@ 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;
+}
+
+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;
+
+ // Make sure the MessagePump does some work for us.
+ PostWorkMessage();
+
+ // 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 current
+ // global timer precision and therefore depends on whether Chrome or any
+ // other process has raised the timer frequency with timeBeginPeriod."
+
+ // We should set the timer only once for each iteration. The
+ // InterlockedExchange call below achieves that.
+ if (::InterlockedExchange(&force_fallback_timer_for_tasks_, 0))
+ SetWakeupTimer(kDefaultUIWorkerThreadWakeupTimerMs);
}
+}
- DCHECK(!have_message || kMsgHaveWork != msg.message ||
- msg.hwnd != message_hwnd_);
+// 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);
+ }
+}
- // Since we discarded a kMsgHaveWork message, we must update the flag.
- int old_have_work = InterlockedExchange(&have_work_, 0);
- DCHECK(old_have_work);
+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);
+}
- // We don't need a special time slice if we didn't have_message to process.
- if (!have_message)
- return false;
+void MessagePumpForUI::ScheduleWorkHelper() {
+ // Set the flag which allows the UI worker thread to repost the timer to
+ // process tasks which may not have been ready to run in the first iteration.
+ ::InterlockedExchange(&force_fallback_timer_for_tasks_, 1);
- // 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);
+ // Set a one shot timer to fire after 3 milliseconds. The actual resolution
+ // of the timer is dependent on the current global timer precision and
+ // therefore depends on whether Chrome or any other process has raised the
+ // timer frequency with timeBeginPeriod."
+ SetWakeupTimer(kDefaultUIWorkerThreadWakeupTimerMs);
}
//-----------------------------------------------------------------------------
diff --git a/base/message_loop/message_pump_win.h b/base/message_loop/message_pump_win.h
index 00a1e77..b1324f8 100644
--- a/base/message_loop/message_pump_win.h
+++ b/base/message_loop/message_pump_win.h
@@ -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"
@@ -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.
@@ -135,11 +138,48 @@ 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);
+
+ // Helper function to ensure that the message pump processes tasks and
+ // delayed tasks.
+ void ScheduleWorkHelper();
+
// 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_;
+
+ // This flag controls whether the UI worker thread sets the waitable timer
+ // to ensure that tasks missed in one timer iteration get picked in the
+ // second.
+ long force_fallback_timer_for_tasks_;
};
//-----------------------------------------------------------------------------