Auto merge of #17294 - emilio:less-refcount-churn, r=bholley

style: Less refcount churn while inserting in the rule tree.

There's no need for it since we know no GC is happening while we're doing it.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/17294)
<!-- Reviewable:end -->

components/style/rule_tree/mod.rs

@@ -572,6 +572,17 @@ impl RuleNode {
         self.parent.is_none()
     }
 
+    fn next_sibling(&self) -> Option<WeakRuleNode> {
+        // We use acquire semantics here to ensure proper synchronization while
+        // inserting in the child list.
+        let ptr = self.next_sibling.load(Ordering::Acquire);
+        if ptr.is_null() {
+            None
+        } else {
+            Some(WeakRuleNode::from_ptr(ptr))
+        }
+    }
+
     /// Remove this rule node from the child list.
     ///
     /// This method doesn't use proper synchronization, and it's expected to be
@@ -637,7 +648,7 @@ impl RuleNode {
 
         let _ = write!(writer, "\n");
         for child in self.iter_children() {
-            child.get().dump(guards, writer, indent + INDENT_INCREMENT);
+            child.upgrade().get().dump(guards, writer, indent + INDENT_INCREMENT);
         }
     }
 
@@ -692,31 +703,30 @@ impl StrongRuleNode {
         WeakRuleNode::from_ptr(self.ptr())
     }
 
-    fn next_sibling(&self) -> Option<WeakRuleNode> {
-        // We use acquire semantics here to ensure proper synchronization while
-        // inserting in the child list.
-        let ptr = self.get().next_sibling.load(Ordering::Acquire);
-        if ptr.is_null() {
-            None
-        } else {
-            Some(WeakRuleNode::from_ptr(ptr))
-        }
-    }
-
     fn parent(&self) -> Option<&StrongRuleNode> {
         self.get().parent.as_ref()
     }
 
-    fn ensure_child(&self,
+    fn ensure_child(
+        &self,
         root: WeakRuleNode,
         source: StyleSource,
-                    level: CascadeLevel) -> StrongRuleNode {
+        level: CascadeLevel
+    ) -> StrongRuleNode {
         let mut last = None;
-        // TODO(emilio): We could avoid all the refcount churn here.
+
+        // NB: This is an iterator over _weak_ nodes.
+        //
+        // It's fine though, because nothing can make us GC while this happens,
+        // and this happens to be hot.
+        //
+        // TODO(emilio): We could actually make this even less hot returning a
+        // WeakRuleNode, and implementing this on WeakRuleNode itself...
         for child in self.get().iter_children() {
-            if child.get().level == level &&
+            let child_node = unsafe { &*child.ptr() };
-                child.get().source.as_ref().unwrap().ptr_equals(&source) {
+            if child_node.level == level &&
-                return child;
+                child_node.source.as_ref().unwrap().ptr_equals(&source) {
+                return child.upgrade();
             }
             last = Some(child);
         }
@@ -728,11 +738,12 @@ impl StrongRuleNode {
         let new_ptr: *mut RuleNode = &mut *node;
 
         loop {
-            let strong;
+            let next;
 
             {
-                let next_sibling_ptr = match last {
+                let last_node = last.as_ref().map(|l| unsafe { &*l.ptr() });
-                    Some(ref l) => &l.get().next_sibling,
+                let next_sibling_ptr = match last_node {
+                    Some(ref l) => &l.next_sibling,
                     None => &self.get().first_child,
                 };
 
@@ -757,17 +768,17 @@ impl StrongRuleNode {
 
                 // Existing is not null: some thread inserted a child node since
                 // we accessed `last`.
-                strong = WeakRuleNode::from_ptr(existing).upgrade();
+                next = WeakRuleNode::from_ptr(existing);
 
-                if strong.get().source.as_ref().unwrap().ptr_equals(&source) {
+                if unsafe { &*next.ptr() }.source.as_ref().unwrap().ptr_equals(&source) {
                     // That node happens to be for the same style source, use
                     // that, and let node fall out of scope.
-                    return strong;
+                    return next.upgrade();
                 }
             }
 
             // Try again inserting after the new last child.
-            last = Some(strong);
+            last = Some(next);
         }
     }
 
@@ -1366,12 +1377,11 @@ struct RuleChildrenListIter {
 }
 
 impl Iterator for RuleChildrenListIter {
-    type Item = StrongRuleNode;
+    type Item = WeakRuleNode;
 
     fn next(&mut self) -> Option<Self::Item> {
         self.current.take().map(|current| {
-            let current = current.upgrade();
+            self.current = unsafe { &*current.ptr() }.next_sibling();
-            self.current = current.next_sibling();
             current
         })
     }