Stop suspending renderer and changing purge interval to 20min

Stop suspending renderer and changing purge interval to 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f. youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJvI/edit?usp=sharing
- the difference between the original purge+suspend and purge+suspend with this patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLjR0/edit?usp=sharing

BUG=675735, 607077
Review-Url: https://codereview.chromium.org/2624063002
Cr-Commit-Position: refs/heads/master@{#444209}
Committed: https://chromium.googlesource.com/chromium/src/+/d224a9dddebc82b819515e4d9c35737c7a92e9d0

[tasak, 2017-01-11 02:21:42]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 02:22:22]

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, 2017-01-11 04:03:10]

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

[commit-bot: I haz the power, 2017-01-11 04:03:11]

Dry run: This issue passed the CQ dry run.

[tasak, 2017-01-11 05:04:04]

Description was changed from

==========
Disable SuspendRenderer() when OnProcessPurgeAndSuspend is invoked.

BUG=
==========

to

==========
Disable SuspendRenderer() when OnProcessPurgeAndSuspend is invoked.

Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable SuspendRenderer.

BUG=675735, 675811
==========

[tasak, 2017-01-11 05:04:53]

Description was changed from

==========
Disable SuspendRenderer() when OnProcessPurgeAndSuspend is invoked.

Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable SuspendRenderer.

BUG=675735, 675811
==========

to

==========
Disable SuspendRenderer() when OnProcessPurgeAndSuspend is invoked.

Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable SuspendRenderer.

BUG=675735, 607077
==========

[tasak, 2017-01-11 05:05:37]

tasak@google.com changed reviewers:
+ bashi@chromium.org, haraken@chromium.org

[tasak, 2017-01-11 05:05:38]

Would you review this CL?

[tasak, 2017-01-11 05:33:58]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 05:34:27]

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

[tasak, 2017-01-11 05:37:14]

Description was changed from

==========
Disable SuspendRenderer() when OnProcessPurgeAndSuspend is invoked.

Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable SuspendRenderer.

BUG=675735, 607077
==========

to

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

BUG=675735, 607077
==========

[commit-bot: I haz the power, 2017-01-11 06:14:48]

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

[commit-bot: I haz the power, 2017-01-11 06:14:49]

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

[tasak, 2017-01-11 11:26:39]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 11:26:47]

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

[tasak, 2017-01-11 11:27:00]

The CQ bit was checked by tasak@google.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-11 11:27: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, 2017-01-11 12:38:42]

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

[commit-bot: I haz the power, 2017-01-11 12:38:43]

Dry run: This issue passed the CQ dry run.

[bashi, 2017-01-13 02:11:43]

Thanks for doing this!

The subject sounds a bit strange to me because we already purges caches. Rather,
this CL stops suspending renderers.

Also we would want to update the design doc and put the link to the doc in the
description.

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.cc (right):

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:89: base::TimeDelta::FromSeconds(1200);
We no longer suspend renderers. Shouldn't we change the name of this const and
related flags? I understand the cost of renaming as it affects a lot of things
including experiments, but I'd prefer avoiding confusions.

I think it's fine to rename this in follow-up CLs.

[haraken, 2017-01-13 02:23:25]

LGTM with bashi-san's comments addressed in a follow-up.

[tasak, 2017-01-13 03:18:43]

Description was changed from

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

BUG=675735, 607077
==========

to

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch:
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

[tasak, 2017-01-13 03:19:03]

Description was changed from

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch:
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

to

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

[tasak, 2017-01-13 03:26:40]

Description was changed from

==========
Purge background tab's memory every 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

to

==========
Stop suspending renderer and changing purge interval to 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

[tasak, 2017-01-13 03:27:04]

Thank you for review.

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.cc (right):

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:89: base::TimeDelta::FromSeconds(1200);
On 2017/01/13 02:11:42, bashi wrote:
> We no longer suspend renderers. Shouldn't we change the name of this const and
> related flags? I understand the cost of renaming as it affects a lot of things
> including experiments, but I'd prefer avoiding confusions.
> 
> I think it's fine to rename this in follow-up CLs.

Acknowledged.
But currelty it is difficult to rename this, because there is no way to purge
cache except changing MC state from NORMAL to SUSPENDED.

[bashi, 2017-01-13 04:36:23]

non-owner lgtm

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
File chrome/browser/memory/tab_manager.cc (right):

https://codereview.chromium.org/2624063002/diff/40001/chrome/browser/memory/t...
chrome/browser/memory/tab_manager.cc:89: base::TimeDelta::FromSeconds(1200);
On 2017/01/13 03:27:04, tasak wrote:
> On 2017/01/13 02:11:42, bashi wrote:
> > We no longer suspend renderers. Shouldn't we change the name of this const
and
> > related flags? I understand the cost of renaming as it affects a lot of
things
> > including experiments, but I'd prefer avoiding confusions.
> > 
> > I think it's fine to rename this in follow-up CLs.
> 
> Acknowledged.
> But currelty it is difficult to rename this, because there is no way to purge
> cache except changing MC state from NORMAL to SUSPENDED.

Yeah, currently it's difficult to rename as purging is depend on
MemoryCoordinatorClient::OnMemoryStateChange(). However at least we should stop
using "suspend" outside memory coordinator (and clients). We can rename
variables and functions in RenderThreadImpl. (Though we still call
base::MemoryCoordinatorClientRegistry::GetInstance()->Notify(base::MemoryState::SUSPENDED)).

[tasak, 2017-01-13 09:40:30]

The CQ bit was checked by tasak@google.com

[commit-bot: I haz the power, 2017-01-13 09:40:59]

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

[tasak, 2017-01-13 09:43:40]

The CQ bit was unchecked by tasak@google.com

[tasak, 2017-01-13 09:44:19]

tasak@google.com changed reviewers:
+ avi@chromium.org

[tasak, 2017-01-13 09:44:19]

avi, would you review this CL?

I need content/ owner's lgtm.

[Avi (use Gerrit), 2017-01-13 16:48:55]

lgtm

stamp

[haraken, 2017-01-14 01:04:20]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-01-14 01:04:45]

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, 2017-01-14 01:12:57]

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

[commit-bot: I haz the power, 2017-01-14 01:12:58]

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...)

[haraken, 2017-01-14 02:29:32]

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

[haraken, 2017-01-14 02:29:33]

chrisha@: PTAL at chrome/browser/memory ?

[chrisha, 2017-01-17 18:19:22]

So we've completely given up on suspend? No anticipated ways to work around the
drawbacks?

(rubber stamp lgtm)

[haraken, 2017-01-17 23:41:01]

On 2017/01/17 18:19:22, chrisha (slow) wrote:
> So we've completely given up on suspend? No anticipated ways to work around
the
> drawbacks?

Once Blink Scheduler exposes an API for the budget-based throttling, we could
throttle background tabs more aggressively (e.g., 0.1 Hz).

The key thing we learned here is that the tab suspension (stopping the tab for 2
mins) is too aggressive.

[haraken, 2017-01-17 23:41:06]

The CQ bit was checked by haraken@chromium.org

[commit-bot: I haz the power, 2017-01-17 23:41:59]

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, 2017-01-18 00:44:33]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1484696466005090,
"parent_rev": "96c89c1cd14f092f9879407f85d7001c246e9687", "commit_rev":
"d224a9dddebc82b819515e4d9c35737c7a92e9d0"}

[commit-bot: I haz the power, 2017-01-18 00:45:22]

Description was changed from

==========
Stop suspending renderer and changing purge interval to 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077
==========

to

==========
Stop suspending renderer and changing purge interval to 20min

- Since SuspendRenderer sometimes causes very long style recalc and layout (c.f.
youtube.com/), disable tab suspension and use default background task throttle.
- also changed time-to-resuspension to 20min. This means, purge background tab's
memory every 20min(to be precise 20min+10sec).

- the original document about purge+suspend is
https://docs.google.com/document/d/1EgLimgxWK5DGhptnNVbEGSvVn6Q609ZJaBkLjEPRJ...
- the difference between the original purge+suspend and purge+suspend with this
patch is 
https://docs.google.com/document/d/1qIrXsi9BuoAgNu6lVGTcGhbAm2TLPRb_JZ0OhynLj...

BUG=675735, 607077

Review-Url: https://codereview.chromium.org/2624063002
Cr-Commit-Position: refs/heads/master@{#444209}
Committed:
https://chromium.googlesource.com/chromium/src/+/d224a9dddebc82b819515e4d9c35...
==========

[commit-bot: I haz the power, 2017-01-18 00:45:23]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/d224a9dddebc82b819515e4d9c35...