Use even fewer bits for source order and shrink ApplicableDeclarationsBlock by another word.

MozReview-Commit-ID: 7B1i1g0HLTj

components/style/rule_tree/mod.rs

@@ -12,6 +12,7 @@ use properties::{AnimationRules, Importance, LonghandIdSet, PropertyDeclarationB
 use shared_lock::{Locked, StylesheetGuards, SharedRwLockReadGuard};
 use smallvec::SmallVec;
 use std::io::{self, Write};
+use std::mem;
 use std::ptr;
 use std::sync::atomic::{AtomicPtr, AtomicUsize, Ordering};
 use stylearc::{Arc, NonZeroPtrMut};
@@ -229,7 +230,7 @@ impl RuleTree {
                              guards: &StylesheetGuards)
                              -> StrongRuleNode
     {
-        let rules = applicable_declarations.drain().map(|d| (d.source, d.level));
+        let rules = applicable_declarations.drain().map(|d| d.source_and_level());
         let rule_node = self.insert_ordered_rules_with_important(rules, guards);
         rule_node
     }
@@ -425,10 +426,18 @@ pub enum CascadeLevel {
     /// User-agent important rules.
     UAImportant,
     /// Transitions
+    ///
+    /// NB: If this changes from being last, change from_byte below.
     Transitions,
 }
 
 impl CascadeLevel {
+    /// Converts a raw byte to a CascadeLevel.
+    pub unsafe fn from_byte(byte: u8) -> Self {
+        debug_assert!(byte <= CascadeLevel::Transitions as u8);
+        mem::transmute(byte)
+    }
+
     /// Select a lock guard for this level
     pub fn guard<'a>(&self, guards: &'a StylesheetGuards<'a>) -> &'a SharedRwLockReadGuard<'a> {
         match *self {

components/style/selector_map.rs

@@ -163,7 +163,7 @@ impl SelectorMap<Rule> {
 
         // Sort only the rules we just added.
         sort_by_key(&mut matching_rules_list[init_len..],
-                    |block| (block.specificity, block.source_order));
+                    |block| (block.specificity, block.source_order()));
     }
 
     /// Check whether we have rules for the given id
@@ -190,7 +190,7 @@ impl SelectorMap<Rule> {
         }
 
         sort_by_key(&mut rules_list,
-                    |block| (block.specificity, block.source_order));
+                    |block| (block.specificity, block.source_order()));
 
         rules_list
     }

components/style/stylist.rs

@@ -34,8 +34,10 @@ use selectors::visitor::SelectorVisitor;
 use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
 use sink::Push;
 use smallvec::{SmallVec, VecLike};
+use std::fmt::{Debug, self};
 #[cfg(feature = "servo")]
 use std::marker::PhantomData;
+use std::mem;
 use style_traits::viewport::ViewportConstraints;
 use stylearc::Arc;
 #[cfg(feature = "gecko")]
@@ -568,7 +570,7 @@ impl Stylist {
                 // FIXME(emilio): When we've taken rid of the cascade we can just
                 // use into_iter.
                 self.rule_tree.insert_ordered_rules_with_important(
-                    declarations.into_iter().map(|a| (a.source.clone(), a.level)),
+                    declarations.into_iter().map(|a| (a.source.clone(), a.level())),
                     guards)
             }
             None => self.rule_tree.root(),
@@ -754,7 +756,7 @@ impl Stylist {
 
         let rule_node =
             self.rule_tree.insert_ordered_rules_with_important(
-                declarations.into_iter().map(|a| (a.source, a.level)),
+                declarations.into_iter().map(|a| a.source_and_level()),
                 guards);
         if rule_node == self.rule_tree.root() {
             None
@@ -959,7 +961,7 @@ impl Stylist {
                                         context: &mut MatchingContext,
                                         flags_setter: &mut F)
         where E: TElement,
-              V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> + ::std::fmt::Debug,
+              V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> + Debug,
               F: FnMut(&E, ElementSelectorFlags),
     {
         debug_assert!(!self.is_device_dirty);
@@ -1011,7 +1013,7 @@ impl Stylist {
             if applicable_declarations.len() != length_before_preshints {
                 if cfg!(debug_assertions) {
                     for declaration in &applicable_declarations[length_before_preshints..] {
-                        assert_eq!(declaration.level, CascadeLevel::PresHints);
+                        assert_eq!(declaration.level(), CascadeLevel::PresHints);
                     }
                 }
                 // Note the existence of presentational attributes so that the
@@ -1206,7 +1208,7 @@ impl Stylist {
                                                           CascadeLevel::StyleAttributeNormal)
         ];
         let rule_node =
-            self.rule_tree.insert_ordered_rules(v.into_iter().map(|a| (a.source, a.level)));
+            self.rule_tree.insert_ordered_rules(v.into_iter().map(|a| a.source_and_level()));
 
         // This currently ignores visited styles.  It appears to be used for
         // font styles in <canvas> via Servo_StyleSet_ResolveForDeclarations.
@@ -1497,7 +1499,9 @@ pub struct Rule {
     /// The ancestor hashes associated with the selector.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "No heap data")]
     pub hashes: AncestorHashes,
-    /// The source order this style rule appears in.
+    /// The source order this style rule appears in. Note that we only use
+    /// three bytes to store this value in ApplicableDeclarationsBlock, so
+    /// we could repurpose that storage here if we needed to.
     pub source_order: u32,
     /// The actual style rule.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
@@ -1527,9 +1531,8 @@ impl Rule {
                                            -> ApplicableDeclarationBlock {
         ApplicableDeclarationBlock {
             source: StyleSource::Style(self.style_rule.clone()),
-            source_order: self.source_order,
+            source_and_level: SourceOrderAndCascadeLevel::new(self.source_order, level),
             specificity: self.specificity(),
-            level: level,
         }
     }
 
@@ -1549,6 +1552,55 @@ impl Rule {
     }
 }
 
+/// Blink uses 18 bits to store source order, and does not check overflow [1].
+/// That's a limit that could be reached in realistic webpages, so we use
+/// 24 bits and enforce defined behavior in the overflow case.
+///
+/// Note that the value of 24 is also hard-coded into the level() accessor,
+/// which does a byte-aligned load of the 4th byte. If you change this value
+/// you'll need to change that as well.
+///
+/// [1] https://cs.chromium.org/chromium/src/third_party/WebKit/Source/core/css/
+///     RuleSet.h?l=128&rcl=90140ab80b84d0f889abc253410f44ed54ae04f3
+const SOURCE_ORDER_BITS: usize = 24;
+const SOURCE_ORDER_MASK: u32 = (1 << SOURCE_ORDER_BITS) - 1;
+const SOURCE_ORDER_MAX: u32 = SOURCE_ORDER_MASK;
+
+/// Stores the source order of a block and the cascade level it belongs to.
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Copy, Clone, Eq, PartialEq)]
+struct SourceOrderAndCascadeLevel(u32);
+
+impl SourceOrderAndCascadeLevel {
+    fn new(source_order: u32, cascade_level: CascadeLevel) -> SourceOrderAndCascadeLevel {
+        let mut bits = ::std::cmp::min(source_order, SOURCE_ORDER_MAX);
+        bits |= (cascade_level as u8 as u32) << SOURCE_ORDER_BITS;
+        SourceOrderAndCascadeLevel(bits)
+    }
+
+    fn order(&self) -> u32 {
+        self.0 & SOURCE_ORDER_MASK
+    }
+
+    fn level(&self) -> CascadeLevel {
+        unsafe {
+            // Transmute rather than shifting so that we're sure the compiler
+            // emits a simple byte-aligned load.
+            let as_bytes: [u8; 4] = mem::transmute(self.0);
+            CascadeLevel::from_byte(as_bytes[3])
+        }
+    }
+}
+
+impl Debug for SourceOrderAndCascadeLevel {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        f.debug_struct("SourceOrderAndCascadeLevel")
+            .field("order", &self.order())
+            .field("level", &self.level())
+            .finish()
+    }
+}
+
 /// A property declaration together with its precedence among rules of equal
 /// specificity so that we can sort them.
 ///
@@ -1560,12 +1612,10 @@ pub struct ApplicableDeclarationBlock {
     /// The style source, either a style rule, or a property declaration block.
     #[cfg_attr(feature = "servo", ignore_heap_size_of = "Arc")]
     pub source: StyleSource,
-    /// The source order of this block.
+    /// The source order of the block, and the cascade level it belongs to.
-    pub source_order: u32,
+    source_and_level: SourceOrderAndCascadeLevel,
     /// The specificity of the selector this block is represented by.
     pub specificity: u32,
-    /// The cascade level this applicable declaration block is in.
-    pub level: CascadeLevel,
 }
 
 impl ApplicableDeclarationBlock {
@@ -1577,9 +1627,28 @@ impl ApplicableDeclarationBlock {
                              -> Self {
         ApplicableDeclarationBlock {
             source: StyleSource::Declarations(declarations),
-            source_order: 0,
+            source_and_level: SourceOrderAndCascadeLevel::new(0, level),
             specificity: 0,
-            level: level,
         }
     }
+
+    /// Returns the source order of the block.
+    #[inline]
+    pub fn source_order(&self) -> u32 {
+        self.source_and_level.order()
+    }
+
+    /// Returns the cascade level of the block.
+    #[inline]
+    pub fn level(&self) -> CascadeLevel {
+        self.source_and_level.level()
+    }
+
+    /// Convenience method to consume self and return the source alongside the
+    /// level.
+    #[inline]
+    pub fn source_and_level(self) -> (StyleSource, CascadeLevel) {
+        let level = self.level();
+        (self.source, level)
+    }
 }

tests/unit/stylo/size_of.rs

@@ -37,7 +37,7 @@ size_of_test!(test_size_of_element_data, ElementData, 56);
 
 size_of_test!(test_size_of_property_declaration, style::properties::PropertyDeclaration, 32);
 
-size_of_test!(test_size_of_application_declaration_block, ApplicableDeclarationBlock, 32);
+size_of_test!(test_size_of_application_declaration_block, ApplicableDeclarationBlock, 24);
 
 // This is huge, but we allocate it on the stack and then never move it,
 // we only pass `&mut SourcePropertyDeclaration` references around.