New issue
Advanced search Search tips

Issue 921789 link

Starred by 1 user

Issue metadata

Status: Started
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

Large paragraph with explicit line breaks is slow for accessibility

Project Member Reported by dmazz...@chromium.org, Jan 14

Issue description

Example:

<p>
  Line 1
  <br>
  Line 2
  <br>
  ...
  <br>
  Line 2000
</p>

There are some O(n^2) loops that cause this type of page to get slower and slower as the size increases, when accessibility is enabled.

 
Labels: -Pri-3 Pri-2
See the attached file line-breaks.html as an example to test. It's very slow to load when accessibility is enabled.

line-breaks.html
153 KB View Download
Project Member

Comment 4 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98f103ba355d07d060516f769f7ce2a44697bdae

commit 98f103ba355d07d060516f769f7ce2a44697bdae
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Jan 16 18:04:07 2019

Replace calls to NodeIndex() with nextSibling/previousSibling.

NodeIndex() is O(n) and the difference is measurable if a single node
has many siblings, like in a paragraph with a lot of explicit line breaks.

Bug: 921789
Change-Id: I0540105cb2947efffda6958797d16cbcfaa2c583
Reviewed-on: https://chromium-review.googlesource.com/c/1410175
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623289}
[modify] https://crrev.com/98f103ba355d07d060516f769f7ce2a44697bdae/docs/accessibility/perf.md
[add] https://crrev.com/98f103ba355d07d060516f769f7ce2a44697bdae/third_party/blink/perf_tests/accessibility/line-breaks.html
[modify] https://crrev.com/98f103ba355d07d060516f769f7ce2a44697bdae/third_party/blink/renderer/modules/accessibility/ax_position.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Jan 16 (6 days ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c9218a18e39eb3e42245ee74f159594611d1e36f

commit c9218a18e39eb3e42245ee74f159594611d1e36f
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Wed Jan 16 22:36:11 2019

Fix bottleneck in AXObject::GetWordBoundaries

GetWordBoundaries is only used on an AXInlineTextBox, so it only needs
to return integer start and end offsets within a single node. It was
inefficient for it to return AXRanges, since AXRanges span multiple nodes
and do lots of validation and normalization.

This change leads to a ~3x speedup of this benchmark
(added in http://crrev.com/c/1410175):
  third_party/blink/perf_tests/accessibility/line-breaks.html

TBR=mkwst@chromium.org

Bug: 921789
Change-Id: I826feedc7863caa5ce4baa11de0971e68b1bf114
Reviewed-on: https://chromium-review.googlesource.com/c/1411212
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#623411}
[modify] https://crrev.com/c9218a18e39eb3e42245ee74f159594611d1e36f/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.cc
[modify] https://crrev.com/c9218a18e39eb3e42245ee74f159594611d1e36f/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.h
[modify] https://crrev.com/c9218a18e39eb3e42245ee74f159594611d1e36f/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/c9218a18e39eb3e42245ee74f159594611d1e36f/third_party/blink/renderer/modules/accessibility/ax_object.h
[modify] https://crrev.com/c9218a18e39eb3e42245ee74f159594611d1e36f/third_party/blink/renderer/modules/exported/web_ax_object.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Yesterday (30 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/3064da20981be73b7acf2128dd407a5f3da78165

commit 3064da20981be73b7acf2128dd407a5f3da78165
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Jan 22 00:10:04 2019

Revert "Fix bottleneck in AXObject::GetWordBoundaries"

This reverts commit c9218a18e39eb3e42245ee74f159594611d1e36f.

Reason for revert: Temporarily reverting in order to see
if the performance on chromeperf changes.

Original change's description:
> Fix bottleneck in AXObject::GetWordBoundaries
> 
> GetWordBoundaries is only used on an AXInlineTextBox, so it only needs
> to return integer start and end offsets within a single node. It was
> inefficient for it to return AXRanges, since AXRanges span multiple nodes
> and do lots of validation and normalization.
> 
> This change leads to a ~3x speedup of this benchmark
> (added in http://crrev.com/c/1410175):
>   third_party/blink/perf_tests/accessibility/line-breaks.html
> 
> TBR=mkwst@chromium.org
> 
> Bug: 921789
> Change-Id: I826feedc7863caa5ce4baa11de0971e68b1bf114
> Reviewed-on: https://chromium-review.googlesource.com/c/1411212
> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#623411}

TBR=dmazzoni@chromium.org,aboxhall@chromium.org,mkwst@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 921789
Change-Id: Icdb625c4563691cb4771dd829f49f06cc165181c
Reviewed-on: https://chromium-review.googlesource.com/c/1426170
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624700}
[modify] https://crrev.com/3064da20981be73b7acf2128dd407a5f3da78165/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.cc
[modify] https://crrev.com/3064da20981be73b7acf2128dd407a5f3da78165/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.h
[modify] https://crrev.com/3064da20981be73b7acf2128dd407a5f3da78165/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/3064da20981be73b7acf2128dd407a5f3da78165/third_party/blink/renderer/modules/accessibility/ax_object.h
[modify] https://crrev.com/3064da20981be73b7acf2128dd407a5f3da78165/third_party/blink/renderer/modules/exported/web_ax_object.cc

Project Member

Comment 7 by bugdroid1@chromium.org, Today (23 hours ago)

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/474733a1f1007ba9c84e27603863a27a8066d2b2

commit 474733a1f1007ba9c84e27603863a27a8066d2b2
Author: Dominic Mazzoni <dmazzoni@chromium.org>
Date: Tue Jan 22 07:13:23 2019

Reland "Fix bottleneck in AXObject::GetWordBoundaries"

This reverts commit 3064da20981be73b7acf2128dd407a5f3da78165.

Reason for revert: Purpose of original revert was just to
see that it was reflected on chromeperf. I can now confirm
that reverting this change does result in a significant
performance drop, so relanding it is worth it.

Original change's description:
> Revert "Fix bottleneck in AXObject::GetWordBoundaries"
> 
> This reverts commit c9218a18e39eb3e42245ee74f159594611d1e36f.
> 
> Reason for revert: Temporarily reverting in order to see
> if the performance on chromeperf changes.
> 
> Original change's description:
> > Fix bottleneck in AXObject::GetWordBoundaries
> > 
> > GetWordBoundaries is only used on an AXInlineTextBox, so it only needs
> > to return integer start and end offsets within a single node. It was
> > inefficient for it to return AXRanges, since AXRanges span multiple nodes
> > and do lots of validation and normalization.
> > 
> > This change leads to a ~3x speedup of this benchmark
> > (added in http://crrev.com/c/1410175):
> >   third_party/blink/perf_tests/accessibility/line-breaks.html
> > 
> > TBR=mkwst@chromium.org
> > 
> > Bug: 921789
> > Change-Id: I826feedc7863caa5ce4baa11de0971e68b1bf114
> > Reviewed-on: https://chromium-review.googlesource.com/c/1411212
> > Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> > Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> > Cr-Commit-Position: refs/heads/master@{#623411}
> 
> TBR=dmazzoni@chromium.org,aboxhall@chromium.org,mkwst@chromium.org
> 
> # Not skipping CQ checks because original CL landed > 1 day ago.
> 
> Bug: 921789
> Change-Id: Icdb625c4563691cb4771dd829f49f06cc165181c
> Reviewed-on: https://chromium-review.googlesource.com/c/1426170
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Reviewed-by: Alice Boxhall <aboxhall@chromium.org>
> Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#624700}

TBR=dmazzoni@chromium.org,aboxhall@chromium.org,mkwst@chromium.org

Change-Id: I775847259836684f1212610c66f5b0f0cbf3893c
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 921789
Reviewed-on: https://chromium-review.googlesource.com/c/1426520
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Commit-Queue: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#624737}
[modify] https://crrev.com/474733a1f1007ba9c84e27603863a27a8066d2b2/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.cc
[modify] https://crrev.com/474733a1f1007ba9c84e27603863a27a8066d2b2/third_party/blink/renderer/modules/accessibility/ax_inline_text_box.h
[modify] https://crrev.com/474733a1f1007ba9c84e27603863a27a8066d2b2/third_party/blink/renderer/modules/accessibility/ax_object.cc
[modify] https://crrev.com/474733a1f1007ba9c84e27603863a27a8066d2b2/third_party/blink/renderer/modules/accessibility/ax_object.h
[modify] https://crrev.com/474733a1f1007ba9c84e27603863a27a8066d2b2/third_party/blink/renderer/modules/exported/web_ax_object.cc

Sign in to add a comment