Adding new sync performance tests to chromium perf dashboard

Adding new sync performance tests to chromium perf dashboard

BUG=90441
TEST=sync_performance_tests
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=94323

[braffert, 2011-07-26 00:24:14]

The sync performance tests are a new addition, and have recently been moved out
of the sync integration tests and into a new, separate target.

I have attempted to make the changes needed to add the sync performance tests by
observing the existing code.  However this code is unfamiliar and scary to me,
so there will most likely be things wrong with or missing from my first shot
here.

I have another CL up for review that makes the sync performance tests print out
their measurements in a format that should be consistent with the perf
dashboard's expectations (http://codereview.chromium.org/7464046/).

[cmp, 2011-07-26 00:52:39]

Impressed with your nearly complete change.  The only thing that needs to be
changed is the test executable target.  I presume the sync perf tests launch a
copy of Chrome and then control that program using the automation proxy?  We
want all of the performance tests to be called from performance_ui_tests now on
instead of from their own executables, and especially so for tests that make
straightforward use of the automation proxy.

http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
File build/scripts/master/factory/chromium_commands.py (right):

http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
build/scripts/master/factory/chromium_commands.py:328: cmd =
self.GetTestCommand('sync_performance_tests', options)
The sync performance tests should be included in the performance_ui_tests
target, which will need to be in a separate CL in src/.  Then, this CL should be
updated so this command calls that executable.

http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
build/scripts/master/factory/chromium_commands.py:329: self.AddTestStep(c,
'sync_test', cmd,
sync_test -> sync

[anna, 2011-07-26 18:39:12]

On 2011/07/26 00:52:39, cmp wrote:
> Impressed with your nearly complete change.  The only thing that needs to be
> changed is the test executable target.  I presume the sync perf tests launch a
> copy of Chrome and then control that program using the automation proxy?  

Actually this is not the case for the sync integration tests (these are not to
be confused with the sync UI tests that reside with the rest of the Chrome UI
automation). 

Furthermore as the sync engine is comprised from the client and the sync server,
we have plans on augmenting the sync perf suite with server specific tests. With
that in mind, it is more meaningful to have them in their own target.

> want all of the performance tests to be called from performance_ui_tests now
on
> instead of from their own executables, and especially so for tests that make
> straightforward use of the automation proxy.
> 
>
http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
> File build/scripts/master/factory/chromium_commands.py (right):
> 
>
http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
> build/scripts/master/factory/chromium_commands.py:328: cmd =
> self.GetTestCommand('sync_performance_tests', options)
> The sync performance tests should be included in the performance_ui_tests
> target, which will need to be in a separate CL in src/.  Then, this CL should
be
> updated so this command calls that executable.
> 
>
http://codereview.chromium.org/7465024/diff/8/build/scripts/master/factory/ch...
> build/scripts/master/factory/chromium_commands.py:329: self.AddTestStep(c,
> 'sync_test', cmd,
> sync_test -> sync

[braffert, 2011-07-26 18:47:04]

I'd also like to add that the sync performance tests extend the existing sync
integration test framework.  They do not involve the UIPerfTest class at all. 
Moving the sync perf tests into performance_ui_tests would not be trivial - it
would involve quite a bit more than simply adding the test case source files.

[cmp, 2011-07-26 18:52:24]

To annapop and braffert:

What sort of performance tests are being run?  Can you point me at a live
test?  The chromium.perf waterfall itself has a specific goal we keep in mind
when extending it.  Since we're not just talking about your new tests, but also
discussing the chromium.perf waterfall, I need to ensure that the tests you
plan to add to it fit within that goal.  It may be there is a better location
for your tests, or that your tests can be modified to avoid introducing sources
of noise the tests we have today already exclude.

The significant downside to adding a new target is we experience an increase in
our compile time from the additional link.  We have an on-going effort to reduce
the number of targets we link so we can decrease our cycle time and increase our
number of test runs.  The intended effect is to reduce the number of CLs in each
test run.  Including Chrome-specific performance tests in the
performance_ui_tests
target is the most recent development to address this issue.  We can discuss
other
approaches, too.

On 2011/07/26 18:47:04, braffert wrote:
> I'd also like to add that the sync performance tests extend the existing sync
> integration test framework.  They do not involve the UIPerfTest class at all. 
> Moving the sync perf tests into performance_ui_tests would not be trivial - it
> would involve quite a bit more than simply adding the test case source files.

Thanks.  What's the rietveld CL URL where these tests were added?

[braffert, 2011-07-26 19:29:07]

The current sync performance tests measure the time taken to sync a large batch
of changes from one client to another using the test sync server.  For each
datatype, we measure the time taken to add, update, and delete a set of entries.
 We hope to eventually expand to include other types of performance scenarios as
well.

The sync perf tests can run with the sync_performance_tests target, and the
source is in chrome/test/live_sync/performance.  They extend the LiveSyncTest
class, found in the parent directory.  They were originally added piece by piece
as part of the sync integration tests, and were just recently moved into their
own separate target (http://codereview.chromium.org/7484007/).



On 2011/07/26 18:52:24, cmp wrote:
> To annapop and braffert:
> 
> What sort of performance tests are being run?  Can you point me at a live
> test?  The chromium.perf waterfall itself has a specific goal we keep in mind
> when extending it.  Since we're not just talking about your new tests, but
also
> discussing the chromium.perf waterfall, I need to ensure that the tests you
> plan to add to it fit within that goal.  It may be there is a better location
> for your tests, or that your tests can be modified to avoid introducing
sources
> of noise the tests we have today already exclude.
> 
> The significant downside to adding a new target is we experience an increase
in
> our compile time from the additional link.  We have an on-going effort to
reduce
> the number of targets we link so we can decrease our cycle time and increase
our
> number of test runs.  The intended effect is to reduce the number of CLs in
each
> test run.  Including Chrome-specific performance tests in the
> performance_ui_tests
> target is the most recent development to address this issue.  We can discuss
> other
> approaches, too.
> 
> On 2011/07/26 18:47:04, braffert wrote:
> > I'd also like to add that the sync performance tests extend the existing
sync
> > integration test framework.  They do not involve the UIPerfTest class at
all. 
> > Moving the sync perf tests into performance_ui_tests would not be trivial -
it
> > would involve quite a bit more than simply adding the test case source
files.
> 
> Thanks.  What's the rietveld CL URL where these tests were added?

[cmp, 2011-07-26 22:11:53]

lgtm

Discussed issues offline, next step is to land, verify the masters start, then
update and restart chromium.perf.  Ben, after you land this, you would usually
send an email to chrome-infrastructure-team requesting a chromium.perf master
restart.  Since we're planning to restart the masters tonight, you may not need
to send that email, however.