Distribute extra memory evenly among visible tabs

Distribute extra memory evenly among visible clients

After computing all clients' memory budgets, take any
extra memory left over and distribute it amongst the
visible clients (so that, if their memory requirments
suddenly jump, they don't need to wait for a roundtrip
to the GPU process to get extra memory).

Disallow keeping around backgrounded tabs' contents on
Android, to keep the maximum for the main tab.

Explicitly limit the memory to use for prepainting by
specifying NiceToHave on Mac to avoid performance problems.

BUG=175125
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=181950

[ccameron, 2013-02-08 23:53:55]

epenner, can you see if this helps out the issue at snowbird.com?

[klobag.chromium, 2013-02-09 04:10:10]

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
File content/common/gpu/gpu_memory_manager.cc (right):

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
content/common/gpu/gpu_memory_manager.cc:801: for
(ClientStateList::const_iterator it = clients_visible_mru_.begin();
Should we expect the number of client_visible_mru is same as the number of
visible tabs/windows?

Earlier I notices that I have one tab, but there are two entries in the
clients_visible_mru. I believe I was loading gmail.com in Chrome.

[ccameron, 2013-02-09 04:45:28]

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
File content/common/gpu/gpu_memory_manager.cc (right):

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
content/common/gpu/gpu_memory_manager.cc:801: for
(ClientStateList::const_iterator it = clients_visible_mru_.begin();
On 2013/02/09 04:10:10, klobag.chromium wrote:
> Should we expect the number of client_visible_mru is same as the number of
> visible tabs/windows?
> 
> Earlier I notices that I have one tab, but there are two entries in the
> clients_visible_mru. I believe I was loading http://gmail.com in Chrome.

We should only have one entry in clients_visible_mru, unless there are two
visible compositor instances (that would be strange, but not out of the real of
possibility).  I'll check this when I'm back at my machine.

[ccameron, 2013-02-11 20:55:56]

Ptal -- this should give a little bit more to the main tab as well.

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
File content/common/gpu/gpu_memory_manager.cc (right):

https://codereview.chromium.org/12226082/diff/3001/content/common/gpu/gpu_mem...
content/common/gpu/gpu_memory_manager.cc:801: for
(ClientStateList::const_iterator it = clients_visible_mru_.begin();
On 2013/02/09 04:45:28, ccameron1 wrote:
> On 2013/02/09 04:10:10, klobag.chromium wrote:
> > Should we expect the number of client_visible_mru is same as the number of
> > visible tabs/windows?
> > 
> > Earlier I notices that I have one tab, but there are two entries in the
> > clients_visible_mru. I believe I was loading http://gmail.com in Chrome.
> 
> We should only have one entry in clients_visible_mru, unless there are two
> visible compositor instances (that would be strange, but not out of the real
of
> possibility).  I'll check this when I'm back at my machine.

I double-checked this an only got 1 visible tab when looking at gmail.

[nduca, 2013-02-11 22:11:52]

https://codereview.chromium.org/12226082/diff/10001/content/common/gpu/gpu_me...
File content/common/gpu/gpu_memory_manager.cc (right):

https://codereview.chromium.org/12226082/diff/10001/content/common/gpu/gpu_me...
content/common/gpu/gpu_memory_manager.cc:880:
GpuMemoryAllocationForRenderer::kPriorityCutoffAllowNiceToHave;
Why should we bother having an eventually bin, then? Can you grab me so that I
can understand this?

[epenner, 2013-02-11 23:36:55]

FYI, as of sometime this weekend when I tried this patch, Snowbird.com still had
32MB on Nexus4.  It was much larger when I started using the "dalvik only"
memory estimate, but still not 100% of the limit.

[ccameron, 2013-02-11 23:39:38]

On 2013/02/11 23:36:55, epenner wrote:
> FYI, as of sometime this weekend when I tried this patch, http://Snowbird.com
still had
> 32MB on Nexus4.  It was much larger when I started using the "dalvik only"
> memory estimate, but still not 100% of the limit.

I'd guess that that's because (until version 4 of this patch) we were keeping
around backgrounded tabs' memory. With this, if you have 1 visible tab (sound
an alarm if not), then 100% should go to that tab.

[epenner, 2013-02-11 23:56:06]

I only had one tab in chrome at that time. If I get a chance I'll try the new
patch. I'm also adjusting my dalvik patch as we speak so maybe that will reduce
any android specific issue.

[ccameron, 2013-02-12 01:41:13]

Updated with changes to NTH bin.

nduca and I discussed making a change to not aggressively pre-paint things that
are in the "eventually", to allow keeping resources for back-scroll, but not
pre-painting them. This was also for stability. It happens to be the case that
we are safe for now because we always put things farther than
kMaxDistanceInContentSpace in the NEVER bin.

I'll follow this with a patch that makes kMaxDistanceInContentSpace less
restrictive, and allows keeping things around for scroll-back.

[nduca, 2013-02-12 01:48:52]

LGTM

[ccameron, 2013-02-12 01:52:58]

Adding piman for OWNER

[piman, 2013-02-12 01:59:22]

OWNERs rubber-stamp LGTM. Note: you're potentially asking for trouble lumping in
a Mac change and an Android change, you may want to separate - up to you.

[ccameron, 2013-02-12 02:03:20]

Thanks!!

Good point about Android v Mac. In this particular case, this is taking behavior
that was in the other path and adding it to the new path, so I feel safe it. In
general, agree.

[nduca, 2013-02-12 02:05:14]

Trouble is megapatches middle name. It should be fine, merging wise.

[commit-bot: I haz the power, 2013-02-12 02:09:42]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/12226082/14002

[epenner, 2013-02-12 02:20:03]

On 2013/02/12 02:09:42, I haz the power (commit-bot) wrote:
> CQ is trying da patch. Follow status at
> https://chromium-status.appspot.com/cq/ccameron%40chromium.org/12226082/14002

FYI, I've tried this patch together with my patch to remove Android specific
behavior. I'm still seeing memory limits of 50% of the true limit.

[ccameron, 2013-02-12 02:21:38]

On 2013/02/12 02:20:03, epenner wrote:
> On 2013/02/12 02:09:42, I haz the power (commit-bot) wrote:
> > CQ is trying da patch. Follow status at
> >
https://chromium-status.appspot.com/cq/ccameron%2540chromium.org/12226082/14002
> 
> FYI, I've tried this patch together with my patch to remove Android specific
> behavior. I'm still seeing memory limits of 50% of the true limit.

Huh. Are there multiple visible clients?

[epenner, 2013-02-12 02:26:12]

On 2013/02/12 02:21:38, ccameron1 wrote:
> On 2013/02/12 02:20:03, epenner wrote:
> > On 2013/02/12 02:09:42, I haz the power (commit-bot) wrote:
> > > CQ is trying da patch. Follow status at
> > >
>
https://chromium-status.appspot.com/cq/ccameron%252540chromium.org/12226082/1...
> > 
> > FYI, I've tried this patch together with my patch to remove Android specific
> > behavior. I'm still seeing memory limits of 50% of the true limit.
> 
> Huh. Are there multiple visible clients?

I put the debug output in the LOG. I only have one tab open... Hmmmmm. Does a
browser compositor count?

[epenner, 2013-02-12 02:26:54]

Rather, I put debug output in the bug.

[klobag.chromium, 2013-02-12 02:51:55]

On 2013/02/12 02:26:54, epenner wrote:
> Rather, I put debug output in the bug.

As I mentioned in #2, when I had one tab, I saw two entries in
clients_visible_mru_. Can you check yours? I don't think it is the browser
compositor. I suspect when we resize, both are in the list.

[commit-bot: I haz the power, 2013-02-12 03:12:22]

Retried try job too often on mac_rel for step(s) browser_tests,
interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...