cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/fa1b0c9d81d9225e4d8bec0b3e1c66817e63b3fb
Cr-Commit-Position: refs/heads/master@{#441551}

[vmpstr, 2017-01-04 21:44:59]

Description was changed from

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
==========

to

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[vmpstr, 2017-01-04 21:55:10]

Please take a look.

[sunnyps, 2017-01-04 22:15:22]

https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:777: highest_bin_found = priority.priority_bin;
Shouldn't this be lowest bin (NOW < SOON < EVENTUALLY)? And shouldn't we process
this in another loop so that we log the correct value in the CHECKs below?

[vmpstr, 2017-01-04 22:49:39]

https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.cc
File cc/tiles/tile_manager.cc (right):

https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.c...
cc/tiles/tile_manager.cc:777: highest_bin_found = priority.priority_bin;
On 2017/01/04 22:15:22, sunnyps wrote:
> Shouldn't this be lowest bin (NOW < SOON < EVENTUALLY)? And shouldn't we
process
> this in another loop so that we log the correct value in the CHECKs below?

It's expected that tiles here would be in a higher priority, I just want to log
what the highest one was... for example, was it a SOON bin that appeared before
the NOW or rfa tile, or was it an EVENTUALLY bin? If this value is "NOW" then
the very next tile was rfa/rfd. 

That is, this is just some information about which tiles we looped over before
reaching the problematic one. Once we reach the first problematic one we should
have enough information regardless of the tiles that follow.

I'm not sure this should be a different loop, this is specifically to just the
remainder of the queue.

[sunnyps, 2017-01-04 23:20:53]

On 2017/01/04 22:49:39, vmpstr wrote:
> https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.cc
> File cc/tiles/tile_manager.cc (right):
> 
>
https://codereview.chromium.org/2612123002/diff/40001/cc/tiles/tile_manager.c...
> cc/tiles/tile_manager.cc:777: highest_bin_found = priority.priority_bin;
> On 2017/01/04 22:15:22, sunnyps wrote:
> > Shouldn't this be lowest bin (NOW < SOON < EVENTUALLY)? And shouldn't we
> process
> > this in another loop so that we log the correct value in the CHECKs below?
> 
> It's expected that tiles here would be in a higher priority, I just want to
log
> what the highest one was... for example, was it a SOON bin that appeared
before
> the NOW or rfa tile, or was it an EVENTUALLY bin? If this value is "NOW" then
> the very next tile was rfa/rfd. 
> 
> That is, this is just some information about which tiles we looped over before
> reaching the problematic one. Once we reach the first problematic one we
should
> have enough information regardless of the tiles that follow.
> 
> I'm not sure this should be a different loop, this is specifically to just the
> remainder of the queue.

Yes, that makes sense. Thanks for the explanation. LGTM

[vmpstr, 2017-01-04 23:46:10]

vmpstr@chromium.org changed reviewers:
+ achuith@chromium.org, ccameron@chromium.org, enne@chromium.org

[vmpstr, 2017-01-04 23:46:10]

+achuith for chrome/browser/chromeos/login/chrome_restart_request.cc
+ccameron for content/browser/renderer_host/render_process_host_impl.cc
+enne for content/renderer/gpu/render_widget_compositor.cc

Please take a look.

[achuithb, 2017-01-04 23:47:18]

On 2017/01/04 23:46:10, vmpstr wrote:
> +achuith for chrome/browser/chromeos/login/chrome_restart_request.cc

lgtm c/b/cros/login/chrome_restart_request.cc

> +ccameron for content/browser/renderer_host/render_process_host_impl.cc
> +enne for content/renderer/gpu/render_widget_compositor.cc
> 
> Please take a look.

[achuithb, 2017-01-04 23:47:21]

On 2017/01/04 23:46:10, vmpstr wrote:
> +achuith for chrome/browser/chromeos/login/chrome_restart_request.cc

lgtm c/b/cros/login/chrome_restart_request.cc

> +ccameron for content/browser/renderer_host/render_process_host_impl.cc
> +enne for content/renderer/gpu/render_widget_compositor.cc
> 
> Please take a look.

[ccameron, 2017-01-04 23:47:21]

content/browser lgtm

[enne (OOO), 2017-01-04 23:50:00]

content/renderer/gpu/render_widget_compositor.cc lgtm

[vmpstr, 2017-01-04 23:50:19]

The CQ bit was checked by vmpstr@chromium.org

[commit-bot: I haz the power, 2017-01-04 23:50:47]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 00:04:04]

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

[commit-bot: I haz the power, 2017-01-05 00:04:05]

Try jobs failed on following builders:
  android_compile_dbg on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_comp...)

[vmpstr, 2017-01-05 00:14:44]

The CQ bit was checked by vmpstr@chromium.org

[vmpstr, 2017-01-05 00:14:44]

The patchset sent to the CQ was uploaded after l-g-t-m from
sunnyps@chromium.org, ccameron@chromium.org, achuith@chromium.org,
enne@chromium.org
Link to the patchset: https://codereview.chromium.org/2612123002/#ps60001
(title: "master: compile_fix")

[commit-bot: I haz the power, 2017-01-05 00:15:04]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-01-05 01:46:58]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1483575284146290,
"parent_rev": "4b0275c983beb452cb1ccdb2d39e347c1d075b44", "commit_rev":
"49f169481c12cceca8b2888eb041da953995ca46"}

[commit-bot: I haz the power, 2017-01-05 01:47:26]

Description was changed from

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2612123002
==========

[commit-bot: I haz the power, 2017-01-05 01:47:27]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2017-01-05 01:49:36]

Description was changed from

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2612123002
==========

to

==========
cc: Add a flag to check for tile priority inversion in AssignGpuMemory.

This patch adds a flag and code to ensure that after all of the tiles
have been processed in the tile manager, whatever remains in the queue
is not in NOW bin and not required for either draw or activation.

The flag is --check-tile-priority-inversion, which could be used to
debug various activation issues.

R=danakj@chromium.org, sunnyps@chromium.org
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/fa1b0c9d81d9225e4d8bec0b3e1c66817e63b3fb
Cr-Commit-Position: refs/heads/master@{#441551}
==========

[commit-bot: I haz the power, 2017-01-05 01:49:37]

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