GPU: Adjust memory and use the soft-limit.

GPU: Adjust memory and use the soft-limit.

We now have a soft limit, so we adjust some memory limits up,
and enable the soft-limit to bring the effective usage down.

Initially we bump most limits up a bit (except for really
high-end which is already at 256MB), and then we cut the
soft limit to 50% of that. For low-end, to be extra careful
we only increase the limit from 8MB to 12MB, and set the
soft limit back to 8MB.

BUG=337940, 273127
TBR=piman@chromium.org

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

[epenner, 2014-02-15 00:44:25]

Ptal.

[aelias_OOO_until_Jul13, 2014-02-15 00:55:17]

https://codereview.chromium.org/168053002/diff/1/content/renderer/gpu/render_...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/168053002/diff/1/content/renderer/gpu/render_...
content/renderer/gpu/render_widget_compositor.cc:266: // TODO(boliu): Set this
ratio for Webview.
How about just using the same policy for WebView until we see evidence an
adjustment is needed?

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc
File ui/gl/gl_context_android.cc (right):

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc#...
ui/gl/gl_context_android.cc:108: // Now we take a default of 1/8th of memory on
high-memory devices,
I think we should include screen resolution as part of this calculation as well
as device memory.  On the Nexus 4, 128MB is 32 viewports of prepaint, still
overkill.  Probably we should never go over 10 viewports of prepaint.

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc#...
ui/gl/gl_context_android.cc:120: // NOTE: Non-low-end devices use only 50% of
these limites,
typo: should be "limits"

[epenner, 2014-02-15 01:02:27]

https://codereview.chromium.org/168053002/diff/1/content/renderer/gpu/render_...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/168053002/diff/1/content/renderer/gpu/render_...
content/renderer/gpu/render_widget_compositor.cc:266: // TODO(boliu): Set this
ratio for Webview.
On 2014/02/15 00:55:17, aelias wrote:
> How about just using the same policy for WebView until we see evidence an
> adjustment is needed?

I think their limits are not as excessive as ours. Possibly already on the low
side. I asked Bo and he said to just leave it for him to manually test and
fiddle with.

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc
File ui/gl/gl_context_android.cc (right):

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc#...
ui/gl/gl_context_android.cc:108: // Now we take a default of 1/8th of memory on
high-memory devices,
On 2014/02/15 00:55:17, aelias wrote:
> I think we should include screen resolution as part of this calculation as
well
> as device memory.  On the Nexus 4, 128MB is 32 viewports of prepaint, still
> overkill.  Probably we should never go over 10 viewports of prepaint.

If you look at the history of this code, we have gone back and forth between
resolution and memory quite a bit. Memory was used in the end since this is a
hard limit spread across all GPU clients (in theory), and screen resolution was
hampering high-end devices.

I think the planned 'implicit' limit will solve this without needing to use
screen resolution here. The implicit limit will only use as much as the content
requires. With enough content it will scale up nicely all the way to the soft
limit, and then it has the hard-limit for extremes.

https://codereview.chromium.org/168053002/diff/1/ui/gl/gl_context_android.cc#...
ui/gl/gl_context_android.cc:120: // NOTE: Non-low-end devices use only 50% of
these limites,
On 2014/02/15 00:55:17, aelias wrote:
> typo: should be "limits"

Done.

[aelias_OOO_until_Jul13, 2014-02-15 01:03:45]

Makes sense, lgtm.

[epenner, 2014-02-15 01:09:09]

On 2014/02/15 01:03:45, aelias wrote:
> Makes sense, lgtm.

+boliu. I've added a TODO(boliu), but feel free to shoot me an email and I'll
fix this up for webview. In the long run we should try to converge these on
Android. With the implicit limit I think we can likely have the same limit logic
for everything.

[epenner, 2014-02-15 02:11:37]

I'm TBR'ing since piman hasn't had a problem with similar changes (mostly rubber
stamps), and we want this to land before the branch point on Monday.

Of course, I'll make any changes if anyone has any concerns.

[epenner, 2014-02-15 02:11:46]

The CQ bit was checked by epenner@chromium.org

[commit-bot: I haz the power, 2014-02-15 02:14:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/168053002/70001

[commit-bot: I haz the power, 2014-02-15 05:40:29]

Change committed as 251544

[klobag.chromium, 2014-02-15 07:27:20]

https://codereview.chromium.org/168053002/diff/70001/content/renderer/gpu/ren...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/168053002/diff/70001/content/renderer/gpu/ren...
content/renderer/gpu/render_widget_compositor.cc:269: // apps. So initially we
use 50% more memory to avoid flickering
comment 50% is not matching code 67%.

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android.cc
File ui/gl/gl_context_android.cc (right):

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android...
ui/gl/gl_context_android.cc:117: // pre-painting and uses the rest only for
'emergencies'.
Can we move this two lines up and say the number is emergency (pre-painting)?

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android...
ui/gl/gl_context_android.cc:126: limit_bytes = physical_memory_mb / 8; // >144MB
merge the above two?

[piman, 2014-02-15 23:14:50]

post-facto OWNER lgtm.

[epenner, 2014-02-17 05:43:22]

Thanks all. I'll fix up the remaining comments. Just preferred this to be in
before branch cut.

https://codereview.chromium.org/168053002/diff/70001/content/renderer/gpu/ren...
File content/renderer/gpu/render_widget_compositor.cc (right):

https://codereview.chromium.org/168053002/diff/70001/content/renderer/gpu/ren...
content/renderer/gpu/render_widget_compositor.cc:269: // apps. So initially we
use 50% more memory to avoid flickering
On 2014/02/15 07:27:20, klobag.chromium wrote:
> comment 50% is not matching code 67%.

Oh, yes this comment is very confusing. We do actually use 50% more (12MB vs
8MB), so this is 8/12 = 67%.  I'll clean this up to be more clear.

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android.cc
File ui/gl/gl_context_android.cc (right):

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android...
ui/gl/gl_context_android.cc:117: // pre-painting and uses the rest only for
'emergencies'.
On 2014/02/15 07:27:20, klobag.chromium wrote:
> Can we move this two lines up and say the number is emergency (pre-painting)? 

Yes, much better way to explain the numbers.

https://codereview.chromium.org/168053002/diff/70001/ui/gl/gl_context_android...
ui/gl/gl_context_android.cc:126: limit_bytes = physical_memory_mb / 8; // >144MB
On 2014/02/15 07:27:20, klobag.chromium wrote:
> merge the above two?

Good point will do. I think this number is common way to differentiate some
devices, but no longer useful here.