Closed Bug 1778275 Opened 2 years ago Closed 1 year ago

Track the performance of adding sources after page load in the debugger

Categories

(DevTools :: Debugger, enhancement)

Product:
DevTools
Component:
Debugger
Type:
enhancement

Tracking

(firefox116 fixed)

Status:
RESOLVED FIXED
Milestone:
116 Branch
Tracking Flags:
Tracking Status
firefox116 --- fixed

People

(Reporter: ochameau, Assigned: ochameau)

References

(Blocks 1 open bug)

Details

Attachments

(5 files, 2 obsolete files)

Bug 1778275 - [devtools] Track performance of adding sources in the debugger.
(deleted), text/x-phabricator-request
Details
Bug 1778275 - [devtools] Better handle clearance of reducer per target/thread.
(deleted), text/x-phabricator-request
Details
Bug 1778275 - [devtools] Reduce polling time and immediately resolve in DAMP test helper.
(deleted), text/x-phabricator-request
Details
Bug 1778275 - [devtools] Make selectSource helper select the precise source passed as argument.
(deleted), text/x-phabricator-request
Details
Bug 1778275 - [devtools] Add many more source to the custom page for debugger.
(deleted), text/x-phabricator-request
Details

While investigating bug 1651849, I realized that many slowness were about sources being added after the toolbox opening. Or sources simply not throttled in the massive first bucket.
These late sources, throttled in a couple of subsequent throlling bucket would actually cause reducer and components updates, leading to slow reflows.
Unfortunately, this doesn't seem to be correctly tracked by DAMP as we do not add sources after the page load, so that they are all notified almost all at once.
It would be nice to add a test adding many sources, causing many throttle buckets. This would verify that incremental reducer/selectors/components updates is not extremely costly.

This helps verify that adding new sources incrementaly wouldn't cause
massive slowness in reducer/components/reflows.

Assignee: nobody → poirot.alex
Status: NEW → ASSIGNED

Also remove useless dbg argument to evalInContent.

Depends on: 1835775

Comment on attachment 9321829 [details]
Bug 1778275 - [devtools] Create debugger context only once in DAMP tests.

Revision D171983 was moved to bug 1835775. Setting attachment 9321829 [details] to obsolete.

Attachment #9321829 - Attachment is obsolete: true

We weren't printing the exception message.
Error mesage and its related stacktrace were also off.

Allow exception to be a string.

Now that reducers are cleared per target/thread, listening to NAVIGATE
for the full clearance of reducer data is wrong.
We should rather wait for the REMOVE_THREAD.

Also add an explicit waiting code for the selected source.

Depends on: 1836146

Comment on attachment 9336583 [details]
Bug 1778275 - [devtools] Improve exception message in DAMP.

Revision D179459 was moved to bug 1836146. Setting attachment 9336583 [details] to obsolete.

Attachment #9336583 - Attachment is obsolete: true

(Instead of forcing to select original or generated based on shouldSelectOriginalLocation flag).

This also add an await for the selectLocation action in order to better wait for all async work.
This required to tweak the minified test as we don't want to wait for full selection.

Keywords: leave-open
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/1affd61c6142 [devtools] Better handle clearance of reducer per target/thread. r=devtools-reviewers,perftest-reviewers,sparky,bomsy
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/95ed74f9ad6d [devtools] Reduce polling time and immediately resolve in DAMP test helper. r=devtools-reviewers,perftest-reviewers,nchevobbe,sparky
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/00af548d93ac [devtools] Make selectSource helper select the precise source passed as argument. r=perftest-reviewers,devtools-reviewers,nchevobbe,sparky
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/d132c75f9ca2 [devtools] Add many more source to the custom page for debugger. r=devtools-reviewers,perftest-reviewers,nchevobbe,sparky
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Keywords: leave-open
Resolution: --- → FIXED
status-firefox116: --- → fixed
Target Milestone: --- → 116 Branch

== Change summary for alert #38599 (as of Wed, 07 Jun 2023 22:56:55 GMT) ==

Regressions:

Ratio Test Platform Options Absolute values (old vs new)
160% damp custom.jsdebugger.project-search.DAMP linux1804-64-shippable-qr e10s fission stylo webrender 1,039.22 -> 2,705.13
156% damp custom.jsdebugger.project-search.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 914.66 -> 2,338.51
154% damp custom.jsdebugger.project-search.DAMP windows10-64-shippable-qr e10s fission stylo webrender 919.93 -> 2,340.97
153% damp custom.jsdebugger.project-search.DAMP linux1804-64-shippable-qr e10s fission stylo webrender-sw 976.68 -> 2,473.37
130% damp custom.jsdebugger.project-search.DAMP macosx1015-64-shippable-qr e10s fission stylo webrender-sw 954.98 -> 2,197.40
129% damp custom.jsdebugger.project-search.DAMP macosx1015-64-shippable-qr e10s fission stylo webrender 951.12 -> 2,177.61
41% damp custom.jsdebugger.open.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 632.11 -> 890.01
38% damp custom.jsdebugger.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender 813.86 -> 1,121.56
38% damp custom.jsdebugger.open.DAMP macosx1015-64-shippable-qr e10s fission stylo webrender 581.99 -> 800.48
37% damp custom.jsdebugger.open.DAMP linux1804-64-shippable-qr e10s fission stylo webrender-sw 799.99 -> 1,094.55
... ... ... ... ...
7% damp custom.jsdebugger.close.DAMP windows10-64-shippable-qr e10s fission stylo webrender 29.68 -> 31.68
6% damp custom.jsdebugger.stepOver.DAMP linux1804-64-shippable-qr e10s fission stylo webrender 441.97 -> 466.39
3% damp custom.jsdebugger.pretty-print.DAMP windows10-64-shippable-qr e10s fission stylo webrender 282.95 -> 291.64
3% damp custom.jsdebugger.pause.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 242.34 -> 249.58
2% damp custom.jsdebugger.pretty-print.DAMP windows10-64-shippable-qr e10s fission stylo webrender-sw 284.16 -> 290.06

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

Regressions: 1837936

Oh wait I actually did not land the very last patch...

Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Pushed by apoirot@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/c882e1bf805b [devtools] Track performance of adding sources in the debugger. r=perftest-reviewers,sparky,jdescottes,devtools-reviewers
Regressions: 1837940

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

Status: REOPENED → RESOLVED
Closed: 1 year ago → 1 year ago
status-firefox116: affected → fixed
Resolution: --- → FIXED
Duplicate of this bug: 1837936
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: