Avoid scheduling a message loop that we know is not sleeping

Avoid scheduling a message loop that we know is not sleeping

When tasks are posted to a message loop, we may need to schedule the
message loop to wake up if it was sleeping so it processes the message.
The incoming task queue checks whether the incoming queue was already
empty as a way to avoid scheduling unnecessarily, but we can be more
precise. Once a message loop is scheduled, we know that it will sooner
or later wake up and enter its run loop. In its run loop it will
repeatedly drain the work queue and attempt to reload from the incoming
queue until there is no work immediately available to do. This means that
before waiting for more work the loop will always reload from an empty
incoming queue. Thus, once we schedule a loop we do not need to schedule
the same loop again until after an empty reload occurs.

This adds a bool to IncomingTaskQueue protected by the incoming task
queue lock that keeps track of if the loop has been scheduled since the
last empty reload.

Local testing on Linux desktop and a ChromeShell Android build shows
that this avoids roughly 40% of the ScheduleWork calls when browsing
around to random websites.

This also has the very nice consequence that when running a task and
posting another task to the same thread we *never* need to invoke
ScheduleWork which avoids the cost of the ScheduleWork call and avoids
a spurious wakeup when going to sleep.

BUG=412137
Committed: https://crrev.com/23a5e54a3ae47b3f5dfb9ca62e19f10db02d1b42
Cr-Commit-Position: refs/heads/master@{#308221}

[jamesr, 2014-11-15 01:52:41]

jamesr@chromium.org changed reviewers:
+ danakj@chromium.org, rvargas@chromium.org

[jamesr, 2014-11-15 01:52:42]

Dana and/or Ricardo - would you mind taking a look at this patch?  It should be
a nice speedup in many common cases (not huge, but nice).

[rvargas (doing something else), 2014-11-17 19:18:50]

Cool!. lgtm after nits.

https://codereview.chromium.org/614723003/diff/1/base/message_loop/incoming_t...
File base/message_loop/incoming_task_queue.cc (right):

https://codereview.chromium.org/614723003/diff/1/base/message_loop/incoming_t...
base/message_loop/incoming_task_queue.cc:17: // Returns true if
MessagePump::ScheduleWork() must be called one
nit: add empty line here.

https://codereview.chromium.org/614723003/diff/1/base/message_loop/incoming_t...
base/message_loop/incoming_task_queue.cc:29: }
nit: add empty line and "  // namespace"

https://codereview.chromium.org/614723003/diff/1/base/message_loop/incoming_t...
base/message_loop/incoming_task_queue.cc:77: if (!incoming_queue_.empty()) {
nit: invert the order of the if.

https://codereview.chromium.org/614723003/diff/1/base/message_loop/incoming_t...
base/message_loop/incoming_task_queue.cc:80: // If the loop attempts to reload
but there are no tasks in the incoming
Having to assume what the owner class will do is not very nice, but these two
are strongly tied anyway and the gains seem worth it.

[jamesr, 2014-12-08 22:51:25]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-12-08 22:52:10]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614723003/20001

[commit-bot: I haz the power, 2014-12-08 22:59:17]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-08 22:59:19]

Try jobs failed on following builders:
  android_chromium_gn_compile_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_chromiu...)

[jamesr, 2014-12-08 23:04:34]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-12-08 23:05:17]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614723003/40001

[commit-bot: I haz the power, 2014-12-08 23:20:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-08 23:20:22]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[jamesr, 2014-12-09 01:33:05]

The CQ bit was checked by jamesr@chromium.org

[commit-bot: I haz the power, 2014-12-09 01:34:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614723003/60001

[commit-bot: I haz the power, 2014-12-09 05:37:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-09 05:37:53]

Try jobs failed on following builders:
  android_dbg_tests_recipe on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_dbg_tes...)

[jamesr, 2014-12-10 21:58:46]

Seems to be having trouble on some android tests, which isn't super surprising
since the android java pump behaves very differently from other pumps.  I'll
study and ask for a new review when I have a fix.

[jamesr, 2014-12-11 02:26:39]

I was able to repro the android instrumentation test failures locally and
construct a fix (commented below).  rvargas@ - PTAL and if it makes sense to you
(and is green on the trybots) please click the CQ button.

https://codereview.chromium.org/614723003/diff/80001/base/message_loop/incomi...
File base/message_loop/incoming_task_queue.cc (right):

https://codereview.chromium.org/614723003/diff/80001/base/message_loop/incomi...
base/message_loop/incoming_task_queue.cc:137: if (always_schedule_work_ ||
(!message_loop_scheduled_ && was_empty)) {
this is the diff that fixes android pumps.  Instead of only scheduling android
pumps when we know we aren't scheduled, we always signal.  This matches previous
behavior and is required by the java pump since it uses a token in the java
queue for each task to schedule fairly.

[rvargas (doing something else), 2014-12-11 18:28:32]

lgtm

[rvargas (doing something else), 2014-12-11 18:28:38]

The CQ bit was checked by rvargas@chromium.org

[commit-bot: I haz the power, 2014-12-11 18:29:13]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614723003/80001

[commit-bot: I haz the power, 2014-12-11 21:10:12]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-12-11 21:10:13]

Try jobs failed on following builders:
  linux_chromium_asan_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[jamesr, 2014-12-13 01:52:52]

The CQ bit was checked by jamesr@chromium.org

[jamesr, 2014-12-13 01:53:05]

On 2014/12/11 21:10:13, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   linux_chromium_asan_rel on tryserver.chromium.linux
>
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

Flake - sending the same patch to the same trybot again went green.

[commit-bot: I haz the power, 2014-12-13 01:53:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/614723003/80001

[commit-bot: I haz the power, 2014-12-13 01:55:38]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2014-12-13 01:56:34]

Patchset 5 (id:??) landed as
https://crrev.com/23a5e54a3ae47b3f5dfb9ca62e19f10db02d1b42
Cr-Commit-Position: refs/heads/master@{#308221}