Bug 126970 - [ATK] AXChildrenChanged notification handling is a performance black hole
Summary: [ATK] AXChildrenChanged notification handling is a performance black hole
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-01-14 01:57 PST by Zan Dobersek
Modified: 2014-01-14 07:41 PST (History)
9 users (show)

See Also:

Attachments
Patch proposal (4.77 KB, patch)
2014-01-14 02:03 PST, Mario Sanchez Prada 		no flags Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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.