Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923, 638474
Committed: https://crrev.com/36ef7b3f562ce421927e6abc941e716db2eb9dc6
Cr-Commit-Position: refs/heads/master@{#417073}

[dmazzoni, 2016-08-02 21:31:59]

The CQ bit was checked by dmazzoni@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-02 21:33:05]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[dmazzoni, 2016-08-02 21:34:54]

Description was changed from

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

BUG=631923
==========

to

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923
==========

[dmazzoni, 2016-08-02 21:34:54]

dmazzoni@chromium.org changed reviewers:
+ dtseng@chromium.org

[commit-bot: I haz the power, 2016-08-02 22:07:44]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-02 22:07:45]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[David Tseng, 2016-09-07 18:59:27]

lgtm

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/blink_ax_tree_source.cc (right):

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.cc:118: }
nit: indent

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.cc:155: 
Could we move this to the beginning of the function?

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/blink_ax_tree_source.h (right):

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.h:88: int
accessibility_focus_id_;
Can you remove this now that you keep around |focus_|?

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.h:94: blink::WebAXObject
focus_;
Suggestion: can we get a property accesser for this (similar to GetRoot) where
we CHECK(frozen) and use all three accessers in place of the private members?

[dmazzoni, 2016-09-07 19:29:35]

Description was changed from

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923
==========

to

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923,638474
==========

[dmazzoni, 2016-09-07 19:29:56]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-09-07 19:29:57]

The patchset sent to the CQ was uploaded after l-g-t-m from dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2205083002/#ps40001
(title: "Accessors for document, root, and focus")

[commit-bot: I haz the power, 2016-09-07 19:30:27]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 20:15:48]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-09-07 20:15:50]

Try jobs failed on following builders:
  linux_chromium_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[dmazzoni, 2016-09-07 20:53:02]

The CQ bit was checked by dmazzoni@chromium.org

[dmazzoni, 2016-09-07 20:53:03]

The patchset sent to the CQ was uploaded after l-g-t-m from dtseng@chromium.org
Link to the patchset: https://codereview.chromium.org/2205083002/#ps60001
(title: "Fix pdf accessibility")

[commit-bot: I haz the power, 2016-09-07 20:53:48]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-09-07 22:05:20]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-09-07 22:07:42]

Description was changed from

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923,638474
==========

to

==========
Optimize BlinkAXTreeSource by adding freeze/thaw for things like root, focus.

When profiling some sites that are slow to build an accessibility tree,
a lot of time was spent retrieving the document, root accessibility object,
or focused accessibility object. Introduce the concept of freezing
BlinkAXTreeSource, which precomputes these once, and then allows them to
be reused through the rest of a function scope.

This is safe because the accessibility tree does not change during processing
of an accessibility event.

For one cs.chromium.org page, this sped up initial load by 50% (1000 ms
to 650 ms) and single-node updates by 400% (90 ms to 23 ms).

BUG=631923,638474

Committed: https://crrev.com/36ef7b3f562ce421927e6abc941e716db2eb9dc6
Cr-Commit-Position: refs/heads/master@{#417073}
==========

[commit-bot: I haz the power, 2016-09-07 22:07:43]

Patchset 4 (id:??) landed as
https://crrev.com/36ef7b3f562ce421927e6abc941e716db2eb9dc6
Cr-Commit-Position: refs/heads/master@{#417073}

[dmazzoni, 2016-09-08 05:26:43]

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/blink_ax_tree_source.cc (right):

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.cc:118: }
On 2016/09/07 at 18:59:27, David Tseng wrote:
> nit: indent

I'm not 100% sure what indentation error you saw, but to be safe I just ran "git
cl format" and accepted all of its suggestions, so it should be fine now.

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.cc:155: 
On 2016/09/07 at 18:59:27, David Tseng wrote:
> Could we move this to the beginning of the function?

Sure, done.

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
File content/renderer/accessibility/blink_ax_tree_source.h (right):

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.h:88: int
accessibility_focus_id_;
On 2016/09/07 at 18:59:27, David Tseng wrote:
> Can you remove this now that you keep around |focus_|?

No, because on Android we still use accessibility focus,
and it might be different than focus.

https://codereview.chromium.org/2205083002/diff/1/content/renderer/accessibil...
content/renderer/accessibility/blink_ax_tree_source.h:94: blink::WebAXObject
focus_;
On 2016/09/07 at 18:59:27, David Tseng wrote:
> Suggestion: can we get a property accesser for this (similar to GetRoot) where
we CHECK(frozen) and use all three accessers in place of the private members?

Sure, good idea