Closed Bug 1498235 Opened 3 years ago Closed 3 years ago

Improve accessibility actor startup performance.

Categories

(DevTools :: Accessibility Tools, enhancement)

Product:
DevTools
Component:
Accessibility Tools
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

(firefox65 fixed)

Status:
RESOLVED FIXED
Milestone:
Firefox 65
Tracking Flags:
Tracking Status
firefox65 --- fixed

People

(Reporter: yzen, Assigned: yzen)

References

Details

Attachments

(4 files, 1 obsolete file)

Bug 1498235 - only initialize accessibility actor when the toolbox opens or when a11y panel opens as a default panel. r?gl
46 bytes, text/x-phabricator-request
Details | Review
Bug 1498235 - split accessibility actors into their own files. r?gl
46 bytes, text/x-phabricator-request
Details | Review
Bug 1498235 - lazy load modules in accessibility actors. r?gl
46 bytes, text/x-phabricator-request
Details | Review
Bug 1498235 - lazy load accessibility highlighter in accessibility walker actor. r?gl
46 bytes, text/x-phabricator-request
Details | Review 
We can do several things:

- split a11y actors into separate files
- lazy load as much as possible
- wait until idle to init accessible actor in accessibility startup component.
MozReview-Commit-ID: Gm7ju38mhdD

Depends on D9578
MozReview-Commit-ID: 2qPBmhnd7tb

Depends on D9579
MozReview-Commit-ID: IDpbV4bWKEt

Depends on D9581
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/9e51e9847562
only initialize accessibility actor when the toolbox opens or when a11y panel opens as a default panel. r=gl
https://hg.mozilla.org/integration/autoland/rev/50d5e01b6dda
split accessibility actors into their own files. r=gl
https://hg.mozilla.org/integration/autoland/rev/134717494734
lazy load modules in accessibility actors. r=gl
https://hg.mozilla.org/integration/autoland/rev/8399e5224d69
lazy load accessibility highlighter in accessibility walker actor. r=gl
Attachment #9019469 - Attachment is obsolete: true
Backed out for multiple devtools failures on devtools/client/*

backout: https://hg.mozilla.org/integration/autoland/rev/4ef080fed9d2246509f6ebf04652e60f7d76402e

push with failures: https://treeherder.mozilla.org/#/jobs?repo=autoland&group_state=expanded&revision=8399e5224d6935e06c37eb600b34967ba06b4487

failure log example: https://treeherder.mozilla.org/logviewer.html#?job_id=207680512&repo=autoland&lineNumber=1513

[task 2018-10-25T03:11:09.988Z] 03:11:09     INFO - TEST-PASS | devtools/client/inspector/rules/test/browser_rules_add-property-svg.js | The fill was changed to red - 
[task 2018-10-25T03:11:09.990Z] 03:11:09     INFO - Leaving test bound 
[task 2018-10-25T03:11:09.992Z] 03:11:09     INFO - Removing tab.
[task 2018-10-25T03:11:09.994Z] 03:11:09     INFO - Waiting for event: 'TabClose' on [object XULElement].
[task 2018-10-25T03:11:09.996Z] 03:11:09     INFO - Got event: 'TabClose' on [object XULElement].
[task 2018-10-25T03:11:09.998Z] 03:11:09     INFO - Tab removed and finished closing
[task 2018-10-25T03:11:10.000Z] 03:11:10     INFO - Buffered messages finished
[task 2018-10-25T03:11:10.002Z] 03:11:10     INFO - TEST-UNEXPECTED-FAIL | devtools/client/inspector/rules/test/browser_rules_add-property-svg.js | A promise chain failed to handle a rejection: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMWindowUtils.getScrollbarSize] - stack: getWindowDimensions@resource://devtools/shared/base-loader.js -> resource://devtools/shared/layout/utils.js:730:5
[task 2018-10-25T03:11:10.004Z] 03:11:10     INFO - AutoRefreshHighlighter@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/highlighters/auto-refresh.js:74:25
[task 2018-10-25T03:11:10.005Z] 03:11:10     INFO - AccessibleHighlighter@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/highlighters/accessible.js:65:5
[task 2018-10-25T03:11:10.007Z] 03:11:10     INFO - initialize@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/highlighters.js:458:27
[task 2018-10-25T03:11:10.009Z] 03:11:10     INFO - cls@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:1263:5
[task 2018-10-25T03:11:10.009Z] 03:11:10     INFO - initialize/<@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/accessibility/walker.js:145:23
[task 2018-10-25T03:11:10.009Z] 03:11:10     INFO - get@resource://devtools/shared/base-loader.js -> resource://devtools/shared/DevToolsUtils.js:398:22
[task 2018-10-25T03:11:10.010Z] 03:11:10     INFO - cancelPick@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/accessibility/walker.js:717:5
[task 2018-10-25T03:11:10.010Z] 03:11:10     INFO - reset@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/accessibility/walker.js:178:5
[task 2018-10-25T03:11:10.011Z] 03:11:10     INFO - destroy@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/accessibility/accessibility.js:259:7
[task 2018-10-25T03:11:10.011Z] 03:11:10     INFO - async*destroy@resource://devtools/shared/base-loader.js -> resource://devtools/shared/protocol.js:948:9
[task 2018-10-25T03:11:10.012Z] 03:11:10     INFO - _detach@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/targets/browsing-context.js:883:7
[task 2018-10-25T03:11:10.013Z] 03:11:10     INFO - exit@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/targets/browsing-context.js:513:5
[task 2018-10-25T03:11:10.013Z] 03:11:10     INFO - frameTargetPrototype.exit@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/targets/frame.js:80:3
[task 2018-10-25T03:11:10.013Z] 03:11:10     INFO - destroy@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/targets/browsing-context.js:494:5
[task 2018-10-25T03:11:10.014Z] 03:11:10     INFO - removeActor@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/common.js:67:7
[task 2018-10-25T03:11:10.014Z] 03:11:10     INFO - APDestroy@resource://devtools/shared/base-loader.js -> resource://devtools/server/actors/common.js:29:7
[task 2018-10-25T03:11:10.015Z] 03:11:10     INFO - onClosed/<@resource://devtools/shared/base-loader.js -> resource://devtools/server/main.js:1412:35
[task 2018-10-25T03:11:10.015Z] 03:11:10     INFO - onClosed@resource://devtools/shared/base-loader.js -> resource://devtools/server/main.js:1412:5
[task 2018-10-25T03:11:10.016Z] 03:11:10     INFO - close@resource://devtools/shared/base-loader.js -> resource://devtools/shared/transport/child-transport.js:62:5
[task 2018-10-25T03:11:10.016Z] 03:11:10     INFO - close@resource://devtools/shared/base-loader.js -> resource://devtools/server/main.js:1026:7
[task 2018-10-25T03:11:10.017Z] 03:11:10     INFO - @resource://devtools/server/startup/frame.js:132:9
[task 2018-10-25T03:11:10.017Z] 03:11:10     INFO - _endRemoveTab@chrome://browser/content/tabbrowser.js:3112:5
[task 2018-10-25T03:11:10.018Z] 03:11:10     INFO - removeTab@chrome://browser/content/tabbrowser.js:2810:7
[task 2018-10-25T03:11:10.018Z] 03:11:10     INFO - removeTab@chrome://mochitests/content/browser/devtools/client/shared/test/shared-head.js:192:3
[task 2018-10-25T03:11:10.019Z] 03:11:10     INFO - async*closeTabAndToolbox@chrome://mochitests/content/browser/devtools/client/shared/test/shared-head.js:456:9
[task 2018-10-25T03:11:10.019Z] 03:11:10     INFO - async*cleanup@chrome://mochitests/content/browser/devtools/client/shared/test/shared-head.js:137:11
[task 2018-10-25T03:11:10.020Z] 03:11:10     INFO - async*nextTest@chrome://mochikit/content/browser-test.js:695:30
[task 2018-10-25T03:11:10.021Z] 03:11:10     INFO - async*testScope/test_finish/<@chrome://mochikit/content/browser-test.js:1387:11
[task 2018-10-25T03:11:10.021Z] 03:11:10     INFO - run@chrome://mochikit/content/browser-test.js:1324:9
[task 2018-10-25T03:11:10.022Z] 03:11:10     INFO - Rejection date: Thu Oct 25 2018 03:11:09 GMT+0000 (Coordinated Universal Time) - false == true - JS frame :: resource://testing-common/PromiseTestUtils.jsm :: assertNoUncaughtRejections :: line 257
[task 2018-10-25T03:11:10.022Z] 03:11:10     INFO - Stack trace:
[task 2018-10-25T03:11:10.023Z] 03:11:10     INFO - resource://testing-common/PromiseTestUtils.jsm:assertNoUncaughtRejections:257
[task 2018-10-25T03:11:10.023Z] 03:11:10     INFO - chrome://mochikit/content/browser-test.js:nextTest:735
[task 2018-10-25T03:11:10.024Z] 03:11:10     INFO - chrome://mochikit/content/browser-test.js:testScope/test_finish/<:1387
[task 2018-10-25T03:11:10.024Z] 03:11:10     INFO - chrome://mochikit/content/browser-test.js:run:1324
[task 2018-10-25T03:11:10.025Z] 03:11:10     INFO - GECKO(1061) | JavaScript error: resource://devtools/shared/base-loader.js -> resource://devtools/shared/layout/utils.js, line 730: NS_ERROR_NOT_AVAILABLE: Component returned failure code: 0x80040111 (NS_ERROR_NOT_AVAILABLE) [nsIDOMWindowUtils.getScrollbarSize]
[task 2018-10-25T03:11:10.026Z] 03:11:10     INFO - GECKO(1061) | MEMORY STAT | vsize 1823MB | residentFast 357MB | heapAllocated 138MB
[task 2018-10-25T03:11:10.027Z] 03:11:10     INFO - TEST-OK | devtools/client/inspector/rules/test/browser_rules_add-property-svg.js | took 1843ms
Flags: needinfo?(yzenevich)
Flags: needinfo?(yzenevich)
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/d56a4f994d89
only initialize accessibility actor when the toolbox opens or when a11y panel opens as a default panel. r=gl
https://hg.mozilla.org/integration/autoland/rev/73b9703078a1
split accessibility actors into their own files. r=gl
https://hg.mozilla.org/integration/autoland/rev/633d8d44ad65
lazy load modules in accessibility actors. r=gl
https://hg.mozilla.org/integration/autoland/rev/073f33ff1ecf
lazy load accessibility highlighter in accessibility walker actor. r=gl
Hey folks, it looks like there might be a regression due to this change. Can you take a look?

== Change summary for alert #17257 (as of Tue, 30 Oct 2018 17:39:36 GMT) ==

Regressions:

  8%  damp simple.styleeditor.reload.DAMP osx-10-10 opt e10s stylo         57.29 -> 61.94
  8%  damp simple.styleeditor.reload.DAMP windows10-64 pgo e10s stylo      29.02 -> 31.21
  3%  damp complicated.inspector.open.DAMP windows10-64 pgo e10s stylo     310.23 -> 318.02

Improvements:

  4%  damp complicated.netmonitor.open.DAMP windows10-64 pgo e10s stylo     266.64 -> 257.04
  2%  damp simple.webconsole.open.DAMP windows10-64 pgo e10s stylo          301.11 -> 294.78

For up to date results, see: https://treeherder.mozilla.org/perf.html#/alerts?id=17257
Flags: needinfo?(yura.zenevich)
Will take a look at it this week, thanks!
Flags: needinfo?(yura.zenevich) → needinfo?(yzenevich)
Some more research:

After 1st commit:

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=60c36c83841680966f36af81e6d88c31b965fb41&newProject=try&newRevision=033c1f2e0bce3cf9110723dfd4fc855dd451a5e8&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=12

Note this patch deals with moving the time of a11y serverside bits initialization, after the toolbox is opened.

After 2nd commit:

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=60c36c83841680966f36af81e6d88c31b965fb41&newProject=try&newRevision=636f9daee23b4594cf44835e73badf4644566739&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=12

After 3rd commit:

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=60c36c83841680966f36af81e6d88c31b965fb41&newProject=try&newRevision=47e5ce4192c5743d4fa6dd0023f8711c493b87ad&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=12

After 4th commit:

https://treeherder.mozilla.org/perf.html#/comparesubtest?originalProject=try&originalRevision=60c36c83841680966f36af81e6d88c31b965fb41&newProject=try&newRevision=f7249c7704f0e2f2003f22366a41c71e6c9c6153&originalSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&newSignature=f79b6a4f1f8f53326d3056f6c8008c0ff4de0a94&showOnlyImportant=1&framework=12

As mentioned above, the 1st commit moves initialization order, not necessarily the total time but in terms of rendering and displaying the toolbox it should be better.

Last commit definitely improves startup time because all of the highlighter machinery is not initialized on start, only lazilly.

Commits 2 and 3 do things like splitting a11y actors into separate files and lazyloading modules in them. They are not necessarily making things better, however that was the pattern used in making inspector startup time better by @gl.

I can try only landing 1 and 4 and see if things clearly get better, thoughts ?
Flags: needinfo?(yzenevich) → needinfo?(ystartsev)
One thing to note is that the alert was mostly windows (with styleeditor reload also showing up on osx). No alert was registered for linux. Maybe it could be interesting to rerun your pushes on windows?
I retriggered the jobs on osx and windows platforms, lets see how it looks after the test run
Flags: needinfo?(ystartsev)
You need to log in before you can comment on or make changes to this bug.