Split out Dromaeo DOM and JSLib tests into smaller components. Will remove large tests after buildb…

Split out Dromaeo DOM and JSLib tests into smaller components. Will remove large tests after buildbots are updated to stop running them.


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

[sullivan, 2012-08-27 21:06:42]

Hi Chase,

What I want to do is replace the 2 large dromaeo tests with the 14 smaller ones.
I want to do this without breaking the build or the perf_expectations. I think
adding the right steps are:

1) Code to add the 14 smaller tests (this CL)
2) Code to stop running the larger tests and start running the smaller tests
(https://chromiumcodereview.appspot.com/10875078/)
3) Remove larger tests from perf_expectations.json
4) Update buildbots with changes to start running the smaller tests.
5) Once smaller tests are running, update perf_expectations.json to add them in.
6) Remove code for larger tests.

Let me know if that's right.

Thanks,
Annie

[cmp, 2012-08-28 16:58:32]

lgtm with a question

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
File chrome/test/perf/dromaeo_benchmark_uitest.cc (right):

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
chrome/test/perf/dromaeo_benchmark_uitest.cc:175: RunTest("dom-traverse");
i noticed you were able to split jslib further than just 4 tests.  is it worth
trying to split domcore further?

[cmp, 2012-08-28 16:58:42]

also, will each test produce a total score of its own?

[sullivan, 2012-08-28 17:06:05]

On 2012/08/28 16:58:32, cmp wrote:
> lgtm with a question
> 
>
https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
> File chrome/test/perf/dromaeo_benchmark_uitest.cc (right):
> 
>
https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
> chrome/test/perf/dromaeo_benchmark_uitest.cc:175: RunTest("dom-traverse");
> i noticed you were able to split jslib further than just 4 tests.  is it worth
> trying to split domcore further?

Not without modifying the test--by default, it splits into 4 DOM categories and
10 JSLib categories. Each is supposed to take 30-45 seconds to run, so it
*should* be split evenly. If we're not seeing those times on the perf bots,
hopefully this change will help find a bug in the test.

I'd like to keep it split out the way the test author intended in the first
pass.

About the test output, they all produce buildbot output like the following. I
believe the "*RESULT" line is going to give each test a total score, but you
understand buildbot a lot better than I do so please let me know if I'm
mistaken.

*RESULT jslib: score_ref= 440.7976945446777 runs/s
RESULT jslib_modify_prototype: score_ref= 440.7976945446777 runs/s
RESULT jslib_modify_prototype_Prototype___after: score_ref= 427.48531468531473
runs/s
RESULT jslib_modify_prototype_Prototype___append: score_ref= 431.44116272077434
runs/s
RESULT jslib_modify_prototype_Prototype___before: score_ref= 432.0227772227772
runs/s
RESULT jslib_modify_prototype_Prototype___prepend: score_ref= 429.47790732221875
runs/s
RESULT jslib_modify_prototype_Prototype___update__: score_ref= 486.3022977022977
runs/s

[commit-bot: I haz the power, 2012-08-28 17:06:33]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sullivan@chromium.org/10868114/1

[commit-bot: I haz the power, 2012-08-28 17:06:36]

Presubmit check for 10868114-1 failed and returned exit status 1.


Running presubmit commit checks ...

** Presubmit ERRORS **
Missing LGTM from an OWNER for files in these directories:
    chrome/test

[sullivan, 2012-08-28 17:13:51]

+sky

Scott, looks like I need OWNERS review for this.

[cmp, 2012-08-28 17:20:23]

> *RESULT jslib: score_ref= 440.7976945446777 runs/s
> RESULT jslib_modify_prototype: score_ref= 440.7976945446777 runs/s
> RESULT jslib_modify_prototype_Prototype___after: score_ref= 427.48531468531473
> runs/s
> RESULT jslib_modify_prototype_Prototype___append: score_ref=
431.44116272077434
> runs/s
> RESULT jslib_modify_prototype_Prototype___before: score_ref= 432.0227772227772
> runs/s
> RESULT jslib_modify_prototype_Prototype___prepend: score_ref=
429.47790732221875
> runs/s
> RESULT jslib_modify_prototype_Prototype___update__: score_ref=
486.3022977022977
> runs/s

Let's say the test run is of "JSLibModifyPrototype".  And let's assume this is
on XP Perf (1) whose system name is xp-release-dual-core.  Let's assume that the
step name we configure this to run under is called
dromaeo_jslib_modify_prototype.  This would map to:
  xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib/score_ref
 
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__after/score_ref
 
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__append/score_ref
 
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__before/score_ref
 
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__prepend/score_ref
 
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__update__/score_ref

Are these what you expected?  The system name will be right (that's set
elsewhere).  The step name may need to be adjusted, that will be based on what's
in your scripts/master/factory/ change (separate).  As for the graph part
(that's jslib, jslib_modify_prototype_Prototype__after, etc), that's probably
fine since each of these split up dromaeo tests will also be split up under the
step name (dromaeo_jslib_modify_prototype in this case).  The score_ref part is
fine (that's the trace), as long as there's also a score that gets produced,
too. :)

[sullivan, 2012-08-28 17:35:14]

On 2012/08/28 17:20:23, cmp wrote:
> > *RESULT jslib: score_ref= 440.7976945446777 runs/s
> > RESULT jslib_modify_prototype: score_ref= 440.7976945446777 runs/s
> > RESULT jslib_modify_prototype_Prototype___after: score_ref=
427.48531468531473
> > runs/s
> > RESULT jslib_modify_prototype_Prototype___append: score_ref=
> 431.44116272077434
> > runs/s
> > RESULT jslib_modify_prototype_Prototype___before: score_ref=
432.0227772227772
> > runs/s
> > RESULT jslib_modify_prototype_Prototype___prepend: score_ref=
> 429.47790732221875
> > runs/s
> > RESULT jslib_modify_prototype_Prototype___update__: score_ref=
> 486.3022977022977
> > runs/s
> 
> Let's say the test run is of "JSLibModifyPrototype".  And let's assume this is
> on XP Perf (1) whose system name is xp-release-dual-core.  Let's assume that
the
> step name we configure this to run under is called
> dromaeo_jslib_modify_prototype.  This would map to:
>   xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib/score_ref
>  
>
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__after/score_ref
>  
>
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__append/score_ref
>  
>
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__before/score_ref
>  
>
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__prepend/score_ref
>  
>
xp-release-dual-core/dromaeo_jslib_modify_prototype/jslib_modify_prototype_Prototype__update__/score_ref
> 
> Are these what you expected?  The system name will be right (that's set
> elsewhere).  The step name may need to be adjusted, that will be based on
what's
> in your scripts/master/factory/ change (separate).  As for the graph part
> (that's jslib, jslib_modify_prototype_Prototype__after, etc), that's probably
> fine since each of these split up dromaeo tests will also be split up under
the
> step name (dromaeo_jslib_modify_prototype in this case).  The score_ref part
is
> fine (that's the trace), as long as there's also a score that gets produced,
> too. :)

Yeah, this sounds right to me (and yep, I checked and there is a score and
score_ref for each test).

[sky, 2012-08-28 20:15:13]

I'm not familiar with this code. Maybe Darin?

[darin (slow to review), 2012-08-28 21:04:26]

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
File chrome/test/perf/dromaeo_benchmark_uitest.cc (right):

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
chrome/test/perf/dromaeo_benchmark_uitest.cc:347: if
(!CommandLine::ForCurrentProcess()->HasSwitch(kRunDromaeo))
perhaps you could fold this command line check into the RunTest function to
avoid having to replicate it?

[sullivan, 2012-09-04 15:43:41]

Fixed the flag copy/pasting.

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
File chrome/test/perf/dromaeo_benchmark_uitest.cc (right):

https://chromiumcodereview.appspot.com/10868114/diff/1/chrome/test/perf/droma...
chrome/test/perf/dromaeo_benchmark_uitest.cc:347: if
(!CommandLine::ForCurrentProcess()->HasSwitch(kRunDromaeo))
On 2012/08/28 21:04:26, darin wrote:
> perhaps you could fold this command line check into the RunTest function to
> avoid having to replicate it?

Done.

[darin (slow to review), 2012-09-04 20:08:58]

LGTM

[commit-bot: I haz the power, 2012-09-04 20:11:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/sullivan@chromium.org/10868114/12001

[commit-bot: I haz the power, 2012-09-04 23:46:34]

Change committed as 154854