Add memory limits struct to in-process context

Add memory limits struct to in-process context

Add SharedMemoryLimits struct to in-process command buffer
context. Pick a reasonable value for the reclaim limit for
synchronous compositor

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

[boliu, 2014-08-11 15:19:10]

PTAL.

I thought about sharing the SharedMemoryLimits struct with the cross-process
one, maybe put it under webkit/common/gpu. Not sure if it's worth the effort
though. Thoughts?

[boliu, 2014-08-19 16:09:33]

+sievers or review of gpu parts?

[no sievers, 2014-08-19 17:55:24]

On 2014/08/19 16:09:33, boliu wrote:
> +sievers or review of gpu parts?

lgtm

[boliu, 2014-08-19 21:32:08]

+kbr for webkit/common/gpu

Will tbr for the cc/test change after.

[Ken Russell (switch to Gerrit), 2014-08-19 21:40:00]

lgtm

[boliu, 2014-08-19 21:53:16]

And tbr-ing piman for the cc/test

[boliu, 2014-08-19 21:53:47]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-08-19 21:56:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/boliu@chromium.org/455083002/40001

[piman, 2014-08-19 22:58:14]

lgtm

https://codereview.chromium.org/455083002/diff/40001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
(right):

https://codereview.chromium.org/455083002/diff/40001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_factory_impl.cc:196:
static_cast<size_t>(6.4 * 1024 * 1024);
nit: it's odd to use doubles to select a memory limit. It's not even a multiple
of 4. Is that intended?

[boliu, 2014-08-19 23:06:38]

https://codereview.chromium.org/455083002/diff/40001/content/browser/android/...
File content/browser/android/in_process/synchronous_compositor_factory_impl.cc
(right):

https://codereview.chromium.org/455083002/diff/40001/content/browser/android/...
content/browser/android/in_process/synchronous_compositor_factory_impl.cc:196:
static_cast<size_t>(6.4 * 1024 * 1024);
On 2014/08/19 22:58:13, piman (OOO) wrote:
> nit: it's odd to use doubles to select a memory limit. It's not even a
multiple
> of 4. Is that intended?

It's from this code in render_widget.cc, but using half of that result due to
webview's pipeline being only one frame deep. This was advice from epenner.

https://code.google.com/p/chromium/codesearch#chromium/src/content/renderer/r...

This class and render_widget.cc does duplicate some magic android only code
though. Should eventually clean those up later..

[commit-bot: I haz the power, 2014-08-19 23:07:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  android_aosp on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/android_aosp/bu...)
  ios_dbg_simulator on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_dbg_simulator...)
  ios_rel_device_ninja on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/ios_rel_device_ni...)
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[commit-bot: I haz the power, 2014-08-19 23:16:02]

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

[commit-bot: I haz the power, 2014-08-19 23:16:03]

Try jobs failed on following builders:
  win_chromium_x64_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_x64_...)

[piman, 2014-08-19 23:50:55]

On Tue, Aug 19, 2014 at 4:06 PM, <boliu@chromium.org> wrote:

>
> https://codereview.chromium.org/455083002/diff/40001/
> content/browser/android/in_process/synchronous_compositor_factory_impl.cc
> File
> content/browser/android/in_process/synchronous_compositor_factory_impl.cc
> (right):
>
> https://codereview.chromium.org/455083002/diff/40001/
> content/browser/android/in_process/synchronous_compositor_factory_impl.cc#
> newcode196
> content/browser/android/in_process/synchronous_compositor_factory_impl.cc:
> 196:
> static_cast<size_t>(6.4 * 1024 * 1024);
> On 2014/08/19 22:58:13, piman (OOO) wrote:
>
>> nit: it's odd to use doubles to select a memory limit. It's not even a
>>
> multiple
>
>> of 4. Is that intended?
>>
>
> It's from this code in render_widget.cc, but using half of that result
> due to webview's pipeline being only one frame deep. This was advice
> from epenner.


> https://code.google.com/p/chromium/codesearch#chromium/
> src/content/renderer/render_widget.cc&l=2106
>
>
Note that that code clamps the double value (6.4 in your case) before
multiplying by 1MB, so you end up with 6 MB which is well rounded.


> This class and render_widget.cc does duplicate some magic android only
> code though. Should eventually clean those up later..
>
> https://codereview.chromium.org/455083002/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[boliu, 2014-08-20 00:01:40]

On 2014/08/19 23:50:55, piman (OOO) wrote:
> Note that that code clamps the double value (6.4 in your case) before
> multiplying by 1MB, so you end up with 6 MB which is well rounded.
> 

Oh good point. Changing it to 6MB then. And adding a comment.

win_x64 failure looks real. Although I don't know how
content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.cc is not
failing build. Afaict it's doing a size_t -> int32 conversion for
CommandBufferHelper::Initialize as well. Maybe it's built with different flags

size_t -> unsigned int being a warning seems weird

<starts spamming the x64 bot>

[boliu, 2014-08-20 01:23:24]

Updated the types in GLInProcessContextSharedMemoryLimits to match the expected
argument types to make win_x64 happy.

[boliu, 2014-08-20 01:23:28]

The CQ bit was checked by boliu@chromium.org

[boliu, 2014-08-20 01:23:35]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2014-08-20 01:24:04]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2014-08-20 01:24:47]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/boliu@chromium.org/455083002/80001

[commit-bot: I haz the power, 2014-08-20 03:01:35]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  win_chromium_rel_swarming on tryserver.chromium.win
(http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-20 04:25:10]

Committed patchset #5 (80001) as 290764