cc: Add complex compositor frame to serialization perf tests

cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=699121
R=danakj

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2770753002
Cr-Commit-Position: refs/heads/master@{#461292}
Committed: https://chromium.googlesource.com/chromium/src/+/b24a207b8d1e25b9cfa4ead338809d3decb6a08c

[weiliangc, 2017-03-22 19:06:46]

Description was changed from

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=
==========

to

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[weiliangc, 2017-03-23 21:41:21]

Patchset #3 (id:40001) has been deleted

[weiliangc, 2017-03-23 21:42:04]

Description was changed from

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=699121
R=danakj

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[weiliangc, 2017-03-23 21:42:04]

weiliangc@chromium.org changed reviewers:
+ danakj@chromium.org

[weiliangc, 2017-03-23 21:50:34]

Add serialization perf test for arbitrary compositor frame (that looks a bit
more real).

[weiliangc, 2017-03-23 21:51:32]

Should block on crbug.com/698831 since the cc_perftests aren't running on bots
now.

[danakj, 2017-03-24 15:16:09]

On 2017/03/23 21:51:32, weiliangc wrote:
> Should block on crbug.com/698831 since the cc_perftests aren't running on bots
> now.

I don't think it needs to block, this is useful for you to run locally on
linux/android/chromeos as you see fit, profile, and demonstrate improvements as
motivation for CLs.

[danakj, 2017-03-24 15:29:58]

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
cc/ipc/cc_serialization_perftest.cc:232: // The default metadata should occupy
same space.
I'm not sure what this means?

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
Why is the test different on android?

[weiliangc, 2017-03-27 15:57:07]

Removed the comment that is not useful. Also no need to change the time limit,
so set back to 2 seconds.

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
cc/ipc/cc_serialization_perftest.cc:232: // The default metadata should occupy
same space.
On 2017/03/24 at 15:29:58, danakj wrote:
> I'm not sure what this means?

Sorry, probably left over from my setting this up, will be removed.

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
On 2017/03/24 at 15:29:58, danakj wrote:
> Why is the test different on android?

Android's trasnferable resource is different. It's from this CL:
https://codereview.chromium.org/2508203004.

The setup code I copied from unittest. Should I add comment mention this?

[danakj, 2017-03-28 18:01:56]

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
On 2017/03/27 15:57:07, weiliangc wrote:
> On 2017/03/24 at 15:29:58, danakj wrote:
> > Why is the test different on android?
> 
> Android's trasnferable resource is different. It's from this CL:
> https://codereview.chromium.org/2508203004.
> 
> The setup code I copied from unittest. Should I add comment mention this?

Unit tests are checking for correctness, do the values of any of these matter to
performance? Could it just all be default initialized?

[weiliangc, 2017-03-30 20:59:04]

On 2017/03/28 at 18:01:56, danakj wrote:
>
https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
> File cc/ipc/cc_serialization_perftest.cc (right):
> 
>
https://codereview.chromium.org/2770753002/diff/20001/cc/ipc/cc_serialization...
> cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
> On 2017/03/27 15:57:07, weiliangc wrote:
> > On 2017/03/24 at 15:29:58, danakj wrote:
> > > Why is the test different on android?
> > 
> > Android's trasnferable resource is different. It's from this CL:
> > https://codereview.chromium.org/2508203004.
> > 
> > The setup code I copied from unittest. Should I add comment mention this?
> 
> Unit tests are checking for correctness, do the values of any of these matter
to performance? Could it just all be default initialized?

For default initialized values the test runs much faster. Should I look for what
values make it slower? (I suspect filters could be part of the reason why)

[danakj, 2017-03-30 21:07:08]

On Thu, Mar 30, 2017 at 4:59 PM, weiliangc@chromium.org via
codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:

> On 2017/03/28 at 18:01:56, danakj wrote:
> >
> https://codereview.chromium.org/2770753002/diff/20001/cc/
> ipc/cc_serialization_perftest.cc
> > File cc/ipc/cc_serialization_perftest.cc (right):
> >
> >
> https://codereview.chromium.org/2770753002/diff/20001/cc/
> ipc/cc_serialization_perftest.cc#newcode260
> > cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
> > On 2017/03/27 15:57:07, weiliangc wrote:
> > > On 2017/03/24 at 15:29:58, danakj wrote:
> > > > Why is the test different on android?
> > >
> > > Android's trasnferable resource is different. It's from this CL:
> > > https://codereview.chromium.org/2508203004.
> > >
> > > The setup code I copied from unittest. Should I add comment mention
> this?
> >
> > Unit tests are checking for correctness, do the values of any of these
> matter
> to performance? Could it just all be default initialized?
>
> For default initialized values the test runs much faster. Should I look
> for what
> values make it slower? (I suspect filters could be part of the reason why)
>

Sorry I was basing this on the assumption youd have the same objects being
sent, just the values not mattering. You have to do CreateGrayscaleFilter()
etc to make those object to send, so you'd keep that. I'm looking at the
values of TransferrableResoruce for example, they won't matter right?


>
> https://codereview.chromium.org/2770753002/
>

-- 
You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[weiliangc, 2017-03-30 21:49:21]

On 2017/03/30 at 21:07:08, danakj wrote:
> On Thu, Mar 30, 2017 at 4:59 PM, weiliangc@chromium.org via
> codereview.chromium.org <reply@chromiumcodereview-hr.appspotmail.com> wrote:
> 
> > On 2017/03/28 at 18:01:56, danakj wrote:
> > >
> > https://codereview.chromium.org/2770753002/diff/20001/cc/
> > ipc/cc_serialization_perftest.cc
> > > File cc/ipc/cc_serialization_perftest.cc (right):
> > >
> > >
> > https://codereview.chromium.org/2770753002/diff/20001/cc/
> > ipc/cc_serialization_perftest.cc#newcode260
> > > cc/ipc/cc_serialization_perftest.cc:260: #if defined(OS_ANDROID)
> > > On 2017/03/27 15:57:07, weiliangc wrote:
> > > > On 2017/03/24 at 15:29:58, danakj wrote:
> > > > > Why is the test different on android?
> > > >
> > > > Android's trasnferable resource is different. It's from this CL:
> > > > https://codereview.chromium.org/2508203004.
> > > >
> > > > The setup code I copied from unittest. Should I add comment mention
> > this?
> > >
> > > Unit tests are checking for correctness, do the values of any of these
> > matter
> > to performance? Could it just all be default initialized?
> >
> > For default initialized values the test runs much faster. Should I look
> > for what
> > values make it slower? (I suspect filters could be part of the reason why)
> >
> 
> Sorry I was basing this on the assumption youd have the same objects being
> sent, just the values not mattering. You have to do CreateGrayscaleFilter()
> etc to make those object to send, so you'd keep that. I'm looking at the
> values of TransferrableResoruce for example, they won't matter right?
> 

Yeah you are right the values of TransferrableResources doesn't matter, though
transferring filters and matrices that are not default does affect time. So I
left those arbitrary value for shared quad states and draw quads in.
> 
> >
> > https://codereview.chromium.org/2770753002/
> >
> 
> -- 
> You received this message because you are subscribed to the Google Groups
"Chromium-reviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
email to chromium-reviews+unsubscribe@chromium.org.
>

[danakj, 2017-03-30 22:39:51]

LGTM

[weiliangc, 2017-03-31 18:42:00]

The CQ bit was checked by weiliangc@chromium.org

[commit-bot: I haz the power, 2017-03-31 18:42:33]

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-03-31 19:07:24]

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

[commit-bot: I haz the power, 2017-03-31 19:07:27]

CQ experienced an internal error when committing your CL and the maintainers
were notified. Sorry for the inconvenience.

[weiliangc, 2017-04-01 00:49:47]

The CQ bit was checked by weiliangc@chromium.org

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

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-04-01 00:55:36]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1491007787693390,
"parent_rev": "22b40bba05236fd02732e0011c945c222e4238a6", "commit_rev":
"b24a207b8d1e25b9cfa4ead338809d3decb6a08c"}

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

Description was changed from

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=699121
R=danakj

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
cc: Add complex compositor frame to serialization perf tests

This adds a more complex compositor frame which contains long resource
list, long shared quad state list, as well as a long quad list. The size
of these lists are around reasonable top web pages.

BUG=699121
R=danakj

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2770753002
Cr-Commit-Position: refs/heads/master@{#461292}
Committed:
https://chromium.googlesource.com/chromium/src/+/b24a207b8d1e25b9cfa4ead33880...
==========

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

Committed patchset #4 (id:80001) as
https://chromium.googlesource.com/chromium/src/+/b24a207b8d1e25b9cfa4ead33880...