Bug 1498235

Improve accessibility actor startup performance.

RESOLVED FIXED in Firefox 65

Get help with this page

Status

Product:

Component:

Type:

enhancement

Status:

RESOLVED FIXED

Opened:

10 months ago

Updated:

9 months ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

Version:

unspecified

Target:

Firefox 65

Points:

---

Blocks:

1488009

Dependency tree / graph

Firefox Tracking Flags

(firefox65 fixed)

Details

Attachments

(4 attachments, 1 obsolete attachment)

Bug 1498235 - only initialize accessibility actor when the toolbox opens or when a11y panel opens as a default panel. r?gl 10 months ago Yura Zenevich [:yzen] 46 bytes, text/x-phabricator-request		Details \| Review
Bug 1498235 - split accessibility actors into their own files. r?gl 10 months ago Yura Zenevich [:yzen] 46 bytes, text/x-phabricator-request		Details \| Review
Bug 1498235 - lazy load modules in accessibility actors. r?gl 10 months ago Yura Zenevich [:yzen] 46 bytes, text/x-phabricator-request		Details \| Review
Bug 1498235 - lazy load accessibility highlighter in accessibility walker actor. r?gl 10 months ago Yura Zenevich [:yzen] 46 bytes, text/x-phabricator-request		Details \| Review
Bug 1498235 - lazy load Services in accessibility walker actor. r?gl 10 months ago Yura Zenevich [:yzen] 46 bytes, text/x-phabricator-request		Details \| Review

Yura Zenevich [:yzen]

Assignee

Description

•

10 months ago

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.

Yura Zenevich [:yzen]

Assignee

Comment 1

•

10 months ago

Posted file Bug 1498235 - only initialize accessibility actor when the toolbox opens or when a11y panel opens as a default panel. r?gl — Details

MozReview-Commit-ID: CzOTdDDU8fv

Yura Zenevich [:yzen]

Assignee

Comment 2

•

10 months ago

Posted file Bug 1498235 - split accessibility actors into their own files. r?gl — Details

MozReview-Commit-ID: Gm7ju38mhdD

Depends on D9578

Yura Zenevich [:yzen]

Assignee

Comment 3

•

10 months ago

Posted file Bug 1498235 - lazy load modules in accessibility actors. r?gl — Details

MozReview-Commit-ID: 2qPBmhnd7tb

Depends on D9579

Yura Zenevich [:yzen]

Assignee

Comment 4

•

10 months ago

Posted file Bug 1498235 - lazy load accessibility highlighter in accessibility walker actor. r?gl — Details

MozReview-Commit-ID: 7xFlSocDPVG

Depends on D9580

Yura Zenevich [:yzen]

Assignee

Comment 5

•

10 months ago

Posted file Bug 1498235 - lazy load Services in accessibility walker actor. r?gl (obsolete) — Details

MozReview-Commit-ID: IDpbV4bWKEt

Depends on D9581

Pulsebot

Comment 6

•

10 months ago

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

Phabricator Automation

Updated

•

10 months ago

Attachment #9019469 - Attachment is obsolete: true

Natalia Csoregi [:nataliaCs]

Comment 7

•

10 months ago

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)

Yura Zenevich [:yzen]

Assignee

Updated

•

10 months ago

Flags: needinfo?(yzenevich)

Yura Zenevich [:yzen]

Assignee

Updated

•

10 months ago

Blocks: 1488009

Pulsebot

Comment 8

•

10 months ago

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

Andrei Ciure[:andrei_ciure]

Comment 9

•

10 months ago

bugherder

https://hg.mozilla.org/mozilla-central/rev/d56a4f994d89
https://hg.mozilla.org/mozilla-central/rev/73b9703078a1
https://hg.mozilla.org/mozilla-central/rev/633d8d44ad65
https://hg.mozilla.org/mozilla-central/rev/073f33ff1ecf

Status: ASSIGNED → RESOLVED

Closed: 10 months ago

status-firefox65: --- → fixed

Resolution: --- → FIXED

Target Milestone: --- → Firefox 65

Yulia Startsev [:yulia]

Comment 10

•

10 months ago

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)

Yura Zenevich [:yzen]

Assignee

Comment 11

•

10 months ago

Will take a look at it this week, thanks!

Yura Zenevich [:yzen]

Assignee

Updated

•

10 months ago

Flags: needinfo?(yura.zenevich) → needinfo?(yzenevich)

Yura Zenevich [:yzen]

Assignee

Comment 12

•

9 months ago

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)

Julian Descottes [:jdescottes] (PTO, back Aug 19th)

Comment 13

•

9 months ago

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?

Yulia Startsev [:yulia]

Comment 14

•

9 months ago

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.

Bugzilla

Improve accessibility actor startup performance.

Status

People

(Reporter: yzen, Assigned: yzen)

Tracking

Firefox Tracking Flags

(firefox65 fixed)

Details

Security

(public)

User Story

Attachments

(4 attachments, 1 obsolete attachment)

Description

Comment 1

Comment 2

Comment 3

Comment 4

Comment 5

Comment 6

Updated

Comment 7

Updated

Updated

Comment 8

Comment 9

Comment 10

Comment 11

Updated

Comment 12

Comment 13

Comment 14