1055014 - Avoid pushing unnecessary files during b2g xpcshell setup

Status: RESOLVED FIXED

(Testing :: XPCShell Harness, defect)

Description

[Chris Manchester (limited bugmail, email directly)]

Each chunk of b2g emulator tests is pretty slow due to time spent pushing files. It looks like not all of them are relevant to the set of running tests.

Comment 1

[Chris Manchester (limited bugmail, email directly)]

Attachment: Bug Only push relevant files to device for xpcshell b2g tests.

I started by trying to push only the files necessary for an individual chunk, but this seems to save enough time on un-chunked runs to make the chunks unnecessary.

Try:

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=568a8954e0e4

And with verbose mode on, so we can see what each file is doing (orange due to known failures in logs that usually aren't included):

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4b1183697f86

I don't think this is a great solution, but it seems pretty effective (saves about 20 minutes per run). ahal, what do you think?

Comment 2

[Andrew Halberstadt [:ahal]]

Comment on attachment 8474566 [details] [diff] [review]
Bug Only push relevant files to device for xpcshell b2g tests.

Review of attachment 8474566 [details] [diff] [review]:
-----------------------------------------------------------------

Huh, something regressed either in mozdevice or mozrunner (emulator.py) I think. It used to be the case that pushing busybox to the emulator made the 'unzip' command available. When unzip is unavailable, mozdevice should automatically zip the contents of a directory up, push that one single file, and then unzip it again on the device side. This simple optimization saved us about ~20 minutes in setup for xpcshell in practice.

I think if we find where this regression happened and fix it, then this patch shouldn't be needed. If for some reason we can't fix it, then I guess it is better than nothing.. I'll take a look to see if I can figure out what happened.

Comment 3

[Andrew Halberstadt [:ahal]]

Attachment: Mozdevice should call _verifyZip from 'pushDir' in case the 'unzip' ability was added in the interim

I think I know what's going on. Mozdevice calls a '_verifyZip' function when it is instantiated to test whether or not the device has 'unzip' capability. My theory is that ever since the mozrunner/emulator.py refactor, runtestsb2g.py creates a second dm instance, before the one in mozrunner. When we install busybox via mozrunner, this instance has already called '_verifyZip' and therefore still thinks that the unzip command is unavailable.

We can fix this, by checking whether it is available in 'pushDir' itself. Here's a try run to test if this theory is correct:
https://tbpl.mozilla.org/?tree=Try&rev=8c320778f755

Comment 4

[Andrew Halberstadt [:ahal]]

My theory was at least partially true as dm._useZip was getting set to True in pushDir, but there seems to be an exception, at which point dm._useZip gets set back to False and we recursively call pushDir again:
http://mxr.mozilla.org/mozilla-central/source/testing/mozbase/mozdevice/mozdevice/devicemanagerADB.py#235

This meant my patch caused infinite recursion. Here's a new try run that fixes that, and should also tell us why there is an exception in pushDir:
https://tbpl.mozilla.org/?tree=Try&rev=44e52de5ac97

Comment 5

[Andrew Halberstadt [:ahal]]

Attachment: Remove b2g specific setupTestDir function

Wlach pointed out on irc that dm.pushDir no longer falls back to pushing one file at a time. So zipping things up is likely not even a perf win anymore. Either way we should always use dm.pushDir. I think the b2g 'setupTestDir' function was a hack to fix a problem that no longer exists.

https://tbpl.mozilla.org/?tree=Try&rev=e946b83e747c

Comment 6

[Chris Manchester (limited bugmail, email directly)]

Great, it looks like that gives us about the same speedup. Thanks ahal!

Comment 7

[Chris Manchester (limited bugmail, email directly)]

Comment on attachment 8474727 [details] [diff] [review]
Remove b2g specific setupTestDir function

Review of attachment 8474727 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks again for looking into this.

Comment 8

[Andrew Halberstadt [:ahal]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/133471646752

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Backed out for intermittent B2G xpcshell failures.
https://hg.mozilla.org/integration/mozilla-inbound/rev/39830d7054c6

https://tbpl.mozilla.org/php/getParsedLog.php?id=46275546&tree=Mozilla-Inbound

Comment 10

[Andrew Halberstadt [:ahal]]

Attachment: Fix exception in pushDir

I think this patch will fix the speed issue as well as not cause the intermittent (which I still have no idea why it is happening). Basically we were hitting an exception in the zip/unzip code path of pushDir 100% of the time (ever since we switched to using mozprocess).

I'll wait for some try retriggers before landing.

https://tbpl.mozilla.org/?tree=Try&rev=dd1de2f1079d

Comment 11

[William Lachance (:wlach)]

Comment on attachment 8475316 [details] [diff] [review]
Fix exception in pushDir

What do you think about making proc.run() return 'self', so that the original code works? Other than that, this looks reasonable, though I hope we don't have to keep the zip code around much longer.

Comment 12

[Andrew Halberstadt [:ahal]]

(In reply to William Lachance (:wlach) from comment #11)
> What do you think about making proc.run() return 'self', so that the
> original code works?

Yeah, I like that idea.

Comment 13

[Andrew Halberstadt [:ahal]]

Try + retriggers are green and we still have the 20 minute speedup:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2cd1144d6150

Comment 14

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/2cd1144d6150