1419327 - Run DAMP open/reload/close tests against a document specific to netmonitor

Status: RESOLVED FIXED

(DevTools :: Netmonitor, enhancement, P2)

Description

[Alexandre Poirot [:ochameau]]

Following bug 1415532, we should also run DAMP test against a custom document, made up for the netmonitor.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

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

Comment 2

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Oops, did not meant to ask review yet.

Comment 3

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/202028/diff/1-2/

Comment 4

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/202028/diff/2-3/

Comment 5

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/202028/diff/3-4/

Comment 6

[Alexandre Poirot [:ochameau]]

This test do many things: script/css tag requests, xhr, post data, big files.
I looked into the impact of each of these type of requests:

# With the patch
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=mozilla-central&newProject=try&newRevision=57c7c1e0b443ae5115ed1bae87390be485aa5919&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=netmonitor&framework=1&selectedTimeRange=172800
  close 58.02 ± 2.89%
  open 575.24 ± 0.60%
  reload 311.48 ± 3.84%
  requestsFinished 2,804.40 ± 0.82% 

# Disable big file requests
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=57c7c1e0b443ae5115ed1bae87390be485aa5919&newProject=try&newRevision=4da7095e2e005e137939be2a13da8b81ff7bdce9&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&framework=1
  reload 311.48 ± 3.84% 	< 	386.30 ± 2.06% 	24.02% 	(high)
  requestsFinished 2,804.40 ± 0.82% 	> 	2,064.37 ± 1.05% 	-26.39% (high)

# Disable post data requests
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=57c7c1e0b443&newProject=try&newRevision=5bbe8f6780652612fe6871fd03abf3880659f2b1&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=netmon&framework=1
  requestsFinished 2,804.40 ± 0.82% 	> 	1,775.76 ± 1.41% 	-36.68% (high)
  close 58.02 ± 2.89% 	< 	64.20 ± 3.92% 	10.65% 	(med)

# Disable XHR requests
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=57c7c1e0b443ae5115ed1bae87390be485aa5919&newProject=try&newRevision=947e98437b79c4bb6297c5e0aef1e4e045006d98&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=netmoni&framework=1
  requestsFinished 2,804.40 ± 0.82% 	> 	2,520.42 ± 1.06% 	-10.13% 	(high)

# Disable all statis requests (JS/CSS tags)
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=57c7c1e0b443ae5115ed1bae87390be485aa5919&newProject=try&newRevision=c7ec771a10baa87c9e216931f98cfcb9205dc543&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&filter=netmon&framework=1
  close 58.02 ± 2.89% 	> 	49.42 ± 2.51% 	-14.83% 	(high)
  reload 311.48 ± 3.84% 	> 	92.26 ± 2.69% 	-70.38% 	(high)
  requestsFinished 2,804.40 ± 0.82% 	> 	1,994.23 ± 0.47% 	-28.89%  (high)

These results confirm that each type of request has a significant impact on the test duration,
so we should be able to see if anything regress about them.

Comment 7

[Alexandre Poirot [:ochameau]]

I tested tweaking the number of requests being dispatch to understand the impact of each.

Dispatch 200 big requests instead of 20:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=0ff904181958823e42efce0531bc993bc52d3b1f&newProject=try&newRevision=f5d5daaedd38e9b7fd818ae93249f2a7665cf67a&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&showOnlyConfident=1&framework=1
requestsFinished  3,316.55 ± 0.78% 	< 	13,417.50 ± 0.30% 	304.56%
close 41.75 ± 1.75% 	< 	59.05 ± 1.36% 	41.42%
Small impact on simple.styleditor.open: 2.14%, complicated.webconsole.open: 7.9%

Dispatch 200 post requests instead of 20:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=0ff904181958823e42efce0531bc993bc52d3b1f&newProject=try&newRevision=e600a1ea65edc91f0067f609db8e73b765d213bd&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&showOnlyConfident=1&framework=1
requestsFinished  3,316.55 ± 0.78% 	< 	14,249.02 ± 0.42% 	329.63% 
close 41.75 ± 1.75% 	< 	58.34 ± 2.72% 	39.72% 
Small impact on simple.styleeditor.open: 2.15%

Dispatch 500 xhr requests instead of 50:
https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=0ff904181958823e42efce0531bc993bc52d3b1f&newProject=try&newRevision=e214ee29b7f6557985baa027e5ff4db478e87d67&originalSignature=edaec66500db21d37602c99daa61ac983f21a6ac&newSignature=edaec66500db21d37602c99daa61ac983f21a6ac&showOnlyConfident=1&framework=1
requestsFinished  3,316.55 ± 0.78% 	< 	9,546.23 ± 1.10% 	187.84% 
close  41.75 ± 1.75% 	< 	81.79 ± 2.08% 	95.89%
Small impact on custom.webconsole.open: 3%, console.objectexpand: 2.7%, complicated.jsdebugger: 1.8%

Dispatching signifiant post request or big files seems to have a similar impact on netmonitor performance. Big files have a slightly increased impact on next tests as it most likely introduce more GC work.
XHR requests, without a lot of data in header/post, have a smaller impact on netmonitor performance. With 500 xhr requests, netmonitor updates in 9s whereas with 200 post data requests, it takes 14s. But it seems to introduce more GC work overall as it impacts more the following tests. The bigger increase of close subtest confirms that.

Comment 8

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/202028/diff/4-5/

Comment 9

[Alexandre Poirot [:ochameau]]

Honza, do you see any other way to stress the netmonitor performance from the webpage?
The obvious followup to this would be to look at the performance of sidebar opening, as I would expect some to be slow as I set lots of headers/post data.

Comment 10

[Code Review Bot [:reviewbot]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

https://reviewboard.mozilla.org/r/202028/#review224108


Code analysis found 2 defects in this patch:
 - 2 defects found by mozlint

You can run this analysis locally with:
 - `./mach lint path/to/file` (JS/Python)


If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx


::: testing/talos/talos/tests/devtools/addon/content/damp.js:283
(Diff revision 5)
> +      let mm = tab.linkedBrowser.messageManager;
> +      mm.addMessageListener("damp-tab-loaded", function listener() {
> +        mm.removeMessageListener("damp-tab-loaded", listener);
>          resolve(tab);
> +      });
> +      mm.loadFrameScript("data:,new " + function () {

Error: Unexpected space before function parentheses. [eslint: space-before-function-paren]

::: testing/talos/talos/tests/devtools/addon/content/damp.js:844
(Diff revision 5)
> +  async customNetmonitor() {
> +    // These numbers controls the number of requests we do in the test
> +    let bigFileRequests = 20, postDataRequests = 20, xhrRequests = 50;
> +    // These other numbers only state how many requests the test do,
> +    // we have to keep them in sync with netmonitor.html static content
> +    let syncCssRequests = 10, syncJSRequests = 10, syncIframeRequests = 3*10;

Error: Infix operators must be spaced. [eslint: space-infix-ops]

Comment 11

[Alexandre Poirot [:ochameau]]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/202028/diff/5-6/

Comment 12

[Robert Wood [:rwood]]

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

Comment 13

[Robert Wood [:rwood]]

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

Comment 14

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8934475 [details]
Bug 1419327 - Run DAMP test against a document specific to the netmonitor.

https://reviewboard.mozilla.org/r/202028/#review224464

Looks good to me, R+, but one question inline.

This is the first time I am looking at damp test,
so you might consider asking yet someone else for review.
 
> Honza, do you see any other way to stress the netmonitor performance from the webpage?

* I am not seeing any cookes in the test
* Perhaps loading set of images could be good too (but at least the one video file is there)
* Big query strings in the test could help to see if parsing is efficient.

Thanks!
Honza

::: testing/talos/talos/tests/devtools/addon/content/pages/custom/netmonitor.html:114
(Diff revision 6)
> +
> +// Force some DOM updates to better simulate a single page webapp
> +// which is going to create its DOM dynamically.
> +// This will help seeing if the page load slow down because
> +// of netmonitor listeners.
> +let interval = setInterval(function forcePaint() {

What's the point of this? Shouldn't we rather focus on stressing the Net panel itself and isolate out things like DOM mutatitions?

Comment 15

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1419327 - [devtools] Run DAMP test against a document specific to the netmonitor.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b51315e2bba4
[devtools] Run DAMP test against a document specific to the netmonitor. r=nchevobbe,perftest-reviewers,sparky

Comment 17

[Narcis Beleuzu [:NarcisB]]

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