Validate all flush IDs if the unvalidated list gets too long.

Validate all flush IDs if the unvalidated list gets too long.

The browser process normally never needs to validate sync tokens unless
it's returning resources to a renderer, so it's possible the highest
verified flush ID will almost never be updated and the list of
unvalidated flush IDs will grow indefinitely. Force a validation if that
happens to avoid wasting memory.

BUG=635051
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/0f79f41c428a89593c81deaf5bbbd952b80195d6
Cr-Commit-Position: refs/heads/master@{#417168}

[jbauman, 2016-09-07 22:02:39]

Description was changed from

==========
Validate all flush IDs if the unvalidated list gets too long.

The browser process normally never needs to validate sync tokens unless
it's returning resources to a renderer, so it's possible the highest
verified flush ID will almost never be updated and the list of
unvalidated flush IDs will grow indefinitely. Force a validation if that
happens to avoid wasting memory.

BUG=635051
==========

to

==========
Validate all flush IDs if the unvalidated list gets too long.

The browser process normally never needs to validate sync tokens unless
it's returning resources to a renderer, so it's possible the highest
verified flush ID will almost never be updated and the list of
unvalidated flush IDs will grow indefinitely. Force a validation if that
happens to avoid wasting memory.

BUG=635051
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

[jbauman, 2016-09-07 22:02:45]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 22:03:44]

Dry run: 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, 2016-09-07 23:12:14]

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

[commit-bot: I haz the power, 2016-09-07 23:12:14]

Dry run: This issue passed the CQ dry run.

[jbauman, 2016-09-07 23:14:50]

jbauman@chromium.org changed reviewers:
+ piman@chromium.org

[piman, 2016-09-07 23:49:10]

LGTM. IWBN to be able to validate flush ids when getting a message back from the
GPU (e.g. OnSwapBuffersCompleted) to avoid the unnecessary sync IPC, but
unfortunately it seems a bit hard to do (associate the swap ack back to the
flush id that originally caused the swap).

https://codereview.chromium.org/2318933005/diff/1/gpu/ipc/client/command_buff...
File gpu/ipc/client/command_buffer_proxy_impl.cc (right):

https://codereview.chromium.org/2318933005/diff/1/gpu/ipc/client/command_buff...
gpu/ipc/client/command_buffer_proxy_impl.cc:277:
CleanupFlushedReleases(highest_verified_flush_id);
nit: any reason to only do that when put_offset_changed is true? The
highest_verified_flush_id can change regardless (e.g other context on the same
stream did a verify).

[jbauman, 2016-09-07 23:54:52]

The CQ bit was checked by jbauman@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-09-07 23:55:29]

Dry run: 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, 2016-09-07 23:58:35]

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

[commit-bot: I haz the power, 2016-09-07 23:58:36]

Dry run: Exceeded global retry quota

[jbauman, 2016-09-07 23:58:38]

On 2016/09/07 23:49:10, piman wrote:
> LGTM. IWBN to be able to validate flush ids when getting a message back from
the
> GPU (e.g. OnSwapBuffersCompleted) to avoid the unnecessary sync IPC, but
> unfortunately it seems a bit hard to do (associate the swap ack back to the
> flush id that originally caused the swap).

Yeah, I also looked into sending it through the shared memory, but the plumbing
there's also a bit difficult. I don't think this IPC is going to happen that
often in practice, so I'm not too worried about it.
> 
>
https://codereview.chromium.org/2318933005/diff/1/gpu/ipc/client/command_buff...
> File gpu/ipc/client/command_buffer_proxy_impl.cc (right):
> 
>
https://codereview.chromium.org/2318933005/diff/1/gpu/ipc/client/command_buff...
> gpu/ipc/client/command_buffer_proxy_impl.cc:277:
> CleanupFlushedReleases(highest_verified_flush_id);
> nit: any reason to only do that when put_offset_changed is true? The
> highest_verified_flush_id can change regardless (e.g other context on the same
> stream did a verify).

No real reason to be this way, so fixed it.

[jbauman, 2016-09-08 00:16:16]

The CQ bit was checked by jbauman@chromium.org

[jbauman, 2016-09-08 00:16:16]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/2318933005/#ps20001
(title: "move cleanup")

[commit-bot: I haz the power, 2016-09-08 00:16:43]

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, 2016-09-08 00:20:18]

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

[commit-bot: I haz the power, 2016-09-08 00:20:19]

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

[jbauman, 2016-09-08 01:24:34]

The CQ bit was checked by jbauman@chromium.org

[commit-bot: I haz the power, 2016-09-08 01:25:00]

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, 2016-09-08 02:20:59]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-09-08 02:24:10]

Description was changed from

==========
Validate all flush IDs if the unvalidated list gets too long.

The browser process normally never needs to validate sync tokens unless
it's returning resources to a renderer, so it's possible the highest
verified flush ID will almost never be updated and the list of
unvalidated flush IDs will grow indefinitely. Force a validation if that
happens to avoid wasting memory.

BUG=635051
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
==========

to

==========
Validate all flush IDs if the unvalidated list gets too long.

The browser process normally never needs to validate sync tokens unless
it's returning resources to a renderer, so it's possible the highest
verified flush ID will almost never be updated and the list of
unvalidated flush IDs will grow indefinitely. Force a validation if that
happens to avoid wasting memory.

BUG=635051
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel

Committed: https://crrev.com/0f79f41c428a89593c81deaf5bbbd952b80195d6
Cr-Commit-Position: refs/heads/master@{#417168}
==========

[commit-bot: I haz the power, 2016-09-08 02:24:11]

Patchset 2 (id:??) landed as
https://crrev.com/0f79f41c428a89593c81deaf5bbbd952b80195d6
Cr-Commit-Position: refs/heads/master@{#417168}