Experimental: Chrome side of killing the blink timer heap

Experimental: Chrome side of killing the blink timer heap

Not currently for review

BUG=463143

[Sami, 2015-02-27 18:22:33]

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

[Sami, 2015-02-27 18:22:33]

Doesn't seem too bad to me. Do we need to change the work queue yielding
behavior for this?

https://codereview.chromium.org/962273002/diff/20001/base/debug/task_annotato...
File base/debug/task_annotator.cc (right):

https://codereview.chromium.org/962273002/diff/20001/base/debug/task_annotato...
base/debug/task_annotator.cc:47: "toplevel",
pending_task.posted_from.function_name(),
Wanna pull this out to a separate patch? We may want to land it if it doesn't
turn out to be too confusing.

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:128: while
(!delayed_task_queue_.empty())
Is there a clear() or reset() that does this in one go?

[rmcilroy, 2015-03-02 13:53:57]

rmcilroy@chromium.org changed reviewers:
+ rmcilroy@chromium.org

[rmcilroy, 2015-03-02 13:53:57]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:158: return true;
drive-by comment: I'm wondering if it be simpler just to simply post a wrapper
task which does the following:

void TaskQueue::EnqueueNextPendingTask() {
  base::AutoLock lock(lock_);
  if (!task_queue_manager_)
    return;

  if (delayed_task_queue_.empty())
    return;

  EnqueueTaskLocked(delayed_task_queue_.pop());
}

And simply always post this for delayed task. This would be much more similar to
the previous approach except the wrapper task now doesn't point to the task
itself, allowing us to delete the pending taskson shutdown.  While I like the
idea of trying to merge multiple pending tasks into a single pending task
wrapper on the message loop, I'm not sure there will be enough pending tasks
with exactly the same delayed_runtime to justify the extra complexity (but I
don't know the DOMTimer code well so I may be wrong here).  WDYT?

[alex clarke (OOO till 29th), 2015-03-02 16:08:10]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:158: return true;
On 2015/03/02 13:53:57, rmcilroy wrote:
> drive-by comment: I'm wondering if it be simpler just to simply post a wrapper
> task which does the following:
> 
> void TaskQueue::EnqueueNextPendingTask() {
>   base::AutoLock lock(lock_);
>   if (!task_queue_manager_)
>     return;
> 
>   if (delayed_task_queue_.empty())
>     return;
> 
>   EnqueueTaskLocked(delayed_task_queue_.pop());
> }
> 
> And simply always post this for delayed task. This would be much more similar
to
> the previous approach except the wrapper task now doesn't point to the task
> itself, allowing us to delete the pending taskson shutdown.  While I like the
> idea of trying to merge multiple pending tasks into a single pending task
> wrapper on the message loop, I'm not sure there will be enough pending tasks
> with exactly the same delayed_runtime to justify the extra complexity (but I
> don't know the DOMTimer code well so I may be wrong here).  WDYT?

That's a really interesting idea.  I quite like the simplicity of it but on the
other hand (based on the verge and some other popular heavy sites such as
sports.yahoo.com and latimes):

1. While the page loads it's not uncommon to have several hundred delayed tasks
in flight
2. It's quite common for 10+ of these delays to "expire" simultaneously (I've
seen a couple of times when it reached 20+)
3. Typically we'd only have single digit number of KickDelayedTasks's posted on
the chromium run loop.

This suggests that the more complicated approach taken by this patch has some
merit.

[Sami, 2015-03-02 16:38:33]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:158: return true;
On 2015/03/02 16:08:10, alexclarke1 wrote:
> On 2015/03/02 13:53:57, rmcilroy wrote:
> > drive-by comment: I'm wondering if it be simpler just to simply post a
wrapper
> > task which does the following:
> > 
> > void TaskQueue::EnqueueNextPendingTask() {
> >   base::AutoLock lock(lock_);
> >   if (!task_queue_manager_)
> >     return;
> > 
> >   if (delayed_task_queue_.empty())
> >     return;
> > 
> >   EnqueueTaskLocked(delayed_task_queue_.pop());
> > }
> > 
> > And simply always post this for delayed task. This would be much more
similar
> to
> > the previous approach except the wrapper task now doesn't point to the task
> > itself, allowing us to delete the pending taskson shutdown.  While I like
the
> > idea of trying to merge multiple pending tasks into a single pending task
> > wrapper on the message loop, I'm not sure there will be enough pending tasks
> > with exactly the same delayed_runtime to justify the extra complexity (but I
> > don't know the DOMTimer code well so I may be wrong here).  WDYT?
> 
> That's a really interesting idea.  I quite like the simplicity of it but on
the
> other hand (based on the verge and some other popular heavy sites such as
> http://sports.yahoo.com and latimes):
> 
> 1. While the page loads it's not uncommon to have several hundred delayed
tasks
> in flight
> 2. It's quite common for 10+ of these delays to "expire" simultaneously (I've
> seen a couple of times when it reached 20+)
> 3. Typically we'd only have single digit number of KickDelayedTasks's posted
on
> the chromium run loop.
> 
> This suggests that the more complicated approach taken by this patch has some
> merit.

As one more data point, this comment here suggests that having lots of delayed
tasks that expire simultaneously is a thing that happens:

https://code.google.com/p/chromium/codesearch#chromium/src/base/message_loop/...

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:167: // Enqueue all delayed
tasks that should be running now.
base::MessageLoop always interleaves delayed and non-delayed work, whereas here
we're adding all expired delayed tasks as a monolithic block (which we can still
schedule of course). I wonder if that might have some bad consequences though by
starving other things on the default queue?

Of course if we are moving those tasks off of the default task runner, then
we're kind of relying the scheduler to handle the interleaving for us. Maybe
overall that is the better way to do things anyway.

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:281:
incoming_queue_.back().delayed_run_time = base::TimeTicks();
int: could do this unconditionally now.

[alex clarke (OOO till 29th), 2015-03-02 16:47:50]

On 2015/03/02 16:38:33, Sami wrote:
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> File content/renderer/scheduler/task_queue_manager.cc (right):
> 
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> content/renderer/scheduler/task_queue_manager.cc:158: return true;
> On 2015/03/02 16:08:10, alexclarke1 wrote:
> > On 2015/03/02 13:53:57, rmcilroy wrote:
> > > drive-by comment: I'm wondering if it be simpler just to simply post a
> wrapper
> > > task which does the following:
> > > 
> > > void TaskQueue::EnqueueNextPendingTask() {
> > >   base::AutoLock lock(lock_);
> > >   if (!task_queue_manager_)
> > >     return;
> > > 
> > >   if (delayed_task_queue_.empty())
> > >     return;
> > > 
> > >   EnqueueTaskLocked(delayed_task_queue_.pop());
> > > }
> > > 
> > > And simply always post this for delayed task. This would be much more
> similar
> > to
> > > the previous approach except the wrapper task now doesn't point to the
task
> > > itself, allowing us to delete the pending taskson shutdown.  While I like
> the
> > > idea of trying to merge multiple pending tasks into a single pending task
> > > wrapper on the message loop, I'm not sure there will be enough pending
tasks
> > > with exactly the same delayed_runtime to justify the extra complexity (but
I
> > > don't know the DOMTimer code well so I may be wrong here).  WDYT?
> > 
> > That's a really interesting idea.  I quite like the simplicity of it but on
> the
> > other hand (based on the verge and some other popular heavy sites such as
> > http://sports.yahoo.com and latimes):
> > 
> > 1. While the page loads it's not uncommon to have several hundred delayed
> tasks
> > in flight
> > 2. It's quite common for 10+ of these delays to "expire" simultaneously
(I've
> > seen a couple of times when it reached 20+)
> > 3. Typically we'd only have single digit number of KickDelayedTasks's posted
> on
> > the chromium run loop.
> > 
> > This suggests that the more complicated approach taken by this patch has
some
> > merit.
> 
> As one more data point, this comment here suggests that having lots of delayed
> tasks that expire simultaneously is a thing that happens:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/message_loop/...

It does sound that way.  To me it looks like that code looks like it's trying to
avoid calling Now() a lot, which probably is sensible.

> 
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> content/renderer/scheduler/task_queue_manager.cc:167: // Enqueue all delayed
> tasks that should be running now.
> base::MessageLoop always interleaves delayed and non-delayed work, whereas
here

> we're adding all expired delayed tasks as a monolithic block (which we can
still
> schedule of course). I wonder if that might have some bad consequences though
by
> starving other things on the default queue?

Actually that's a good point.  Previously the blink timer heap would run up to
50 ms of work.  There isn't a limit here (or with Ross's approach either afaik),
maybe there should be.

I suspect adding a TimerQueue would make it much easier to do that.  WDYT?

> 
> Of course if we are moving those tasks off of the default task runner, then
> we're kind of relying the scheduler to handle the interleaving for us. Maybe
> overall that is the better way to do things anyway.
> 
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> content/renderer/scheduler/task_queue_manager.cc:281:
> incoming_queue_.back().delayed_run_time = base::TimeTicks();
> int: could do this unconditionally now.

[alex clarke (OOO till 29th), 2015-03-02 16:49:12]

On 2015/03/02 16:47:50, alexclarke1 wrote:
> On 2015/03/02 16:38:33, Sami wrote:
> >
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> > File content/renderer/scheduler/task_queue_manager.cc (right):
> > 
> >
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> > content/renderer/scheduler/task_queue_manager.cc:158: return true;
> > On 2015/03/02 16:08:10, alexclarke1 wrote:
> > > On 2015/03/02 13:53:57, rmcilroy wrote:
> > > > drive-by comment: I'm wondering if it be simpler just to simply post a
> > wrapper
> > > > task which does the following:
> > > > 
> > > > void TaskQueue::EnqueueNextPendingTask() {
> > > >   base::AutoLock lock(lock_);
> > > >   if (!task_queue_manager_)
> > > >     return;
> > > > 
> > > >   if (delayed_task_queue_.empty())
> > > >     return;
> > > > 
> > > >   EnqueueTaskLocked(delayed_task_queue_.pop());
> > > > }
> > > > 
> > > > And simply always post this for delayed task. This would be much more
> > similar
> > > to
> > > > the previous approach except the wrapper task now doesn't point to the
> task
> > > > itself, allowing us to delete the pending taskson shutdown.  While I
like
> > the
> > > > idea of trying to merge multiple pending tasks into a single pending
task
> > > > wrapper on the message loop, I'm not sure there will be enough pending
> tasks
> > > > with exactly the same delayed_runtime to justify the extra complexity
(but
> I
> > > > don't know the DOMTimer code well so I may be wrong here).  WDYT?
> > > 
> > > That's a really interesting idea.  I quite like the simplicity of it but
on
> > the
> > > other hand (based on the verge and some other popular heavy sites such as
> > > http://sports.yahoo.com and latimes):
> > > 
> > > 1. While the page loads it's not uncommon to have several hundred delayed
> > tasks
> > > in flight
> > > 2. It's quite common for 10+ of these delays to "expire" simultaneously
> (I've
> > > seen a couple of times when it reached 20+)
> > > 3. Typically we'd only have single digit number of KickDelayedTasks's
posted
> > on
> > > the chromium run loop.
> > > 
> > > This suggests that the more complicated approach taken by this patch has
> some
> > > merit.
> > 
> > As one more data point, this comment here suggests that having lots of
delayed
> > tasks that expire simultaneously is a thing that happens:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/message_loop/...
> 
> It does sound that way.  To me it looks like that code looks like it's trying
to
> avoid calling Now() a lot, which probably is sensible.

Actually I think the key thing is that it *isn't* doing this:

while (task.run_time < Now())
  task.Run();


> 
> > 
> >
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> > content/renderer/scheduler/task_queue_manager.cc:167: // Enqueue all delayed
> > tasks that should be running now.
> > base::MessageLoop always interleaves delayed and non-delayed work, whereas
> here
> 
> > we're adding all expired delayed tasks as a monolithic block (which we can
> still
> > schedule of course). I wonder if that might have some bad consequences
though
> by
> > starving other things on the default queue?
> 
> Actually that's a good point.  Previously the blink timer heap would run up to
> 50 ms of work.  There isn't a limit here (or with Ross's approach either
afaik),
> maybe there should be.
> 
> I suspect adding a TimerQueue would make it much easier to do that.  WDYT?
> 
> > 
> > Of course if we are moving those tasks off of the default task runner, then
> > we're kind of relying the scheduler to handle the interleaving for us. Maybe
> > overall that is the better way to do things anyway.
> > 
> >
>
https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
> > content/renderer/scheduler/task_queue_manager.cc:281:
> > incoming_queue_.back().delayed_run_time = base::TimeTicks();
> > int: could do this unconditionally now.

[alex clarke (OOO till 29th), 2015-03-03 10:11:16]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:128: while
(!delayed_task_queue_.empty())
On 2015/02/27 18:22:33, Sami wrote:
> Is there a clear() or reset() that does this in one go?

Irritatingly no :(  

http://en.cppreference.com/w/cpp/container/priority_queue

[rmcilroy, 2015-03-03 10:22:21]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:128: while
(!delayed_task_queue_.empty())
On 2015/03/03 10:11:15, alexclarke1 wrote:
> On 2015/02/27 18:22:33, Sami wrote:
> > Is there a clear() or reset() that does this in one go?
> 
> Irritatingly no :(  
> 
> http://en.cppreference.com/w/cpp/container/priority_queue

One option would be to just make it a scoped_ptr<base::DelayedTaskQueue> and
reset the scoped_ptr - not sure it's worth the bother in this case though?

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:158: return true;
On 2015/03/02 16:38:33, Sami wrote:
> On 2015/03/02 16:08:10, alexclarke1 wrote:
> > On 2015/03/02 13:53:57, rmcilroy wrote:
> > > drive-by comment: I'm wondering if it be simpler just to simply post a
> wrapper
> > > task which does the following:
> > > 
> > > void TaskQueue::EnqueueNextPendingTask() {
> > >   base::AutoLock lock(lock_);
> > >   if (!task_queue_manager_)
> > >     return;
> > > 
> > >   if (delayed_task_queue_.empty())
> > >     return;
> > > 
> > >   EnqueueTaskLocked(delayed_task_queue_.pop());
> > > }
> > > 
> > > And simply always post this for delayed task. This would be much more
> similar
> > to
> > > the previous approach except the wrapper task now doesn't point to the
task
> > > itself, allowing us to delete the pending taskson shutdown.  While I like
> the
> > > idea of trying to merge multiple pending tasks into a single pending task
> > > wrapper on the message loop, I'm not sure there will be enough pending
tasks
> > > with exactly the same delayed_runtime to justify the extra complexity (but
I
> > > don't know the DOMTimer code well so I may be wrong here).  WDYT?
> > 
> > That's a really interesting idea.  I quite like the simplicity of it but on
> the
> > other hand (based on the verge and some other popular heavy sites such as
> > http://sports.yahoo.com and latimes):
> > 
> > 1. While the page loads it's not uncommon to have several hundred delayed
> tasks
> > in flight
> > 2. It's quite common for 10+ of these delays to "expire" simultaneously
(I've
> > seen a couple of times when it reached 20+)
> > 3. Typically we'd only have single digit number of KickDelayedTasks's posted
> on
> > the chromium run loop.
> > 
> > This suggests that the more complicated approach taken by this patch has
some
> > merit.
> 
> As one more data point, this comment here suggests that having lots of delayed
> tasks that expire simultaneously is a thing that happens:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/base/message_loop/...

As discussed offline, the message_loop goes to a fair amount of work to keep
task posting low-overhead when a lot of work is posted together.  I'm not
against doing the more complex approach here but would prefer if we could
justify that it actually makes a performance difference compared to the simple
approach using the optimizations in the message loop before adding the
complexity.

[Sami, 2015-03-03 10:46:48]

BTW, we'll probably want to hook up this timer suspension logic here too:
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/blin...

[alex clarke (OOO till 29th), 2015-03-03 16:40:27]

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
File content/renderer/scheduler/task_queue_manager.cc (right):

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:158: return true;
On 2015/03/02 13:53:57, rmcilroy wrote:
> drive-by comment: I'm wondering if it be simpler just to simply post a wrapper
> task which does the following:
> 
> void TaskQueue::EnqueueNextPendingTask() {
>   base::AutoLock lock(lock_);
>   if (!task_queue_manager_)
>     return;
> 
>   if (delayed_task_queue_.empty())
>     return;
> 
>   EnqueueTaskLocked(delayed_task_queue_.pop());
> }
> 
> And simply always post this for delayed task. This would be much more similar
to
> the previous approach except the wrapper task now doesn't point to the task
> itself, allowing us to delete the pending taskson shutdown.  While I like the
> idea of trying to merge multiple pending tasks into a single pending task
> wrapper on the message loop, I'm not sure there will be enough pending tasks
> with exactly the same delayed_runtime to justify the extra complexity (but I
> don't know the DOMTimer code well so I may be wrong here).  WDYT?

Circling back on this.  I tried it:  https://codereview.chromium.org/972473004

Based on some (admittedly rather contrived) micro benchmarks it seems that there
is a ~30% performance overhead with Ross's approach.

https://codereview.chromium.org/962273002/diff/20001/content/renderer/schedul...
content/renderer/scheduler/task_queue_manager.cc:167: // Enqueue all delayed
tasks that should be running now.
On 2015/03/02 16:38:33, Sami wrote:
> base::MessageLoop always interleaves delayed and non-delayed work, whereas
here
> we're adding all expired delayed tasks as a monolithic block (which we can
still
> schedule of course). I wonder if that might have some bad consequences though
by
> starving other things on the default queue?
> 
> Of course if we are moving those tasks off of the default task runner, then
> we're kind of relying the scheduler to handle the interleaving for us. Maybe
> overall that is the better way to do things anyway.

I think that adding a TimerQueue should help a lot here, since the interleaving
should occur naturally.

[alex clarke (OOO till 29th), 2015-03-05 17:57:53]

Shall we move forward with this patch?  It's only pressing if we refactor the
blink timer heap, although more native handling of delayed tasks is probably a
good thing anyway.

[Sami, 2015-03-05 19:27:55]

I think this is definitely worth doing given the amount of code we can delete on
the Blink side. Before landing however let's make sure we can manage timer
suspension and background ticking in this new world.

[alex clarke (OOO till 29th), 2015-03-06 13:57:56]

On 2015/03/05 19:27:55, Sami wrote:
> I think this is definitely worth doing given the amount of code we can delete
on
> the Blink side. Before landing however let's make sure we can manage timer
> suspension and background ticking in this new world.

That appears to be working out of the box, thanks to
DOMTimerCoordinator::didChangeTimerAlignmentInterval.

I have a trace with the patches applied that illustrates this.

[Sami, 2015-03-06 15:03:55]

On 2015/03/06 13:57:56, alexclarke1 wrote:
> On 2015/03/05 19:27:55, Sami wrote:
> > I think this is definitely worth doing given the amount of code we can
delete
> on
> > the Blink side. Before landing however let's make sure we can manage timer
> > suspension and background ticking in this new world.
> 
> That appears to be working out of the box, thanks to
> DOMTimerCoordinator::didChangeTimerAlignmentInterval.
> 
> I have a trace with the patches applied that illustrates this.

What about completely suspending the timers like here:
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/blin...

[alex clarke (OOO till 29th), 2015-03-06 15:07:15]

On 2015/03/06 15:03:55, Sami wrote:
> On 2015/03/06 13:57:56, alexclarke1 wrote:
> > On 2015/03/05 19:27:55, Sami wrote:
> > > I think this is definitely worth doing given the amount of code we can
> delete
> > on
> > > the Blink side. Before landing however let's make sure we can manage timer
> > > suspension and background ticking in this new world.
> > 
> > That appears to be working out of the box, thanks to
> > DOMTimerCoordinator::didChangeTimerAlignmentInterval.
> > 
> > I have a trace with the patches applied that illustrates this.
> 
> What about completely suspending the timers like here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/blin...

In a follow up patch I'm planning to introduce a new TIMER_TASK_QUEUE and we
could add some simple logic to the scheduler to support that.

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

On 2015/03/06 15:07:15, alexclarke1 wrote:
> On 2015/03/06 15:03:55, Sami wrote:
> > On 2015/03/06 13:57:56, alexclarke1 wrote:
> > > On 2015/03/05 19:27:55, Sami wrote:
> > > > I think this is definitely worth doing given the amount of code we can
> > delete
> > > on
> > > > the Blink side. Before landing however let's make sure we can manage
timer
> > > > suspension and background ticking in this new world.
> > > 
> > > That appears to be working out of the box, thanks to
> > > DOMTimerCoordinator::didChangeTimerAlignmentInterval.
> > > 
> > > I have a trace with the patches applied that illustrates this.
> > 
> > What about completely suspending the timers like here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/child/blin...
> 
> In a follow up patch I'm planning to introduce a new TIMER_TASK_QUEUE and we
> could add some simple logic to the scheduler to support that.

This patch illustrates doing this:  https://codereview.chromium.org/985813002/

Note the diffbase contains rather more in it than I intended but you can see
I've been able to remove quite a lot from BlinkPlatformImpl while adding a
fairly modest amount of code to RendererSchedulerImpl.  WDYT?

[Sami, 2015-03-16 20:17:15]

Here's what we talked about (and tell me if I misinterpreted):

- We seems to prefer a solution that would allow us to do some TQM-wide
optimization of the pending delayed tasks (but probably don't want to do
anything in the initial step).
- The above would suggest posting delayed tasks to the TQM and then back to the
individual work queues, which works but is a little roundabout.
- Having the logic *either* mostly in the TQM or the individual task queues
would nicely keep it in one place.
- I wanted to keep the individual queues unaware of delayed tasks, but I think
I've been convinced this was a pipe dream.
- IMO a nice property of the MessageLoop is that it never does work
O(number_of_pending_tasks) per run loop iteration. We should maintain that if we
can without horrible contortions.
- It's unclear if we need to maintain the interleaving of immediate and delayed
tasks like base does.

Based on the above I think I'm starting to warm up to the original idea of
having a separate delayed task heap in each task queue, because it keeps the
logic in one place and minimizes the back-and-forth task routing while still
allowing for a hypothetical hook in TQM that just coalesces across queues.

I'd suggest a couple of improvements to how this version implements it:  
- Instead of EnqueueTaskLocked try appending expired delayed tasks directly to
the work queue (and rely on DoWork calling us to do it when necessary) because
EnqueueTaskLocked can cause a DoWork to get posted.
- See if we can avoid calling Now() when there is no expired delayed work that
needs to be checked.