auto merge of #3835 : pcwalton/servo/script-microoptzns, r=Ms2ger

This is a grab bag of various microoptimizations for script that I came across when profiling our performance on RoboHornet.

r? @jdm

components/script/dom/bindings/js.rs

@@ -53,7 +53,8 @@ use js::jsapi::JSObject;
 use layout_interface::TrustedNodeAddress;
 use script_task::StackRoots;
 
-use std::cell::{Cell, RefCell};
+use servo_util::smallvec::{SmallVec, SmallVec16};
+use std::cell::{Cell, UnsafeCell};
 use std::default::Default;
 use std::kinds::marker::ContravariantLifetime;
 use std::mem;
@@ -419,14 +420,14 @@ impl<T: Assignable<U>, U: Reflectable> TemporaryPushable<T> for Vec<JS<U>> {
 
 /// An opaque, LIFO rooting mechanism.
 pub struct RootCollection {
-    roots: RefCell<Vec<*mut JSObject>>,
+    roots: UnsafeCell<SmallVec16<*mut JSObject>>,
 }
 
 impl RootCollection {
     /// Create an empty collection of roots
     pub fn new() -> RootCollection {
         RootCollection {
-            roots: RefCell::new(vec!()),
+            roots: UnsafeCell::new(SmallVec16::new()),
         }
     }
 
@@ -438,17 +439,23 @@ impl RootCollection {
 
     /// Track a stack-based root to ensure LIFO root ordering
     fn root<'a, 'b, T: Reflectable>(&self, untracked: &Root<'a, 'b, T>) {
-        let mut roots = self.roots.borrow_mut();
+        unsafe {
-        roots.push(untracked.js_ptr);
+            let roots = self.roots.get();
+            (*roots).push(untracked.js_ptr);
             debug!("  rooting {:?}", untracked.js_ptr);
         }
+    }
 
     /// Stop tracking a stack-based root, asserting if LIFO root ordering has been violated
     fn unroot<'a, 'b, T: Reflectable>(&self, rooted: &Root<'a, 'b, T>) {
-        let mut roots = self.roots.borrow_mut();
+        unsafe {
-        debug!("unrooting {:?} (expecting {:?}", roots.last().unwrap(), rooted.js_ptr);
+            let roots = self.roots.get();
-        assert!(*roots.last().unwrap() == rooted.js_ptr);
+            debug!("unrooting {:?} (expecting {:?}",
-        roots.pop().unwrap();
+                   (*roots).as_slice().last().unwrap(),
+                   rooted.js_ptr);
+            assert!(*(*roots).as_slice().last().unwrap() == rooted.js_ptr);
+            (*roots).pop().unwrap();
+        }
     }
 }
 

components/script/dom/bindings/trace.rs

@@ -39,7 +39,7 @@ use msg::constellation_msg::{PipelineId, SubpageId, WindowSizeData};
 use net::image_cache_task::ImageCacheTask;
 use script_traits::ScriptControlChan;
 use std::collections::hashmap::HashMap;
-use collections::hash::Hash;
+use collections::hash::{Hash, Hasher};
 use style::PropertyDeclarationBlock;
 use std::comm::{Receiver, Sender};
 use string_cache::{Atom, Namespace};
@@ -170,7 +170,9 @@ impl<T: JSTraceable> JSTraceable for Option<T> {
     }
 }
 
-impl<K: Eq+Hash+JSTraceable, V: JSTraceable> JSTraceable for HashMap<K, V> {
+impl<K,V,S,H> JSTraceable for HashMap<K, V, H> where K: Eq + Hash<S> + JSTraceable,
+                                                     V: JSTraceable,
+                                                     H: Hasher<S> {
     #[inline]
     fn trace(&self, trc: *mut JSTracer) {
         for e in self.iter() {

components/script/dom/eventtarget.rs

@@ -18,6 +18,7 @@ use dom::xmlhttprequest::XMLHttpRequestId;
 use dom::virtualmethods::VirtualMethods;
 use js::jsapi::{JS_CompileUCFunction, JS_GetFunctionObject, JS_CloneFunctionObject};
 use js::jsapi::{JSContext, JSObject};
+use servo_util::fnv::FnvHasher;
 use servo_util::str::DOMString;
 use libc::{c_char, size_t};
 use std::ptr;
@@ -69,7 +70,7 @@ pub struct EventListenerEntry {
 pub struct EventTarget {
     type_id: EventTargetTypeId,
     reflector_: Reflector,
-    handlers: DOMRefCell<HashMap<DOMString, Vec<EventListenerEntry>>>,
+    handlers: DOMRefCell<HashMap<DOMString, Vec<EventListenerEntry>, FnvHasher>>,
 }
 
 impl EventTarget {
@@ -77,7 +78,7 @@ impl EventTarget {
         EventTarget {
             type_id: type_id,
             reflector_: Reflector::new(),
-            handlers: DOMRefCell::new(HashMap::new()),
+            handlers: DOMRefCell::new(HashMap::with_hasher(FnvHasher)),
         }
     }
 

components/script/dom/htmltabledatacellelement.rs

@@ -32,9 +32,13 @@ impl HTMLTableDataCellElement {
     }
 
     #[allow(unrooted_must_root)]
-    pub fn new(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>) -> Temporary<HTMLTableDataCellElement> {
+    pub fn new(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>)
-        let element = HTMLTableDataCellElement::new_inherited(localName, prefix, document);
+               -> Temporary<HTMLTableDataCellElement> {
-        Node::reflect_node(box element, document, HTMLTableDataCellElementBinding::Wrap)
+        Node::reflect_node(box HTMLTableDataCellElement::new_inherited(localName,
+                                                                       prefix,
+                                                                       document),
+                           document,
+                           HTMLTableDataCellElementBinding::Wrap)
     }
 }
 

components/script/dom/htmltablerowelement.rs

@@ -32,9 +32,11 @@ impl HTMLTableRowElement {
     }
 
     #[allow(unrooted_must_root)]
-    pub fn new(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>) -> Temporary<HTMLTableRowElement> {
+    pub fn new(localName: DOMString, prefix: Option<DOMString>, document: JSRef<Document>)
-        let element = HTMLTableRowElement::new_inherited(localName, prefix, document);
+               -> Temporary<HTMLTableRowElement> {
-        Node::reflect_node(box element, document, HTMLTableRowElementBinding::Wrap)
+        Node::reflect_node(box HTMLTableRowElement::new_inherited(localName, prefix, document),
+                           document,
+                           HTMLTableRowElementBinding::Wrap)
     }
 }
 

components/script/dom/node.rs

@@ -620,6 +620,10 @@ impl<'a> NodeHelpers<'a> for JSRef<'a, Node> {
         // 1. Dirty self.
         self.set_has_changed(true);
 
+        if self.get_is_dirty() {
+            return
+        }
+
         // 2. Dirty descendants.
         fn dirty_subtree(node: JSRef<Node>) {
             // Stop if this subtree is already dirty.
@@ -1141,7 +1145,7 @@ impl Node {
 
             layout_data: LayoutDataRef::new(),
 
-            unique_id: DOMRefCell::new("".to_string()),
+            unique_id: DOMRefCell::new(String::new()),
         }
     }
 
@@ -1327,29 +1331,8 @@ impl Node {
               parent: JSRef<Node>,
               child: Option<JSRef<Node>>,
               suppress_observers: SuppressObserver) {
-        // XXX assert owner_doc
+        fn do_insert(node: JSRef<Node>, parent: JSRef<Node>, child: Option<JSRef<Node>>) {
-        // Step 1-3: ranges.
+            parent.add_child(node, child);
-        // Step 4.
-        let mut nodes = match node.type_id() {
-            DocumentFragmentNodeTypeId => node.children().collect(),
-            _ => vec!(node.clone()),
-        };
-
-        // Step 5: DocumentFragment, mutation records.
-        // Step 6: DocumentFragment.
-        match node.type_id() {
-            DocumentFragmentNodeTypeId => {
-                for c in node.children() {
-                    Node::remove(c, node, Suppressed);
-                }
-            },
-            _ => (),
-        }
-
-        // Step 7: mutation records.
-        // Step 8.
-        for node in nodes.iter_mut() {
-            parent.add_child(*node, child);
             let is_in_doc = parent.is_in_doc();
             for kid in node.traverse_preorder() {
                 let mut flags = kid.flags.get();
@@ -1362,17 +1345,50 @@ impl Node {
             }
         }
 
-        // Step 9.
+        fn fire_observer_if_necessary(node: JSRef<Node>, suppress_observers: SuppressObserver) {
             match suppress_observers {
-            Unsuppressed => {
+                Unsuppressed => node.node_inserted(),
-                for node in nodes.iter() {
-                    node.node_inserted();
-                }
-            }
                 Suppressed => ()
             }
         }
 
+        // XXX assert owner_doc
+        // Step 1-3: ranges.
+
+        match node.type_id() {
+            DocumentFragmentNodeTypeId => {
+                // Step 4.
+                // Step 5: DocumentFragment, mutation records.
+                // Step 6: DocumentFragment.
+                let mut kids = Vec::new();
+                for kid in node.children() {
+                    kids.push(kid.clone());
+                    Node::remove(kid, node, Suppressed);
+                }
+
+                // Step 7: mutation records.
+                // Step 8.
+                for kid in kids.iter() {
+                    do_insert((*kid).clone(), parent, child);
+                }
+
+                for kid in kids.into_iter() {
+                    fire_observer_if_necessary(kid, suppress_observers);
+                }
+            }
+            _ => {
+                // Step 4.
+                // Step 5: DocumentFragment, mutation records.
+                // Step 6: DocumentFragment.
+                // Step 7: mutation records.
+                // Step 8.
+                do_insert(node, parent, child);
+                // Step 9.
+                fire_observer_if_necessary(node, suppress_observers);
+            }
+        }
+    }
+
     // http://dom.spec.whatwg.org/#concept-node-replace-all
     fn replace_all(node: Option<JSRef<Node>>, parent: JSRef<Node>) {
 

components/script/lib.rs

@@ -5,7 +5,7 @@
 #![comment = "The Servo Parallel Browser Project"]
 #![license = "MPL"]
 
-#![feature(globs, macro_rules, struct_variant, phase, unsafe_destructor)]
+#![feature(default_type_params, globs, macro_rules, struct_variant, phase, unsafe_destructor)]
 
 #![deny(unused_imports, unused_variable)]
 #![allow(non_snake_case)]