1320790 - gcli test helpers are slowing down inspector tests

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Alexandre Poirot [:ochameau]]

It is especially slow when running on linux32 debug or other slow build machines.
And it seems to be used only by one test, a gcli one.

Comment 1

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=117f60cae7ba

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: patch v1

Comment 3

[Alexandre Poirot [:ochameau]]

I'm lost in DAMP!

I don't get why this patch highlight a 5 to 8% win on DAMP, with a remarkable win on netmonitor!
I first thought my base revision on mozilla-central was broken, but I compared again various revisions and the result is the same.
Here is the one I think is the real base revision, the one I have in my local branch:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=05328d3102ef&newProject=try&newRevision=65e61d97720d&framework=1&showOnlyImportant=0&showOnlyConfident=1

But as I used push-to-try --tip, I also compared against whatever was the tip at that time:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-central&originalRevision=8387a4ada9a5c4cab059d8fafe0f8c933e83c149&newProject=try&newRevision=65e61d97720d81104293ffdf83654c2ecbfa2b8f&framework=1&filter=damp

And also used perfherder automatic base commit finder, which found something on inbound:
https://treeherder.mozilla.org/perf.html#/compare?originalProject=mozilla-inbound&originalRevision=a7a8cd0dd192&newProject=try&newRevision=65e61d97720d&framework=1

I don't see how changes made to /inspector/test/head.js makes any difference on DAMP!
And it is even more surprising to see the exact same difference against various base commits.
Or is it just that my try push ran while the build machines were exceptionally fast?!

Comment 4

[Alexandre Poirot [:ochameau]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9694dec0d975

Comment 5

[Alexandre Poirot [:ochameau]]

I just repushed to try the same commit again and will compare the two runs,
and also pushed an empty commit to compare against it.

Comment 6

[Alexandre Poirot [:ochameau]]

I'm afraid that with such behavior I can't use DAMP to highlight the value of all my perf tweaks based on profiler results.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: profile-gclihelper.json

Here is the profiler data that highlights helpers.js while running browser_inspector_initialization.js.
If you expand the tree under "SimpleTest.waitForFocus/..." measured at 421ms,
you end up seeing helpers.js, cli.js and various gcli deps being loaded.

This patch is only usefull on top of bug 1320149, as, otherwise, gcli is loaded by the toolbox anyway. But once we stop loading it by default, I imagine there may be a win to stop loading it in tests.

Now, I can't prove with numbers that this patch brings something.
Should I wait or move forward?

Comment 8

[Alexandre Poirot [:ochameau]]

Brian, Any thought on my experiences around DAMP?
Do you think I should move forward with this patch anyway?

I'm wondering if that could help reduce the timeout intermittents we have on linux32 or windows vm...

Comment 9

[Brian Grinstead [:bgrins]]

Yeah, it's a test only change so it's not going to reflect on DAMP.  If it ends up lowering the # of intermittent timeouts then that would be a great success on its own -- no need to see DAMP changes here.

Comment 10

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1320790 - Only inject gcli test helper in the inspect that use it.

Review commit: https://reviewboard.mozilla.org/r/96560/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96560/

Comment 11

[Patrick Brosset <:pbro>]

Comment on attachment 8815718 [details]
Bug 1320790 - Only inject gcli test helper in the inspect that use it.

https://reviewboard.mozilla.org/r/96560/#review96766

Comment 12

[J. Ryan Stinnett [:jryans] (Use needinfo, replies may be slow)]

Looks like autoland failed here.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cd56360c0615
Only inject gcli test helper in the inspect that use it. r=pbro

Comment 14

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cd56360c0615

Comment 15

[Alexandre Poirot [:ochameau]]

Looks like it succeed here, but keeps failing to land in bug 1320084...