Make tasks cancellable inside the blink scheduler.

Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

BUG=638542, 605718
Committed: https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99
Cr-Commit-Position: refs/heads/master@{#413120}

[alex clarke (OOO till 29th), 2016-08-18 13:27:57]

Description was changed from

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

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

to

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

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

[alex clarke (OOO till 29th), 2016-08-18 13:27:57]

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

[alex clarke (OOO till 29th), 2016-08-18 13:28:02]

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

[alex clarke (OOO till 29th), 2016-08-18 13:28:13]

PTAL :)

[commit-bot: I haz the power, 2016-08-18 13:28:23]

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-08-18 15:42:47]

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

[commit-bot: I haz the power, 2016-08-18 15:42:48]

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

[Sami, 2016-08-18 15:53:42]

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

[Sami, 2016-08-18 15:53:44]

Looks awesome. Thanks for adding the class-level comments too. Added some minor
observations.

I'll cc haraken@ here too since he had some comments on the internals on the
older patch.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:18: // A 64bit
integer used to provide ordering of tasks.
Should we also mention we don't support these wrapping around? I'm not sure if
that was the case previously either...

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:28: static
bool isValidEnqueueOrder(EnqueueOrder enqueue_order) {
nit: IsValidEnqueueOrder

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:249: if
(!main_thread_only().task_queue_manager)
We don't seem to need TQM below anymore, so maybe we can remove this early-out?
Same for IsTaskPending.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:258: Task
fake_task(FROM_HERE, base::Closure(), handle.scheduled_run_time_,
nit: not sure if the compiler is smart enough to eliminate it, but maybe we
could use tracked_object::Location() instead of FROM_HERE?

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:268: Task
fake_task(FROM_HERE, base::Closure(), handle.scheduled_run_time_,
Readability suggestion -- feel free to ignore: Should we have a private
constructor on the task which takes a task handle to make a fake task? Would cut
down on some of the repetition here.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:43: //
immediate_work_queue is swapped with immediate_incoming_queue when it becomes
nit: it => the former

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:54: // queue
is selected, it round-robbins between the immediate_work_queue and
typo: round-robins

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:124: // is
set when posted, but for delayed tasks it's defined until they are
it's defined until => it's not defined until?

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1943:
TEST_F(TaskQueueManagerTest, PostCancellabledTask) {
typo: -d

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1954:
TEST_F(TaskQueueManagerTest, PostCancellableTask_AndCancel_OnIncommingQueue) {
typo: Incoming

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:2304:
TEST_F(TaskQueueManagerTest, TryToCancelDelayedTaskAfterItsRun) {
Is this the same as TryToCancelTaskAfterItsRun? Or did you mean to use a
non-zero delay here?

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc:89:
web_task_runner_.reset();
Did you mean to make this change?

[alex clarke (OOO till 29th), 2016-08-18 16:51:41]

PTAL

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h (right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:18: // A 64bit
integer used to provide ordering of tasks.
On 2016/08/18 15:53:43, Sami wrote:
> Should we also mention we don't support these wrapping around? I'm not sure if
> that was the case previously either...

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/enqueue_order.h:28: static
bool isValidEnqueueOrder(EnqueueOrder enqueue_order) {
On 2016/08/18 15:53:43, Sami wrote:
> nit: IsValidEnqueueOrder

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:249: if
(!main_thread_only().task_queue_manager)
On 2016/08/18 15:53:43, Sami wrote:
> We don't seem to need TQM below anymore, so maybe we can remove this
early-out?
> Same for IsTaskPending.

Right but we do need the immediate_work_queue & delayed_work_queue and (kinda)
time domain.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:258: Task
fake_task(FROM_HERE, base::Closure(), handle.scheduled_run_time_,
On 2016/08/18 15:53:43, Sami wrote:
> nit: not sure if the compiler is smart enough to eliminate it, but maybe we
> could use tracked_object::Location() instead of FROM_HERE?

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.cc:268: Task
fake_task(FROM_HERE, base::Closure(), handle.scheduled_run_time_,
On 2016/08/18 15:53:43, Sami wrote:
> Readability suggestion -- feel free to ignore: Should we have a private
> constructor on the task which takes a task handle to make a fake task? Would
cut
> down on some of the repetition here.

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:43: //
immediate_work_queue is swapped with immediate_incoming_queue when it becomes
On 2016/08/18 15:53:43, Sami wrote:
> nit: it => the former

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:54: // queue
is selected, it round-robbins between the immediate_work_queue and
On 2016/08/18 15:53:43, Sami wrote:
> typo: round-robins

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:124: // is
set when posted, but for delayed tasks it's defined until they are
On 2016/08/18 15:53:43, Sami wrote:
> it's defined until => it's not defined until?

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1943:
TEST_F(TaskQueueManagerTest, PostCancellabledTask) {
On 2016/08/18 15:53:43, Sami wrote:
> typo: -d

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:1954:
TEST_F(TaskQueueManagerTest, PostCancellableTask_AndCancel_OnIncommingQueue) {
On 2016/08/18 15:53:43, Sami wrote:
> typo: Incoming

Done.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/base/task_queue_manager_unittest.cc:2304:
TEST_F(TaskQueueManagerTest, TryToCancelDelayedTaskAfterItsRun) {
On 2016/08/18 15:53:43, Sami wrote:
> Is this the same as TryToCancelTaskAfterItsRun? Or did you mean to use a
> non-zero delay here?

Hmm busted test, fixed now.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc:89:
web_task_runner_.reset();
On 2016/08/18 15:53:44, Sami wrote:
> Did you mean to make this change?

Yes, it's not strictly needed in this patch. I was having crashes in
WebThreadImplForWorkerSchedulerTest with out it in the big patch.

I think it's actually a bug today.  Do you want me to move it to a later patch?

[alex clarke (OOO till 29th), 2016-08-18 16:51:53]

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

[commit-bot: I haz the power, 2016-08-18 16:52:51]

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

[Sami, 2016-08-18 17:12:18]

lgtm -- let's give Kentaro a chance to take a look too.

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
File
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc
(right):

https://codereview.chromium.org/2258713004/diff/1/third_party/WebKit/Source/p...
third_party/WebKit/Source/platform/scheduler/child/webthread_impl_for_worker_scheduler.cc:89:
web_task_runner_.reset();
On 2016/08/18 16:51:41, alex clarke wrote:
> On 2016/08/18 15:53:44, Sami wrote:
> > Did you mean to make this change?
> 
> Yes, it's not strictly needed in this patch. I was having crashes in
> WebThreadImplForWorkerSchedulerTest with out it in the big patch.
> 
> I think it's actually a bug today.  Do you want me to move it to a later
patch?

Okay, just checking it was intended.

[commit-bot: I haz the power, 2016-08-18 17:25:33]

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

[commit-bot: I haz the power, 2016-08-18 17:25:34]

Dry run: Try jobs failed on following builders:
  win_clang on master.tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_clang/builds/...)

[alex clarke (OOO till 29th), 2016-08-18 17:37:02]

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

[commit-bot: I haz the power, 2016-08-18 17:37:40]

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-08-18 19:50:11]

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

[commit-bot: I haz the power, 2016-08-18 19:50:12]

Dry run: Try jobs failed on following builders:
  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-08-18 20:23:19]

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

[commit-bot: I haz the power, 2016-08-18 20:24:05]

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-08-18 21:30:16]

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

[commit-bot: I haz the power, 2016-08-18 21:31:20]

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-08-18 22:43:08]

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

[commit-bot: I haz the power, 2016-08-18 22:44:07]

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-08-18 22:52:08]

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

[commit-bot: I haz the power, 2016-08-18 22:52:10]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[alex clarke (OOO till 29th), 2016-08-19 07:32:59]

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

[commit-bot: I haz the power, 2016-08-19 07:33:32]

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-08-19 07:41:07]

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

[commit-bot: I haz the power, 2016-08-19 07:41:08]

Dry run: Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[alex clarke (OOO till 29th), 2016-08-19 09:15:21]

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

[commit-bot: I haz the power, 2016-08-19 09:15:46]

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

[haraken, 2016-08-19 10:42:51]

Sorry about the delayed review. LGTM.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:129: //
enqueued on the |immediate_incoming_queue_|. This is because otherwise

|immediate_incoming_queue_| => |delayed_work_queue_| ?

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/time_domain.cc (right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/time_domain.cc:103: if
(first_wakeup == prev_first_wakeup)

I'm just curious but what is this check doing?

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/time_domain.cc:110: break;

This for loop finishes when it encounters the first task that is associated with
|queue|. Why is it enough to remove only the task encountered first?

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue.h (right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.h:60: // to be the
origional task posted, rather it must match the key of the task

original

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.h:60: // to be the
origional task posted, rather it must match the key of the task

I don't fully get the meaning of this comment. What's a difference between "the
original task posted" and "the task posted"?

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.h
(right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.h:28: // works
is std::map is an tree based associative container and all the values

a tree-based

[commit-bot: I haz the power, 2016-08-19 10:44:24]

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

[commit-bot: I haz the power, 2016-08-19 10:44:25]

Dry run: This issue passed the CQ dry run.

[alex clarke (OOO till 29th), 2016-08-19 11:02:59]

Thanks, all comments addressed.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h
(right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/task_queue_impl.h:129: //
enqueued on the |immediate_incoming_queue_|. This is because otherwise
On 2016/08/19 10:42:51, haraken wrote:
> 
> |immediate_incoming_queue_| => |delayed_work_queue_| ?

Oops yes. Good catch.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/time_domain.cc (right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/time_domain.cc:103: if
(first_wakeup == prev_first_wakeup)
On 2016/08/19 10:42:51, haraken wrote:
> 
> I'm just curious but what is this check doing?

I added a comment, it's checking if the first entry in
|delayed_wakeup_multimap_| still has the same |delayed_run_time|.  If it hasn't
changed we don't need to reschedule the wakeup.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/time_domain.cc:110: break;
On 2016/08/19 10:42:51, haraken wrote:
> 
> This for loop finishes when it encounters the first task that is associated
with
> |queue|. Why is it enough to remove only the task encountered first?

I added a comment above but there might be other queues that happen to require a
wake up at |delayed_run_time| and we must respect that or timers won't fire at
the expected time.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue.h (right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.h:60: // to be the
origional task posted, rather it must match the key of the task
On 2016/08/19 10:42:51, haraken wrote:
> 
> original

Done.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue.h:60: // to be the
origional task posted, rather it must match the key of the task
On 2016/08/19 10:42:51, haraken wrote:
> 
> I don't fully get the meaning of this comment. What's a difference between
"the
> original task posted" and "the task posted"?

The handle doesn't contain all the information in TaskQueueImpl::Task, and we
use 
TaskQueueImpl::Task::CreateFakeTaskFromHandle to create a fake task which
matches the
key in so far as the comparators are concerned.

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
File third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.h
(right):

https://codereview.chromium.org/2258713004/diff/100001/third_party/WebKit/Sou...
third_party/WebKit/Source/platform/scheduler/base/work_queue_sets.h:28: // works
is std::map is an tree based associative container and all the values
On 2016/08/19 10:42:51, haraken wrote:
> 
> a tree-based

Done.

[alex clarke (OOO till 29th), 2016-08-19 11:03:06]

The CQ bit was checked by alexclarke@chromium.org

[alex clarke (OOO till 29th), 2016-08-19 11:03:06]

The patchset sent to the CQ was uploaded after l-g-t-m from
skyostil@chromium.org, haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/2258713004/#ps140001
(title: "Various comment nits addressed")

[commit-bot: I haz the power, 2016-08-19 11:03:38]

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-08-19 12:55:59]

Description was changed from

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

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

to

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

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

[commit-bot: I haz the power, 2016-08-19 12:56:00]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2016-08-19 12:57:57]

Description was changed from

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

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

to

==========
Make tasks cancellable inside the blink scheduler.

The complexity of WorkQueue got a bit worse O(log n) insert & pop
but this seems unlikely to matter in prartice because the queues
never really get big enough for this to matter.

Follow on patches will enable Timer<> to use the new style of
cancellation which result in smaller queues.  Note the cost of
running a NOP task is currently quite high so overall we should
get a net benefit once Blink tasks use the new API even if this
patch adds extra overhead.

BUG=638542, 605718

Committed: https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99
Cr-Commit-Position: refs/heads/master@{#413120}
==========

[commit-bot: I haz the power, 2016-08-19 12:57:59]

Patchset 8 (id:??) landed as
https://crrev.com/098f0e5e5786dbccfe2aef2d9b8319dc92798b99
Cr-Commit-Position: refs/heads/master@{#413120}