Limit the number of accelerated canvases that can exist in a render process

Limit the number of accelerated canvases that can exist in a render process

To avoid exhausting GPU resource limits, we limit the number of the
accelerated canvas count. Previously, we only limited based on memory
consumption, wich was not enough to prevent recurring GPU context resets.

BUG=620329
Committed: https://crrev.com/316f13fd14e0e9206850c67fa04463b99de0abd8
Cr-Commit-Position: refs/heads/master@{#400190}

[Justin Novosad, 2016-06-15 15:14:18]

junov@chromium.org changed reviewers:
+ xlai@chromium.org

[xlai (Olivia), 2016-06-15 18:54:07]

The previous metric s_globalGPUMemoryUsage is also per-renderer-process-based. I
don't quite understand what's the purpose of adding one more metric to guard
against GPU over-usage. Could you explain what would the scenarios when
shouldAccelerate returns "true" with only s_globalGPUMemoryUsage but return
"false" after having your additional s_globalAcceleratedImageBufferCount? And
the opposite scenario as well. If the scenarios are completely overlapping or
one metric is always stricter than the other metric, we should keep only one of
the two metrics.

https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:753: // When GPU
allocated memory runs low (due to having created too many
Hmmm...duplicate comments?

[Justin Novosad, 2016-06-15 19:12:56]

On 2016/06/15 18:54:07, xlai (Olivia) wrote:
> The previous metric s_globalGPUMemoryUsage is also per-renderer-process-based.
I
> don't quite understand what's the purpose of adding one more metric to guard
> against GPU over-usage. Could you explain what would the scenarios when
> shouldAccelerate returns "true" with only s_globalGPUMemoryUsage but return
> "false" after having your additional s_globalAcceleratedImageBufferCount? And
> the opposite scenario as well. If the scenarios are completely overlapping or
> one metric is always stricter than the other metric, we should keep only one
of
> the two metrics.

Basically there are two resource exhaustion scenarios: maxing out GPU memory,
and maxing-out the number of resource. The previous logic was just tracking
memory, not resource count

[Justin Novosad, 2016-06-15 19:17:34]

https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):

https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:753: // When GPU
allocated memory runs low (due to having created too many
On 2016/06/15 18:54:06, xlai (Olivia) wrote:
> Hmmm...duplicate comments?

Done.

[xlai (Olivia), 2016-06-15 19:31:39]

On 2016/06/15 19:17:34, Justin Novosad wrote:
>
https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp (right):
> 
>
https://codereview.chromium.org/2069143002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/html/HTMLCanvasElement.cpp:753: // When GPU
> allocated memory runs low (due to having created too many
> On 2016/06/15 18:54:06, xlai (Olivia) wrote:
> > Hmmm...duplicate comments?
> 
> Done.

I see... lgtm but you might need to fix the minor bug reported in bots.

[Justin Novosad, 2016-06-15 19:36:46]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-06-15 19:36:48]

The patchset sent to the CQ was uploaded after l-g-t-m from xlai@chromium.org
Link to the patchset: https://codereview.chromium.org/2069143002/#ps40001
(title: "'u' is for unsigned")

[commit-bot: I haz the power, 2016-06-15 19:37:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2069143002/40001

[commit-bot: I haz the power, 2016-06-15 20:46:57]

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

[commit-bot: I haz the power, 2016-06-15 20:46:58]

Try jobs failed on following builders:
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[Justin Novosad, 2016-06-16 15:34:51]

The CQ bit was checked by junov@chromium.org

[Justin Novosad, 2016-06-16 15:34:52]

The patchset sent to the CQ was uploaded after l-g-t-m from xlai@chromium.org
Link to the patchset: https://codereview.chromium.org/2069143002/#ps60001
(title: "fix layout test")

[commit-bot: I haz the power, 2016-06-16 15:36:11]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/2069143002/60001

[commit-bot: I haz the power, 2016-06-16 17:32:38]

Description was changed from

==========
Limit the number of accelerated canvases that can exist in a render process

To avoid exhausting GPU resource limits, we limit the number of the
accelerated canvas count. Previously, we only limited based on memory
consumption, wich was not enough to prevent recurring GPU context resets.

BUG=620329
==========

to

==========
Limit the number of accelerated canvases that can exist in a render process

To avoid exhausting GPU resource limits, we limit the number of the
accelerated canvas count. Previously, we only limited based on memory
consumption, wich was not enough to prevent recurring GPU context resets.

BUG=620329

Committed: https://crrev.com/316f13fd14e0e9206850c67fa04463b99de0abd8
Cr-Commit-Position: refs/heads/master@{#400190}
==========

[commit-bot: I haz the power, 2016-06-16 17:32:39]

Patchset 4 (id:??) landed as
https://crrev.com/316f13fd14e0e9206850c67fa04463b99de0abd8
Cr-Commit-Position: refs/heads/master@{#400190}