Suspend more task queues for background renderers (experimental)

Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a

Committed: https://crrev.com/e9897950a1574e0842dc86e0c50e6695773edf41
Cr-Commit-Position: refs/heads/master@{#397990}

[hajimehoshi, 2016-05-27 09:33:04]

Patchset #1 (id:1) has been deleted

[hajimehoshi, 2016-05-27 09:33:43]

hajimehoshi@chromium.org changed reviewers:
+ chrisha@chromium.org, georgesak@chromium.org, skyostil@chromium.org

[hajimehoshi, 2016-05-27 09:34:24]

Sami: I'm not confident with suspending control(_after_wakeup)_tq. Is this OK?

PTAL

[hajimehoshi, 2016-05-27 09:35:11]

Description was changed from

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * compositor_tq
  * control_tq
  * control_after_wakeup_tq
  * (default|frame)_loading_tq

All of them might call V8 functions, which means any purged cache can
be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches).

BUG=607077
TEST=n/a
==========

to

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * compositor_tq
  * control_tq
  * control_after_wakeup_tq
  * (default|frame)_loading_tq

Some of them might call V8 functions, which means any purged cache can
be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches).

BUG=607077
TEST=n/a
==========

[Sami, 2016-05-27 14:08:57]

I think we can avoid suspending any of the control task queues, because those
are only used for internal scheduler housekeeping tasks which will never call
outside code.

Similarly it's probably not useful to suspend the compositor task queue because
there won't be any compositing activity happening for backgrounded renderers.

Something we could consider is adding UMA and/or tracing to measure how often
tasks are executed in suspended renderers.

https://codereview.chromium.org/2017763003/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.h (right):

https://codereview.chromium.org/2017763003/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.h:341: bool
queue_suspended;
Maybe renderer_suspended would be a more descriptive name for this? Also maybe
move it together with the other renderer_* flags above.

[hajimehoshi, 2016-05-31 08:08:24]

Description was changed from

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * compositor_tq
  * control_tq
  * control_after_wakeup_tq
  * (default|frame)_loading_tq

Some of them might call V8 functions, which means any purged cache can
be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches).

BUG=607077
TEST=n/a
==========

to

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a
==========

[hajimehoshi, 2016-05-31 08:09:01]

Thank you! I also fixed not to suspend controller_*_tq and compositor_tq.

https://codereview.chromium.org/2017763003/diff/20001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.h (right):

https://codereview.chromium.org/2017763003/diff/20001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.h:341: bool
queue_suspended;
On 2016/05/27 14:08:57, Sami wrote:
> Maybe renderer_suspended would be a more descriptive name for this? Also maybe
> move it together with the other renderer_* flags above.

Done.

[Sami, 2016-05-31 10:11:22]

https://codereview.chromium.org/2017763003/diff/40001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2017763003/diff/40001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:1223:
MainThreadOnly().renderer_suspended = true;
I guess this is just for testing, but ultimately you'd want to do this in
RendererSchedulerImpl::SuspendRenderer().

[hajimehoshi, 2016-05-31 11:10:07]

https://codereview.chromium.org/2017763003/diff/40001/components/scheduler/re...
File components/scheduler/renderer/renderer_scheduler_impl.cc (right):

https://codereview.chromium.org/2017763003/diff/40001/components/scheduler/re...
components/scheduler/renderer/renderer_scheduler_impl.cc:1223:
MainThreadOnly().renderer_suspended = true;
On 2016/05/31 10:11:21, Sami wrote:
> I guess this is just for testing, but ultimately you'd want to do this in
> RendererSchedulerImpl::SuspendRenderer().

Ah, this was my mistake: Setting this variable should be in SuspendRenderer as
you suggested. Done.

[Sami, 2016-05-31 14:21:03]

Thanks, looks great. Please add a test if you want to land this for real.

[hajimehoshi, 2016-06-02 09:12:37]

On 2016/05/31 14:21:03, Sami wrote:
> Thanks, looks great. Please add a test if you want to land this for real.

Thanks, Done.

[Sami, 2016-06-03 15:59:06]

Thanks, lgtm.

[hajimehoshi, 2016-06-06 04:49:19]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2016-06-06 04:49:30]

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

[commit-bot: I haz the power, 2016-06-06 07:36:59]

Description was changed from

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a
==========

to

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a
==========

[commit-bot: I haz the power, 2016-06-06 07:37:00]

Committed patchset #5 (id:100001)

[commit-bot: I haz the power, 2016-06-06 07:38:41]

Description was changed from

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a
==========

to

==========
Suspend more task queues for background renderers (experimental)

Backgrounded-tab suspending was introduced at crrev/1914143002, where
only the timer task queue was suspended. We knew this was not enough and
we need to suspend more task queues to prevent the return of purged
cache.

This CL adds the feature to suspend these task queues besides timer task
queues when backgrounded-tab suspending starts:

  * default_loading_tq
  * frame_loading_tq

The above task queues might call V8 functions, which means any purged cache
can be reverted.

This CL doesn't suspend "default_tq" (which we can't suspend since this
conveys critial tasks) and "idle_tq" (which I think is not harmful for
purged caches), "control_(after_wakeup_)tq" (which is only for internal
scheduler housekeeping tasks) and "compositor_tq" (which is not used when
the renderer is backgrounded).

BUG=607077
TEST=n/a

Committed: https://crrev.com/e9897950a1574e0842dc86e0c50e6695773edf41
Cr-Commit-Position: refs/heads/master@{#397990}
==========

[commit-bot: I haz the power, 2016-06-06 07:38:42]

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

[haraken, 2016-10-07 06:34:39]

Sami: I'm just curious, but does this CL suspend not only the default timer &
loading task runners but also per-frame timer & loading task runners?

[Sami, 2016-10-07 10:04:15]

On 2016/10/07 06:34:39, haraken wrote:
> Sami: I'm just curious, but does this CL suspend not only the default timer &
> loading task runners but also per-frame timer & loading task runners?

Yes, see these loops here where the policies are applied to all task queues of
the respective type:

https://cs.chromium.org/chromium/src/third_party/WebKit/Source/platform/sched...