Bug 126970

Summary: [ATK] AXChildrenChanged notification handling is a performance black hole
Product: WebKit Reporter: Zan Dobersek <zan>
Component: WebKitGTKAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, cfleizach, commit-queue, dmazzoni, jcraig, jdiggs, mario, samuel_white
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch proposal none

Description Zan Dobersek 2014-01-14 01:57:04 PST 
in AXObjectCache::postPlatformNotification, handling the AXChildrenChanged is causing great performace reduction when loading the single-page HTML Living Standard document.
http://trac.webkit.org/browser/trunk/Source/WebCore/accessibility/atk/AXObjectCacheAtk.cpp#L196
http://www.whatwg.org/specs/web-apps/current-work/
Comment 1 Zan Dobersek 2014-01-14 01:58:53 PST 
Commenting out that code block reduces the time of load from 60 seconds down to 15 seconds.

The ref-deref cycle is fired more than a million times during the load, and AccessibilityObject::updateBackingStore() is called twice per cycle, causing additional work that hinders the performance.
Comment 2 Mario Sanchez Prada 2014-01-14 02:03:58 PST 
Created attachment 221130 [details]
Patch proposal

This should be enough to fix this issue. Sorry for the issue in the first place (and thanks Zan for reporting it)
Comment 3 Gustavo Noronha (kov) 2014-01-14 05:40:21 PST 
Comment on attachment 221130 [details]
Patch proposal

View in context: https://bugs.webkit.org/attachment.cgi?id=221130&action=review

> LayoutTests/accessibility/children-changed-sends-notification-expected.txt:12
> -PARAGRAPH notification: AXChildrenAdded
> -GLOBAL notification: AXChildrenAdded on element with role AXRole: AXParagraph
>  PARAGRAPH notification: AXChildrenRemoved
>  GLOBAL notification: AXChildrenRemoved on element with role AXRole: AXParagraph
> +PARAGRAPH notification: AXChildrenAdded
> +GLOBAL notification: AXChildrenAdded on element with role AXRole: AXParagraph

Does this not affect Mac? Is this test specific for ATK?
Comment 4 Mario Sanchez Prada 2014-01-14 07:12:11 PST 
Comment on attachment 221130 [details]
Patch proposal

(In reply to comment #3)
> (From update of attachment 221130 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=221130&action=review
> 
> > LayoutTests/accessibility/children-changed-sends-notification-expected.txt:12
> > -PARAGRAPH notification: AXChildrenAdded
> > -GLOBAL notification: AXChildrenAdded on element with role AXRole: AXParagraph
> >  PARAGRAPH notification: AXChildrenRemoved
> >  GLOBAL notification: AXChildrenRemoved on element with role AXRole: AXParagraph
> > +PARAGRAPH notification: AXChildrenAdded
> > +GLOBAL notification: AXChildrenAdded on element with role AXRole: AXParagraph
> 
> Does this not affect Mac? Is this test specific for ATK?

The mac is currently skipping this test, so yes, this test is specific to ATK.

Thanks for the review!
Comment 5 WebKit Commit Bot 2014-01-14 07:41:24 PST 
Comment on attachment 221130 [details]
Patch proposal

Clearing flags on attachment: 221130

Committed r161966: <http://trac.webkit.org/changeset/161966>
Comment 6 WebKit Commit Bot 2014-01-14 07:41:26 PST 
All reviewed patches have been landed.  Closing bug.