563331 - Speed up or find alternative for HasRelatedContent/FindNeighbourPointingToNode

Status: RESOLVED WORKSFORME

(Core :: Disability Access APIs, defect)

Description

[David Bolter [:davidb] (NeedInfo me for attention)]

Spin off from bug 538594.

Brainstorm:
What if we perform the FindNeighbourPointingToNode more lazily? Like during GetName and GetDescription? We could do it once and set a member flag on the accessible object?

What if we just accept any node with an id? Probably the a11y tree will be too cluttered.

Comment 1

[alexander :surkov (:asurkov)]

ARIA attributes (like aria-labelledby, aria-describedby and etc) are used to create accessible relations for accessibles, for example,

<span id="label">it's a description</span>
<input aria-describedby="label">

We need to get related accessibles each by other, i.e. in the example above we need to get span by input and input by span.

As side effect of relation computation we make sure that node that is related with another one is accessible what leads perf problems since we inspect the subtree around the node to see if it has ARIA attribute. For example, mozilla mxr pages may be loading significant time.

We could cache either accessible relations or DOM structures that allows us to find related nodes.

We need to create relations when DOM document is loading. Also we need to use nsIMutationObserver to keep relation updated. The problem is if the document was loaded before a11y started then we need to either traverse the whole DOM document before accessible tree creation and cache relations or make sure we create whole accessible tree and remember elements with ID since they can be referred by ARIA attributes somewhere in the tree and then insert their accessibles into tree.

Comment 2

[alexander :surkov (:asurkov)]

this bug has a twin bug 381599

Comment 3

[David Bolter [:davidb] (NeedInfo me for attention)]

I'm not sure I agree with the morphing of this bug. How about morphing it back and doing any relation caching on that twin bug?

Comment 4

[alexander :surkov (:asurkov)]

(In reply to comment #3)
> I'm not sure I agree with the morphing of this bug. How about morphing it back
> and doing any relation caching on that twin bug?

I'm fine with that.

Comment 5

[alexander :surkov (:asurkov)]

done

Comment 6

[David Bolter [:davidb] (NeedInfo me for attention)]

Note Michael Kraft provides a good test case on dupe bug 581281:
"http://mxr.mozilla.org/mozilla1.9.2/source/browser/base/content/browser.js#5822
and then click and drag on the page, the browser will hang nearly 100% of time."
(He also graciously attaches a debug stack on that bug)

Alexander did you start a WIP for this one yet? I'm leaning towards taking an ax to HasRelatedContent ;)

Comment 8

[alexander :surkov (:asurkov)]

(In reply to comment #6)
> Alexander did you start a WIP for this one yet? I'm leaning towards taking an
> ax to HasRelatedContent ;)

No, especially if you keep this bug for functionality axing :) I'm planing to work on the problem in bug 573469.

Comment 9

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #8)
> (In reply to comment #6)
> > Alexander did you start a WIP for this one yet? I'm leaning towards taking an
> > ax to HasRelatedContent ;)
> 
> No, especially if you keep this bug for functionality axing :) I'm planing to
> work on the problem in bug 573469.

Great!

Comment 10

[David Bolter [:davidb] (NeedInfo me for attention)]

Approving blocking2.0 (nomination migrated from dependency bug 573469).

Comment 11

[David Bolter [:davidb] (NeedInfo me for attention)]

I'll own this for now.

Comment 12

[David Bolter [:davidb] (NeedInfo me for attention)]

Weird. I'm having trouble getting HasRelatedContent or FindNeighbourPointingToNode to show up in profiling... Is this now a WFM?

Comment 13

[David Bolter [:davidb] (NeedInfo me for attention)]

Still not seeing this on xperf (sure wish it showed the call stack like shark).

Comment 14

[alexander :surkov (:asurkov)]

(In reply to comment #12)
> Weird. I'm having trouble getting HasRelatedContent or
> FindNeighbourPointingToNode to show up in profiling... Is this now a WFM?

It can't be wfm even you don't have steps to reproduce. Obviously we traverse large hung of DOM tree for every node under certain circumstances.

Comment 15

[alexander :surkov (:asurkov)]

(In reply to comment #14)
> (In reply to comment #12)
> > Weird. I'm having trouble getting HasRelatedContent or
> > FindNeighbourPointingToNode to show up in profiling... Is this now a WFM?
> 
> It can't be wfm even you don't have steps to reproduce. Obviously we traverse
> large hung of DOM tree for every node under certain circumstances.

David, what's your testcase?

Comment 16

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #15)
> (In reply to comment #14)
> > (In reply to comment #12)
> > > Weird. I'm having trouble getting HasRelatedContent or
> > > FindNeighbourPointingToNode to show up in profiling... Is this now a WFM?
> > 
> > It can't be wfm even you don't have steps to reproduce. Obviously we traverse
> > large hung of DOM tree for every node under certain circumstances.
> 
> David, what's your testcase?

I simply don't have a good test case. I tried mxr for large files. I'm still looking for a good test case.

Comment 17

[Michael Kraft [:morac]]

I'm taking you can't reproduce this issue with the steps from comment #6?

Comment 18

[alexander :surkov (:asurkov)]

(In reply to comment #17)
> I'm taking you can't reproduce this issue with the steps from comment #6?

btw, today I saw perf problem on mxr pages with FindDescedantPointingToID as pointed in summary of bug 581281 (inside of accessible name calculation). I was confused by this bug summary and therefore I said I wasn't able to reproduce this bug when I talk to David on irc today.

I think the good testcase will be a huge amount of spans with ID.

Comment 19

[David Bolter [:davidb] (NeedInfo me for attention)]

(In reply to comment #17)
> I'm taking you can't reproduce this issue with the steps from comment #6?

That is correct, but I have only tried profiling with xperf. Michael can you still confirm the hang with Firefox trunk?
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

Comment 20

[Michael Kraft [:morac]]

(In reply to comment #19)
> That is correct, but I have only tried profiling with xperf. Michael can you
> still confirm the hang with Firefox trunk?
> http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-trunk/

No I can't get it to hang on the trunk load, but I never could even back when I filed bug 581281.  It would freeze for about 10 seconds, but it would recover. Now there's no freeze at all (at least on the PC I'm using currently).  

Maybe it was "accidentally" fixed elsewhere or the new optimized JavaScript engine is really that much better.

Comment 21

[David Bolter [:davidb] (NeedInfo me for attention)]

Unblocking 2.0. We can reconsider if the severity resurfaces.

Comment 22

[David Bolter [:davidb] (NeedInfo me for attention)]

I wonder if we can just check HasID for HTML, and keep our DOM look-around for XUL?

Comment 23

[David Bolter [:davidb] (NeedInfo me for attention)]

Note that question is for myself ;)

Comment 24

[alexander :surkov (:asurkov)]

HasRelatedContent is much improved by bug 573469. We still get used to traverse the tree from bottom to top to search active descendants.

FindNeighbourPointingToNode issue is fixed, it's completely removed in bug 381599.

We need to estimate our resources and mark this bug blocking if we can fix it in reasonable time. This is really wanted, without it fixed we can't be fast.

Comment 25

[alexander :surkov (:asurkov)]

eventually aria-activedescendant stuff referred in comment #24 were gone, not sure at which point. We can close this bug as worksforme.