cc: Add command line switch to control resource usage for impl-side painting.

cc: Add command line switch to control resource usage for impl-side painting.

Reduce unused resource pool memory to a percentage of memory limit.

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

[reveman, 2013-03-25 05:00:24]

We should probably get this into M27. Wdyt?

[piman, 2013-03-25 18:13:46]

I'll let Nat review the actual code, just one question/comment.

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc
File cc/resources/resource_pool.cc (right):

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool...
cc/resources/resource_pool.cc:100: size_t ten_percent_of_memory =
max_memory_usage_bytes_ / 10;
What would happen if we didn't try to keep anything?
10% of 1GB (the current limit on Chrome OS) is 100MB. I want to make sure 100MB
allocated per visible tab is worth whatever we hope to save in perf.

[reveman, 2013-03-25 18:36:06]

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc
File cc/resources/resource_pool.cc (right):

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool...
cc/resources/resource_pool.cc:100: size_t ten_percent_of_memory =
max_memory_usage_bytes_ / 10;
On 2013/03/25 18:13:46, piman wrote:
> What would happen if we didn't try to keep anything?
> 10% of 1GB (the current limit on Chrome OS) is 100MB. I want to make sure
100MB
> allocated per visible tab is worth whatever we hope to save in perf.

we have to pay the cost of allocating texture, pixel-transfer-buffer and query
more frequently. so basically some shallow flushes and whatever driver side cost
we have for creating a new texture. this might not be too bad on chromeos but
excessive amounts of cache misses for each pending tree activation will
undoubtedly have a performance impact.

I picked 10% as that's what we use in the old texture manager. I don't have any
data showing that this is the right number. I've only done some minimal testing
and not seen a significant perf impact.

[nduca, 2013-03-25 18:37:49]

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc
File cc/resources/resource_pool.cc (right):

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool...
cc/resources/resource_pool.cc:100: size_t ten_percent_of_memory =
max_memory_usage_bytes_ / 10;
How does this compare to a "please drop everything" policy? I'm really leery of
a memory managment strategy that drops some but not everything.

If you see a difference between the two, can we use the
allow_nothing/minimum/prepaint/anything policy instead? And ideally tie them
together more closely?

[reveman, 2013-03-25 18:47:33]

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc
File cc/resources/resource_pool.cc (right):

https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool...
cc/resources/resource_pool.cc:100: size_t ten_percent_of_memory =
max_memory_usage_bytes_ / 10;
On 2013/03/25 18:37:49, nduca wrote:
> How does this compare to a "please drop everything" policy? I'm really leery
of
> a memory managment strategy that drops some but not everything.

if we drop everything then initializing a new pending tree will be more costly
as we'll have to create new resources for every single tile. it might affect
active tree performance a bit too with the current read lock fences as we can't
reuse textures that we recently used for drawing.

> 
> If you see a difference between the two, can we use the
> allow_nothing/minimum/prepaint/anything policy instead? And ideally tie them
> together more closely?

sg, I forgot that we had that.

[nduca, 2013-03-25 19:06:41]

maybe we can make the pool accept the enum, then have idleness cause an
allow_prepainting mode, which internally would be defined as 10% as before.

[nduca, 2013-03-25 19:07:22]

also should we inherit the "temporary reduce to this policy" vs "please use this
as your policy"

and in that context, should the 10% be defined as 10% of the limit given to us
on the global policy call?

[piman, 2013-03-25 19:19:29]

On Mon, Mar 25, 2013 at 11:36 AM, <reveman@chromium.org> wrote:

>
> https://codereview.chromium.**org/12502016/diff/1001/cc/**
>
resources/resource_pool.cc<https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc>
> File cc/resources/resource_pool.cc (right):
>
> https://codereview.chromium.**org/12502016/diff/1001/cc/**
>
resources/resource_pool.cc#**newcode100<https://codereview.chromium.org/12502016/diff/1001/cc/resources/resource_pool.cc#newcode100>
> cc/resources/resource_pool.cc:**100: size_t ten_percent_of_memory =
> max_memory_usage_bytes_ / 10;
> On 2013/03/25 18:13:46, piman wrote:
>
>> What would happen if we didn't try to keep anything?
>> 10% of 1GB (the current limit on Chrome OS) is 100MB. I want to make
>>
> sure 100MB
>
>> allocated per visible tab is worth whatever we hope to save in perf.
>>
>
> we have to pay the cost of allocating texture, pixel-transfer-buffer and
> query more frequently. so basically some shallow flushes and whatever
> driver side cost we have for creating a new texture. this might not be
> too bad on chromeos but excessive amounts of cache misses for each
> pending tree activation will undoubtedly have a performance impact.


> I picked 10% as that's what we use in the old texture manager. I don't
> have any data showing that this is the right number. I've only done some
> minimal testing and not seen a significant perf impact.
>

Allocating the texture, including the shallow flush, seems comparatively
insignificant compared to the cost of painting it and uploading it. 100MB
seems very high to amortize this cost - about 5 times the size of the
viewport on Pixel. It doesn't seem legit that we'll need to re-allocate
that much in the critical path.
If we know that that memory will be used for prepainting, it makes sense,
but isn't the concept here that we're idle?

In comparison, take transfer buffers, we aggressively release them, they're
a lot more costly to reallocate, but it hasn't showed up as a problem in
the critical path.


>
>
https://codereview.chromium.**org/12502016/<https://codereview.chromium.org/1...
>

[reveman, 2013-03-26 04:51:37]

latest patch sets the unused memory limit based on viewport size and it doesn't
care about being idle. it might be a bit too aggressive for android. I'll do
more testing tomorrow.

[nduca, 2013-03-26 05:46:15]

Gagh, thats memory manager's job [and we've seen countless cases where viewport
heurstics are wrong, e.g. viewport is funky during init]!! Why can't it use a
ratio relative to the budget given to us by the memory manager?

[piman, 2013-03-26 20:32:37]

On Mon, Mar 25, 2013 at 10:46 PM, <nduca@chromium.org> wrote:

> Gagh, thats memory manager's job [and we've seen countless cases where
> viewport
> heurstics are wrong, e.g. viewport is funky during init]!! Why can't it
> use a
> ratio relative to the budget given to us by the memory manager?
>

What we're talking about here is memory we keep around to avoid paying the
cost to reallocate it. It doesn't hold valid data.
There's a strong argument that you shouldn't be uploading more than the
visible data in the critical path. We shouldn't be wasting memory for
offscreen/prerendering. Hence the relationship with the viewport if we want
something positive. Though 0 is possibly a valid number too.

The memory manager gives a memory allocation that's unrelated to the
viewport/visible size. In particular, 10% of 1000MB is 100MB and that's
ridiculously high for the amount of benefits we get. We shouldn't be
wasting 100MB just because we're lazy.

We need something for M27, or impl-side will probably be a large regression
in terms of memory.



>
>
https://codereview.chromium.**org/12502016/<https://codereview.chromium.org/1...
>

[epenner, 2013-03-27 02:52:33]

This is a case where Android has pretty different needs. On Android we are much
more concerned about avoiding jank and can live with a fixed GPU memory cost
allocated to a tab (the entire memory budget basically).

I understand that's not the case on all other platforms, where tabs need to
share and co-exist.

Once concern with a small buffer is that it quickly goes away in the most common
use-case (scrolling to new content), and then it may as well have been zero. We
can somewhat absorb the cost of allocations on some GPUs, but unfortunately not
all.

[reveman, 2013-03-27 23:51:47]

Added the command line switch as discussed. PTAL.

[piman, 2013-03-28 00:00:04]

LGTM for the flag, content/ and chrome/, someone else please review the rest.

[nduca, 2013-03-28 02:51:23]

lgtm and lets do a follow up maybe later to see if we can get somewhere on the
overall pooling topic.

[commit-bot: I haz the power, 2013-03-28 18:23:44]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/12502016/23001

[commit-bot: I haz the power, 2013-03-28 23:27:03]

Change committed as 191239