Closed
Bug 452861
Opened 16 years ago
Closed 15 years ago
unittest runs should be split up and parallelized
Categories
(Release Engineering :: General, defect, P3)
Release Engineering
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: Dolske, Assigned: catlee)
References
Details
Attachments
(1 file)
(deleted),
patch
|
bhearsum
:
review+
nthomas
:
review+
catlee
:
checked-in+
|
Details | Diff | Splinter Review |
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•16 years ago
|
||
Getting bug 445622 done would help the compile time part of this.
Updated•16 years ago
|
Component: Release Engineering → Release Engineering: Future
Priority: -- → P3
Reporter | ||
Comment 2•16 years ago
|
||
Perhaps bug 383136 too, if that could allows running (some?) tests with normal packaged builds from some other box.
Depends on: 383136
Comment 3•16 years ago
|
||
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.
Assignee | ||
Comment 4•16 years ago
|
||
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•16 years ago
|
||
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•16 years ago
|
||
un-duped bug 406906, per catlee, because bug 452861 is about *unit tests* and bug 406906 is about perf tests.
Assignee | ||
Comment 8•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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•15 years ago
|
||
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.
Depends on: 487689
Assignee | ||
Updated•15 years ago
|
Assignee: nobody → catlee
Component: Release Engineering: Future → Release Engineering
Assignee | ||
Comment 15•15 years ago
|
||
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
Reporter | ||
Comment 16•15 years ago
|
||
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.
Assignee | ||
Comment 17•15 years ago
|
||
Attachment #421144 -
Flags: review?(nrthomas)
Attachment #421144 -
Flags: review?(bhearsum)
Comment 18•15 years ago
|
||
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.
Assignee | ||
Comment 19•15 years ago
|
||
(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•15 years ago
|
||
(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!
Updated•15 years ago
|
Attachment #421144 -
Flags: review?(bhearsum) → review+
Updated•15 years ago
|
Attachment #421144 -
Flags: review?(nrthomas) → review+
Assignee | ||
Comment 21•15 years ago
|
||
Comment on attachment 421144 [details] [diff] [review]
split up everything else into mochitest-other, reftest, crashtest, xpcshell, and jsreftest
changeset: 1944:ccdfd6860f6e
Attachment #421144 -
Flags: checked-in+
Assignee | ||
Comment 22•15 years ago
|
||
I'm going to call this one fixed. Further tweaks to how the tests are split up should happen in new bugs.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•11 years ago
|
Product: mozilla.org → Release Engineering
You need to log in
before you can comment on or make changes to this bug.
Description
•