Fixed tough texture upload benchmark on low end devices.

Fixed tough texture upload benchmark on low end devices.

Fast extra shared memory texture upload path is now disabled for low
end devices, they don't have enough memory to allocate this.

R=sievers@chromium.org
BUG=543067, 462078
Committed: https://crrev.com/13ac193d767c089c720497398a1dec39c07e2929
Cr-Commit-Position: refs/heads/master@{#354797}

[David Yen, 2015-10-15 21:11:03]

dyen@chromium.org changed reviewers:
+ nednguyen@google.com

[David Yen, 2015-10-15 21:11:03]

+nednguyen@ for telemetry page change

[David Yen, 2015-10-15 21:15:15]

dyen@chromium.org changed reviewers:
- nednguyen@google.com

[David Yen, 2015-10-15 21:16:00]

I moved the javascript changes to another CL instead since nednguyen@ is OOO,
that is less important.

[no sievers, 2015-10-15 22:33:30]

https://codereview.chromium.org/1408843002/diff/40001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/40001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:2443: const uint32_t
max_extra_transfer_buffer_size =
can you make it a constant in this class?

[David Yen, 2015-10-15 23:01:55]

https://codereview.chromium.org/1408843002/diff/40001/gpu/command_buffer/clie...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/40001/gpu/command_buffer/clie...
gpu/command_buffer/client/gles2_implementation.cc:2443: const uint32_t
max_extra_transfer_buffer_size =
On 2015/10/15 22:33:30, sievers wrote:
> can you make it a constant in this class?

I made it a static constant instead for the file.

[no sievers, 2015-10-15 23:24:45]

https://codereview.chromium.org/1408843002/diff/100001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/100001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:2222: const int64_t
kPhysicalMemory = base::SysInfo::AmountOfPhysicalMemory();
This might not be safe, because it's unclear when this runs.

Can you just make it a member var and set it from the constructor or Init?

[David Yen, 2015-10-19 15:25:53]

Hopefully making it a non-static const will avoid the "< 0 is always false"
warning...

https://codereview.chromium.org/1408843002/diff/100001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/100001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:2222: const int64_t
kPhysicalMemory = base::SysInfo::AmountOfPhysicalMemory();
On 2015/10/15 23:24:45, sievers wrote:
> This might not be safe, because it's unclear when this runs.
> 
> Can you just make it a member var and set it from the constructor or Init?

Done.

[no sievers, 2015-10-19 15:55:05]

lgtm, but should we also set the reclaim limit (at the very least on <=1GB
devices)? can be a separate patch though

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:120: // use any extra for
memory contrained devices (<1GB).
nit: <=GB

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:122: ?
static_cast<uint32_t>(base::SysInfo::AmountOfPhysicalMemory() /
maybe be paranoid and do saturated_cast<>?

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.h (right):

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.h:749: // Maximum amount of extra
shared memory to use when needing to transfer
nit: maybe say 'extra memory from the mapped memory pool', since it's
technically not entirely 'extra' but part of an existing pool

[David Yen, 2015-10-19 16:02:18]

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.cc (right):

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:120: // use any extra for
memory contrained devices (<1GB).
On 2015/10/19 15:55:04, sievers wrote:
> nit: <=GB

Done.

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.cc:122: ?
static_cast<uint32_t>(base::SysInfo::AmountOfPhysicalMemory() /
On 2015/10/19 15:55:04, sievers wrote:
> maybe be paranoid and do saturated_cast<>?

Done.

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
File gpu/command_buffer/client/gles2_implementation.h (right):

https://codereview.chromium.org/1408843002/diff/120001/gpu/command_buffer/cli...
gpu/command_buffer/client/gles2_implementation.h:749: // Maximum amount of extra
shared memory to use when needing to transfer
On 2015/10/19 15:55:04, sievers wrote:
> nit: maybe say 'extra memory from the mapped memory pool', since it's
> technically not entirely 'extra' but part of an existing pool

Done.

[David Yen, 2015-10-19 16:35:36]

The CQ bit was checked by dyen@chromium.org

[David Yen, 2015-10-19 16:35:36]

The patchset sent to the CQ was uploaded after l-g-t-m from sievers@chromium.org
Link to the patchset: https://codereview.chromium.org/1408843002/#ps140001
(title: "Improved comments, use saturated cast")

[commit-bot: I haz the power, 2015-10-19 16:35:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1408843002/140001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1408843002/140001

[commit-bot: I haz the power, 2015-10-19 17:23:54]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-10-19 17:24:40]

Patchset 8 (id:??) landed as
https://crrev.com/13ac193d767c089c720497398a1dec39c07e2929
Cr-Commit-Position: refs/heads/master@{#354797}