diff --git a/components/style/rule_tree/mod.rs b/components/style/rule_tree/mod.rs index be563d2230d..66e5bd214f0 100644 --- a/components/style/rule_tree/mod.rs +++ b/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 { diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs index a65b664ad38..8bb9eeb8e31 100644 --- a/components/style/selector_map.rs +++ b/components/style/selector_map.rs @@ -163,7 +163,7 @@ impl SelectorMap { // 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 { } sort_by_key(&mut rules_list, - |block| (block.specificity, block.source_order)); + |block| (block.specificity, block.source_order())); rules_list } diff --git a/components/style/stylist.rs b/components/style/stylist.rs index 5db9b14c8f2..ecda82f269e 100644 --- a/components/style/stylist.rs +++ b/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 + VecLike + ::std::fmt::Debug, + V: Push + VecLike + 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 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. - pub source_order: u32, + /// The source order of the block, and the cascade level it belongs to. + 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) + } } diff --git a/tests/unit/stylo/size_of.rs b/tests/unit/stylo/size_of.rs index af23e38a43e..6c90f53ffe6 100644 --- a/tests/unit/stylo/size_of.rs +++ b/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.