servo: Merge #16874 - Use a SmallVec when gathering applicable declarations (from bholley:applicable_declarations_smallvec); r=emilio
authorBobby Holley <bobbyholley@gmail.com>
Mon, 15 May 2017 23:19:43 -0500
changeset 358575 0e29c045a214ed58d1f1e800b67774d851d1c471
parent 358571 bd4e12a3fed9a082fde9f54e2093dd81af078951
child 358576 161b368464fbbcdf1b9d8778a86709950eac8891
push id90353
push userryanvm@gmail.com
push dateWed, 17 May 2017 00:10:47 +0000
treeherdermozilla-inbound@41958333867b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #16874 - Use a SmallVec when gathering applicable declarations (from bholley:applicable_declarations_smallvec); r=emilio https://bugzilla.mozilla.org/show_bug.cgi?id=1364952 Source-Repo: https://github.com/servo/servo Source-Revision: 7ca393a9603d7fa72e58a36bd53c29db396f6ea4
servo/components/style/matching.rs
servo/components/style/stylist.rs
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -24,17 +24,17 @@ use restyle_hints::{RESTYLE_STYLE_ATTRIB
 use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode};
 use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl};
 use selectors::bloom::BloomFilter;
 use selectors::matching::{ElementSelectorFlags, StyleRelations};
 use selectors::matching::AFFECTED_BY_PSEUDO_ELEMENTS;
 use shared_lock::StylesheetGuards;
 use sink::ForgetfulSink;
 use stylearc::Arc;
-use stylist::ApplicableDeclarationBlock;
+use stylist::ApplicableDeclarationList;
 
 /// The way a style should be inherited.
 enum InheritMode {
     /// Inherit from the parent element, as normal CSS dictates, _or_ from the
     /// closest non-Native Anonymous element in case this is Native Anonymous
     /// Content.
     Normal,
     /// Inherit from the primary style, this is used while computing eager
@@ -860,21 +860,21 @@ trait PrivateMatchMethods: TElement {
                                               -> Result<ComputedStyle, CacheMiss> {
         let candidate_element = *candidate.element;
         element_matches_candidate(self, candidate, &candidate_element,
                                   shared, bloom, info, selector_flags_map)
     }
 }
 
 fn compute_rule_node<E: TElement>(rule_tree: &RuleTree,
-                                  applicable_declarations: &mut Vec<ApplicableDeclarationBlock>,
+                                  applicable_declarations: &mut ApplicableDeclarationList,
                                   guards: &StylesheetGuards)
                                   -> StrongRuleNode
 {
-    let rules = applicable_declarations.drain(..).map(|d| (d.source, d.level));
+    let rules = applicable_declarations.drain().map(|d| (d.source, d.level));
     let rule_node = rule_tree.insert_ordered_rules_with_important(rules, guards);
     rule_node
 }
 
 impl<E: TElement> PrivateMatchMethods for E {}
 
 /// Controls whether the style sharing cache is used.
 #[derive(Clone, Copy, PartialEq)]
@@ -989,18 +989,17 @@ pub trait MatchMethods : TElement {
                         rules = node;
                     }
                 }
 
                 return data.set_primary_rules(rules);
             }
         }
 
-        let mut applicable_declarations =
-            Vec::<ApplicableDeclarationBlock>::with_capacity(16);
+        let mut applicable_declarations = ApplicableDeclarationList::new();
 
         let stylist = &context.shared.stylist;
         let style_attribute = self.style_attribute();
         let smil_override = self.get_smil_override();
         let animation_rules = self.get_animation_rules();
         let bloom = context.thread_local.bloom_filter.filter();
 
         let map = &mut context.thread_local.selector_flags;
@@ -1049,18 +1048,17 @@ pub trait MatchMethods : TElement {
                      data: &mut ElementData)
                      -> bool
     {
         if self.implemented_pseudo_element().is_some() {
             // Element pseudos can't have any other pseudo.
             return false;
         }
 
-        let mut applicable_declarations =
-            Vec::<ApplicableDeclarationBlock>::with_capacity(16);
+        let mut applicable_declarations = ApplicableDeclarationList::new();
 
         let map = &mut context.thread_local.selector_flags;
         let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
             self.apply_selector_flags(map, element, flags);
         };
 
         // Borrow the stuff we need here so the borrow checker doesn't get mad
         // at us later in the closure.
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -28,32 +28,41 @@ use selectors::Element;
 use selectors::bloom::BloomFilter;
 use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
 use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_selector};
 use selectors::parser::{AttrSelector, Combinator, Component, Selector, SelectorInner, SelectorIter};
 use selectors::parser::{SelectorMethods, LocalName as LocalNameSelector};
 use selectors::visitor::SelectorVisitor;
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use sink::Push;
-use smallvec::VecLike;
+use smallvec::{SmallVec, VecLike};
 use std::borrow::Borrow;
 use std::collections::HashMap;
 use std::hash::Hash;
 #[cfg(feature = "servo")]
 use std::marker::PhantomData;
 use style_traits::viewport::ViewportConstraints;
 use stylearc::Arc;
 use stylesheets::{CssRule, FontFaceRule, Origin, StyleRule, Stylesheet, UserAgentStylesheets};
 #[cfg(feature = "servo")]
 use stylesheets::NestedRulesResult;
 use thread_state;
 use viewport::{self, MaybeNew, ViewportRule};
 
 pub use ::fnv::FnvHashMap;
 
+/// List of applicable declaration. This is a transient structure that shuttles
+/// declarations between selector matching and inserting into the rule tree, and
+/// therefore we want to avoid heap-allocation where possible.
+///
+/// In measurements on wikipedia, we pretty much never have more than 8 applicable
+/// declarations, so we could consider making this 8 entries instead of 16.
+/// However, it may depend a lot on workload, and stack space is cheap.
+pub type ApplicableDeclarationList = SmallVec<[ApplicableDeclarationBlock; 16]>;
+
 /// This structure holds all the selectors and device characteristics
 /// for a given document. The selectors are converted into `Rule`s
 /// (defined in rust-selectors), and introduced in a `SelectorMap`
 /// depending on the pseudo-element (see `PerPseudoElementSelectorMap`),
 /// and stylesheet origin (see the fields of `PerPseudoElementSelectorMap`).
 ///
 /// This structure is effectively created once per pipeline, in the
 /// LayoutThread corresponding to that pipeline.
@@ -673,17 +682,17 @@ impl Stylist {
             if !parent_flags.is_empty() {
                 if let Some(p) = element.parent_element() {
                     unsafe { p.set_selector_flags(parent_flags); }
                 }
             }
         };
 
 
-        let mut declarations = vec![];
+        let mut declarations = ApplicableDeclarationList::new();
         self.push_applicable_declarations(element,
                                           None,
                                           None,
                                           None,
                                           AnimationRules(None, None),
                                           Some((pseudo, pseudo_state)),
                                           &mut declarations,
                                           &mut set_selector_flags);