Improve performance of the accessibility highlighter.

RESOLVED FIXED in Firefox 66

Status

enhancement
RESOLVED FIXED
5 months ago
4 months ago

People

(Reporter: yzen, Assigned: yzen)

Tracking

unspecified
Firefox 66

Firefox Tracking Flags

(firefox66 fixed)

Details

Attachments

(3 attachments)

Assignee

Description

5 months ago
Right now there are some website that highlighter is can choke on with a lot of moving around with mouse (cnn.com is a great example). We can improve this by:
* moving image data traversal loop into the worker thread.
* do 1 accessible highlighter at a time (instead of highlighting every accessible on hover).
Assignee

Comment 4

5 months ago
Another major optimization is for text that is larger than minimum large size or if the document is zoomed in. Since when calculating contrast, we use original sizes, this results in unnecessary large arrays of image data. We can scale down the image to a point where its text is equal to the minimum large text size.
Attachment #9032725 - Attachment description: Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=pbro → Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes
Attachment #9032725 - Attachment description: Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes → Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes, ochameau
Assignee

Comment 5

4 months ago

(In reply to Yura Zenevich [:yzen] from comment #1)

Created attachment 9032724 [details]
Bug 1514856 - make sure a11y highlighter does not do unnecessary when
highlighted accessible gets updated. r=pbro

MozReview-Commit-ID: AhfECIMsDPm

Here are 2 profiles performing the traversal of the whole a11y tree in the panel twice (top to bottom and bottom to top):

Before: https://perfht.ml/2VWoDjb (traversal happens between ~6s and 10.5s)

After: https://perfht.ml/2VTxqT0 (traversal happens between 5s and 10s)

You can see jank (2 red markers) in the before profile. If you look at just those intervals (just zoom in bound by the jank), and reverse the call stack, you will see that most of that time is spent on calculating the RGBA values (min,max) for backgrounds. This jank is not present in the second profile because this patch prevents highlighting for accessible objects that are no longer considered "current".

Assignee

Comment 6

4 months ago

(In reply to Yura Zenevich [:yzen] from comment #2)

Created attachment 9032725 [details]
Bug 1514856 - move image data array traversal to worker thread when
calculating contrast ratio for text nodes. r=jdescottes, ochameau

MozReview-Commit-ID: K3twiMih7e9

Depends on D15112

Regarding the second patch, this is the best 2 profiles I could manage:

Before: https://perfht.ml/2QR6nEq
After: https://perfht.ml/2QQFX5w

They both describe the same action - highlighting of a fairly large text node. The noticeable difference is the much longer jank in the content process (server side). The the biggest contrast computation step runs in the worker (see "after"). There is still some jank due to image data generation that still happens on the main thread in content in both cases and in the future could potentially move into offscreen canvas if it ever supports 2D.

Assignee

Comment 7

4 months ago

(In reply to Yura Zenevich [:yzen] from comment #3)

Created attachment 9032726 [details]
Bug 1514856 - scale the text node snapshots down to normal size when text is
too big or is zoomed in. r=pbro

MozReview-Commit-ID: IzRo5nA94Fd

Depends on D15113

The latest profile: https://perfht.ml/2QRChAm has no jank visible with the "after" profile from comment 6.

Attachment #9032725 - Attachment description: Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes, ochameau → Bug 1514856 - move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes,ochameau
Attachment #9032726 - Attachment description: Bug 1514856 - scale the text node snapshots down to normal size when text is too big or is zoomed in. r=pbro → Bug 1514856 - scale the text node snapshots down to normal size when text is too big or is zoomed in. r=pbro MozReview-Commit-ID: IzRo5nA94Fd

Thanks a lot Yura for these reports, they are all perfect!

With such details, I'm highly confident that each individual patch is having the expected positive impact on performance.

Now, if you want to go further and ensure this work won't regress, you can contribute to DAMP to cover the accessibility panel and the few interactions you care about.

Attachment #9032726 - Attachment description: Bug 1514856 - scale the text node snapshots down to normal size when text is too big or is zoomed in. r=pbro MozReview-Commit-ID: IzRo5nA94Fd → Bug 1514856 - scale the text node snapshots down to normal size when text is too big or is zoomed in. r=pbro

Comment 9

4 months ago
Pushed by yura.zenevich@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/ee36ea46016d
make sure a11y highlighter does not do unnecessary when highlighted accessible gets updated. r=pbro
https://hg.mozilla.org/integration/autoland/rev/4fe5d0b72572
move image data array traversal to worker thread when calculating contrast ratio for text nodes. r=jdescottes,ochameau
https://hg.mozilla.org/integration/autoland/rev/50582093318a
scale the text node snapshots down to normal size when text is too big or is zoomed in. r=pbro

Comment 10

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Last Resolved: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 66
You need to log in before you can comment on or make changes to this bug.