Test end-to-end startup performance of loading several tabs.

Test end-to-end startup performance of loading several tabs.

This test is similar to other startup tests, with the exception that it loads multible tabs at once, and it tests both with and without the session restore command line flag. In both cases, WaitForInitialLoads waits for all the tabs to finish loading before returning.

BUG=44129
TEST=none

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=49827

[Patrick Horn, 2010-06-08 02:16:47]

This is a much simplified end-to-end version of the test in cl 2559001.
Currently I only test the time for 20 tabs to load. Would versions of this for 1
(with the restore flag) and 5 tabs be useful too?

I wrote it in a similar style to the other startup_tests. While it depends on
IPC latency and time to execute base::LaunchApp, the tests get run repeatedly.

The test is intended to be a test that does not alter chromium code, and thus
can be used with old Chrome releases, hopefully dating back from the earliest
version with the --restore-last-session command line argument.

[Paweł Hajdan Jr., 2010-06-08 06:46:02]

http://codereview.chromium.org/2753001/diff/1/2
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/1/2#newcode150
chrome/test/startup/startup_test.cc:150: int tab_count = 20;
nit: Shouldn't this be a constant (kTabCount maybe)?

[huanr, 2010-06-08 18:26:27]

I think this end to end test is useful. It does not change IPC message so we can
use it to benchmark older builds.

http://codereview.chromium.org/2753001/diff/1/2
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/1/2#newcode150
chrome/test/startup/startup_test.cc:150: int tab_count = 20;
We may want to test for both 5 tabs and 20 tabs.

http://codereview.chromium.org/2753001/diff/1/2#newcode153
chrome/test/startup/startup_test.cc:153: SetUpWithFileURL();
It loads only "simple.html" which does not have any content in it. We can use
the pages in page cycler test. see GetDataPath in page_cycler_test.cc

http://codereview.chromium.org/2753001/diff/1/2#newcode166
chrome/test/startup/startup_test.cc:166: // The session will be restored once
per test, and each time,
per cycle for numCycles test cycles,

http://codereview.chromium.org/2753001/diff/1/2#newcode169
chrome/test/startup/startup_test.cc:169: false /* cold */, false /* not
important */,
I think you meant /* warm */ here?

On a separate note, cold is more interesting case but requires extra care to
clean up cache. We can do that in later check in.

[Peter Kasting, 2010-06-08 18:29:35]

http://codereview.chromium.org/2753001/diff/1/2
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/1/2#newcode152
chrome/test/startup/startup_test.cc:152: for (int i = 0; i < tab_count; ++i) {
Nit: No {}

http://codereview.chromium.org/2753001/diff/1/2#newcode169
chrome/test/startup/startup_test.cc:169: false /* cold */, false /* not
important */,
On 2010/06/08 18:26:27, huanr wrote:
> I think you meant /* warm */ here?

Indeed; this points out that a bunch of the comments below are wrong.

Personally I think it would be better to simply eliminate all these /* */
comments, as it's not hard to look at the function declaration when you need to
know, and as we see here, these sorts of comments can become out-of-date and/or
wrong easily.

[Patrick Horn, 2010-06-09 01:49:58]

Another small edit.

I decided to make a test for session restore, and for command line arguments: I
noticed a huge difference between them. Session restore is about twice as slow,
and it seems to load each tab completely serially.

I checked, and command line arguments also creates an InitialLoadObserver, with
the number of tabs to wait for equal to the number of command line arguments,
which is exactly what we want. So I would think that the two tests should run at
approximately the same speed, and if anything, session restore should be faster
since it can use the cache?

http://codereview.chromium.org/2753001/diff/1/2
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/1/2#newcode150
chrome/test/startup/startup_test.cc:150: int tab_count = 20;
On 2010/06/08 18:26:27, huanr wrote:
> We may want to test for both 5 tabs and 20 tabs.

Done.

http://codereview.chromium.org/2753001/diff/1/2#newcode152
chrome/test/startup/startup_test.cc:152: for (int i = 0; i < tab_count; ++i) {
On 2010/06/08 18:29:35, Peter Kasting wrote:
> Nit: No {}

Done.

http://codereview.chromium.org/2753001/diff/1/2#newcode153
chrome/test/startup/startup_test.cc:153: SetUpWithFileURL();
On 2010/06/08 18:26:27, huanr wrote:
> It loads only "simple.html" which does not have any content in it. We can use
> the pages in page cycler test. see GetDataPath in page_cycler_test.cc

I looked at page_cycler_test but I'm not sure that's appropriate for this test:
they have javascript code to redirect to another page. Also, we would have to
pick and choose a good benchmark page, since they will all be subtly different.

Are there any simple performance tests for a moderately complex page but nothing
else? Should we write our own sample html file to run this test with?

http://codereview.chromium.org/2753001/diff/1/2#newcode166
chrome/test/startup/startup_test.cc:166: // The session will be restored once
per test, and each time,
On 2010/06/08 18:26:27, huanr wrote:
> per cycle for numCycles test cycles,

Done.

[Patrick Horn, 2010-06-09 21:17:45]

Huan,
I added a function to reference page_cycler URLs using the ?skip argument so
they don't refresh. This avoids having to check in additional code into the data
repository.

The tests run approximately 2-3 times slower than using simple.html, so all the
startup tests take 170 seconds on my computer without any code to limit to two
cores.

[huanr, 2010-06-09 22:06:57]

http://codereview.chromium.org/2753001/diff/17001/18001
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/17001/18001#newcode46
chrome/test/startup/startup_test.cc:46: const char * const
kTestPageCyclerDomains[] = {
char*

http://codereview.chromium.org/2753001/diff/17001/18001#newcode48
chrome/test/startup/startup_test.cc:48: "www.yahoo.com", "espn.go.com",
"www.amazon.com"
Great. These are better than simple.html.

http://codereview.chromium.org/2753001/diff/17001/18001#newcode51
chrome/test/startup/startup_test.cc:51: const char *this_domain =
kTestPageCyclerDomains[which_el];
use char* to follow the style in this file.

http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...

http://codereview.chromium.org/2753001/diff/17001/18001#newcode192
chrome/test/startup/startup_test.cc:192: IntToWString(tab_count));
Do you still need those URLs on the command line if kRestoreLastSession is
provided?

[Patrick Horn, 2010-06-09 22:28:10]

http://codereview.chromium.org/2753001/diff/17001/18001
File chrome/test/startup/startup_test.cc (right):

http://codereview.chromium.org/2753001/diff/17001/18001#newcode46
chrome/test/startup/startup_test.cc:46: const char * const
kTestPageCyclerDomains[] = {
On 2010/06/09 22:06:58, huanr wrote:
> char*

Done.

http://codereview.chromium.org/2753001/diff/17001/18001#newcode51
chrome/test/startup/startup_test.cc:51: const char *this_domain =
kTestPageCyclerDomains[which_el];
On 2010/06/09 22:06:58, huanr wrote:
> use char* to follow the style in this file.
> 
>
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Pointer_and_Re...

Done.

http://codereview.chromium.org/2753001/diff/17001/18001#newcode192
chrome/test/startup/startup_test.cc:192: IntToWString(tab_count));
On 2010/06/09 22:06:58, huanr wrote:
> Do you still need those URLs on the command line if kRestoreLastSession is
> provided?

I tried running it without clearing command line options, and the command line
arguments would be added in addition to the restored tabs, so each test run, the
number of tabs would increase.

[huanr, 2010-06-09 22:36:43]

LGTM