Skip to content

Commit

Permalink
more efficient preorder DOM traversals
Browse files Browse the repository at this point in the history
  • Loading branch information
Clark Gaebel committed Oct 21, 2014
1 parent 156ca98 commit a5bb2f2
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 97 deletions.
87 changes: 45 additions & 42 deletions components/layout/wrapper.rs
View file
Expand Up @@ -214,28 +214,36 @@ impl<'ln> LayoutNode<'ln> {
}
}

/// Iterates over this node and all its descendants, in preorder.
///
/// FIXME(pcwalton): Terribly inefficient. We should use parallelism.
pub fn traverse_preorder(self) -> LayoutTreeIterator<'ln> {
let mut nodes = vec!();
gather_layout_nodes(self, &mut nodes, false);
LayoutTreeIterator::new(nodes)
LayoutTreeIterator::new(self)
}

fn last_child(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.get_jsmanaged().last_child_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

/// Returns an iterator over this node's children.
pub fn children(self) -> LayoutNodeChildrenIterator<'ln> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn first_child<T: TLayoutNode>(this: &T) -> Option<T> {
fn first_child<T: TLayoutNode>(this: T) -> Option<T> {
this.first_child()
}

LayoutNodeChildrenIterator {
current_node: first_child(&self),
current: first_child(self),
}
}

pub fn rev_children(self) -> LayoutNodeReverseChildrenIterator<'ln> {
LayoutNodeReverseChildrenIterator {
current: self.last_child()
}

}

pub unsafe fn get_jsmanaged<'a>(&'a self) -> &'a JS<Node> {
&self.node
}
Expand Down Expand Up @@ -291,6 +299,12 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
}
}

fn last_child(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.last_child_ref().map(|node| self.new_with_this_lifetime(&node))
}
}

fn prev_sibling(self) -> Option<LayoutNode<'ln>> {
unsafe {
self.node.prev_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
Expand Down Expand Up @@ -390,59 +404,48 @@ impl<'ln> TNode<'ln, LayoutElement<'ln>> for LayoutNode<'ln> {
}

pub struct LayoutNodeChildrenIterator<'a> {
current_node: Option<LayoutNode<'a>>,
current: Option<LayoutNode<'a>>,
}

impl<'a> Iterator<LayoutNode<'a>> for LayoutNodeChildrenIterator<'a> {
fn next(&mut self) -> Option<LayoutNode<'a>> {
let node = self.current_node.clone();
self.current_node = node.clone().and_then(|node| {
node.next_sibling()
});
let node = self.current;
self.current = node.and_then(|node| node.next_sibling());
node
}
}

pub struct LayoutNodeReverseChildrenIterator<'a> {
current: Option<LayoutNode<'a>>,
}

impl<'a> Iterator<LayoutNode<'a>> for LayoutNodeReverseChildrenIterator<'a> {
fn next(&mut self) -> Option<LayoutNode<'a>> {
let node = self.current;
self.current = node.and_then(|node| node.prev_sibling());
node
}
}

// FIXME: Do this without precomputing a vector of refs.
// Easy for preorder; harder for postorder.
//
// FIXME(pcwalton): Parallelism! Eventually this should just be nuked.
pub struct LayoutTreeIterator<'a> {
nodes: Vec<LayoutNode<'a>>,
index: uint,
stack: Vec<LayoutNode<'a>>,
}

impl<'a> LayoutTreeIterator<'a> {
fn new(nodes: Vec<LayoutNode<'a>>) -> LayoutTreeIterator<'a> {
fn new(root: LayoutNode<'a>) -> LayoutTreeIterator<'a> {
let mut stack = vec!();
stack.push(root);
LayoutTreeIterator {
nodes: nodes,
index: 0,
stack: stack
}
}
}

impl<'a> Iterator<LayoutNode<'a>> for LayoutTreeIterator<'a> {
fn next(&mut self) -> Option<LayoutNode<'a>> {
if self.index >= self.nodes.len() {
None
} else {
let v = self.nodes[self.index].clone();
self.index += 1;
Some(v)
}
}
}

/// FIXME(pcwalton): This is super inefficient.
fn gather_layout_nodes<'a>(cur: LayoutNode<'a>, refs: &mut Vec<LayoutNode<'a>>, postorder: bool) {
if !postorder {
refs.push(cur.clone());
}
for kid in cur.children() {
gather_layout_nodes(kid, refs, postorder)
}
if postorder {
refs.push(cur.clone());
let ret = self.stack.pop();
ret.map(|node| self.stack.extend(node.rev_children()));
ret
}
}

Expand Down
102 changes: 48 additions & 54 deletions components/script/dom/node.rs
View file
Expand Up @@ -381,6 +381,7 @@ impl<'a> PrivateNodeHelpers for JSRef<'a, Node> {
pub trait NodeHelpers<'a> {
fn ancestors(self) -> AncestorIterator<'a>;
fn children(self) -> AbstractNodeChildrenIterator<'a>;
fn rev_children(self) -> ReverseChildrenIterator<'a>;
fn child_elements(self) -> ChildElementIterator<'a>;
fn following_siblings(self) -> AbstractNodeChildrenIterator<'a>;
fn is_in_doc(self) -> bool;
Expand Down Expand Up @@ -441,7 +442,6 @@ pub trait NodeHelpers<'a> {
fn debug_str(self) -> String;

fn traverse_preorder(self) -> TreeIterator<'a>;
fn sequential_traverse_postorder(self) -> TreeIterator<'a>;
fn inclusively_following_siblings(self) -> AbstractNodeChildrenIterator<'a>;

fn to_trusted_node_address(self) -> TrustedNodeAddress;
Expand Down Expand Up @@ -658,21 +658,12 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {

/// Iterates over this node and all its descendants, in preorder.
fn traverse_preorder(self) -> TreeIterator<'a> {
let mut nodes = vec!();
gather_abstract_nodes(self, &mut nodes, false);
TreeIterator::new(nodes)
}

/// Iterates over this node and all its descendants, in postorder.
fn sequential_traverse_postorder(self) -> TreeIterator<'a> {
let mut nodes = vec!();
gather_abstract_nodes(self, &mut nodes, true);
TreeIterator::new(nodes)
TreeIterator::new(self)
}

fn inclusively_following_siblings(self) -> AbstractNodeChildrenIterator<'a> {
AbstractNodeChildrenIterator {
current_node: Some(self.clone()),
current: Some(self.clone()),
}
}

Expand All @@ -682,7 +673,7 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {

fn following_siblings(self) -> AbstractNodeChildrenIterator<'a> {
AbstractNodeChildrenIterator {
current_node: self.next_sibling().root().map(|next| next.clone()),
current: self.next_sibling().root().map(|next| next.clone()),
}
}

Expand Down Expand Up @@ -774,7 +765,13 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {

fn children(self) -> AbstractNodeChildrenIterator<'a> {
AbstractNodeChildrenIterator {
current_node: self.first_child.get().map(|node| (*node.root()).clone()),
current: self.first_child.get().map(|node| (*node.root()).clone()),
}
}

fn rev_children(self) -> ReverseChildrenIterator<'a> {
ReverseChildrenIterator {
current: self.last_child.get().map(|node| *node.root().deref()),
}
}

Expand Down Expand Up @@ -974,15 +971,25 @@ pub type ChildElementIterator<'a> = Map<'a, JSRef<'a, Node>,
Filter<'a, JSRef<'a, Node>, AbstractNodeChildrenIterator<'a>>>;

pub struct AbstractNodeChildrenIterator<'a> {
current_node: Option<JSRef<'a, Node>>,
current: Option<JSRef<'a, Node>>,
}

impl<'a> Iterator<JSRef<'a, Node>> for AbstractNodeChildrenIterator<'a> {
fn next(&mut self) -> Option<JSRef<'a, Node>> {
let node = self.current_node.clone();
self.current_node = node.clone().and_then(|node| {
node.next_sibling().map(|node| (*node.root()).clone())
});
let node = self.current;
self.current = node.and_then(|node| node.next_sibling().map(|node| *node.root().deref()));
node
}
}

pub struct ReverseChildrenIterator<'a> {
current: Option<JSRef<'a, Node>>,
}

impl<'a> Iterator<JSRef<'a, Node>> for ReverseChildrenIterator<'a> {
fn next(&mut self) -> Option<JSRef<'a, Node>> {
let node = self.current;
self.current = node.and_then(|node| node.prev_sibling().map(|node| *node.root().deref()));
node
}
}
Expand All @@ -993,43 +1000,32 @@ pub struct AncestorIterator<'a> {

impl<'a> Iterator<JSRef<'a, Node>> for AncestorIterator<'a> {
fn next(&mut self) -> Option<JSRef<'a, Node>> {
if self.current.is_none() {
return None;
}

// FIXME: Do we need two clones here?
let x = self.current.as_ref().unwrap().clone();
self.current = x.parent_node().map(|node| (*node.root()).clone());
Some(x)
let node = self.current;
self.current = node.and_then(|node| node.parent_node().map(|node| *node.root().deref()));
node
}
}

// FIXME: Do this without precomputing a vector of refs.
// Easy for preorder; harder for postorder.
pub struct TreeIterator<'a> {
nodes: Vec<JSRef<'a, Node>>,
index: uint,
stack: Vec<JSRef<'a, Node>>,
}

impl<'a> TreeIterator<'a> {
fn new(nodes: Vec<JSRef<'a, Node>>) -> TreeIterator<'a> {
fn new(root: JSRef<'a, Node>) -> TreeIterator<'a> {
let mut stack = vec!();
stack.push(root);

TreeIterator {
nodes: nodes,
index: 0,
stack: stack,
}
}
}

impl<'a> Iterator<JSRef<'a, Node>> for TreeIterator<'a> {
fn next(&mut self) -> Option<JSRef<'a, Node>> {
if self.index >= self.nodes.len() {
None
} else {
let v = self.nodes[self.index];
let v = v.clone();
self.index += 1;
Some(v)
}
let ret = self.stack.pop();
ret.map(|node| self.stack.extend(node.rev_children()));
ret
}
}

Expand Down Expand Up @@ -1116,18 +1112,6 @@ impl<'a> Iterator<JSRef<'a, Node>> for NodeIterator {
}
}

fn gather_abstract_nodes<'a>(cur: JSRef<'a, Node>, refs: &mut Vec<JSRef<'a, Node>>, postorder: bool) {
if !postorder {
refs.push(cur.clone());
}
for kid in cur.children() {
gather_abstract_nodes(kid, refs, postorder)
}
if postorder {
refs.push(cur.clone());
}
}

/// Specifies whether children must be recursively cloned or not.
#[deriving(PartialEq)]
pub enum CloneChildrenFlag {
Expand Down Expand Up @@ -2209,6 +2193,16 @@ impl<'a> style::TNode<'a, JSRef<'a, Element>> for JSRef<'a, Node> {
first_child(self).map(|node| *node.root())
}

fn last_child(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
fn last_child<'a, T: NodeHelpers<'a>>(this: T) -> Option<Temporary<Node>> {
this.last_child()
}

last_child(self).map(|node| *node.root())
}

fn prev_sibling(self) -> Option<JSRef<'a, Node>> {
// FIXME(zwarich): Remove this when UFCS lands and there is a better way
// of disambiguating methods.
Expand Down
2 changes: 1 addition & 1 deletion components/style/node.rs
View file
Expand Up @@ -13,6 +13,7 @@ use string_cache::{Atom, Namespace};
pub trait TNode<'a, E: TElement<'a>> : Clone + Copy {
fn parent_node(self) -> Option<Self>;
fn first_child(self) -> Option<Self>;
fn last_child(self) -> Option<Self>;
fn prev_sibling(self) -> Option<Self>;
fn next_sibling(self) -> Option<Self>;
fn is_document(self) -> bool;
Expand Down Expand Up @@ -57,4 +58,3 @@ pub trait TElementAttributes : Copy {
fn get_length_attribute(self, attribute: LengthAttribute) -> LengthOrPercentageOrAuto;
fn get_integer_attribute(self, attribute: IntegerAttribute) -> Option<i32>;
}

5 comments on commit a5bb2f2

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

saw approval from pcwalton
at cgaebel@a5bb2f2

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merging cgaebel/servo/efficient-preorder-traversal = a5bb2f2 into auto

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cgaebel/servo/efficient-preorder-traversal = a5bb2f2 merged ok, testing candidate = 2bc4ffe

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all tests pass:
success: http://build.servo.org/builders/linux/builds/385
success: http://build.servo.org/builders/mac/builds/392

@bors-servo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fast-forwarding master to auto = 2bc4ffe

Please sign in to comment.