Auto merge of #18431 - mbrubeck:pdb-size, r=SimonSapin

Use SmallBitVec to optimize size of PropertyDeclarationBlock

https://bugzilla.mozilla.org/show_bug.cgi?id=1398322

<!-- 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/18431)
<!-- Reviewable:end -->

Cargo.lock

@@ -3027,6 +3027,11 @@ name = "slab"
 version = "0.3.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 
+[[package]]
+name = "smallbitvec"
+version = "1.0.3"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+
 [[package]]
 name = "smallvec"
 version = "0.4.3"
@@ -3085,7 +3090,6 @@ dependencies = [
  "arrayvec 0.3.23 (registry+https://github.com/rust-lang/crates.io-index)",
  "atomic_refcell 0.1.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "bindgen 0.29.0 (registry+https://github.com/rust-lang/crates.io-index)",
- "bit-vec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "bitflags 0.7.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "byteorder 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "cfg-if 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -3121,6 +3125,7 @@ dependencies = [
  "servo_atoms 0.0.1",
  "servo_config 0.0.1",
  "servo_url 0.0.1",
+ "smallbitvec 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
  "style_derive 0.0.1",
  "style_traits 0.0.1",
@@ -3957,6 +3962,7 @@ dependencies = [
 "checksum siphasher 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "0df90a788073e8d0235a67e50441d47db7c8ad9debd91cbf43736a2a92d36537"
 "checksum skeptic 0.9.0 (registry+https://github.com/rust-lang/crates.io-index)" = "dd7d8dc1315094150052d0ab767840376335a98ac66ef313ff911cdf439a5b69"
 "checksum slab 0.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "17b4fcaed89ab08ef143da37bc52adbcc04d4a69014f4c1208d6b51f0c47bc23"
+"checksum smallbitvec 1.0.3 (registry+https://github.com/rust-lang/crates.io-index)" = "c1ebf4681dc284c22efb7248986bbdf8aa23c2749ea85a0107e0e787038d303e"
 "checksum smallvec 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "8fcd03faf178110ab0334d74ca9631d77f94c8c11cc77fcb59538abf0025695d"
 "checksum stable_deref_trait 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "15132e0e364248108c5e2c02e3ab539be8d6f5d52a01ca9bbf27ed657316f02b"
 "checksum string_cache 0.6.2 (registry+https://github.com/rust-lang/crates.io-index)" = "413fc7852aeeb5472f1986ef755f561ddf0c789d3d796e65f0b6fe293ecd4ef8"

components/script/dom/cssstyledeclaration.rs

@@ -410,10 +410,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
     // https://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
     fn IndexedGetter(&self, index: u32) -> Option<DOMString> {
         self.owner.with_block(|pdb| {
-            pdb.declarations().get(index as usize).map(|entry| {
+            pdb.declarations().get(index as usize).map(|declaration| {
-                let (ref declaration, importance) = *entry;
+                let important = pdb.declarations_importance().get(index);
                 let mut css = declaration.to_css_string();
-                if importance.important() {
+                if important {
                     css += " !important";
                 }
                 DOMString::from(css)

components/style/Cargo.toml

@@ -34,7 +34,6 @@ arrayvec = "0.3.20"
 arraydeque = "0.2.3"
 atomic_refcell = "0.1"
 bitflags = "0.7"
-bit-vec = "0.4.3"
 byteorder = "1.0"
 cfg-if = "0.1.0"
 cssparser = "0.20"
@@ -66,6 +65,7 @@ serde = {version = "1.0", optional = true, features = ["derive"]}
 servo_arc = { path = "../servo_arc" }
 servo_atoms = {path = "../atoms", optional = true}
 servo_config = {path = "../config", optional = true}
+smallbitvec = "1.0"
 smallvec = "0.4"
 style_derive = {path = "../style_derive"}
 style_traits = {path = "../style_traits"}

components/style/animation.rs

@@ -10,7 +10,7 @@ use bezier::Bezier;
 use context::SharedStyleContext;
 use dom::OpaqueNode;
 use font_metrics::FontMetricsProvider;
-use properties::{self, CascadeFlags, ComputedValues, Importance};
+use properties::{self, CascadeFlags, ComputedValues};
 use properties::animated_properties::{AnimatableLonghand, AnimatedProperty, TransitionProperty};
 use properties::longhands::animation_direction::computed_value::single_value::T as AnimationDirection;
 use properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
@@ -482,12 +482,11 @@ fn compute_style_for_animation_step(context: &SharedStyleContext,
             let guard = declarations.read_with(context.guards.author);
 
             // No !important in keyframes.
-            debug_assert!(guard.declarations().iter()
+            debug_assert!(!guard.any_important());
-                            .all(|&(_, importance)| importance == Importance::Normal));
 
             let iter = || {
                 guard.declarations().iter().rev()
-                     .map(|&(ref decl, _importance)| (decl, CascadeLevel::Animations))
+                     .map(|decl| (decl, CascadeLevel::Animations))
             };
 
             // This currently ignores visited styles, which seems acceptable,

components/style/lib.rs

@@ -40,7 +40,6 @@
 extern crate app_units;
 extern crate arrayvec;
 extern crate atomic_refcell;
-extern crate bit_vec;
 #[macro_use]
 extern crate bitflags;
 #[allow(unused_extern_crates)] extern crate byteorder;
@@ -81,6 +80,7 @@ pub extern crate servo_arc;
 #[cfg(feature = "servo")] #[macro_use] extern crate servo_atoms;
 #[cfg(feature = "servo")] extern crate servo_config;
 #[cfg(feature = "servo")] extern crate servo_url;
+extern crate smallbitvec;
 extern crate smallvec;
 #[macro_use]
 extern crate style_derive;

components/style/properties/declaration_block.rs

@@ -14,8 +14,10 @@ use parser::{ParserContext, ParserErrorContext};
 use properties::animated_properties::AnimationValue;
 use selectors::parser::SelectorParseError;
 use shared_lock::Locked;
+use smallbitvec::{self, SmallBitVec};
 use smallvec::SmallVec;
 use std::fmt;
+use std::iter::{DoubleEndedIterator, Zip};
 use std::slice::Iter;
 use style_traits::{PARSING_MODE_DEFAULT, ToCss, ParseError, ParsingMode, StyleParseError};
 use stylesheets::{CssRuleType, Origin, UrlExtraData};
@@ -73,10 +75,10 @@ pub struct PropertyDeclarationBlock {
     /// The group of declarations, along with their importance.
     ///
     /// Only deduplicated declarations appear here.
-    declarations: Vec<(PropertyDeclaration, Importance)>,
+    declarations: Vec<PropertyDeclaration>,
 
-    /// The number of entries in `self.declaration` with `Importance::Important`
+    /// The "important" flag for each declaration in `declarations`.
-    important_count: usize,
+    declarations_importance: SmallBitVec,
 
     longhands: LonghandIdSet,
 }
@@ -87,23 +89,39 @@ impl MallocSizeOf for PropertyDeclarationBlock {
     }
 }
 
-/// Iterator for PropertyDeclaration to be generated from PropertyDeclarationBlock.
+/// Iterator over `(PropertyDeclaration, Importance)` pairs.
-#[derive(Clone)]
+pub struct DeclarationImportanceIterator<'a> {
-pub struct PropertyDeclarationIterator<'a> {
+    iter: Zip<Iter<'a, PropertyDeclaration>, smallbitvec::Iter<'a>>,
-    iter: Iter<'a, (PropertyDeclaration, Importance)>,
 }
 
-impl<'a> Iterator for PropertyDeclarationIterator<'a> {
+impl<'a> DeclarationImportanceIterator<'a> {
-    type Item = &'a PropertyDeclaration;
+    /// Constructor
-    #[inline]
+    pub fn new(declarations: &'a [PropertyDeclaration], important: &'a SmallBitVec) -> Self {
-    fn next(&mut self) -> Option<&'a PropertyDeclaration> {
+        DeclarationImportanceIterator {
-        self.iter.next().map(|&(ref decl, _)| decl)
+            iter: declarations.iter().zip(important.iter()),
+        }
+    }
+}
+
+impl<'a> Iterator for DeclarationImportanceIterator<'a> {
+    type Item = (&'a PropertyDeclaration, Importance);
+
+    fn next(&mut self) -> Option<Self::Item> {
+        self.iter.next().map(|(decl, important)|
+            (decl, if important { Importance::Important } else { Importance::Normal }))
+    }
+}
+
+impl<'a> DoubleEndedIterator for DeclarationImportanceIterator<'a> {
+    fn next_back(&mut self) -> Option<Self::Item> {
+        self.iter.next_back().map(|(decl, important)|
+            (decl, if important { Importance::Important } else { Importance::Normal }))
     }
 }
 
 /// Iterator for AnimationValue to be generated from PropertyDeclarationBlock.
 pub struct AnimationValueIterator<'a, 'cx, 'cx_a:'cx> {
-    iter: Iter<'a, (PropertyDeclaration, Importance)>,
+    iter: DeclarationImportanceIterator<'a>,
     context: &'cx mut Context<'cx_a>,
     default_values: &'a ComputedValues,
 }
@@ -113,7 +131,7 @@ impl<'a, 'cx, 'cx_a:'cx> AnimationValueIterator<'a, 'cx, 'cx_a> {
            context: &'cx mut Context<'cx_a>,
            default_values: &'a ComputedValues) -> AnimationValueIterator<'a, 'cx, 'cx_a> {
         AnimationValueIterator {
-            iter: declarations.declarations().iter(),
+            iter: declarations.declaration_importance_iter(),
             context: context,
             default_values: default_values,
         }
@@ -124,12 +142,10 @@ impl<'a, 'cx, 'cx_a:'cx> Iterator for AnimationValueIterator<'a, 'cx, 'cx_a> {
     type Item = (AnimatableLonghand, AnimationValue);
     #[inline]
     fn next(&mut self) -> Option<Self::Item> {
-        use properties::Importance;
-
         loop {
             let next = self.iter.next();
             match next {
-                Some(&(ref decl, importance)) => {
+                Some((decl, importance)) => {
                     if importance == Importance::Normal {
                         let property = AnimatableLonghand::from_declaration(decl);
                         let animation = AnimationValue::from_declaration(decl, &mut self.context,
@@ -166,7 +182,7 @@ impl PropertyDeclarationBlock {
     pub fn new() -> Self {
         PropertyDeclarationBlock {
             declarations: Vec::new(),
-            important_count: 0,
+            declarations_importance: SmallBitVec::new(),
             longhands: LonghandIdSet::new(),
         }
     }
@@ -178,22 +194,25 @@ impl PropertyDeclarationBlock {
             longhands.insert(id);
         }
         PropertyDeclarationBlock {
-            declarations: vec![(declaration, importance)],
+            declarations: vec![declaration],
-            important_count: if importance.important() { 1 } else { 0 },
+            declarations_importance: SmallBitVec::from_elem(1, importance.important()),
             longhands: longhands,
         }
     }
 
     /// The declarations in this block
-    pub fn declarations(&self) -> &[(PropertyDeclaration, Importance)] {
+    pub fn declarations(&self) -> &[PropertyDeclaration] {
         &self.declarations
     }
 
-    /// Iterate over only PropertyDeclaration.
+    /// The `important` flags for declarations in this block
-    pub fn declarations_iter(&self) -> PropertyDeclarationIterator {
+    pub fn declarations_importance(&self) -> &SmallBitVec {
-        PropertyDeclarationIterator {
+        &self.declarations_importance
-            iter: self.declarations.iter(),
+    }
-        }
+
+    /// Iterate over `(PropertyDeclaration, Importance)` pairs
+    pub fn declaration_importance_iter(&self) -> DeclarationImportanceIterator {
+        DeclarationImportanceIterator::new(&self.declarations, &self.declarations_importance)
     }
 
     /// Return an iterator of (AnimatableLonghand, AnimationValue).
@@ -206,27 +225,32 @@ impl PropertyDeclarationBlock {
 
     /// Returns whether this block contains any declaration with `!important`.
     ///
-    /// This is based on the `important_count` counter,
+    /// This is based on the `declarations_importance` bit-vector,
     /// which should be maintained whenever `declarations` is changed.
-    // FIXME: make fields private and maintain it here in methods?
     pub fn any_important(&self) -> bool {
-        self.important_count > 0
+        !self.declarations_importance.all_false()
     }
 
     /// Returns whether this block contains any declaration without `!important`.
     ///
-    /// This is based on the `important_count` counter,
+    /// This is based on the `declarations_importance` bit-vector,
     /// which should be maintained whenever `declarations` is changed.
-    // FIXME: make fields private and maintain it here in methods?
     pub fn any_normal(&self) -> bool {
-        self.declarations.len() > self.important_count
+        !self.declarations_importance.all_true()
     }
 
     /// Get a declaration for a given property.
     ///
     /// NOTE: This is linear time.
-    pub fn get(&self, property: PropertyDeclarationId) -> Option< &(PropertyDeclaration, Importance)> {
+    pub fn get(&self, property: PropertyDeclarationId) -> Option<(&PropertyDeclaration, Importance)> {
-        self.declarations.iter().find(|&&(ref decl, _)| decl.id() == property)
+        self.declarations.iter().enumerate().find(|&(_, decl)| decl.id() == property).map(|(i, decl)| {
+            let importance = if self.declarations_importance.get(i as u32) {
+                Importance::Important
+            } else {
+                Importance::Normal
+            };
+            (decl, importance)
+        })
     }
 
     /// Find the value of the given property in this block and serialize it
@@ -251,7 +275,7 @@ impl PropertyDeclarationBlock {
 
                     // Step 1.2.2.2 & 1.2.2.3
                     match declaration {
-                        Some(&(ref declaration, importance)) => {
+                        Some((declaration, importance)) => {
                             list.push(declaration);
                             if importance.important() {
                                 important_count += 1;
@@ -277,7 +301,7 @@ impl PropertyDeclarationBlock {
                 }
             }
             Err(longhand_or_custom) => {
-                if let Some(&(ref value, _importance)) = self.get(longhand_or_custom) {
+                if let Some((value, _importance)) = self.get(longhand_or_custom) {
                     // Step 2
                     value.to_css(dest)
                 } else {
@@ -298,7 +322,7 @@ impl PropertyDeclarationBlock {
                 // Step 2.1 & 2.2 & 2.3
                 if shorthand.longhands().iter().all(|&l| {
                     self.get(PropertyDeclarationId::Longhand(l))
-                        .map_or(false, |&(_, importance)| importance.important())
+                        .map_or(false, |(_, importance)| importance.important())
                 }) {
                     Importance::Important
                 } else {
@@ -307,7 +331,7 @@ impl PropertyDeclarationBlock {
             }
             Err(longhand_or_custom) => {
                 // Step 3
-                self.get(longhand_or_custom).map_or(Importance::Normal, |&(_, importance)| importance)
+                self.get(longhand_or_custom).map_or(Importance::Normal, |(_, importance)| importance)
             }
         }
     }
@@ -409,25 +433,24 @@ impl PropertyDeclarationBlock {
         if !definitely_new {
             let mut index_to_remove = None;
             for (i, slot) in self.declarations.iter_mut().enumerate() {
-                if slot.0.id() == declaration.id() {
+                if slot.id() == declaration.id() {
-                    match (slot.1, importance) {
+                    let important = self.declarations_importance.get(i as u32);
-                        (Importance::Normal, Importance::Important) => {
+                    match (important, importance.important()) {
-                            self.important_count += 1;
+                        (false, true) => {}
-                        }
+
-                        (Importance::Important, Importance::Normal) => {
+                        (true, false) => {
-                            if overwrite_more_important_and_reuse_slot {
+                            if !overwrite_more_important_and_reuse_slot {
-                                self.important_count -= 1;
-                            } else {
                                 return false
                             }
                         }
-                        _ => if slot.0 == declaration {
+                        _ => if *slot == declaration {
                             return false;
                         }
                     }
 
                     if overwrite_more_important_and_reuse_slot {
-                        *slot = (declaration, importance);
+                        *slot = declaration;
+                        self.declarations_importance.set(i as u32, importance.important());
                         return true;
                     }
 
@@ -441,7 +464,9 @@ impl PropertyDeclarationBlock {
 
             if let Some(index) = index_to_remove {
                 self.declarations.remove(index);
-                self.declarations.push((declaration, importance));
+                self.declarations_importance.remove(index as u32);
+                self.declarations.push(declaration);
+                self.declarations_importance.push(importance.important());
                 return true;
             }
         }
@@ -449,10 +474,8 @@ impl PropertyDeclarationBlock {
         if let Some(id) = longhand_id {
             self.longhands.insert(id);
         }
-        self.declarations.push((declaration, importance));
+        self.declarations.push(declaration);
-        if importance.important() {
+        self.declarations_importance.push(importance.important());
-            self.important_count += 1;
-        }
         true
     }
 
@@ -461,21 +484,13 @@ impl PropertyDeclarationBlock {
     /// Returns whether any declaration was updated.
     pub fn set_importance(&mut self, property: &PropertyId, new_importance: Importance) -> bool {
         let mut updated_at_least_one = false;
-        for &mut (ref declaration, ref mut importance) in &mut self.declarations {
+        for (i, declaration) in self.declarations.iter().enumerate() {
             if declaration.id().is_or_is_longhand_of(property) {
-                match (*importance, new_importance) {
+                let is_important = new_importance.important();
-                    (Importance::Normal, Importance::Important) => {
+                if self.declarations_importance.get(i as u32) != is_important {
-                        self.important_count += 1;
+                    self.declarations_importance.set(i as u32, is_important);
-                    }
+                    updated_at_least_one = true;
-                    (Importance::Important, Importance::Normal) => {
-                        self.important_count -= 1;
-                    }
-                    _ => {
-                        continue;
-                    }
                 }
-                updated_at_least_one = true;
-                *importance = new_importance;
             }
         }
         updated_at_least_one
@@ -490,20 +505,21 @@ impl PropertyDeclarationBlock {
                 return false
             }
         }
-        let important_count = &mut self.important_count;
         let mut removed_at_least_one = false;
         let longhands = &mut self.longhands;
-        self.declarations.retain(|&(ref declaration, importance)| {
+        let declarations_importance = &mut self.declarations_importance;
+        let mut i = 0;
+        self.declarations.retain(|declaration| {
             let id = declaration.id();
             let remove = id.is_or_is_longhand_of(property);
             if remove {
                 removed_at_least_one = true;
                 if let PropertyDeclarationId::Longhand(id) = id {
-                    longhands.remove(id)
+                    longhands.remove(id);
-                }
-                if importance.important() {
-                    *important_count -= 1
                 }
+                declarations_importance.remove(i);
+            } else {
+                i += 1;
             }
             !remove
         });
@@ -527,7 +543,7 @@ impl PropertyDeclarationBlock {
         match property.as_shorthand() {
             Err(_longhand_or_custom) => {
                 if self.declarations.len() == 1 {
-                    let declaration = &self.declarations[0].0;
+                    let declaration = &self.declarations[0];
                     // If we have a longhand declaration with variables, those variables will be
                     // stored as unparsed values. As a temporary measure to produce sensible results
                     // in Gecko's getKeyframes() implementation for CSS animations, if
@@ -549,10 +565,10 @@ impl PropertyDeclarationBlock {
                 }
             }
             Ok(shorthand) => {
-                if !self.declarations.iter().all(|decl| decl.0.shorthands().contains(&shorthand)) {
+                if !self.declarations.iter().all(|decl| decl.shorthands().contains(&shorthand)) {
                     return Err(fmt::Error)
                 }
-                let iter = self.declarations_iter();
+                let iter = self.declarations.iter();
                 match shorthand.get_shorthand_appendable_value(iter) {
                     Some(AppendableValue::Css { css, .. }) => {
                         dest.write_str(css)
@@ -569,17 +585,18 @@ impl PropertyDeclarationBlock {
     /// Convert AnimationValueMap to PropertyDeclarationBlock.
     #[cfg(feature = "gecko")]
     pub fn from_animation_value_map(animation_value_map: &AnimationValueMap) -> Self {
-        let mut declarations = vec![];
+        let len = animation_value_map.len();
+        let mut declarations = Vec::with_capacity(len);
         let mut longhands = LonghandIdSet::new();
 
         for (property, animation_value) in animation_value_map.iter() {
           longhands.set_animatable_longhand_bit(property);
-          declarations.push((animation_value.uncompute(), Importance::Normal));
+          declarations.push(animation_value.uncompute());
         }
 
         PropertyDeclarationBlock {
             declarations: declarations,
-            important_count: 0,
+            declarations_importance: SmallBitVec::from_elem(len as u32, false),
             longhands: longhands,
         }
     }
@@ -593,7 +610,7 @@ impl PropertyDeclarationBlock {
                 return false
             }
         }
-        self.declarations.iter().any(|&(ref decl, _)|
+        self.declarations.iter().any(|decl|
             decl.id().is_or_is_longhand_of(property) &&
             decl.get_css_wide_keyword().is_some()
         )
@@ -613,7 +630,7 @@ impl ToCss for PropertyDeclarationBlock {
         let mut already_serialized = PropertyDeclarationIdSet::new();
 
         // Step 3
-        for &(ref declaration, importance) in &*self.declarations {
+        for (declaration, importance) in self.declaration_importance_iter() {
             // Step 3.1
             let property = declaration.id();
 
@@ -639,13 +656,13 @@ impl ToCss for PropertyDeclarationBlock {
 
                     let is_system_font =
                         shorthand == ShorthandId::Font &&
-                        self.declarations.iter().any(|&(ref l, _)| {
+                        self.declarations.iter().any(|l| {
                             !already_serialized.contains(l.id()) &&
                             l.get_system().is_some()
                         });
 
                     if is_system_font {
-                        for &(ref longhand, longhand_importance) in &self.declarations {
+                        for (longhand, importance) in self.declaration_importance_iter() {
                             if already_serialized.contains(longhand.id()) {
                                 continue;
                             }
@@ -655,20 +672,20 @@ impl ToCss for PropertyDeclarationBlock {
                                 if found_system.is_none() {
                                    found_system = longhand.get_system();
                                 }
-                                if longhand_importance.important() {
+                                if importance.important() {
                                     important_count += 1;
                                 }
                             }
                         }
                     } else {
-                        for &(ref longhand, longhand_importance) in &self.declarations {
+                        for (longhand, importance) in self.declaration_importance_iter() {
                             if already_serialized.contains(longhand.id()) {
                                 continue;
                             }
 
                             if longhand.id().is_longhand_of(shorthand) {
                                 current_longhands.push(longhand);
-                                if longhand_importance.important() {
+                                if importance.important() {
                                     important_count += 1;
                                 }
                             }

components/style/properties/properties.mako.rs

@@ -12,6 +12,7 @@
 
 #[cfg(feature = "servo")] use app_units::Au;
 use servo_arc::{Arc, UniqueArc};
+use smallbitvec::SmallBitVec;
 use std::borrow::Cow;
 use hash::HashSet;
 use std::{fmt, mem, ops};
@@ -3028,25 +3029,26 @@ pub fn cascade(
                   ptr::eq(parent_style.unwrap(),
                           parent_style_ignoring_first_line.unwrap()) ||
                   parent_style.unwrap().pseudo() == Some(PseudoElement::FirstLine));
+    let empty = SmallBitVec::new();
     let iter_declarations = || {
         rule_node.self_and_ancestors().flat_map(|node| {
             let cascade_level = node.cascade_level();
             let source = node.style_source();
+
             let declarations = if source.is_some() {
-                source.read(cascade_level.guard(guards)).declarations()
+                source.read(cascade_level.guard(guards)).declaration_importance_iter()
             } else {
                 // The root node has no style source.
-                &[]
+                DeclarationImportanceIterator::new(&[], &empty)
             };
             let node_importance = node.importance();
 
             let property_restriction = pseudo.and_then(|p| p.property_restriction());
 
             declarations
-                .iter()
                 // Yield declarations later in source order (with more precedence) first.
                 .rev()
-                .filter_map(move |&(ref declaration, declaration_importance)| {
+                .filter_map(move |(declaration, declaration_importance)| {
                     if let Some(property_restriction) = property_restriction {
                         // declaration.id() is either a longhand or a custom
                         // property.  Custom properties are always allowed, but

components/style/rule_tree/mod.rs

@@ -1211,15 +1211,15 @@ impl StrongRuleNode {
             for node in element_rule_node.self_and_ancestors() {
                 let source = node.style_source();
                 let declarations = if source.is_some() {
-                    source.read(node.cascade_level().guard(guards)).declarations()
+                    source.read(node.cascade_level().guard(guards)).declaration_importance_iter()
                 } else {
                     continue
                 };
 
                 // Iterate over declarations of the longhands we care about.
                 let node_importance = node.importance();
-                let longhands = declarations.iter().rev()
+                let longhands = declarations.rev()
-                    .filter_map(|&(ref declaration, importance)| {
+                    .filter_map(|(declaration, importance)| {
                         if importance != node_importance { return None }
                         match declaration.id() {
                             PropertyDeclarationId::Longhand(id) => {
@@ -1329,9 +1329,8 @@ impl StrongRuleNode {
         let mut result = (LonghandIdSet::new(), false);
         for node in iter {
             let style = node.style_source();
-            for &(ref decl, important) in style.read(node.cascade_level().guard(guards))
+            for (decl, important) in style.read(node.cascade_level().guard(guards))
-                                               .declarations()
+                                               .declaration_importance_iter() {
-                                               .iter() {
                 // Although we are only iterating over cascade levels that
                 // override animations, in a given property declaration block we
                 // can have a mixture of !important and non-!important

components/style/sharing/mod.rs

@@ -67,7 +67,6 @@
 use Atom;
 use applicable_declarations::ApplicableDeclarationBlock;
 use atomic_refcell::{AtomicRefCell, AtomicRefMut};
-use bit_vec::BitVec;
 use bloom::StyleBloom;
 use cache::{LRUCache, LRUCacheMutIterator};
 use context::{SelectorFlagsMap, SharedStyleContext, StyleContext};
@@ -78,6 +77,7 @@ use owning_ref::OwningHandle;
 use properties::ComputedValues;
 use selectors::matching::{ElementSelectorFlags, VisitedHandlingMode};
 use servo_arc::Arc;
+use smallbitvec::SmallBitVec;
 use smallvec::SmallVec;
 use std::marker::PhantomData;
 use std::mem;
@@ -123,7 +123,7 @@ pub struct ValidationData {
 
     /// The cached result of matching this entry against the revalidation
     /// selectors.
-    revalidation_match_results: Option<BitVec>,
+    revalidation_match_results: Option<SmallBitVec>,
 }
 
 impl ValidationData {
@@ -177,7 +177,7 @@ impl ValidationData {
         bloom: &StyleBloom<E>,
         bloom_known_valid: bool,
         flags_setter: &mut F
-    ) -> &BitVec
+    ) -> &SmallBitVec
         where E: TElement,
               F: FnMut(&E, ElementSelectorFlags),
     {
@@ -256,7 +256,7 @@ impl<E: TElement> StyleSharingCandidate<E> {
         &mut self,
         stylist: &Stylist,
         bloom: &StyleBloom<E>,
-    ) -> &BitVec {
+    ) -> &SmallBitVec {
         self.validation_data.revalidation_match_results(
             self.element,
             stylist,
@@ -309,7 +309,7 @@ impl<E: TElement> StyleSharingTarget<E> {
         stylist: &Stylist,
         bloom: &StyleBloom<E>,
         selector_flags_map: &mut SelectorFlagsMap<E>
-    ) -> &BitVec {
+    ) -> &SmallBitVec {
         // It's important to set the selector flags. Otherwise, if we succeed in
         // sharing the style, we may not set the slow selector flags for the
         // right elements (which may not necessarily be |element|), causing

components/style/stylesheets/keyframes_rule.rs

@@ -299,7 +299,7 @@ impl KeyframesStep {
            guard: &SharedRwLockReadGuard) -> Self {
         let declared_timing_function = match value {
             KeyframesStepValue::Declarations { ref block } => {
-                block.read_with(guard).declarations().iter().any(|&(ref prop_decl, _)| {
+                block.read_with(guard).declarations().iter().any(|prop_decl| {
                     match *prop_decl {
                         PropertyDeclaration::AnimationTimingFunction(..) => true,
                         _ => false,
@@ -325,7 +325,7 @@ impl KeyframesStep {
         match self.value {
             KeyframesStepValue::Declarations { ref block } => {
                 let guard = block.read_with(guard);
-                let &(ref declaration, _) =
+                let (declaration, _) =
                     guard.get(PropertyDeclarationId::Longhand(LonghandId::AnimationTimingFunction)).unwrap();
                 match *declaration {
                     PropertyDeclaration::AnimationTimingFunction(ref value) => {
@@ -369,7 +369,7 @@ fn get_animated_properties(keyframes: &[Arc<Locked<Keyframe>>], guard: &SharedRw
     for keyframe in keyframes {
         let keyframe = keyframe.read_with(&guard);
         let block = keyframe.block.read_with(guard);
-        for &(ref declaration, importance) in block.declarations().iter() {
+        for (declaration, importance) in block.declaration_importance_iter() {
             assert!(!importance.important());
 
             if let Some(property) = AnimatableLonghand::from_declaration(declaration) {

components/style/stylist.rs

@@ -6,7 +6,6 @@
 
 use {Atom, LocalName, Namespace};
 use applicable_declarations::{ApplicableDeclarationBlock, ApplicableDeclarationList};
-use bit_vec::BitVec;
 use context::{CascadeInputs, QuirksMode};
 use dom::TElement;
 use element_state::ElementState;
@@ -35,6 +34,7 @@ use selectors::sink::Push;
 use selectors::visitor::SelectorVisitor;
 use servo_arc::{Arc, ArcBorrow};
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
+use smallbitvec::SmallBitVec;
 use smallvec::VecLike;
 use std::fmt::Debug;
 use std::ops;
@@ -1560,7 +1560,7 @@ impl Stylist {
         element: &E,
         bloom: Option<&BloomFilter>,
         flags_setter: &mut F
-    ) -> BitVec
+    ) -> SmallBitVec
     where
         E: TElement,
         F: FnMut(&E, ElementSelectorFlags),
@@ -1575,7 +1575,7 @@ impl Stylist {
         // This means we're guaranteed to get the same rulehash buckets for all
         // the lookups, which means that the bitvecs are comparable. We verify
         // this in the caller by asserting that the bitvecs are same-length.
-        let mut results = BitVec::new();
+        let mut results = SmallBitVec::new();
         for (data, _) in self.cascade_data.iter_origins() {
             data.selectors_for_cache_revalidation.lookup(
                 *element,

ports/geckolib/glue.rs

@@ -2261,7 +2261,7 @@ pub extern "C" fn Servo_SerializeFontValueForCanvas(
     use style::properties::shorthands::font;
 
     read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
-        let longhands = match font::LonghandsToSerialize::from_iter(decls.declarations_iter()) {
+        let longhands = match font::LonghandsToSerialize::from_iter(decls.declarations().iter()) {
             Ok(l) => l,
             Err(()) => {
                 warn!("Unexpected property!");
@@ -2289,7 +2289,7 @@ pub extern "C" fn Servo_DeclarationBlock_Count(declarations: RawServoDeclaration
 pub extern "C" fn Servo_DeclarationBlock_GetNthProperty(declarations: RawServoDeclarationBlockBorrowed,
                                                         index: u32, result: *mut nsAString) -> bool {
     read_locked_arc(declarations, |decls: &PropertyDeclarationBlock| {
-        if let Some(&(ref decl, _)) = decls.declarations().get(index as usize) {
+        if let Some(decl) = decls.declarations().get(index as usize) {
             let result = unsafe { result.as_mut().unwrap() };
             result.assign_utf8(&decl.id().name());
             true
@@ -3407,8 +3407,8 @@ pub extern "C" fn Servo_AnimationValue_Compute(element: RawGeckoElementBorrowed,
     let guard = global_style_data.shared_lock.read();
     let declarations = Locked::<PropertyDeclarationBlock>::as_arc(&declarations);
     // We only compute the first element in declarations.
-    match declarations.read_with(&guard).declarations().first() {
+    match declarations.read_with(&guard).declaration_importance_iter().next() {
-        Some(&(ref decl, imp)) if imp == Importance::Normal => {
+        Some((decl, imp)) if imp == Importance::Normal => {
             let animation = AnimationValue::from_declaration(decl, &mut context, default_values);
             animation.map_or(RawServoAnimationValueStrong::null(), |value| {
                 Arc::new(value).into_strong()
@@ -3554,11 +3554,9 @@ pub extern "C" fn Servo_StyleSet_GetKeyframesForName(raw_data: RawServoStyleSetB
                 let animatable =
                     guard.declarations()
                          .iter()
-                         .filter(|&&(ref declaration, _)| {
+                         .filter(|declaration| declaration.is_animatable());
-                             declaration.is_animatable()
-                         });
 
-                for &(ref declaration, _) in animatable {
+                for declaration in animatable {
                     let property = AnimatableLonghand::from_declaration(declaration).unwrap();
                     // Skip the 'display' property because although it is animatable from SMIL,
                     // it should not be animatable from CSS Animations.