Skip to content

GitLab

T
tangled
Commit 99c20732 authored by dominicc's avatar dominicc Committed by Commit bot
Browse files
Options

Use the sibling limit to decide whether to create layout for whitespace 

Text::textLayoutObjectIsNeeded is an optimization which avoids
creating layout objects for whitespace text nodes. However
LayoutTreeBuilderTraversal::nextSiblingLayoutObject traverses all
siblings. For long lists of N space-separated siblings, this
degenerates into an N^2 DOM walk. Oops.

Text::textLayoutObjectIsNeeded already has a sibling limit; this
passes the limit through to
prevSiblingLayoutObject/nextSiblingLayoutObject.

BUG=650938

Review-Url: https://codereview.chromium.org/2379483002
Cr-Commit-Position: refs/heads/master@{#422631}
parent 89b1e152
Hide whitespace changes
Inline Side-by-side
Showing with 51 additions and 11 deletions
third_party/WebKit/PerformanceTests/DOM/long-sibling-list.html 0 → 100644
View file @ 99c20732
<!DOCTYPE html>
<script src="../resources/runner.js"></script>
<div id="container"></div>
<script>
'use strict';
PerfTestRunner.measureRunsPerSecond({
description: 'Adds, then lays out, a long list of sibling elements ' +
'separated by spaces',
setup: () => {
container.parentNode.replaceChild(container.cloneNode(false), container);
PerfTestRunner.gc();
},
run: () => {
const num_words = 1000;
for (let i = 0; i < num_words; i++) {
let a = document.createElement('a');
a.append(i);
container.append(a);
container.append(' ');
}
PerfTestRunner.forceLayout();
}
});
</script>
third_party/WebKit/Source/core/dom/LayoutTreeBuilderTraversal.cpp
View file @ 99c20732
......@@ -214,8 +214,10 @@ Node* next(const Node& node, const Node* stayWithin) {
return nextSkippingChildren(node, stayWithin);
}
LayoutObject* nextSiblingLayoutObject(const Node& node) {
for (Node* sibling = LayoutTreeBuilderTraversal::nextSibling(node); sibling;
LayoutObject* nextSiblingLayoutObject(const Node& node, int32_t limit) {
DCHECK(limit == kTraverseAllSiblings || limit >= 0) << limit;
for (Node* sibling = LayoutTreeBuilderTraversal::nextSibling(node);
sibling && limit-- != 0;
sibling = LayoutTreeBuilderTraversal::nextSibling(*sibling)) {
LayoutObject* layoutObject = sibling->layoutObject();
if (layoutObject && !isLayoutObjectReparented(layoutObject))
......@@ -224,9 +226,10 @@ LayoutObject* nextSiblingLayoutObject(const Node& node) {
return 0;
}
LayoutObject* previousSiblingLayoutObject(const Node& node) {
LayoutObject* previousSiblingLayoutObject(const Node& node, int32_t limit) {
DCHECK(limit == kTraverseAllSiblings || limit >= 0) << limit;
for (Node* sibling = LayoutTreeBuilderTraversal::previousSibling(node);
sibling;
sibling && limit-- != 0;
sibling = LayoutTreeBuilderTraversal::previousSibling(*sibling)) {
LayoutObject* layoutObject = sibling->layoutObject();
if (layoutObject && !isLayoutObjectReparented(layoutObject))
......
third_party/WebKit/Source/core/dom/LayoutTreeBuilderTraversal.h
View file @ 99c20732
......@@ -30,6 +30,7 @@
#include "core/CoreExport.h"
#include "core/dom/Element.h"
#include "core/dom/shadow/InsertionPoint.h"
#include <cstdint>
namespace blink {
......@@ -37,6 +38,8 @@ class LayoutObject;
namespace LayoutTreeBuilderTraversal {
const int32_t kTraverseAllSiblings = -2;
class ParentDetails {
STACK_ALLOCATED();
......@@ -62,8 +65,10 @@ Node* previousSibling(const Node&);
Node* previous(const Node&, const Node* stayWithin);
Node* next(const Node&, const Node* stayWithin);
Node* nextSkippingChildren(const Node&, const Node* stayWithin);
LayoutObject* nextSiblingLayoutObject(const Node&);
LayoutObject* previousSiblingLayoutObject(const Node&);
LayoutObject* nextSiblingLayoutObject(const Node&,
int32_t limit = kTraverseAllSiblings);
LayoutObject* previousSiblingLayoutObject(const Node&,
int32_t limit = kTraverseAllSiblings);
LayoutObject* nextInTopLayer(const Element&);
inline Element* parentElement(const Node& node) {
......
third_party/WebKit/Source/core/dom/Text.cpp
View file @ 99c20732
......@@ -286,8 +286,13 @@ bool Text::textLayoutObjectIsNeeded(const ComputedStyle& style,
if (document().childNeedsDistributionRecalc())
return true;
// Avoiding creation of a layoutObject for the text node is a non-essential memory optimization.
// So to avoid blowing up on very wide DOMs, we limit the number of siblings to visit.
unsigned maxSiblingsToVisit = 50;
const LayoutObject* prev =
LayoutTreeBuilderTraversal::previousSiblingLayoutObject(*this);
LayoutTreeBuilderTraversal::previousSiblingLayoutObject(
*this, maxSiblingsToVisit);
if (prev && prev->isBR()) // <span><br/> <br/></span>
return false;
......@@ -306,11 +311,13 @@ bool Text::textLayoutObjectIsNeeded(const ComputedStyle& style,
unsigned maxSiblingsToVisit = 50;
LayoutObject* first = parent.slowFirstChild();
while (first && first->isFloatingOrOutOfFlowPositioned() &&
maxSiblingsToVisit--)
first = first->nextSibling();
for (; first && first->isFloatingOrOutOfFlowPositioned() &&
maxSiblingsToVisit;
first = first->nextSibling(), --maxSiblingsToVisit) {
}
if (!first || first == layoutObject() ||
LayoutTreeBuilderTraversal::nextSiblingLayoutObject(*this) == first) {
LayoutTreeBuilderTraversal::nextSiblingLayoutObject(
*this, maxSiblingsToVisit) == first) {
// If we're adding children to this flow our previous siblings are not in
// the layout tree yet so we cannot know if we will be the first child in
// the line and collapse away. We have to assume we need a layout object.
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment