Closed Bug 992366 Opened 11 years ago Closed 11 years ago

run browser-chrome with --chunk-by-dir to reduce tests moving between chunks

Categories

(Testing :: Mochitest, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jmaher, Assigned: jmaher)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

simple change, lets try it out and fix what few failures crop up
and I thought this was easy but it wasn't: https://tbpl.mozilla.org/?tree=Cedar&showall=1&rev=81c24e006114 some concerns: dt2 failed - could be a misconfig in buildbot vs mozharness- I cannot reproduce locally bc2 is running 2K tests and bc3 is running 14k tests so the chunking is very imbalanced right now. The question is what level should I set chunkbydir to, for mochitest-plain it is '--chunk-by-dir 4', I suspect we want '--chunk-by-dir 3' for devtools and maybe browser chrome. I am looking into it more. Here is the chunkify code: http://dxr.mozilla.org/mozilla-central/source/testing/mochitest/chunkifyTests.js?from=chunkifytests.js#1
learned more about this, we need --chunk-by-dir=5. the number is the number of directories to skip (i.e. chrome://mochitests/browser/browser/devtools/styleinspector/test/testname.js has 6 directories by the way we parse the test path) would be 5 and that makes sense as mochitest-plain doesn't have the additional paths in the front. On my previous push, this was finding devtools and toolkit (2 total dirs to run) for 3 chunks. Obviously there would be a problem. We will see how well this works for browser-chrome, it is pushed to the tip of cedar.
w7 debug time in minutes base C=5 C=4 bc1 21 41 41 bc2 22 7 5 bc3 24 19 19 dt1 32 38 N/A dt2 24 15 N/A dt3 N/A tbd C=5 == chunk-by-dir=5 this is quite an imbalance, really on bc2. dt2 is orange on all platforms, I suspect a single test disable will result in green, if not maybe 2 or 3 tests are causing all the problems there. Our biggest concerns for time is dt1 on linux debug (32 and 64). Ryan, what are you thoughts here?
Flags: needinfo?(ryanvm)
I think the time imbalance is a bigger issue than the leaks. Like you said, I think we could get around that quickly. Unfortunately, I'm not sure how much we can do about it for now? (Though I have some ideas once we go instance-per-directory)
Flags: needinfo?(ryanvm)
Gavin, let me know your thoughts on going forward with --chunk-by-dir?
Flags: needinfo?(gavin.sharp)
Joel and I talked a bit more about this. While the current split isn't overly optimal, I think it'll work for now. We can work on balancing it better in phase 2 with the instance-per-directory work. That said, we do need to resolve the new leak that appears in dt2 with per-directory chunking turned on.
Flags: needinfo?(gavin.sharp)
We can fix the imbalance by moving tests or by improving the chunking somehow (I noted this issue in bug 883314 comment 13). We have a lot of reasons to move tests anyhow (the current bunching is far from ideal). Is there a bug filed about the leak?
Depends on: 993580
(In reply to :Gavin Sharp (email gavin@gavinsharp.com) from comment #7) > We can fix the imbalance by moving tests or by improving the chunking > somehow (I noted this issue in bug 883314 comment 13). We have a lot of > reasons to move tests anyhow (the current bunching is far from ideal). Moving tests is probably our best option if we want to do something now. Once we're in a instance-per-directory world, I think we can give ourselves more knobs to fiddle in the harness itself. Right now, I'd be worried about upsetting the balance, though. > Is there a bug filed about the leak? Filed bug 993580.
this patch was easier than I thought it would be. Right now we only run browser chrome (including devtools) chunks en masse on Cedar. There is an exception, linux 32/64 debug runs browser chrome (no devtools) in 3 chunks on all branches. There is a slight chance that the act of rearranging these tests will cause oranges to show up. Our ultimate goal is to get these this rolled out tomorrow on all project/integration branches, so any regressions would be gone there. That would leave esr24/release/beta/aurora. I can fix esr24, release, beta and we plan to backport (halfway there already) test fixes from fx-team and cedar to aurora.
Assignee: nobody → jmaher
Status: NEW → ASSIGNED
Attachment #8404738 - Flags: review?(bhearsum)
Comment on attachment 8404738 [details] [diff] [review] add chunk-by-dir=5 to bc related chunks (1.0) LGTM
Attachment #8404738 - Flags: review?(bhearsum) → review+
landed on mozharness default: https://hg.mozilla.org/build/mozharness/rev/11061e1cebb8 will wait for it to get in production before resolving this.
Merged to production
The max runtime for the bc1 chunk had to be increased by bug 995060 to fix bustage. I'll file a bug for lowering this once devtools is split out, so we don't accidentally regress runtimes across the board later.
Depends on: 995060
(In reply to Ed Morley [:edmorley UTC+0] from comment #13) > The max runtime for the bc1 chunk had to be increased by bug 995060 to fix > bustage. I'll file a bug for lowering this once devtools is split out, so we > don't accidentally regress runtimes across the board later. Not helping our cause is that the debugger tests missed the devtools subsuite annotation when this landed on trunk, so bc1 didn't get as much relief from bug 984930 as it should have. Bug 996003 tracks fixing that (along with some other annoyances). Anyway, everything here is in production now on trunk, so we can close this bug. Bug 996240 tracks enabling this on Aurora30 as well.
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Blocks: 996504
Component: Mochitest Chrome → Mochitest
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: