CC serialization perftests: more accurate "num runs in 2 seconds".

CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[yzshen1, 2016-07-29 21:53:52]

Description was changed from

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because in some
test cases, 2 seconds can only run ~30 runs. The number is too rough to be
useful.

BUG=624459
==========

to

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because in some
test cases, 2 seconds can only run ~30 runs. The number is too rough to be
useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[yzshen1, 2016-07-29 21:55:14]

Description was changed from

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because in some
test cases, 2 seconds can only run ~30 runs. The number is too rough to be
useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases
can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

[yzshen1, 2016-07-29 21:55:15]

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

[yzshen1, 2016-07-29 21:55:46]

Hi, Dana

Would you please take a look? Thanks!

[yzshen1, 2016-07-29 21:57:15]

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

[commit-bot: I haz the power, 2016-07-29 21:57:46]

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

[danakj, 2016-07-29 22:04:56]

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
cc/ipc/cc_serialization_perftest.cc:134: for (int i = 0; i < kTimeCheckInterval;
++i) {
Just decrease kTimeCheckInterval for those tests and maybe EXPECT that it's
larger than some function of the number the test reports?

[yzshen1, 2016-07-29 22:12:12]

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
cc/ipc/cc_serialization_perftest.cc:134: for (int i = 0; i < kTimeCheckInterval;
++i) {
On 2016/07/29 22:04:56, danakj wrote:
> Just decrease kTimeCheckInterval for those tests and maybe EXPECT that it's
> larger than some function of the number the test reports?

Decreasing the number of kTimeCheckInterval may affect the min_frame_*_time,
because that is an average of kTimeCheckInterval runs.

Is it a concern that we may invoke base::TimeTicks::Now() too many times? I
compared the numbers before/after the change, it seems the difference is
negligible.

WDYT? Thanks!

[danakj, 2016-07-29 22:14:02]

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
cc/ipc/cc_serialization_perftest.cc:134: for (int i = 0; i < kTimeCheckInterval;
++i) {
On 2016/07/29 22:12:12, yzshen1 wrote:
> On 2016/07/29 22:04:56, danakj wrote:
> > Just decrease kTimeCheckInterval for those tests and maybe EXPECT that it's
> > larger than some function of the number the test reports?
> 
> Decreasing the number of kTimeCheckInterval may affect the min_frame_*_time,
> because that is an average of kTimeCheckInterval runs.
> 
> Is it a concern that we may invoke base::TimeTicks::Now() too many times? I
> compared the numbers before/after the change, it seems the difference is
> negligible.
> 
> WDYT? Thanks!

Ah ok.

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
cc/ipc/cc_serialization_perftest.cc:137: ++count;
Can you do this at the bottom of the loop. And you should be able to reuse this
Now() value below the loop if you scope the variable right.

[commit-bot: I haz the power, 2016-07-29 22:40:37]

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

[commit-bot: I haz the power, 2016-07-29 22:40:38]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2016-07-31 06:54:18]

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

[commit-bot: I haz the power, 2016-07-31 06:54:27]

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

[yzshen1, 2016-07-31 06:57:17]

Thanks! PTAL

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
File cc/ipc/cc_serialization_perftest.cc (right):

https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
cc/ipc/cc_serialization_perftest.cc:137: ++count;
On 2016/07/29 22:14:02, danakj wrote:
> Can you do this at the bottom of the loop. And you should be able to reuse
this
> Now() value below the loop if you scope the variable right.

Done.

[commit-bot: I haz the power, 2016-07-31 07:35:46]

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

[commit-bot: I haz the power, 2016-07-31 07:35:47]

Dry run: This issue passed the CQ dry run.

[yzshen1, 2016-08-02 22:08:08]

On 2016/07/31 06:57:17, yzshen1 wrote:
> Thanks! PTAL
> 
>
https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
> File cc/ipc/cc_serialization_perftest.cc (right):
> 
>
https://codereview.chromium.org/2196873002/diff/1/cc/ipc/cc_serialization_per...
> cc/ipc/cc_serialization_perftest.cc:137: ++count;
> On 2016/07/29 22:14:02, danakj wrote:
> > Can you do this at the bottom of the loop. And you should be able to reuse
> this
> > Now() value below the loop if you scope the variable right.
> 
> Done.

Friendly ping, Dana.

[yzshen1, 2016-08-02 22:10:27]

yzshen@chromium.org changed reviewers:
+ enne@chromium.org

[yzshen1, 2016-08-02 22:10:27]

Just realized that Dana is OOO currently.

+enne@
Hi, Adrienne. Woud you please take a look? Thanks!

[enne (OOO), 2016-08-03 17:22:37]

I am also.  I'm going to punt and just say wait for danakj.

[yzshen1, 2016-08-09 16:59:12]

Friendly ping, Dana. :)

[danakj, 2016-08-09 21:50:39]

Thanks, can you jsut add a comment on each of those saying "We don't count
iterations after the end time" cuz I think that's not super obvious. LGTM

[yzshen1, 2016-08-09 23:07:54]

The CQ bit was checked by yzshen@chromium.org

[yzshen1, 2016-08-09 23:07:55]

The patchset sent to the CQ was uploaded after l-g-t-m from danakj@chromium.org
Link to the patchset: https://codereview.chromium.org/2196873002/#ps40001
(title: ".")

[yzshen1, 2016-08-09 23:08:09]

On 2016/08/09 21:50:39, danakj wrote:
> Thanks, can you jsut add a comment on each of those saying "We don't count
> iterations after the end time" cuz I think that's not super obvious. LGTM

Thanks Dana! Done.

[commit-bot: I haz the power, 2016-08-09 23:08:32]

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-08-10 00:00:00]

Description was changed from

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases
can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases
can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

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

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-10 00:01:31]

Description was changed from

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases
can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel
==========

to

==========
CC serialization perftests: more accurate "num runs in 2 seconds".

Previously, the code checked elapsed time every 10 runs. Because some test cases
can only do ~30 runs in 2 seconds. The number is too rough to be useful.

BUG=624459
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_precise_blink_rel

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

[commit-bot: I haz the power, 2016-08-10 00:01:33]

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