Auto merge of #6778 - nox:childnodes, r=jdm

Optimise Node.childNodes



<!-- Reviewable:start -->
[<img src="https://reviewable.io/review_button.png" height=40 alt="Review on Reviewable"/>](https://reviewable.io/reviews/servo/servo/6778)
<!-- Reviewable:end -->

components/layout/parallel.rs

@@ -123,8 +123,7 @@ pub trait ParallelPreorderDomTraversal : PreorderDomTraversal {
             // Perform the appropriate traversal.
             self.process(node);
 
-            // NB: O(n).
+            let child_count = node.children_count();
-            let child_count = node.children().count();
 
             // Reset the count of children.
             {

components/layout/wrapper.rs

@@ -237,6 +237,10 @@ impl<'ln> LayoutNode<'ln> {
             self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node))
         }
     }
+
+    pub fn children_count(&self) -> u32 {
+        unsafe { self.node.children_count() }
+    }
 }
 
 impl<'ln> LayoutNode<'ln> {

components/script/dom/node.rs

@@ -40,7 +40,7 @@ use dom::element::{AttributeHandlers, Element, ElementCreator, ElementTypeId};
 use dom::element::ElementHelpers;
 use dom::eventtarget::{EventTarget, EventTargetTypeId};
 use dom::htmlelement::HTMLElementTypeId;
-use dom::nodelist::NodeList;
+use dom::nodelist::{NodeList, NodeListHelpers};
 use dom::processinginstruction::{ProcessingInstruction, ProcessingInstructionHelpers};
 use dom::text::Text;
 use dom::virtualmethods::{VirtualMethods, vtable_for};
@@ -107,6 +107,9 @@ pub struct Node {
     /// The live list of children return by .childNodes.
     child_list: MutNullableHeap<JS<NodeList>>,
 
+    /// The live count of children of this node.
+    children_count: Cell<u32>,
+
     /// A bitfield of flags for node items.
     flags: Cell<NodeFlags>,
 
@@ -437,6 +440,7 @@ pub trait NodeHelpers {
     fn type_id(self) -> NodeTypeId;
     fn len(self) -> u32;
     fn index(self) -> u32;
+    fn children_count(self) -> u32;
 
     fn owner_doc(self) -> Root<Document>;
     fn set_owner_doc(self, document: &Document);
@@ -574,7 +578,7 @@ impl<'a> NodeHelpers for &'a Node {
             NodeTypeId::CharacterData(_) => {
                 CharacterDataCast::to_ref(self).unwrap().Length()
             },
-            _ => self.children().count() as u32
+            _ => self.children_count(),
         }
     }
 
@@ -583,6 +587,10 @@ impl<'a> NodeHelpers for &'a Node {
         self.preceding_siblings().count() as u32
     }
 
+    fn children_count(self) -> u32 {
+        self.children_count.get()
+    }
+
     #[inline]
     fn is_anchor_element(self) -> bool {
         self.type_id == NodeTypeId::Element(ElementTypeId::HTMLElement(HTMLElementTypeId::HTMLAnchorElement))
@@ -1083,36 +1091,26 @@ pub fn from_untrusted_node_address(_runtime: *mut JSRuntime, candidate: Untruste
     }
 }
 
+#[allow(unsafe_code)]
 pub trait LayoutNodeHelpers {
-    #[allow(unsafe_code)]
     unsafe fn type_id_for_layout(&self) -> NodeTypeId;
 
-    #[allow(unsafe_code)]
     unsafe fn parent_node_ref(&self) -> Option<LayoutJS<Node>>;
-    #[allow(unsafe_code)]
     unsafe fn first_child_ref(&self) -> Option<LayoutJS<Node>>;
-    #[allow(unsafe_code)]
     unsafe fn last_child_ref(&self) -> Option<LayoutJS<Node>>;
-    #[allow(unsafe_code)]
     unsafe fn prev_sibling_ref(&self) -> Option<LayoutJS<Node>>;
-    #[allow(unsafe_code)]
     unsafe fn next_sibling_ref(&self) -> Option<LayoutJS<Node>>;
 
-    #[allow(unsafe_code)]
     unsafe fn owner_doc_for_layout(&self) -> LayoutJS<Document>;
 
-    #[allow(unsafe_code)]
     unsafe fn is_element_for_layout(&self) -> bool;
-    #[allow(unsafe_code)]
     unsafe fn get_flag(&self, flag: NodeFlags) -> bool;
-    #[allow(unsafe_code)]
     unsafe fn set_flag(&self, flag: NodeFlags, value: bool);
 
-    #[allow(unsafe_code)]
+    unsafe fn children_count(&self) -> u32;
+
     unsafe fn layout_data(&self) -> Ref<Option<LayoutData>>;
-    #[allow(unsafe_code)]
     unsafe fn layout_data_mut(&self) -> RefMut<Option<LayoutData>>;
-    #[allow(unsafe_code)]
     unsafe fn layout_data_unchecked(&self) -> *const Option<LayoutData>;
 
     fn get_hover_state_for_layout(&self) -> bool;
@@ -1191,6 +1189,12 @@ impl LayoutNodeHelpers for LayoutJS<Node> {
         (*this).flags.set(flags);
     }
 
+    #[inline]
+    #[allow(unsafe_code)]
+    unsafe fn children_count(&self) -> u32 {
+        (*self.unsafe_get()).children_count.get()
+    }
+
     #[inline]
     #[allow(unsafe_code)]
     unsafe fn layout_data(&self) -> Ref<Option<LayoutData>> {
@@ -1489,6 +1493,7 @@ impl Node {
             prev_sibling: Default::default(),
             owner_doc: MutNullableHeap::new(doc.map(JS::from_ref)),
             child_list: Default::default(),
+            children_count: Cell::new(0u32),
             flags: Cell::new(NodeFlags::new(type_id)),
 
             layout_data: LayoutDataRef::new(),
@@ -2411,7 +2416,7 @@ impl<'a> NodeMethods for &'a Node {
             }
 
             // Step 5.
-            if this.children().count() != node.children().count() {
+            if this.children_count() != node.children_count() {
                 return false;
             }
 
@@ -2564,6 +2569,30 @@ impl<'a> VirtualMethods for &'a Node {
         let eventtarget: &&EventTarget = EventTargetCast::from_borrowed_ref(self);
         Some(eventtarget as &VirtualMethods)
     }
+
+    fn children_changed(&self, mutation: &ChildrenMutation) {
+        if let Some(ref s) = self.super_type() {
+            s.children_changed(mutation);
+        }
+        match *mutation {
+            ChildrenMutation::Append { added, .. } |
+            ChildrenMutation::Insert { added, .. } |
+            ChildrenMutation::Prepend { added, .. } => {
+                self.children_count.set(
+                    self.children_count.get() + added.len() as u32);
+            },
+            ChildrenMutation::Replace { added, .. } => {
+                self.children_count.set(
+                    self.children_count.get() - 1u32 + added.len() as u32);
+            },
+            ChildrenMutation::ReplaceAll { added, .. } => {
+                self.children_count.set(added.len() as u32);
+            },
+        }
+        if let Some(list) = self.child_list.get().map(|list| list.root()) {
+            list.as_children_list().children_changed(mutation);
+        }
+    }
 }
 
 pub trait DisabledStateHelpers {

components/script/dom/nodelist.rs

@@ -2,19 +2,22 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
+use dom::bindings::codegen::Bindings::NodeBinding::NodeMethods;
 use dom::bindings::codegen::Bindings::NodeListBinding;
 use dom::bindings::codegen::Bindings::NodeListBinding::NodeListMethods;
 use dom::bindings::global::GlobalRef;
-use dom::bindings::js::{JS, Root};
+use dom::bindings::js::{JS, MutNullableHeap, Root};
 use dom::bindings::utils::{Reflector, reflect_dom_object};
-use dom::node::{Node, NodeHelpers};
+use dom::node::{ChildrenMutation, Node, NodeHelpers};
 use dom::window::Window;
 
+use std::cell::Cell;
+
 #[derive(JSTraceable)]
 #[must_root]
 pub enum NodeListType {
     Simple(Vec<JS<Node>>),
-    Children(JS<Node>)
+    Children(ChildrenList),
 }
 
 // https://dom.spec.whatwg.org/#interface-nodelist
@@ -45,7 +48,7 @@ impl NodeList {
     }
 
     pub fn new_child_list(window: &Window, node: &Node) -> Root<NodeList> {
-        NodeList::new(window, NodeListType::Children(JS::from_ref(node)))
+        NodeList::new(window, NodeListType::Children(ChildrenList::new(node)))
     }
 }
 
@@ -54,22 +57,17 @@ impl<'a> NodeListMethods for &'a NodeList {
     fn Length(self) -> u32 {
         match self.list_type {
             NodeListType::Simple(ref elems) => elems.len() as u32,
-            NodeListType::Children(ref node) => {
+            NodeListType::Children(ref list) => list.len(),
-                let node = node.root();
-                node.r().children().count() as u32
-            }
         }
     }
 
     // https://dom.spec.whatwg.org/#dom-nodelist-item
     fn Item(self, index: u32) -> Option<Root<Node>> {
         match self.list_type {
-            _ if index >= self.Length() => None,
+            NodeListType::Simple(ref elems) => {
-            NodeListType::Simple(ref elems) => Some(elems[index as usize].root()),
+                elems.get(index as usize).map(|node| Root::from_rooted(*node))
-            NodeListType::Children(ref node) => {
+            },
-                let node = node.root();
+            NodeListType::Children(ref list) => list.item(index),
-                node.r().children().nth(index as usize)
-            }
         }
     }
 
@@ -81,3 +79,193 @@ impl<'a> NodeListMethods for &'a NodeList {
     }
 }
 
+pub trait NodeListHelpers<'a> {
+    fn as_children_list(self) -> &'a ChildrenList;
+}
+
+impl<'a> NodeListHelpers<'a> for &'a NodeList {
+    fn as_children_list(self) -> &'a ChildrenList {
+        if let NodeListType::Children(ref list) = self.list_type {
+            list
+        } else {
+            panic!("called as_children_list() on a simple node list")
+        }
+    }
+}
+
+#[derive(JSTraceable)]
+#[must_root]
+pub struct ChildrenList {
+    node: JS<Node>,
+    last_visited: MutNullableHeap<JS<Node>>,
+    last_index: Cell<u32>,
+}
+
+impl ChildrenList {
+    fn new(node: &Node) -> ChildrenList {
+        let last_visited = node.GetFirstChild();
+        ChildrenList {
+            node: JS::from_ref(node),
+            last_visited:
+                MutNullableHeap::new(last_visited.as_ref().map(JS::from_rooted)),
+            last_index: Cell::new(0u32),
+        }
+    }
+
+    pub fn len(&self) -> u32 {
+        self.node.root().children_count()
+    }
+
+    pub fn item(&self, index: u32) -> Option<Root<Node>> {
+        // This always start traversing the children from the closest element
+        // among parent's first and last children and the last visited one.
+        let len = self.len() as u32;
+        if index >= len {
+            return None;
+        }
+        if index == 0u32 {
+            // Item is first child if any, not worth updating last visited.
+            return self.node.root().GetFirstChild();
+        }
+        let last_index = self.last_index.get();
+        if index == last_index {
+            // Item is last visited child, no need to update last visited.
+            return Some(self.last_visited.get().unwrap().root());
+        }
+        let last_visited = if index - 1u32 == last_index {
+            // Item is last visited's next sibling.
+            self.last_visited.get().unwrap().root().GetNextSibling().unwrap()
+        } else if last_index > 0 && index == last_index - 1u32 {
+            // Item is last visited's previous sibling.
+            self.last_visited.get().unwrap().root().GetPreviousSibling().unwrap()
+        } else if index > last_index {
+            if index == len - 1u32 {
+                // Item is parent's last child, not worth updating last visited.
+                return Some(self.node.root().GetLastChild().unwrap());
+            }
+            if index <= last_index + (len - last_index) / 2u32 {
+                // Item is closer to the last visited child and follows it.
+                self.last_visited.get().unwrap().root()
+                                 .inclusively_following_siblings()
+                                 .nth((index - last_index) as usize).unwrap()
+            } else {
+                // Item is closer to parent's last child and obviously
+                // precedes it.
+                self.node.root().GetLastChild().unwrap()
+                    .inclusively_preceding_siblings()
+                    .nth((len - index - 1u32) as usize).unwrap()
+            }
+        } else if index >= last_index / 2u32 {
+            // Item is closer to the last visited child and precedes it.
+            self.last_visited.get().unwrap().root()
+                             .inclusively_preceding_siblings()
+                             .nth((last_index - index) as usize).unwrap()
+        } else {
+            // Item is closer to parent's first child and obviously follows it.
+            debug_assert!(index < last_index / 2u32);
+            self.node.root().GetFirstChild().unwrap()
+                     .inclusively_following_siblings()
+                     .nth(index as usize)
+                     .unwrap()
+        };
+        self.last_visited.set(Some(JS::from_rooted(&last_visited)));
+        self.last_index.set(index);
+        Some(last_visited)
+    }
+
+    pub fn children_changed(&self, mutation: &ChildrenMutation) {
+        fn prepend(list: &ChildrenList, added: &[&Node], next: &Node) {
+            let len = added.len() as u32;
+            if len == 0u32 {
+                return;
+            }
+            let index = list.last_index.get();
+            if index < len {
+                list.last_visited.set(Some(JS::from_ref(added[index as usize])));
+            } else if index / 2u32 >= len {
+                // If last index is twice as large as the number of added nodes,
+                // updating only it means that less nodes will be traversed if
+                // caller is traversing the node list linearly.
+                list.last_index.set(len + index);
+            } else {
+                // If last index is not twice as large but still larger,
+                // it's better to update it to the number of added nodes.
+                list.last_visited.set(Some(JS::from_ref(next)));
+                list.last_index.set(len);
+            }
+        }
+
+        fn replace(list: &ChildrenList,
+                   prev: Option<&Node>,
+                   removed: &Node,
+                   added: &[&Node],
+                   next: Option<&Node>) {
+            let index = list.last_index.get();
+            if removed == &*list.last_visited.get().unwrap().root() {
+                let visited = match (prev, added, next) {
+                    (None, _, None) => {
+                        // Such cases where parent had only one child should
+                        // have been changed into ChildrenMutation::ReplaceAll
+                        // by ChildrenMutation::replace().
+                        unreachable!()
+                    },
+                    (_, [node, ..], _) => node,
+                    (_, [], Some(next)) => next,
+                    (Some(prev), [], None) => {
+                        list.last_index.set(index - 1u32);
+                        prev
+                    },
+                };
+                list.last_visited.set(Some(JS::from_ref(visited)));
+            } else {
+                match (prev, next) {
+                    (Some(_), None) => {},
+                    (None, Some(next)) => {
+                        list.last_index.set(index - 1);
+                        prepend(list, added, next);
+                    },
+                    (Some(_), Some(_)) => {
+                        list.reset();
+                    },
+                    (None, None) => unreachable!(),
+                }
+            }
+        }
+
+        match *mutation {
+            ChildrenMutation::Append { .. } => {},
+            ChildrenMutation::Insert { .. } => {
+                self.reset();
+            },
+            ChildrenMutation::Prepend { added, next } => {
+                prepend(self, added, next);
+            },
+            ChildrenMutation::Replace { prev, removed, added, next } => {
+                replace(self, prev, removed, added, next);
+            },
+            ChildrenMutation::ReplaceAll { added, .. } => {
+                let len = added.len();
+                let index = self.last_index.get();
+                if len == 0 {
+                    self.last_visited.set(None);
+                    self.last_index.set(0u32);
+                } else if index < len as u32 {
+                    self.last_visited.set(Some(JS::from_ref(added[index as usize])));
+                } else {
+                    // Setting last visited to parent's last child serves no purpose,
+                    // so the middle is arbitrarily chosen here in case the caller
+                    // wants random access.
+                    let middle = len / 2;
+                    self.last_visited.set(Some(JS::from_ref(added[middle])));
+                    self.last_index.set(middle as u32);
+                }
+            },
+        }
+    }
+
+    fn reset(&self) {
+        self.last_visited.set(
+            self.node.root().GetFirstChild().map(|node| JS::from_rooted(&node)));
+        self.last_index.set(0u32);
+    }
+}

components/script/lib.rs

@@ -22,6 +22,7 @@
 #![feature(plugin)]
 #![feature(ref_slice)]
 #![feature(rc_unique)]
+#![feature(slice_patterns)]
 #![feature(str_utf16)]
 #![feature(unicode)]
 #![feature(vec_push_all)]