Joel Maher ( :jmaher ) (UTC -8) Assignee
	Description • 10 years ago

simple change, lets try it out and fix what few failures crop up

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 1 • 10 years ago

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

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 2 • 10 years ago

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.

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 3 • 10 years ago

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?

	Ryan VanderMeulen [:RyanVM]
	Comment 4 • 10 years ago

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)

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 5 • 10 years ago

Gavin, let me know your thoughts on going forward with --chunk-by-dir?

	Ryan VanderMeulen [:RyanVM]
	Comment 6 • 10 years ago

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.

	:Gavin Sharp [email: gavin@gavinsharp.com]
	Comment 7 • 10 years ago

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?

	Ryan VanderMeulen [:RyanVM]
	Comment 8 • 10 years ago

(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.

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 9 • 10 years ago

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.

	Armen [:armenzg]
	Comment 10 • 10 years ago

Comment on attachment 8404738 [details] [diff] [review]
add chunk-by-dir=5 to bc related chunks (1.0)

LGTM

	Joel Maher ( :jmaher ) (UTC -8) Assignee
	Comment 11 • 10 years ago

landed on mozharness default:
https://hg.mozilla.org/build/mozharness/rev/11061e1cebb8

will wait for it to get in production before resolving this.

	Justin Wood (:Callek)
	Comment 12 • 10 years ago

Merged to production

	Ed Morley [:emorley]
	Comment 13 • 10 years ago

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.

	Ryan VanderMeulen [:RyanVM]
	Comment 14 • 10 years ago

(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.