Prevent redundant DoWorks due to canceled delayed tasks

Prevent redundant DoWorks due to canceled delayed tasks

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication
logic to only post a delayed DoWork if the task is meant to run before
any previously registered delayed DoWorks.

BUG=638542, 605718
Committed: https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013
Cr-Commit-Position: refs/heads/master@{#418556}

[alex clarke (OOO till 29th), 2016-09-09 16:33:26]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 16:33:42]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alex clarke (OOO till 29th), 2016-09-09 16:33:44]

alexclarke@chromium.org changed reviewers:
+ skyostil@chromium.org

[alex clarke (OOO till 29th), 2016-09-09 16:33:45]

PTAL!

[commit-bot: I haz the power, 2016-09-09 16:35:39]

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

[commit-bot: I haz the power, 2016-09-09 16:35:40]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[alex clarke (OOO till 29th), 2016-09-09 17:04:26]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-09 17:04:43]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-09 17:06:35]

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

[commit-bot: I haz the power, 2016-09-09 17:06:35]

Dry run: Try jobs failed on following builders:
  ios-device on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-device/builds...)
  ios-simulator on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/ios-simulator/bui...)
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[alex clarke (OOO till 29th), 2016-09-12 14:49:14]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-12 14:49:31]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Sami, 2016-09-12 15:30:32]

Here's a dumb question: when scheduling the next wake up in the time domain, why
don't we check for cancelled tasks at that point and skip forward until we find
a task which wasn't cancelled? It doesn't seem like sweeping the tasks away
eagerly buys us much on top of making sure we don't schedule a wake up for a
task which was cancelled.

[commit-bot: I haz the power, 2016-09-12 16:23:02]

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

[commit-bot: I haz the power, 2016-09-12 16:23:04]

Dry run: This issue passed the CQ dry run.

[alex clarke (OOO till 29th), 2016-09-13 14:40:07]

Description was changed from

==========
Periodically sweep away incoming canceled delayed tasks

This helps avoid pointless wakeups and reduces the size of the
incoming delayed task queues which would often otherwise contain a
lot of canceled delayed tasks.

BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all of
them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next wakeup (if
any).  Since it removes all canceled
delayed tasks from the front of the priority queue this has the effect of not
scheduling wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication logic to
only post a delayed DoWork if the task is meant to run before any previously
registered delayed DoWorks.



BUG=638542
==========

[alex clarke (OOO till 29th), 2016-09-13 14:40:15]

The CQ bit was checked by alexclarke@chromium.org to run a CQ dry run

[alex clarke (OOO till 29th), 2016-09-13 14:40:34]

On 2016/09/12 15:30:32, Sami wrote:
> Here's a dumb question: when scheduling the next wake up in the time domain,
why
> don't we check for cancelled tasks at that point and skip forward until we
find
> a task which wasn't cancelled? It doesn't seem like sweeping the tasks away
> eagerly buys us much on top of making sure we don't schedule a wake up for a
> task which was cancelled.

Turns out it was easier to do that than anticipated. PTAL.

[commit-bot: I haz the power, 2016-09-13 14:40:39]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-13 16:16:35]

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

[commit-bot: I haz the power, 2016-09-13 16:16:36]

Dry run: This issue passed the CQ dry run.

[alex clarke (OOO till 29th), 2016-09-13 17:09:02]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all of
them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next wakeup (if
any).  Since it removes all canceled
delayed tasks from the front of the priority queue this has the effect of not
scheduling wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication logic to
only post a delayed DoWork if the task is meant to run before any previously
registered delayed DoWorks.



BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all of
them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks 
from the front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

[alex clarke (OOO till 29th), 2016-09-13 17:09:13]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all of
them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks 
from the front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks 
from the front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

[alex clarke (OOO till 29th), 2016-09-13 17:09:34]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks 
from the front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks from the front of the priority queue this has the effect of
not scheduling wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

[alex clarke (OOO till 29th), 2016-09-13 17:10:11]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled
delayed tasks from the front of the priority queue this has the effect of
not scheduling wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

[Sami, 2016-09-13 17:42:54]

lgtm -- I like how this clarifies the responsibility of the TimeDomain.

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:177: void
MoveReadyDelayedTasksToDelayedWorkQueue(LazyNow* lazy_now);
Not sure if we need to do this now, but maybe we could rename this function to
something like ProcessDelayedWork() or something else that better captures its
new role.

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
(right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1894:
for (;;) {
nit: could extract this to a helper method -- up to you.

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/time_domain.h:34: // have
nit: reformat

[alex clarke (OOO till 29th), 2016-09-14 09:04:24]

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:177: void
MoveReadyDelayedTasksToDelayedWorkQueue(LazyNow* lazy_now);
On 2016/09/13 17:42:53, Sami wrote:
> Not sure if we need to do this now, but maybe we could rename this function to
> something like ProcessDelayedWork() or something else that better captures its
> new role.

Done.

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
(right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1894:
for (;;) {
On 2016/09/13 17:42:53, Sami wrote:
> nit: could extract this to a helper method -- up to you.

Done.

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
File third_party/WebKit/Source/platform/scheduler/base/time_domain.h (right):

https://codereview.chromium.org/2320403003/diff/80001/third_party/WebKit/Sour...
third_party/WebKit/Source/platform/scheduler/base/time_domain.h:34: // have
On 2016/09/13 17:42:53, Sami wrote:
> nit: reformat

Done.

[alex clarke (OOO till 29th), 2016-09-14 09:04:30]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2016-09-14 09:04:30]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org
Link to the patchset: https://codereview.chromium.org/2320403003/#ps100001
(title: "Changes for Sami")

[commit-bot: I haz the power, 2016-09-14 09:04:41]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[alex clarke (OOO till 29th), 2016-09-14 09:08:36]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542, 605718
==========

[commit-bot: I haz the power, 2016-09-14 11:32:58]

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

[commit-bot: I haz the power, 2016-09-14 11:32:59]

Try jobs failed on following builders:
  win_chromium_rel_ng on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[alex clarke (OOO till 29th), 2016-09-14 12:54:50]

The CQ bit was checked by alexclarke@chromium.org

[commit-bot: I haz the power, 2016-09-14 12:55:06]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-14 14:30:04]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542, 605718
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542, 605718
==========

[commit-bot: I haz the power, 2016-09-14 14:30:06]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-09-14 14:32:07]

Description was changed from

==========
Prevent redundant DoWorks due to canceled delayed tasks 

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all 
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next 
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling 
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication 
logic to only post a delayed DoWork if the task is meant to run before 
any previously registered delayed DoWorks.


BUG=638542, 605718
==========

to

==========
Prevent redundant DoWorks due to canceled delayed tasks

To achieve this we make a few changes:

1. We only register the next wakeup with the TimeDomain, rather than all
of them.
2. MoveReadyDelayedTasksToDelayedWorkQueue now registers the next
wakeup (if any).  Since it removes all canceled delayed tasks from the
front of the priority queue this has the effect of not scheduling
wakeups for cancelled tasks.
3. Tweaking the TaskQueueManager level delayed DoWork de-duplication
logic to only post a delayed DoWork if the task is meant to run before
any previously registered delayed DoWorks.

BUG=638542, 605718

Committed: https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013
Cr-Commit-Position: refs/heads/master@{#418556}
==========

[commit-bot: I haz the power, 2016-09-14 14:32:08]

Patchset 6 (id:??) landed as
https://crrev.com/929cbb9f92b5570867c3842c80778243db81a013
Cr-Commit-Position: refs/heads/master@{#418556}

[Ken Russell (switch to Gerrit), 2016-09-19 18:20:11]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.chromium.org/2353473003/ by kbr@chromium.org.

The reason for reverting is: Suspect that this CL caused breakage in Gmail and
Hangouts per http://crbug.com/647484 .
.