[CTW] Terrible performance in NVDA vbuf loading using View Source
Categories
(Core :: Disability Access APIs, defect)
Tracking
()
Tracking | Status | |
---|---|---|
firefox102 | --- | fixed |
People
(Reporter: Jamie, Assigned: Jamie)
References
(Blocks 1 open bug)
Details
(Whiteboard: [ctw-m2])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
Details |
STR (with NVDA):
- Enable CTW and restart Firefox.
- Open https://www.jantrid.net/
- View source.
- Press NVDA+f5 to refresh NVDA's buffer.
- Expected: It takes a fraction of a second.
- Actual: It takes more than a second.
- Repeat with CTW disabled.
- Observe: It takes a fraction of a second.
This is the first time I've seen CTW perform worse than non-CTW on Windows. However, it's likely that there are significant perf gains to be had across the board based on the learnings from this.
Profile: https://share.firefox.dev/3LnP6zF
This is much, much worse on bigger pages. That page only has a few hundred lines of code. Even on a Bugzilla bug with about 20 comments, there are several thousand lines of code, and the buffer takes minutes to render.
In the spirit of "start simple and optimise later", I didn't port the HyperText offset cache from HyperTextAccessible when I was implementing HyperTextAccessibleBase. It wasn't clear to me how much we needed this optimisation. Well, it turns out we do.
The thing that's unique about view source is that it has one single container with many children (6403 in this example). That's because of all the syntax highlighting, which creates many little text nodes. Because this is such a large HyperText container, calculating the offset for each node is pretty expensive, since we have to walk through the child nodes accumulating the text length. That's O(n^2) at best, and on a page like this, n is biiig.
The algorithm for this offset cache isn't particularly difficult. What's tricky here is that it's hard to use a common cache across the local and remote code because we cache differently for local and remote. Local doesn't have mCachedFields, which is a nice place to store this so we don't waste memory in RemoteAccessibles that don't need this.
Assignee | ||
Comment 1•3 years ago
|
||
While caching HyperText offsets will obviously improve this, this profile clearly shows that searching cache hash tables definitely incurs a cost. That's the nature of hash tables, of course, but I don't think I realised how high that cost could get.
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Updated•3 years ago
|
Assignee | ||
Comment 2•3 years ago
|
||
We already have a similar cache in local HyperTextAccessible.
Unfortunately, we can't use common code here because we don't want to waste memory having a member variable on all RemoteAccessibles, but local HyperTextAccessibles don't have mCachedFields.
This improves performance significantly when walking the text attributes of a container with a large number of text leaf children, such as is encountered when using view source.
Comment 4•3 years ago
|
||
Backed out for causing build bustages.
Backout link: https://hg.mozilla.org/integration/autoland/rev/b6daea0eb787ae4b418eb494e2d25f3b2444325c
Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&revision=4e3e696ff0543b405ba49b65cbdd278b322b0dde
Failure log: https://treeherder.mozilla.org/logviewer?job_id=376564838&repo=autoland&lineNumber=7803
Comment 7•3 years ago
|
||
Backed out for causing marionette failures.
Backout link: https://hg.mozilla.org/integration/autoland/rev/f83a7083c09a707cc27851fef33b652027d7d345
Failure log: https://treeherder.mozilla.org/logviewer?job_id=376901537&repo=autoland&lineNumber=26687
Assignee | ||
Updated•3 years ago
|
Comment 9•3 years ago
|
||
bugherder |
Updated•2 years ago
|
Assignee | ||
Updated•2 years ago
|
Description
•