Added methods to generate sync tokens for resource id(s).

Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6aa453542a8bb25a5616151d9444022d50ed9e98
Cr-Commit-Position: refs/heads/master@{#376039}

[David Yen, 2016-02-17 21:51:34]

Description was changed from

==========
Added method to generate sync tokens for resource id(s).

BUG=584381
==========

to

==========
Added method to generate sync tokens for resource id(s).

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[David Yen, 2016-02-17 21:53:29]

https://codereview.chromium.org/1707143002/diff/1/cc/resources/resource_provi...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/1707143002/diff/1/cc/resources/resource_provi...
cc/resources/resource_provider.cc:756: void
ResourceProvider::GenerateSyncTokenForResource(ResourceId resource_id) {
From piman@ (originally inside CopyToResource where I had inserted a fence sync
after every copy):

Most callers are debug/fallback paths and/or relatively rare calls (UI
resources), but one is the video path for software decode, where this would be
called 3 times per video frame (once for each plane). If there are concurrent
rasterization tasks, the ordering barriers can become (or cause) real flushes,
so that would be a regression I think.

Maybe we could make the caller responsible for doing the synchronization, and
provide a SynchronizeResources that could do a single fence/OB and set the token
on all resources?

[David Yen, 2016-02-17 21:54:36]

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

[David Yen, 2016-02-17 21:54:36]

https://codereview.chromium.org/1707143002/diff/1/cc/resources/resource_provi...
File cc/resources/resource_provider.cc (right):

https://codereview.chromium.org/1707143002/diff/1/cc/resources/resource_provi...
cc/resources/resource_provider.cc:756: void
ResourceProvider::GenerateSyncTokenForResource(ResourceId resource_id) {
On 2016/02/17 21:53:28, David Yen wrote:
> From piman@ (originally inside CopyToResource where I had inserted a fence
sync
> after every copy):
> 
> Most callers are debug/fallback paths and/or relatively rare calls (UI
> resources), but one is the video path for software decode, where this would be
> called 3 times per video frame (once for each plane). If there are concurrent
> rasterization tasks, the ordering barriers can become (or cause) real flushes,
> so that would be a regression I think.
> 
> Maybe we could make the caller responsible for doing the synchronization, and
> provide a SynchronizeResources that could do a single fence/OB and set the
token
> on all resources?

Done. There are probably other places CopyToResource() is used where this
function needs to be called, but I just added the 2 most obvious ones first.
Once I add in the DCHECK it will be more obvious where else we need to add sync
token generation.

[piman, 2016-02-17 22:47:05]

lgtm

https://codereview.chromium.org/1707143002/diff/1/cc/layers/video_layer_impl.cc
File cc/layers/video_layer_impl.cc (right):

https://codereview.chromium.org/1707143002/diff/1/cc/layers/video_layer_impl....
cc/layers/video_layer_impl.cc:127: ResourceProvider::ResourceIdArray
resource_ids;
nit: resource_ids.reserve(external_resources.mailboxes.size()); to avoid
reallocs.

[David Yen, 2016-02-17 22:52:53]

https://codereview.chromium.org/1707143002/diff/1/cc/layers/video_layer_impl.cc
File cc/layers/video_layer_impl.cc (right):

https://codereview.chromium.org/1707143002/diff/1/cc/layers/video_layer_impl....
cc/layers/video_layer_impl.cc:127: ResourceProvider::ResourceIdArray
resource_ids;
On 2016/02/17 22:47:04, piman wrote:
> nit: resource_ids.reserve(external_resources.mailboxes.size()); to avoid
> reallocs.

Done.

[David Yen, 2016-02-17 22:53:26]

The CQ bit was checked by dyen@chromium.org

[David Yen, 2016-02-17 22:53:27]

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/1707143002/#ps20001
(title: "Added reserve")

[commit-bot: I haz the power, 2016-02-17 22:55:23]

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

[David Yen, 2016-02-17 22:56:00]

Description was changed from

==========
Added method to generate sync tokens for resource id(s).

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-02-18 00:01:39]

Description was changed from

==========
Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

[commit-bot: I haz the power, 2016-02-18 00:01:40]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-18 00:02:32]

Description was changed from

==========
Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel
==========

to

==========
Added methods to generate sync tokens for resource id(s).

New functions have been added so a sync token can represent
multiple resources at once. Adding a single sync token at
the end of generating a multitude of resources can be a win.

The most obvious use cases of this is the video layer which
generates a resource for each layer. A single sync token is
now generated at the end of the layer generation.

The layer tree UI resource also generates its own sync token.

BUG=584381
CQ_INCLUDE_TRYBOTS=tryserver.blink:linux_blink_rel

Committed: https://crrev.com/6aa453542a8bb25a5616151d9444022d50ed9e98
Cr-Commit-Position: refs/heads/master@{#376039}
==========

[commit-bot: I haz the power, 2016-02-18 00:02:33]

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