style: Rework how precomputed pseudo stuff works, to avoid malloc/fre…

…e churn. This showed up in a few profiles, and was an easy improvement. MozReview-Commit-ID: HVqATaSB2Ak Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
servo · Aug 8, 2017 · 01cf3c5 · 01cf3c5
1 parent 4fdc571
commit 01cf3c5
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 67 deletions.
diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs
@@ -201,29 +201,6 @@ impl SelectorMap<Rule> {
                     |block| (block.specificity, block.source_order()));
     }
 
-    /// Append to `rule_list` all universal Rules (rules with selector `*|*`) in
-    /// `self` sorted by specificity and source order.
-    pub fn get_universal_rules(&self,
-                               cascade_level: CascadeLevel)
-                               -> Vec<ApplicableDeclarationBlock> {
-        debug_assert!(!cascade_level.is_important());
-        if self.is_empty() {
-            return vec![];
-        }
-
-        let mut rules_list = vec![];
-        for rule in self.other.iter() {
-            if rule.selector.is_universal() {
-                rules_list.push(rule.to_applicable_declaration_block(cascade_level))
-            }
-        }
-
-        sort_by_key(&mut rules_list,
-                    |block| (block.specificity, block.source_order()));
-
-        rules_list
-    }
-
     /// Adds rules in `rules` that match `element` to the `matching_rules` list.
     fn get_matching_rules<E, V, F>(element: &E,
                                    rules: &[Rule],

diff --git a/components/style/stylist.rs b/components/style/stylist.rs
@@ -403,13 +403,6 @@ impl Stylist {
             self.add_stylesheet(stylesheet, guards.author, extra_data);
         }
 
-        SelectorImpl::each_precomputed_pseudo_element(|pseudo| {
-            if let Some(map) = self.cascade_data.user_agent.pseudos_map.remove(&pseudo) {
-                let declarations = map.get_universal_rules(CascadeLevel::UANormal);
-                self.precomputed_pseudo_element_decls.insert(pseudo, declarations);
-            }
-        });
-
         self.is_device_dirty = false;
         true
     }
@@ -470,17 +463,49 @@ impl Stylist {
                     for selector in &style_rule.selectors.0 {
                         self.num_selectors += 1;
 
+                        let map = match selector.pseudo_element() {
+                            None => &mut origin_cascade_data.element_map,
+                            Some(pseudo) => {
+                                if pseudo.is_precomputed() {
+                                    if !selector.is_universal() ||
+                                       !matches!(origin, Origin::UserAgent) {
+                                        // ::-moz-tree selectors may appear in
+                                        // non-UA sheets (even though they never
+                                        // match).
+                                        continue;
+                                    }
+
+                                    self.precomputed_pseudo_element_decls
+                                        .entry(pseudo.canonical())
+                                        .or_insert_with(Vec::new)
+                                        .push(ApplicableDeclarationBlock::new(
+                                            StyleSource::Style(locked.clone()),
+                                            self.rules_source_order,
+                                            CascadeLevel::UANormal,
+                                            selector.specificity()
+                                        ));
+
+                                    continue;
+                                } else {
+                                    origin_cascade_data
+                                        .pseudos_map
+                                        .entry(pseudo.canonical())
+                                        .or_insert_with(SelectorMap::new)
+                                }
+                            }
+                        };
+
                         let hashes =
                             AncestorHashes::new(&selector, self.quirks_mode);
 
-                        origin_cascade_data
-                            .borrow_mut_for_pseudo_or_insert(selector.pseudo_element())
-                            .insert(
-                                Rule::new(selector.clone(),
-                                          hashes.clone(),
-                                          locked.clone(),
-                                          self.rules_source_order),
-                                self.quirks_mode);
+                        let rule = Rule::new(
+                            selector.clone(),
+                            hashes.clone(),
+                            locked.clone(),
+                            self.rules_source_order
+                        );
+
+                        map.insert(rule, self.quirks_mode);
 
                         self.invalidation_map.note_selector(selector, self.quirks_mode);
                         let mut visitor = StylistSelectorVisitor {
@@ -1660,18 +1685,6 @@ impl PerOriginCascadeData {
         }
     }
 
-    #[inline]
-    fn borrow_mut_for_pseudo_or_insert(&mut self, pseudo: Option<&PseudoElement>) -> &mut SelectorMap<Rule> {
-        match pseudo {
-            Some(pseudo) => {
-                self.pseudos_map
-                    .entry(pseudo.canonical())
-                    .or_insert_with(SelectorMap::new)
-            }
-            None => &mut self.element_map,
-        }
-    }
-
     fn clear(&mut self) {
         *self = Self::new();
     }
@@ -1720,14 +1733,17 @@ impl Rule {
 
     /// Turns this rule into an `ApplicableDeclarationBlock` for the given
     /// cascade level.
-    pub fn to_applicable_declaration_block(&self,
-                                           level: CascadeLevel)
-                                           -> ApplicableDeclarationBlock {
+    pub fn to_applicable_declaration_block(
+        &self,
+        level: CascadeLevel
+    ) -> ApplicableDeclarationBlock {
         let source = StyleSource::Style(self.style_rule.clone());
-        ApplicableDeclarationBlock::new(source,
-                                        self.source_order,
-                                        level,
-                                        self.specificity())
+        ApplicableDeclarationBlock::new(
+            source,
+            self.source_order,
+            level,
+            self.specificity()
+        )
     }
 
     /// Creates a new Rule.

diff --git a/tests/unit/style/stylist.rs b/tests/unit/style/stylist.rs
@@ -222,16 +222,6 @@ fn test_insert() {
     assert!(selector_map.class_hash.get(&Atom::from("foo"), QuirksMode::NoQuirks).is_none());
 }
 
-#[test]
-fn test_get_universal_rules() {
-    thread_state::initialize(thread_state::LAYOUT);
-    let (map, _shared_lock) = get_mock_map(&["*|*", "#foo > *|*", "*|* > *|*", ".klass", "#id"]);
-
-    let decls = map.get_universal_rules(CascadeLevel::UserNormal);
-
-    assert_eq!(decls.len(), 1, "{:?}", decls);
-}
-
 fn mock_stylist() -> Stylist {
     let device = Device::new(MediaType::Screen, TypedSize2D::new(0f32, 0f32), ScaleFactor::new(1.0));
     Stylist::new(device, QuirksMode::NoQuirks)