Commit 29800115 authored by Johann Koenig's avatar Johann Koenig Committed by Commit Bot

Revert "[base] Experiment with only invoking ::PeekMessage when there are pending messages"

This reverts commit d35a8977.

Reason for revert: Causes test failures on Win 7
BUG=chromium:1098531

Original change's description:
> [base] Experiment with only invoking ::PeekMessage when there are pending messages
> 
> ::PeekMessage can process sent-messages and internal events. It seems
> the kernel also uses it as a heuristic to deschedule the main thread when
> it thinks it's out of messages. In MessagePumpForUI's design, it's
> possible to have application tasks without pending native tasks
> (kMsgHaveWork is dropped when running out of native tasks).
> 
> This CL uses ::GetQueueStatus() to probe the presence of messages before
> invoking ::PeekMessage().
> 
> Bug: 1075960
> Change-Id: I4a93b22adf225ae2d62aabeb1a7a734232cf7fb6
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2258456
> Auto-Submit: Gabriel Charette <gab@chromium.org>
> Reviewed-by: François Doray <fdoray@chromium.org>
> Commit-Queue: François Doray <fdoray@chromium.org>
> Commit-Queue: Gabriel Charette <gab@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#781498}

TBR=gab@chromium.org,fdoray@chromium.org

Change-Id: Ib1e700621eef27b1ed8cfd1c50b3019836f6c41b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1075960
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2262160Reviewed-by: default avatarJohann Koenig <johannkoenig@google.com>
Commit-Queue: Johann Koenig <johannkoenig@google.com>
Cr-Commit-Position: refs/heads/master@{#781736}
parent 4004f2ba
...@@ -463,6 +463,10 @@ void MessagePumpForUI::KillNativeTimer() { ...@@ -463,6 +463,10 @@ void MessagePumpForUI::KillNativeTimer() {
bool MessagePumpForUI::ProcessNextWindowsMessage() { bool MessagePumpForUI::ProcessNextWindowsMessage() {
DCHECK_CALLED_ON_VALID_THREAD(bound_thread_); DCHECK_CALLED_ON_VALID_THREAD(bound_thread_);
// If there are sent messages in the queue then PeekMessage internally
// dispatches the message and returns false. We return true in this
// case to ensure that the message loop peeks again instead of calling
// MsgWaitForMultipleObjectsEx.
bool more_work_is_plausible = false; bool more_work_is_plausible = false;
{ {
// Individually trace ::GetQueueStatus and ::PeekMessage because sampling // Individually trace ::GetQueueStatus and ::PeekMessage because sampling
...@@ -473,22 +477,7 @@ bool MessagePumpForUI::ProcessNextWindowsMessage() { ...@@ -473,22 +477,7 @@ bool MessagePumpForUI::ProcessNextWindowsMessage() {
// profiler's thread while the sampled thread is swapped out on this frame). // profiler's thread while the sampled thread is swapped out on this frame).
TRACE_EVENT0("base", TRACE_EVENT0("base",
"MessagePumpForUI::ProcessNextWindowsMessage GetQueueStatus"); "MessagePumpForUI::ProcessNextWindowsMessage GetQueueStatus");
DWORD queue_status = ::GetQueueStatus(QS_SENDMESSAGE);
// Experiment with querying all messages and not invoking PeekMessage at all
// if there's nothing there.
static const auto get_queue_status_flag =
FeatureList::IsEnabled(kPreventMessagePumpHangs) ? QS_ALLINPUT
: QS_SENDMESSAGE;
const DWORD queue_status = ::GetQueueStatus(get_queue_status_flag);
if (get_queue_status_flag == QS_ALLINPUT && !queue_status)
return false;
// If there were sent messages in the queue then ::PeekMessage internally
// dispatches the message and returns false. We return true in this case to
// ensure that the message loop peeks again instead of calling
// MsgWaitForMultipleObjectsEx.
if (HIWORD(queue_status) & QS_SENDMESSAGE) if (HIWORD(queue_status) & QS_SENDMESSAGE)
more_work_is_plausible = true; more_work_is_plausible = true;
} }
...@@ -563,45 +552,32 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() { ...@@ -563,45 +552,32 @@ bool MessagePumpForUI::ProcessPumpReplacementMessage() {
// that peeked replacement. Note that the re-post of kMsgHaveWork may be // that peeked replacement. Note that the re-post of kMsgHaveWork may be
// asynchronous to this thread!! // asynchronous to this thread!!
// Bump the work id since ::PeekMessage may process internal events.
state_->delegate->BeforeDoInternalWork();
// The system headers don't define this; it's equivalent to PM_QS_INPUT |
// PM_QS_PAINT | PM_QS_POSTMESSAGE. i.e., anything but QS_SENDMESSAGE. Since
// we're looking to replace our kMsgHaveWork posted message, we can ignore
// sent messages (which never compete with posted messages in the initial
// PeekMessage call).
constexpr auto PM_QS_ALLEVENTS = QS_ALLEVENTS << 16;
static_assert(
PM_QS_ALLEVENTS == (PM_QS_INPUT | PM_QS_PAINT | PM_QS_POSTMESSAGE), "");
static_assert((PM_QS_ALLEVENTS & PM_QS_SENDMESSAGE) == 0, "");
MSG msg; MSG msg;
bool have_message = false; bool have_message = false;
{ {
// The system headers don't define this; it's equivalent to PM_QS_INPUT | TRACE_EVENT0("base",
// PM_QS_PAINT | PM_QS_POSTMESSAGE. i.e., anything but QS_SENDMESSAGE. Since "MessagePumpForUI::ProcessPumpReplacementMessage PeekMessage");
// we're looking to replace our kMsgHaveWork posted message, we can ignore
// sent messages (which never compete with posted messages in the initial
// PeekMessage call).
constexpr auto PM_QS_ALLEVENTS = QS_ALLEVENTS << 16;
static_assert(
PM_QS_ALLEVENTS == (PM_QS_INPUT | PM_QS_PAINT | PM_QS_POSTMESSAGE), "");
static_assert((PM_QS_ALLEVENTS & PM_QS_SENDMESSAGE) == 0, "");
static const auto peek_replacement_message_modifier = static const auto peek_replacement_message_modifier =
FeatureList::IsEnabled(kPreventMessagePumpHangs) ? PM_QS_ALLEVENTS : 0; base::FeatureList::IsEnabled(kPreventMessagePumpHangs) ? PM_QS_ALLEVENTS
: 0;
// Always check messages under the non-experimental arm but skip in the
// experimental arm if GetQueueStatus() doesn't think there are any.
bool check_messages = true;
{
if (peek_replacement_message_modifier == PM_QS_ALLEVENTS) {
TRACE_EVENT0(
"base",
"MessagePumpForUI::ProcessPumpReplacementMessage GetQueueStatus");
check_messages = ::GetQueueStatus(QS_ALLEVENTS);
}
}
if (check_messages) {
TRACE_EVENT0(
"base",
"MessagePumpForUI::ProcessPumpReplacementMessage PeekMessage");
// Bump the work id since ::PeekMessage may process internal events. have_message =
state_->delegate->BeforeDoInternalWork(); ::PeekMessage(&msg, nullptr, 0, 0,
have_message = PM_REMOVE | peek_replacement_message_modifier) != FALSE;
::PeekMessage(&msg, nullptr, 0, 0,
PM_REMOVE | peek_replacement_message_modifier) != FALSE;
}
} }
// Expect no message or a message different than kMsgHaveWork. // Expect no message or a message different than kMsgHaveWork.
......
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