1854941 - Expand DAMP coverage around pretty printed sources

Status: RESOLVED FIXED

(DevTools :: Debugger, task)

Description

[Alexandre Poirot [:ochameau]]

Bug 1843454's STR isn't properly coveraged on DAMP:

• the pretty printed source may be too small
• we don't set breakpoint on it and don't experience the slow column breakpoint selectors
• we don't reload the page while pausing on a pretty printed source. This particular scenario experience the slow selectors, but also the even slower parser worker as well as re-trigerring the pretty print worker.

Comment 1

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Wait for all sources for the custom DAMP reload test.

We weren't waiting for all sources to be rendered on reload.
But for some reason, it doesn't seem to impact the duration of the resolution.
Most likely, some other updates that we are waiting for are slower.

Comment 2

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Load DAMP's minified file from the page.

In following changesets, this will help use this large file while reloading the page.

Comment 3

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Use the large minified file to cover pretty print performance

Use this larger file to extend coverage on complex STRs.

Comment 4

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Cover reloading with a pretty printed source.

This will help cover re-pretty-printing the source after reloading the page.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Cover pausing on a pretty printed location on reload.

This will highlight all the slow operations happening on reloading a pretty printed location:

• re-pretty printing the source
• slow column breakpoint selectors when a breakpoint is set on a large file
• slow parser worker to compute symbols for scope on pause

Comment 6

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Wait for scopes to be fully expanded in DAMP tests.

We weren't really waiting for full expand and could miss some async updates on pause.

Comment 7

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Fix closing tabs and unselecting pretty printed source.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: Bug 1854941 - [devtools] Ensure resolving only once the request source is selected.

selectSource helper may resolve too early, against a previously selected source
if the source was already fully fetched (text content and symbols)

Comment 9

[Alexandre Poirot [:ochameau]]

I'm moving the last patch, still having unexpected impact on opening minified file subtest to bug 1855910.

Here is the final expected impact of the to-be-landed patch queue:
https://treeherder.mozilla.org/perfherder/comparesubtest?originalProject=try&newProject=try&newRevision=c9ad3925648bfd82ea3ea88e64dd4d16cf7d04c3&originalSignature=4763542&newSignature=4763542&framework=12&application=firefox&originalRevision=6ed2eb00e973d86c9fda8df9438f8da5f2860484&page=1&showOnlyConfident=1&pageTitle=Bug+1854941+patch+queue

Lots of regressions on random following subtests as the pretty print subtest moves from 300ms to 2500ms and most likely cause some GC overhead.
Also some impact on the custom.jsdebugger.open-large-minified-file subtest, most likely related to delay coming up from the parser worker who suffer from very long GC pauses.

There is only https://phabricator.services.mozilla.com/D189127 introducing an improvement on the pause test, which I wasn't able to explain.

Comment 10

[Phabricator Automation]

Comment on attachment 9354881 [details]
Bug 1854941 - [devtools] Cover pausing on a pretty printed location on reload.

Revision D189130 was moved to bug 1855910. Setting attachment 9354881 [details] to obsolete.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/afa3f66d7270
[devtools] Wait for all sources for the custom DAMP reload test. r=perftest-reviewers,devtools-reviewers,nchevobbe,sparky
https://hg.mozilla.org/integration/autoland/rev/38c31d682646
[devtools] Wait for scopes to be fully expanded in DAMP tests. r=perftest-reviewers,devtools-reviewers,nchevobbe,AlexandruIonescu
https://hg.mozilla.org/integration/autoland/rev/8eb3332de6c8
[devtools] Load DAMP's minified file from the page. r=perftest-reviewers,devtools-reviewers,sparky,bomsy
https://hg.mozilla.org/integration/autoland/rev/8bf909f9e919
[devtools] Use the large minified file to cover pretty print performance r=perftest-reviewers,devtools-reviewers,nchevobbe,sparky
https://hg.mozilla.org/integration/autoland/rev/c956bee3cc23
[devtools] Cover reloading with a pretty printed source. r=perftest-reviewers,devtools-reviewers,nchevobbe,sparky
https://hg.mozilla.org/integration/autoland/rev/a50ae02ffea2
[devtools] Fix closing tabs and unselecting pretty printed source. r=perftest-reviewers,devtools-reviewers,bomsy
https://hg.mozilla.org/integration/autoland/rev/eee85ca883b5
[devtools] Ensure resolving only once the request source is selected. r=perftest-reviewers,devtools-reviewers,nchevobbe,AlexandruIonescu

Comment 12

[Cristian Tuns]

https://hg.mozilla.org/mozilla-central/rev/afa3f66d7270
https://hg.mozilla.org/mozilla-central/rev/38c31d682646
https://hg.mozilla.org/mozilla-central/rev/8eb3332de6c8
https://hg.mozilla.org/mozilla-central/rev/8bf909f9e919
https://hg.mozilla.org/mozilla-central/rev/c956bee3cc23
https://hg.mozilla.org/mozilla-central/rev/a50ae02ffea2
https://hg.mozilla.org/mozilla-central/rev/eee85ca883b5

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Expected DAMP impact:

== Change summary for alert #39756 (as of Mon, 02 Oct 2023 21:12:15 GMT) ==

Regressions:

Ratio	Test	Platform	Options	Absolute values (old vs new)
611%	damp custom.jsdebugger.pretty-print.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	361.68 -> 2,569.87
596%	damp custom.jsdebugger.pretty-print.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	360.88 -> 2,510.97
550%	damp custom.jsdebugger.pretty-print.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	273.97 -> 1,781.87
498%	damp custom.jsdebugger.pretty-print.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	260.87 -> 1,559.57
497%	damp custom.jsdebugger.pretty-print.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	262.14 -> 1,564.28
71%	damp simple.netmonitor.reload.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	31.56 -> 54.02
26%	damp custom.jsdebugger.open-large-minified-file.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	819.86 -> 1,031.86
24%	damp custom.jsdebugger.open-large-minified-file.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	830.46 -> 1,026.20
17%	damp simple.netmonitor.requestsFinished.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	129.01 -> 150.78
14%	damp custom.netmonitor.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	18.89 -> 21.56
...	...	...	...	...
5%	damp custom.netmonitor.requestsFinished.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	2,817.92 -> 2,947.36
5%	damp custom.jsdebugger.open-large-minified-file.full-selection.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	4,362.96 -> 4,561.08
4%	damp custom.netmonitor.open.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	198.34 -> 207.18
4%	damp custom.jsdebugger.reload.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	821.22 -> 854.28
4%	damp custom.jsdebugger.reload.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	828.89 -> 861.28

Improvements:

Ratio	Test	Platform	Options	Absolute values (old vs new)
58%	damp custom.jsdebugger.close.DAMP	macosx1015-64-shippable-qr	e10s fission stylo webrender	47.53 -> 19.91
52%	damp custom.jsdebugger.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender-sw	60.18 -> 28.73
52%	damp custom.jsdebugger.close.DAMP	windows10-64-shippable-qr	e10s fission stylo webrender	60.48 -> 28.93
48%	damp custom.jsdebugger.close.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	74.88 -> 38.59
48%	damp custom.jsdebugger.close.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender-sw	73.63 -> 38.63
...	...	...	...	...
4%	damp source-map.eachMapping.DAMP	linux1804-64-shippable-qr	e10s fission stylo webrender	327.05 -> 313.01

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=39756