Add full frame measurement mode for some blink_perf tests

Add full frame measurement mode for some blink_perf tests

This is like virtual test suites for layout tests: some tests can also run
with special settings to cover special cases.

Added _BlinkPerfFullFrameMeasurement which forces browser_type to use
content_shell with --expose-internals-for-testing.

The original blink_perf.svg and blink_perf.layout suites are now also run
as blink_perf.svg_full_frame and blink_perf.layout_full_frame with the
new measurement class.

This doesn't affect existing tests and their results.

BUG=426599
Committed: https://crrev.com/0990ab3a30d3edb742d341bb79500866bd4231ff
Cr-Commit-Position: refs/heads/master@{#305550}

[Xianzhu, 2014-10-24 21:39:05]

wangxianzhu@chromium.org changed reviewers:
+ ernstm@chromium.org, tonyg@chromium.org

[tonyg, 2014-10-27 19:11:57]

https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
File tools/perf/benchmarks/blink_perf.py (right):

https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
tools/perf/benchmarks/blink_perf.py:74: def CustomizeFinderOptions(self,
options):
This doesn't seem right. We shouldn't just change the browser that the user is
running against.

If some tests require content shell, then they should fatally exit with an error
message explaining that when they're run against an unsupported browser and the
bots should be changed to pass the desired browser.

But maybe we should talk more about what we're relying on in
--expose-internals-for-testing? It'd be nice to be able to run these tests
against chrome too.

[Xianzhu, 2014-10-27 20:43:28]

https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
File tools/perf/benchmarks/blink_perf.py (right):

https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
tools/perf/benchmarks/blink_perf.py:74: def CustomizeFinderOptions(self,
options):
On 2014/10/27 19:11:57, tonyg wrote:
> This doesn't seem right. We shouldn't just change the browser that the user is
> running against.
> 
> If some tests require content shell, then they should fatally exit with an
error
> message explaining that when they're run against an unsupported browser and
the
> bots should be changed to pass the desired browser.
>
> But maybe we should talk more about what we're relying on in
> --expose-internals-for-testing? It'd be nice to be able to run these tests
> against chrome too.

We want to let the tests cover the cost of compositing update and paint
invalidation if possible. Now these operations can't be triggered with public
JavaScript API, but only with window.internals.forceCompositingUpdate() API.

Actually the tests can still run in chrome, but the performance results won't
reflect cost of compositing update and paint invalidation.

How about the following:
1. change code to display a warning when the browser_type is not as expected;
2. change build bot code to use content_shell for blink_perf tests.

[Xianzhu, 2014-10-28 15:55:50]

On 2014/10/27 20:43:28, Xianzhu wrote:
>
https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
> File tools/perf/benchmarks/blink_perf.py (right):
> 
>
https://codereview.chromium.org/681493002/diff/1/tools/perf/benchmarks/blink_...
> tools/perf/benchmarks/blink_perf.py:74: def CustomizeFinderOptions(self,
> options):
> On 2014/10/27 19:11:57, tonyg wrote:
> > This doesn't seem right. We shouldn't just change the browser that the user
is
> > running against.
> > 
> > If some tests require content shell, then they should fatally exit with an
> error
> > message explaining that when they're run against an unsupported browser and
> the
> > bots should be changed to pass the desired browser.
> >
> > But maybe we should talk more about what we're relying on in
> > --expose-internals-for-testing? It'd be nice to be able to run these tests
> > against chrome too.
> 
> We want to let the tests cover the cost of compositing update and paint
> invalidation if possible. Now these operations can't be triggered with public
> JavaScript API, but only with window.internals.forceCompositingUpdate() API.
> 
> Actually the tests can still run in chrome, but the performance results won't
> reflect cost of compositing update and paint invalidation.
> 
> How about the following:
> 1. change code to display a warning when the browser_type is not as expected;
> 2. change build bot code to use content_shell for blink_perf tests.

PTAL. Patch Set 2 just gives warning. If this looked good, could you hint me
where to modify switches for the bots?

About avoiding using internals API, thought of using requestAnimationFrame, but
it seems not applicable to the current tests because most of the tests just take
take milliseconds or sub-milliseconds to run one iteration. To make the tests
suitable for rAF, we need to increase the complexity of the tests which would be
a big amount of work.

Any idea will be welcomed.

[Xianzhu, 2014-10-28 18:12:26]

PTAL. The latest patch set adds full layout measurement mode for blink_perf.svg
and blink_perf.layout suites (like virtual test suites in layout tests), keeping
existing tests and results unaffected.

[Xianzhu, 2014-10-29 16:54:58]

https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
File tools/perf/benchmarks/blink_perf.py (right):

https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
tools/perf/benchmarks/blink_perf.py:113: options.browser_type =
'content-shell-debug'
Here browser_type is still forced, but it is for the new virtual test suite
configuration only.

Another choices are:

1. to issue warning here, and modify bot code to pass
--browser=content-shell-release for blink_perf.layout_full_layout and
blink_perf.svg_full_layout. The con is that the bot must know the names of
specific test suites that need --browser=content-shell-release.

2. based on 1, but add blink_perf_full_layout.py which adds
blink_perf_full_layout.layout and blink_perf_full_layout.svg suites, and let the
bot pass --browser=content-shell-release for blink_perf_layout_full.* suites.

wdyt?

[Xianzhu, 2014-10-30 15:52:46]

On 2014/10/29 16:54:58, Xianzhu wrote:
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> File tools/perf/benchmarks/blink_perf.py (right):
> 
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> tools/perf/benchmarks/blink_perf.py:113: options.browser_type =
> 'content-shell-debug'
> Here browser_type is still forced, but it is for the new virtual test suite
> configuration only.
> 
> Another choices are:
> 
> 1. to issue warning here, and modify bot code to pass
> --browser=content-shell-release for blink_perf.layout_full_layout and
> blink_perf.svg_full_layout. The con is that the bot must know the names of
> specific test suites that need --browser=content-shell-release.
> 
> 2. based on 1, but add blink_perf_full_layout.py which adds
> blink_perf_full_layout.layout and blink_perf_full_layout.svg suites, and let
the
> bot pass --browser=content-shell-release for blink_perf_layout_full.* suites.
> 
> wdyt?

ping...

[tonyg, 2014-10-30 16:33:45]

On 2014/10/30 15:52:46, Xianzhu wrote:
> On 2014/10/29 16:54:58, Xianzhu wrote:
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > File tools/perf/benchmarks/blink_perf.py (right):
> > 
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > tools/perf/benchmarks/blink_perf.py:113: options.browser_type =
> > 'content-shell-debug'
> > Here browser_type is still forced, but it is for the new virtual test suite
> > configuration only.
> > 
> > Another choices are:
> > 
> > 1. to issue warning here, and modify bot code to pass
> > --browser=content-shell-release for blink_perf.layout_full_layout and
> > blink_perf.svg_full_layout. The con is that the bot must know the names of
> > specific test suites that need --browser=content-shell-release.

I think this option is more along the right lines. The @Enabled/@Disabled
benchmark decorators support enabling/disabling for a particular browser type.
So you could just make this benchmark @Enabled('content-shell-release',
'content-shell-debug'). Then we could modify the bb code to tell it to just run
everything that's enabled for content-shell-release only in addition to release.
That code is here:
https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl...

> > 
> > 2. based on 1, but add blink_perf_full_layout.py which adds
> > blink_perf_full_layout.layout and blink_perf_full_layout.svg suites, and let
> the
> > bot pass --browser=content-shell-release for blink_perf_layout_full.*
suites.
> > 
> > wdyt?
> 
> ping...

[Xianzhu, 2014-10-30 17:08:17]

On 2014/10/30 16:33:45, tonyg wrote:
> On 2014/10/30 15:52:46, Xianzhu wrote:
> > On 2014/10/29 16:54:58, Xianzhu wrote:
> > >
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > > File tools/perf/benchmarks/blink_perf.py (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > > tools/perf/benchmarks/blink_perf.py:113: options.browser_type =
> > > 'content-shell-debug'
> > > Here browser_type is still forced, but it is for the new virtual test
suite
> > > configuration only.
> > > 
> > > Another choices are:
> > > 
> > > 1. to issue warning here, and modify bot code to pass
> > > --browser=content-shell-release for blink_perf.layout_full_layout and
> > > blink_perf.svg_full_layout. The con is that the bot must know the names of
> > > specific test suites that need --browser=content-shell-release.
> 
> I think this option is more along the right lines. The @Enabled/@Disabled
> benchmark decorators support enabling/disabling for a particular browser type.
> So you could just make this benchmark @Enabled('content-shell-release',
> 'content-shell-debug'). Then we could modify the bb code to tell it to just
run
> everything that's enabled for content-shell-release only in addition to
release.
> That code is here:
>
https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl...

Great! Will upload a patch using this method soon.

Some questions:
1. What are the browser types with _x64 postfix? Are they actually used?
2. Because there are also android-content-shell, content-shell-release_x64,
etc., would it look good to let the decorators support regex?
   e.g. @Enabled(re.compile('content-shell'))
   Existing string values still exactly match whole strings.

[tonyg, 2014-10-31 01:47:23]

On 2014/10/30 17:08:17, Xianzhu wrote:
> On 2014/10/30 16:33:45, tonyg wrote:
> > On 2014/10/30 15:52:46, Xianzhu wrote:
> > > On 2014/10/29 16:54:58, Xianzhu wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > > > File tools/perf/benchmarks/blink_perf.py (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/681493002/diff/60001/tools/perf/benchmarks/bl...
> > > > tools/perf/benchmarks/blink_perf.py:113: options.browser_type =
> > > > 'content-shell-debug'
> > > > Here browser_type is still forced, but it is for the new virtual test
> suite
> > > > configuration only.
> > > > 
> > > > Another choices are:
> > > > 
> > > > 1. to issue warning here, and modify bot code to pass
> > > > --browser=content-shell-release for blink_perf.layout_full_layout and
> > > > blink_perf.svg_full_layout. The con is that the bot must know the names
of
> > > > specific test suites that need --browser=content-shell-release.
> > 
> > I think this option is more along the right lines. The @Enabled/@Disabled
> > benchmark decorators support enabling/disabling for a particular browser
type.
> > So you could just make this benchmark @Enabled('content-shell-release',
> > 'content-shell-debug'). Then we could modify the bb code to tell it to just
> run
> > everything that's enabled for content-shell-release only in addition to
> release.
> > That code is here:
> >
>
https://code.google.com/p/chromium/codesearch#chromium/tools/build/scripts/sl...
> 
> Great! Will upload a patch using this method soon.
> 
> Some questions:
> 1. What are the browser types with _x64 postfix? Are they actually used?

Yes, for 64-bit builds. Currently only the windows bot uses it.

> 2. Because there are also android-content-shell, content-shell-release_x64,
> etc., would it look good to let the decorators support regex?
>    e.g. @Enabled(re.compile('content-shell'))
>    Existing string values still exactly match whole strings.

Good question. I'd recommend getting dtu's opinion on the decorator syntax.

[tonyg, 2014-10-31 01:47:33]

tonyg@chromium.org changed reviewers:
+ dtu@chromium.org

[Xianzhu, 2014-10-31 16:45:55]

> Good question. I'd recommend getting dtu's opinion on the decorator syntax.

dtu@, I'm reading telemetry/decorators.py to see the possibility to support
regex.

Saw the following in doc of Disabled():

    @Disabled('canary')        # Disabled for canary browsers

How is the above supposed to work?

[Xianzhu, 2014-10-31 17:51:53]

PTAL.

The new patch uses a method simpler than @Enabled(regex). In IsEnabled()
'content-shell' will be added into platform_attributes if browser_type string
contains 'content-shell'.

[Xianzhu, 2014-11-17 20:48:49]

On 2014/10/31 17:51:53, Xianzhu wrote:
> PTAL.
> 
> The new patch uses a method simpler than @Enabled(regex). In IsEnabled()
> 'content-shell' will be added into platform_attributes if browser_type string
> contains 'content-shell'.

PTAL. The latest patch set renamed 'full_layout' to 'full_frame', according to
the blink-side change https://codereview.chromium.org/675983004/.

[Xianzhu, 2014-11-18 17:17:59]

On 2014/11/17 20:48:49, Xianzhu wrote:
> On 2014/10/31 17:51:53, Xianzhu wrote:
> > PTAL.
> > 
> > The new patch uses a method simpler than @Enabled(regex). In IsEnabled()
> > 'content-shell' will be added into platform_attributes if browser_type
string
> > contains 'content-shell'.
> 
> PTAL. The latest patch set renamed 'full_layout' to 'full_frame', according to
> the blink-side change https://codereview.chromium.org/675983004/.

ping...

[dtu, 2014-11-24 22:39:19]

Seems like a reasonable way of doing this to me. lgtm

[Xianzhu, 2014-11-25 00:17:34]

The CQ bit was checked by wangxianzhu@chromium.org

[commit-bot: I haz the power, 2014-11-25 00:19:04]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/681493002/100001

[commit-bot: I haz the power, 2014-11-25 01:25:15]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2014-11-25 01:25:54]

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