Add initial framework for performance tests that measure frame rate.

Add initial framework for performance tests that measure frame rate.

Add the "blank page" test, which just scrolls a page with a very large body.
The actual frame rate measurement is based on the webkitRequestAnimationFrame
callback.  The framework records how frequently that API generates a callback.
If it generates a callback at 60 FPS, then we know nothing is slowing us down.
This of course assumes that webkitRequestAnimationFrame is well implemented.

The advantage of this test framework over one that directly reads a framerate
from Chromium is that this one works outside of the test harness.  You can just
navigate your browser to the test file and run the test manually.

R=brettw,nduca
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=86568

[cmp, 2011-05-21 02:22:19]

nice, some general comments

http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi
File chrome/chrome_tests.gypi (right):

http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi#newcod...
chrome/chrome_tests.gypi:2873: 'target_name': 'frame_rate_tests',
Adding another UIPerfTest exe to our build will have a noticeable effect on
build time.  (The cost is highest in terms of linking against the UIPerfTest
code and everything it depends on.)  Underscores the necessity for pyauto-based
perf tests.

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
chrome/test/perf/frame_rate/frame_rate_tests.cc:56: tab, L"",
L"window.domAutomationController.send(!__running);",
Our perf tests have been flaky in domAutomationController tests.  I haven't
tracked down yet why.  Keep an eye out for that.  There's a bug on file for
that, it's led to marking some perf tests as FLAKY_ (yours might get that
treatment, too).

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
chrome/test/perf/frame_rate/frame_rate_tests.cc:75: "frames-per-second", true);
I would not bother with setting important to true here.  The waterfall is
overloaded with numeric data that I suspect no one watches (if someone watches
it at the start they eventually don't watch it).  The signal should come from
the color of the step (green, orange, red) which comes from perf expectations,
and from the perf dashboard itself.

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
chrome/test/perf/frame_rate/frame_rate_tests.cc:104: // the
webkitRequestAnimationFrame API.
We also need to update the reference build to pick up a copy of pyauto.  We'd
like to use builds that come off of the build systems this time (and from now
on).  Issues:
1. Verify those builds have everything we need enabled.
2. No recent regressions.
3. Official Mac tests still use a different branding name to reach the
executable (solved previously with a wholly separate reference build for
official mac).
4. Perf expectations will need updating after the reference builds land.

Obviously doesn't block landing this, just blocks uncommenting the reference
tests.

[darin (slow to review), 2011-05-21 04:26:53]

On Fri, May 20, 2011 at 7:22 PM, <cmp@chromium.org> wrote:

> nice, some general comments
>
>
> http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi
> File chrome/chrome_tests.gypi (right):
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi#newcod...
> chrome/chrome_tests.gypi:2873: 'target_name': 'frame_rate_tests',
> Adding another UIPerfTest exe to our build will have a noticeable effect
> on build time.  (The cost is highest in terms of linking against the
> UIPerfTest code and everything it depends on.)  Underscores the
> necessity for pyauto-based perf tests.
>

pyauto is one approach.  Another approach is to reduce the amount of code
we need to link in.  I suspect we are linking in way WAY more than we need.



>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> chrome/test/perf/frame_rate/frame_rate_tests.cc:56: tab, L"",
> L"window.domAutomationController.send(!__running);",
> Our perf tests have been flaky in domAutomationController tests.  I
> haven't tracked down yet why.  Keep an eye out for that.  There's a bug
> on file for that, it's led to marking some perf tests as FLAKY_ (yours
> might get that treatment, too).
>

Thanks for the heads up.


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> chrome/test/perf/frame_rate/frame_rate_tests.cc:75: "frames-per-second",
> true);
> I would not bother with setting important to true here.  The waterfall
> is overloaded with numeric data that I suspect no one watches (if
> someone watches it at the start they eventually don't watch it).  The
> signal should come from the color of the step (green, orange, red) which
> comes from perf expectations, and from the perf dashboard itself.
>

OK, I wasn't sure what important meant.  Should we delete that attribute?


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> chrome/test/perf/frame_rate/frame_rate_tests.cc:104: // the
> webkitRequestAnimationFrame API.
> We also need to update the reference build to pick up a copy of pyauto.
> We'd like to use builds that come off of the build systems this time
> (and from now on).  Issues:
> 1. Verify those builds have everything we need enabled.
> 2. No recent regressions.
> 3. Official Mac tests still use a different branding name to reach the
> executable (solved previously with a wholly separate reference build for
> official mac).
> 4. Perf expectations will need updating after the reference builds land.
>
> Obviously doesn't block landing this, just blocks uncommenting the
> reference tests.


Thanks.  Yeah, the test isn't useful (and can't be on the main
waterfall) until it has a reference.

-Darin



>
>
> http://codereview.chromium.org/7038052/
>

[darin (slow to review), 2011-05-21 04:41:48]

On Fri, May 20, 2011 at 9:26 PM, Darin Fisher <darin@chromium.org> wrote:

>
>
> On Fri, May 20, 2011 at 7:22 PM, <cmp@chromium.org> wrote:
>
>> nice, some general comments
>>
>>
>> http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi
>> File chrome/chrome_tests.gypi (right):
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi#newcod...
>> chrome/chrome_tests.gypi:2873: 'target_name': 'frame_rate_tests',
>> Adding another UIPerfTest exe to our build will have a noticeable effect
>> on build time.  (The cost is highest in terms of linking against the
>> UIPerfTest code and everything it depends on.)  Underscores the
>> necessity for pyauto-based perf tests.
>>
>
> pyauto is one approach.  Another approach is to reduce the amount of code
> we need to link in.  I suspect we are linking in way WAY more than we need.
>

Yes, looking at this, it is crazy how much the test_support_common target
brings into the binary.  A binary that just depends on chrome automation IPC
should have a very small footprint (base + net + automation).  It seems like
there is some low hanging fruit here.



>
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:56: tab, L"",
>> L"window.domAutomationController.send(!__running);",
>> Our perf tests have been flaky in domAutomationController tests.  I
>> haven't tracked down yet why.  Keep an eye out for that.  There's a bug
>> on file for that, it's led to marking some perf tests as FLAKY_ (yours
>> might get that treatment, too).
>>
>
> Thanks for the heads up.
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:75: "frames-per-second",
>> true);
>> I would not bother with setting important to true here.  The waterfall
>> is overloaded with numeric data that I suspect no one watches (if
>> someone watches it at the start they eventually don't watch it).  The
>> signal should come from the color of the step (green, orange, red) which
>> comes from perf expectations, and from the perf dashboard itself.
>>
>
> OK, I wasn't sure what important meant.  Should we delete that attribute?
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:104: // the
>> webkitRequestAnimationFrame API.
>> We also need to update the reference build to pick up a copy of pyauto.
>> We'd like to use builds that come off of the build systems this time
>> (and from now on).  Issues:
>> 1. Verify those builds have everything we need enabled.
>> 2. No recent regressions.
>> 3. Official Mac tests still use a different branding name to reach the
>> executable (solved previously with a wholly separate reference build for
>> official mac).
>> 4. Perf expectations will need updating after the reference builds land.
>>
>> Obviously doesn't block landing this, just blocks uncommenting the
>> reference tests.
>
>
> Thanks.  Yeah, the test isn't useful (and can't be on the main
> waterfall) until it has a reference.
>
> -Darin
>
>
>
>>
>>
>> http://codereview.chromium.org/7038052/
>>
>
>

[darin (slow to review), 2011-05-21 05:20:54]

On Fri, May 20, 2011 at 9:26 PM, Darin Fisher <darin@chromium.org> wrote:

>
>
> On Fri, May 20, 2011 at 7:22 PM, <cmp@chromium.org> wrote:
>
>> nice, some general comments
>>
>>
>> http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi
>> File chrome/chrome_tests.gypi (right):
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/chrome_tests.gypi#newcod...
>> chrome/chrome_tests.gypi:2873: 'target_name': 'frame_rate_tests',
>> Adding another UIPerfTest exe to our build will have a noticeable effect
>> on build time.  (The cost is highest in terms of linking against the
>> UIPerfTest code and everything it depends on.)  Underscores the
>> necessity for pyauto-based perf tests.
>>
>
> pyauto is one approach.  Another approach is to reduce the amount of code
> we need to link in.  I suspect we are linking in way WAY more than we need.
>

We can also easily consolidate all of the UIPerfTest subclasses into the
same
executable.

-Darin



>
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:56: tab, L"",
>> L"window.domAutomationController.send(!__running);",
>> Our perf tests have been flaky in domAutomationController tests.  I
>> haven't tracked down yet why.  Keep an eye out for that.  There's a bug
>> on file for that, it's led to marking some perf tests as FLAKY_ (yours
>> might get that treatment, too).
>>
>
> Thanks for the heads up.
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:75: "frames-per-second",
>> true);
>> I would not bother with setting important to true here.  The waterfall
>> is overloaded with numeric data that I suspect no one watches (if
>> someone watches it at the start they eventually don't watch it).  The
>> signal should come from the color of the step (green, orange, red) which
>> comes from perf expectations, and from the perf dashboard itself.
>>
>
> OK, I wasn't sure what important meant.  Should we delete that attribute?
>
>
>>
>>
>>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
>> chrome/test/perf/frame_rate/frame_rate_tests.cc:104: // the
>> webkitRequestAnimationFrame API.
>> We also need to update the reference build to pick up a copy of pyauto.
>> We'd like to use builds that come off of the build systems this time
>> (and from now on).  Issues:
>> 1. Verify those builds have everything we need enabled.
>> 2. No recent regressions.
>> 3. Official Mac tests still use a different branding name to reach the
>> executable (solved previously with a wholly separate reference build for
>> official mac).
>> 4. Perf expectations will need updating after the reference builds land.
>>
>> Obviously doesn't block landing this, just blocks uncommenting the
>> reference tests.
>
>
> Thanks.  Yeah, the test isn't useful (and can't be on the main
> waterfall) until it has a reference.
>
> -Darin
>
>
>
>>
>>
>> http://codereview.chromium.org/7038052/
>>
>
>

[nduca, 2011-05-22 04:33:41]

Psyched to see this moving along. Thanks Darin.
Needless to say, LGTM.

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
File chrome/test/data/perf/frame_rate/head.js (right):

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
chrome/test/data/perf/frame_rate/head.js:8: 
Eg, on Nytimes (www/~nduca/test/perf/nytimes.html) i have this set up to
scrollBy 10. How do we customize this per page?

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
chrome/test/data/perf/frame_rate/head.js:53: document.title = "FPS: " + (fps |
0);
avoid updating the title at all? Its not strictly needed, right?

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
chrome/test/data/perf/frame_rate/head.js:56: __t_est_squared_total += t_delta *
t_delta;
Would be curious what the t_est_squared is on nytimes on chrome canary vs chrome
12. Not part of the review, but the answer might tell us how effective sigma is
here.

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
chrome/test/data/perf/frame_rate/head.js:63:
webkitRequestAnimationFrame(function() {
I think RAF passes in the frame begin time to the callback. That should give us
better results in the case our RAF runs after someone else's.

http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
chrome/test/data/perf/frame_rate/head.js:72: function __start() {
Maybe pass in scroll-by and some "am I done?" so that we can modify it per page?
No big deal.

[cmp, 2011-05-23 23:56:50]

On 2011/05/21 04:26:53, darin wrote:
> pyauto is one approach.  Another approach is to reduce the amount of code
> we need to link in.  I suspect we are linking in way WAY more than we need.

Agree, thanks for looking into it.

> OK, I wasn't sure what important meant.  Should we delete that attribute?

No rush to delete it, I suggest setting this value to false for your test,
though.

[darin (slow to review), 2011-05-24 05:32:08]

On Sat, May 21, 2011 at 9:33 PM, <nduca@chromium.org> wrote:

> Psyched to see this moving along. Thanks Darin.
> Needless to say, LGTM.
>
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> File chrome/test/data/perf/frame_rate/head.js (right):
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> chrome/test/data/perf/frame_rate/head.js:8:
> Eg, on Nytimes (www/~nduca/test/perf/nytimes.html) i have this set up to
> scrollBy 10. How do we customize this per page?
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> chrome/test/data/perf/frame_rate/head.js:53: document.title = "FPS: " +
> (fps | 0);
> avoid updating the title at all? Its not strictly needed, right?
>

i find it useful for interactive use of the tests, but yeah... not strictly
needed.


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> chrome/test/data/perf/frame_rate/head.js:56: __t_est_squared_total +=
> t_delta * t_delta;
> Would be curious what the t_est_squared is on nytimes on chrome canary
> vs chrome 12. Not part of the review, but the answer might tell us how
> effective sigma is here.
>

good idea


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> chrome/test/data/perf/frame_rate/head.js:63:
> webkitRequestAnimationFrame(function() {
> I think RAF passes in the frame begin time to the callback. That should
> give us better results in the case our RAF runs after someone else's.
>

cool


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/data/perf/frame_rat...
> chrome/test/data/perf/frame_rate/head.js:72: function __start() {
> Maybe pass in scroll-by and some "am I done?" so that we can modify it
> per page? No big deal.


I was thinking that each page could have a custom <script> block to override
defaults like this,
so configuring the scrollBy amount would be done in test.html instead of
frame_rate_tests.cc.

-darin



>
>
> http://codereview.chromium.org/7038052/
>

[nduca, 2011-05-24 05:59:20]

> I was thinking that each page could have a custom <script> block to override
> defaults like this,
> so configuring the scrollBy amount would be done in test.html instead of
> frame_rate_tests.cc.
Makes total sense! Needless to say, sheep!

[brettw, 2011-05-24 16:39:40]

LGTM, I didn't look at the JS.

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
chrome/test/perf/frame_rate/frame_rate_tests.cc:28: // Make sure the test data
is checked out
Ah HA! I found a comment missing a period.

http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
chrome/test/perf/frame_rate/frame_rate_tests.cc:48:
AUTOMATION_MSG_NAVIGATION_SUCCESS,
Why not put this on the previous line and then have the next arg aligned to it?

[darin (slow to review), 2011-05-24 18:28:50]

On Tue, May 24, 2011 at 9:39 AM, <brettw@chromium.org> wrote:

> LGTM, I didn't look at the JS.
>
>
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> File chrome/test/perf/frame_rate/frame_rate_tests.cc (right):
>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> chrome/test/perf/frame_rate/frame_rate_tests.cc:28: // Make sure the
> test data is checked out
> Ah HA! I found a comment missing a period.
>

copy/paste :-)


>
>
>
http://codereview.chromium.org/7038052/diff/1/chrome/test/perf/frame_rate/fra...
> chrome/test/perf/frame_rate/frame_rate_tests.cc:48:
> AUTOMATION_MSG_NAVIGATION_SUCCESS,
> Why not put this on the previous line and then have the next arg aligned
> to it?
>
>
done.


>
> http://codereview.chromium.org/7038052/
>

[darin (slow to review), 2011-05-24 19:18:53]

The latest version includes a couple of changes:

1- Executable renamed to performance_ui_tests.exe, with the intention of merging
in all other such UI tests.

2- Support for mozRequestAnimationFrame, so that FF4 can also be tested.