chromium/src
0
Fork 0
Code Activity

Replace calls to NodeIndex() with nextSibling/previousSibling.

Browse Source 
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}
This commit is contained in:
Dominic Mazzoni
2019-01-16 18:04:07 +00:00
committed by Commit Bot
parent a8eadaf186
commit 98f103ba35
3 changed files with 67 additions and 3 deletions
docs/accessibility
perf.md
third_party/blink
perf_tests
accessibility
line-breaks.html
renderer
modules
accessibility
ax_position.cc

8
docs/accessibility/perf.md

@ -37,6 +37,10 @@ locally using the currently installed Canary browser:
```tools/perf/run_benchmark system_health.common_desktop --story-filter="accessibility.*" --browser canary```
To run the same set of tests on your own compiled version of Chrome:
```tools/perf/run_benchmark system_health.common_desktop --story-filter="accessibility.*" --browser=exact --browser-executable=out/Release/chrome```
See the [documentation](https://github.com/catapult-project/catapult/blob/master/telemetry/docs/run_benchmarks_locally.md)
or command-line help for tools/perf/run_benchmark for
more command-line arguments.
@ -61,6 +65,10 @@ in isolation. You can find these tests here:
```third_party/blink/perf_tests/accessibility/```
Example command line to run these tests locally on your own compiled Chrome:
```tools/perf/run_benchmark blink_perf.accessibility --browser=exact --browser-executable=out/Release/chrome```
## Results
The results can be found at

57
third_party/blink/perf_tests/accessibility/line-breaks.html vendored Normal file

@ -0,0 +1,57 @@
<!DOCTYPE html>
<html>
<body>
<script src="../resources/runner.js"></script>
<p id="testElement" spellcheck=false style="height:90vh"></p>
<script>
var isDone = false;
var startTime;
function randomString() {
return '' + Math.floor(1000000 * Math.random());
}
function runTest() {
if (startTime) {
PerfTestRunner.measureValueAsync(PerfTestRunner.now() - startTime);
PerfTestRunner.addRunTestEndMarker();
}
if (!isDone) {
PerfTestRunner.addRunTestStartMarker();
startTime = PerfTestRunner.now();
// Fill the paragraph with 1000 lines of text with <br> elements
// in-between.
var testElement = document.getElementById('testElement');
var html = '';
for (var i = 0; i < 1000; i++) {
html += randomString() + ' ' + randomString() + '<br>';
}
testElement.innerHTML = html;
// Wait to allow the asynchronous accessibility code that's
// covered by traceEventsToMeasure to have a chance to run.
setTimeout(runTest, 1500);
}
}
PerfTestRunner.startMeasureValuesAsync({
description: 'Test accessibility performance of many line breaks in one paragraph.',
unit: 'ms',
done: function () {
isDone = true;
},
run: function() {
runTest();
},
iterationCount: 6,
tracingCategories: 'accessibility',
traceEventsToMeasure: [
'RenderAccessibilityImpl::SendPendingAccessibilityEvents',
]
});
</script>
</html>

5
third_party/blink/renderer/modules/accessibility/ax_position.cc vendored

@ -661,7 +661,7 @@ const PositionWithAffinity AXPosition::ToPositionWithAffinity(
DCHECK(child_node) << "AX objects used in AX positions that are valid "
"DOM positions should always be connected to their "
"DOM nodes.";
if (child_node->NodeIndex() == 0) {
if (!child_node->previousSibling()) {
// Creates a |PositionAnchorType::kBeforeChildren| position.
container_node = child_node->parentNode();
DCHECK(container_node);
@ -683,8 +683,7 @@ const PositionWithAffinity AXPosition::ToPositionWithAffinity(
"connected to their DOM nodes.";
// Check if this is an "after children" position in the DOM as well.
if ((last_child_node->NodeIndex() + 1) ==
container_node->CountChildren()) {
if (!last_child_node->nextSibling()) {
// Creates a |PositionAnchorType::kAfterChildren| position.
container_node = last_child_node->parentNode();
DCHECK(container_node);