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)

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Dolske, Assigned: catlee)

References

Details

Attachments

(1 file)

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.
Getting bug 445622 done would help the compile time part of this.
Component: Release Engineering → Release Engineering: Future
Priority: -- → P3
Perhaps bug 383136 too, if that could allows running (some?) tests with normal packaged builds from some other box.
Depends on: 383136
No longer blocks: 443323
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.
IIRC the other test suites range from 1 to 15 minutes. I'd aim for something like 10 minutes per chunk for mochitests.
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?
un-duped bug 406906, per catlee, because bug 452861 is about *unit tests* and bug 406906 is about perf tests.
Depends on: 494165
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.
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 :)
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)
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?
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.
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: nobody → catlee
Component: Release Engineering: Future → Release Engineering
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
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 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.
(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.
(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!
Attachment #421144 - Flags: review?(bhearsum) → review+
Attachment #421144 - Flags: review?(nrthomas) → review+
Blocks: 539135
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+
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
Depends on: 539838
Product: mozilla.org → Release Engineering
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: