452861 - unittest runs should be split up and parallelized

Status: RESOLVED FIXED

(Release Engineering :: General, defect, P3)

Description

[Justin Dolske [:Dolske]]

The unit test boxes take a long time to cycle -- looking at recent activity, it takes 1-2 hours (varies by box, platform, and phase of moon). We often have to wait up to twice as long as the slowest box to see if backout of a bad commit fixes the problem (or if a failure is random or not), and longer still if the cause isn't definitively known.

Looking at the breakdown for one recent run that took 1:50 to complete: 0:13 for "make check", 0:20 for reftests, 0:25 for mochitests, 0:30 for the compile, and 0:15 minutes for the initial setup (notably, hg pull). The remaining 0:07 was misc stuff (including the test for browser chrome, mochichrome, and crashtests).

It would help if this was parallelized. For example, 3 boxes -- 1 to run reftests, 1 to run mochitests, and 1 to run the rest. That turns 1 hour of serialized work into a 15-30 minute cycle (50% faster, more if the test of interest is in a fast test suite).

There's some diminishing returns here, in that the first 0:45 of this run was spent making a build to test. Might help to do something like Talos does -- have the unit test boxes just run tests, and have a separate box (w/ smokin' fast CPU+disk) crank out builds. Parallelizing the build would also help with getting finer regression ranges, even if the overall pipeline is longer.

Comment 1

[Nick Thomas [:nthomas] (UTC+12)]

Getting bug 445622 done would help the compile time part of this.

Comment 2

[Justin Dolske [:Dolske]]

Perhaps bug 383136 too, if that could allows running (some?) tests with normal packaged builds from some other box.

Comment 3

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

As well as running each suite concurrently, we should split up the longest test (mochitest) into smaller chunks. 

How small is small enough? I'd guess small enough to complete in the same length of time as all the other unittest suites being run concurrently. Ideally, all suites could start, and finally end, about the same time.

See separate bug#487689 for details.

Comment 4

[Chris AtLee [:catlee]]

IIRC the other test suites range from 1 to 15 minutes.

I'd aim for something like 10 minutes per chunk for mochitests.

Comment 6

[Dietrich Ayala (:dietrich)]

yes, still a pain-point (per bug 406906). in comment #0 dolske talks about the unit tests, but the perf tests are not interdependent either.

given the lack of interdependency, is there anything blocking doing something like this, besides than hardware requests and management?

Comment 7

[Dietrich Ayala (:dietrich)]

un-duped bug 406906, per catlee, because bug 452861 is about *unit tests* and bug 406906 is about perf tests.

Comment 8

[Chris AtLee [:catlee]]

This is mostly done.  We're currently running mochitests as one job, and all the other tests as a second job.

Bug 494165 is for splitting up individual suites (mochitest in particular) into smaller chunks so we can get better parallelization.  Once we get that working, we can update our code to run mochitest in smaller chunks.

Comment 10

[Dietrich Ayala (:dietrich)]

Chris, what's the status of this?

Today we've had the tree effectively closed for hours waiting for unit test runs to complete. This would've helped tremendously :)

Comment 11

[Aki Sasaki (not active)]

catlee is on paternity leave.

We've got stuff running on http://tinderbox.mozilla.org/showbuilds.cgi?tree=Firefox-Unittest ; I think this is still open to split it up even further (points at comment 8)

Comment 12

[Dietrich Ayala (:dietrich)]

Thanks for the update Aki!

What's the plan for getting this onto the mozilla-central tree? Is that going to be done a separate bug?

Comment 13

[Aki Sasaki (not active)]

I'm under the impression catlee split this into its own tinderbox tree to avoid widening the main tree.

If the convenience of one-stop-status shopping, and/or tbpl logic overrules that, it shouldn't be difficult to send these to the respective main tinderbox trees.  However, be forewarned that there are more columns to come (debug unit tests, multiplied by number of platforms, multiplied by number of "splits"; plus possibly release builds x number of platforms x number of splits,  Also, we're currently at 2 columns per platform per build type; that may change to 3 or more columns per.  At 3 columns, 3 platforms, and 3 build types that would add 27 columns to the Firefox tinderbox tree).

Until then we have the Firefox-Unittest, Firefox3.6-Unittest, and Firefox3.5-Unittest trees.

Comment 14

[John O'Duinn [:joduinn] (please use "needinfo?" flag)]

catlee split out mochitest today in production today; still working with developers investigating tests that are green when run in all-in-one mochitest, but are orange when run as split-out-mochitest suites.

Comment 15

[Chris AtLee [:catlee]]

So it looks like 'everythingelse' is the long pole here by a significant margin.  I suggest splitting it up thusly:

new suite mochitest-other runs all mochitest suites currently in 'everythingelse'
xpcshell, crashtest, jsreftest and reftest would each be run separately.

Average times for the various suites in mozilla-central for the past month:
	                    win32   win32-debug	osx	osx-debug   linux   linux-debug
mochitest 1	            351	    1180        434	1341	    508	    1177
mochitest 2	            358	    1399        386	1582	    434	    1452
mochitest 3	            300	    1650        306	1738	    392	    1729
mochitest 4	            443	    1388        717	2136	    870	    2270
mochitest 5	            216	    697	        312	1084	    314	    791
reftest	                    345	    1116        423	1017	    1073    1751
crashtest	            108	    856	        137	840	    175	    437
mochitest-chrome	    175	    411	        198	572	    224	    436
mochitest-browser-chrome    332	    545	        272	1078	    266	    654
mochitest-a11y	            106	    243		                    153	    363
xpcshell	            722	    1874	466	1435	    576     1458
jsreftest	            498	    1823	638	1356	    901	    1691
mochitest-ipcplugins	    85	    152	        24	262	    27	    58

Comment 16

[Justin Dolske [:Dolske]]

Bugzilla fail! Reformatted:

                      win win-dbg osx osx-dbg lin lin-dbg
mochi 1               351   1180  434  1341   508  1177
mochi 2               358   1399  386  1582   434  1452
mochi 3               300   1650  306  1738   392  1729
mochi 4               443   1388  717  2136   870  2270
mochi 5               216    697  312  1084   314   791
reftest               345   1116  423  1017  1073  1751
crashtest             108    856  137   840   175   437
mochi-chrome          175    411  198   572   224   436
mochi-browser-chrome  332    545  272  1078   266   654
mochi-a11y            106    243              153   363
xpcshell              722   1874  466  1435   576  1458
jsreftest             498   1823  638  1356   901  1691
mochi-ipcplugins       85    152   24   262    27    58

Anyway, makes sense to me. Although I suspect we'll soon need to split up the revised "everythingelse", since it will still be the long pole (mostly due to the browser-chrome tests). But that could be done as a separate step if you want.

We can probably close this bug as-is, in any case, since the original goal has been accomplished.

Comment 17

[Chris AtLee [:catlee]]

Attachment: split up everything else into mochitest-other, reftest, crashtest, xpcshell, and jsreftest

Comment 18

[[e-mail] (:bhearsum)]

Comment on attachment 421144 [details] [diff] [review]
split up everything else into mochitest-other, reftest, crashtest, xpcshell, and jsreftest

>diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
> ######## places
> BRANCHES['places']['repo_path'] = 'projects/places'
> BRANCHES['places']['major_version'] = '1.9.2'
> BRANCHES['places']['product_name'] = 'firefox'
> BRANCHES['places']['app_name'] = 'browser'
> BRANCHES['places']['brand_name'] = 'Minefield'
>-BRANCHES['places']['start_hour'] = [4] 
>-BRANCHES['places']['start_minute'] = [2] 
>+BRANCHES['places']['start_hour'] = [4]
>+BRANCHES['places']['start_minute'] = [2]
>+BRANCHES['places']['unittest_suites'].append( ('jsreftest', ['jsreftest']) )
> for suite in BRANCHES['places']['unittest_suites']:
>-    if suite[0] == 'everythingelse':
>-        suite[1].append('jsreftest')
>+    if suite[0] == 'mochitest-other':
>         suite[1].append('mochitest-ipcplugins')

I realize this isn't your code originally, but can you update it to be less opaque? I'm not sure why what's here is better than:
BRANCHES['places']['unittest_suites']['mochitest-other'].append('mochitest-ipcplugins')

Same comment for similar code in other spots.

Comment 19

[Chris AtLee [:catlee]]

(In reply to comment #18)
> (From update of attachment 421144 [details] [diff] [review])
> >diff --git a/mozilla2-staging/config.py b/mozilla2-staging/config.py
> > ######## places
> > BRANCHES['places']['repo_path'] = 'projects/places'
> > BRANCHES['places']['major_version'] = '1.9.2'
> > BRANCHES['places']['product_name'] = 'firefox'
> > BRANCHES['places']['app_name'] = 'browser'
> > BRANCHES['places']['brand_name'] = 'Minefield'
> >-BRANCHES['places']['start_hour'] = [4] 
> >-BRANCHES['places']['start_minute'] = [2] 
> >+BRANCHES['places']['start_hour'] = [4]
> >+BRANCHES['places']['start_minute'] = [2]
> >+BRANCHES['places']['unittest_suites'].append( ('jsreftest', ['jsreftest']) )
> > for suite in BRANCHES['places']['unittest_suites']:
> >-    if suite[0] == 'everythingelse':
> >-        suite[1].append('jsreftest')
> >+    if suite[0] == 'mochitest-other':
> >         suite[1].append('mochitest-ipcplugins')
> 
> I realize this isn't your code originally, but can you update it to be less
> opaque? I'm not sure why what's here is better than:
> BRANCHES['places']['unittest_suites']['mochitest-other'].append('mochitest-ipcplugins')
> 
> Same comment for similar code in other spots.

unittest_suites isn't a dictionary, so BRANCHES['places']['unittest_suites']['mochitest-other'].append('mochitest-ipcplugins') wouldn't work.

Comment 20

[[e-mail] (:bhearsum)]

(In reply to comment #19)
> > > for suite in BRANCHES['places']['unittest_suites']:
> > >-    if suite[0] == 'everythingelse':
> > >-        suite[1].append('jsreftest')
> > >+    if suite[0] == 'mochitest-other':
> > >         suite[1].append('mochitest-ipcplugins')
> > 
> > I realize this isn't your code originally, but can you update it to be less
> > opaque? I'm not sure why what's here is better than:
> > BRANCHES['places']['unittest_suites']['mochitest-other'].append('mochitest-ipcplugins')
> > 
> > Same comment for similar code in other spots.
> 
> unittest_suites isn't a dictionary, so
> BRANCHES['places']['unittest_suites']['mochitest-other'].append('mochitest-ipcplugins')
> wouldn't work.

Oh, duh. Sorry about that. carry on!

Comment 21

[Chris AtLee [:catlee]]

Comment on attachment 421144 [details] [diff] [review]
split up everything else into mochitest-other, reftest, crashtest, xpcshell, and jsreftest

changeset:   1944:ccdfd6860f6e

Comment 22

[Chris AtLee [:catlee]]

I'm going to call this one fixed.  Further tweaks to how the tests are split up should happen in new bugs.