From 62419adaaaeefd9da21c32899af2a5e1555fe8de Mon Sep 17 00:00:00 2001 From: Cameron McCormack Date: Mon, 16 Jul 2018 12:15:47 +1000 Subject: [PATCH] style: Shrink selectors::Component to 24 bytes. This saves about 37 KiB of memory across the UA style sheets. Bug: 1475197 Reviewed-by: emilio --- components/malloc_size_of/Cargo.toml | 1 + components/malloc_size_of/lib.rs | 21 ++++- components/selectors/Cargo.toml | 1 + components/selectors/attr.rs | 12 +-- components/selectors/lib.rs | 1 + components/selectors/matching.rs | 13 +++- components/selectors/parser.rs | 76 +++++++++++-------- components/style/Cargo.toml | 1 + components/style/gecko/pseudo_element.rs | 1 + .../gecko/pseudo_element_definition.mako.rs | 6 +- components/style/gecko/selector_parser.rs | 12 +-- components/style/lib.rs | 1 + 12 files changed, 96 insertions(+), 50 deletions(-) diff --git a/components/malloc_size_of/Cargo.toml b/components/malloc_size_of/Cargo.toml index 746b1566b141..dafeb9381ecf 100644 --- a/components/malloc_size_of/Cargo.toml +++ b/components/malloc_size_of/Cargo.toml @@ -37,6 +37,7 @@ servo_arc = { path = "../servo_arc" } smallbitvec = "2.1.0" smallvec = "0.6" string_cache = { version = "0.7", optional = true } +thin-slice = "0.1.0" time = { version = "0.1.17", optional = true } url = { version = "1.2", optional = true } webrender_api = { git = "https://github.com/servo/webrender", features = ["ipc"], optional = true } diff --git a/components/malloc_size_of/lib.rs b/components/malloc_size_of/lib.rs index 641d276483ee..f4774d66a3e6 100644 --- a/components/malloc_size_of/lib.rs +++ b/components/malloc_size_of/lib.rs @@ -63,6 +63,7 @@ extern crate smallbitvec; extern crate smallvec; #[cfg(feature = "servo")] extern crate string_cache; +extern crate thin_slice; #[cfg(feature = "servo")] extern crate time; #[cfg(feature = "url")] @@ -231,6 +232,24 @@ impl MallocSizeOf for Box { } } +impl MallocShallowSizeOf for thin_slice::ThinBoxedSlice { + fn shallow_size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + let mut n = 0; + unsafe { + n += thin_slice::ThinBoxedSlice::spilled_storage(self) + .map_or(0, |ptr| ops.malloc_size_of(ptr)); + n += ops.malloc_size_of(&**self); + } + n + } +} + +impl MallocSizeOf for thin_slice::ThinBoxedSlice { + fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize { + self.shallow_size_of(ops) + (**self).size_of(ops) + } +} + impl MallocSizeOf for () { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { 0 @@ -770,7 +789,7 @@ where } impl MallocSizeOf - for selectors::attr::AttrSelectorWithNamespace + for selectors::attr::AttrSelectorWithOptionalNamespace { fn size_of(&self, _ops: &mut MallocSizeOfOps) -> usize { 0 diff --git a/components/selectors/Cargo.toml b/components/selectors/Cargo.toml index 3663fcb76893..20ca61fb7847 100644 --- a/components/selectors/Cargo.toml +++ b/components/selectors/Cargo.toml @@ -30,6 +30,7 @@ phf = "0.7.18" precomputed-hash = "0.1" servo_arc = { version = "0.1", path = "../servo_arc" } smallvec = "0.6.2" +thin-slice = "0.1.0" [build-dependencies] phf_codegen = "0.7.18" diff --git a/components/selectors/attr.rs b/components/selectors/attr.rs index b36be8a0a683..c0f5fe731859 100644 --- a/components/selectors/attr.rs +++ b/components/selectors/attr.rs @@ -7,20 +7,20 @@ use parser::SelectorImpl; use std::fmt; #[derive(Clone, Eq, PartialEq)] -pub struct AttrSelectorWithNamespace { - pub namespace: NamespaceConstraint<(Impl::NamespacePrefix, Impl::NamespaceUrl)>, +pub struct AttrSelectorWithOptionalNamespace { + pub namespace: Option>, pub local_name: Impl::LocalName, pub local_name_lower: Impl::LocalName, pub operation: ParsedAttrSelectorOperation, pub never_matches: bool, } -impl AttrSelectorWithNamespace { - pub fn namespace(&self) -> NamespaceConstraint<&Impl::NamespaceUrl> { - match self.namespace { +impl AttrSelectorWithOptionalNamespace { + pub fn namespace(&self) -> Option> { + self.namespace.as_ref().map(|ns| match ns { NamespaceConstraint::Any => NamespaceConstraint::Any, NamespaceConstraint::Specific((_, ref url)) => NamespaceConstraint::Specific(url), - } + }) } } diff --git a/components/selectors/lib.rs b/components/selectors/lib.rs index a41a71ef8fd4..aa30604bbd1a 100644 --- a/components/selectors/lib.rs +++ b/components/selectors/lib.rs @@ -19,6 +19,7 @@ extern crate phf; extern crate precomputed_hash; extern crate servo_arc; extern crate smallvec; +extern crate thin_slice; pub mod attr; pub mod bloom; diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index fd4765c2dff4..1fe54b217d23 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -699,7 +699,6 @@ where }, Component::AttributeInNoNamespace { ref local_name, - ref local_name_lower, ref value, operator, case_sensitivity, @@ -711,7 +710,7 @@ where let is_html = element.is_html_element_in_html_document(); element.attr_matches( &NamespaceConstraint::Specific(&::parser::namespace_empty_string::()), - select_name(is_html, local_name, local_name_lower), + local_name, &AttrSelectorOperation::WithValue { operator: operator, case_sensitivity: case_sensitivity.to_unconditional(is_html), @@ -724,8 +723,16 @@ where return false; } let is_html = element.is_html_element_in_html_document(); + let empty_string; + let namespace = match attr_sel.namespace() { + Some(ns) => ns, + None => { + empty_string = ::parser::namespace_empty_string::(); + NamespaceConstraint::Specific(&empty_string) + } + }; element.attr_matches( - &attr_sel.namespace(), + &namespace, select_name(is_html, &attr_sel.local_name, &attr_sel.local_name_lower), &match attr_sel.operation { ParsedAttrSelectorOperation::Exists => AttrSelectorOperation::Exists, diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 399f21e6361b..d0f98cde57bb 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -2,8 +2,9 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use attr::{AttrSelectorOperator, AttrSelectorWithNamespace, ParsedAttrSelectorOperation}; -use attr::{NamespaceConstraint, ParsedCaseSensitivity, SELECTOR_WHITESPACE}; +use attr::{AttrSelectorOperator, AttrSelectorWithOptionalNamespace}; +use attr::{NamespaceConstraint, ParsedAttrSelectorOperation}; +use attr::{ParsedCaseSensitivity, SELECTOR_WHITESPACE}; use bloom::BLOOM_HASH_MASK; use builder::{SelectorBuilder, SpecificityAndFlags}; use context::QuirksMode; @@ -19,6 +20,7 @@ use std::borrow::{Borrow, Cow}; use std::fmt::{self, Debug, Display, Write}; use std::iter::Rev; use std::slice; +use thin_slice::ThinBoxedSlice; pub use visitor::{SelectorVisitor, Visit}; /// A trait that represents a pseudo-element. @@ -45,6 +47,8 @@ pub trait NonTSPseudoClass: Sized + ToCss { fn is_active_or_hover(&self) -> bool; } +/// Returns a Cow::Borrowed if `s` is already ASCII lowercase, and a +/// Cow::Owned if `s` had to be converted into ASCII lowercase. fn to_ascii_lowercase(s: &str) -> Cow { if let Some(first_uppercase) = s.bytes().position(|byte| byte >= b'A' && byte <= b'Z') { let mut string = s.to_owned(); @@ -428,7 +432,6 @@ where }, AttributeInNoNamespace { ref local_name, - ref local_name_lower, never_matches, .. } if !never_matches => @@ -436,14 +439,22 @@ where if !visitor.visit_attribute_selector( &NamespaceConstraint::Specific(&namespace_empty_string::()), local_name, - local_name_lower, + local_name, ) { return false; } }, AttributeOther(ref attr_selector) if !attr_selector.never_matches => { + let empty_string; + let namespace = match attr_selector.namespace() { + Some(ns) => ns, + None => { + empty_string = ::parser::namespace_empty_string::(); + NamespaceConstraint::Specific(&empty_string) + } + }; if !visitor.visit_attribute_selector( - &attr_selector.namespace(), + &namespace, &attr_selector.local_name, &attr_selector.local_name_lower, ) { @@ -815,16 +826,16 @@ pub enum Component { local_name: Impl::LocalName, local_name_lower: Impl::LocalName, }, + // Used only when local_name is already lowercase. AttributeInNoNamespace { local_name: Impl::LocalName, - local_name_lower: Impl::LocalName, operator: AttrSelectorOperator, value: Impl::AttrValue, case_sensitivity: ParsedCaseSensitivity, never_matches: bool, }, // Use a Box in the less common cases with more data to keep size_of::() small. - AttributeOther(Box>), + AttributeOther(Box>), /// Pseudo-classes /// @@ -836,7 +847,7 @@ pub enum Component { /// need to think about how this should interact with /// visit_complex_selector, and what the consumers of those APIs should do /// about the presence of combinators in negation. - Negation(Box<[Component]>), + Negation(ThinBoxedSlice>), FirstChild, LastChild, OnlyChild, @@ -948,7 +959,7 @@ impl Debug for Component { self.to_css(f) } } -impl Debug for AttrSelectorWithNamespace { +impl Debug for AttrSelectorWithOptionalNamespace { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { self.to_css(f) } @@ -1238,18 +1249,19 @@ impl ToCss for Component { } } -impl ToCss for AttrSelectorWithNamespace { +impl ToCss for AttrSelectorWithOptionalNamespace { fn to_css(&self, dest: &mut W) -> fmt::Result where W: fmt::Write, { dest.write_char('[')?; match self.namespace { - NamespaceConstraint::Specific((ref prefix, _)) => { + Some(NamespaceConstraint::Specific((ref prefix, _))) => { display_to_css_identifier(prefix, dest)?; dest.write_char('|')? }, - NamespaceConstraint::Any => dest.write_str("*|")?, + Some(NamespaceConstraint::Any) => dest.write_str("*|")?, + None => {} } display_to_css_identifier(&self.local_name, dest)?; match self.operation { @@ -1628,8 +1640,8 @@ where let local_name = local_name.as_ref().into(); if let Some(namespace) = namespace { return Ok(Component::AttributeOther(Box::new( - AttrSelectorWithNamespace { - namespace: namespace, + AttrSelectorWithOptionalNamespace { + namespace: Some(namespace), local_name: local_name, local_name_lower: local_name_lower, operation: ParsedAttrSelectorOperation::Exists, @@ -1685,6 +1697,7 @@ where let value = value.as_ref().into(); let local_name_lower; + let local_name_is_ascii_lowercase; { let local_name_lower_cow = to_ascii_lowercase(&local_name); if let ParsedCaseSensitivity::CaseSensitive = case_sensitivity { @@ -1699,15 +1712,16 @@ where } } local_name_lower = local_name_lower_cow.as_ref().into(); + local_name_is_ascii_lowercase = matches!(local_name_lower_cow, Cow::Borrowed(..)); } let local_name = local_name.as_ref().into(); - if let Some(namespace) = namespace { + if namespace.is_some() || !local_name_is_ascii_lowercase { Ok(Component::AttributeOther(Box::new( - AttrSelectorWithNamespace { - namespace: namespace, - local_name: local_name, - local_name_lower: local_name_lower, - never_matches: never_matches, + AttrSelectorWithOptionalNamespace { + namespace, + local_name, + local_name_lower, + never_matches, operation: ParsedAttrSelectorOperation::WithValue { operator: operator, case_sensitivity: case_sensitivity, @@ -1718,7 +1732,6 @@ where } else { Ok(Component::AttributeInNoNamespace { local_name: local_name, - local_name_lower: local_name_lower, operator: operator, value: value, case_sensitivity: case_sensitivity, @@ -1785,7 +1798,7 @@ where } // Success. - Ok(Component::Negation(sequence.into_vec().into_boxed_slice())) + Ok(Component::Negation(sequence.into_vec().into_boxed_slice().into())) } /// simple_selector_sequence @@ -2625,7 +2638,7 @@ pub mod tests { vec![ Component::DefaultNamespace(MATHML.into()), Component::Negation( - vec![Component::Class(DummyAtom::from("cl"))].into_boxed_slice(), + vec![Component::Class(DummyAtom::from("cl"))].into_boxed_slice().into(), ), ], specificity(0, 1, 0), @@ -2642,7 +2655,7 @@ pub mod tests { vec![ Component::DefaultNamespace(MATHML.into()), Component::ExplicitUniversalType, - ].into_boxed_slice(), + ].into_boxed_slice().into(), ), ], specificity(0, 0, 0), @@ -2662,7 +2675,7 @@ pub mod tests { name: DummyAtom::from("e"), lower_name: DummyAtom::from("e"), }), - ].into_boxed_slice(), + ].into_boxed_slice().into(), ), ], specificity(0, 0, 1), @@ -2676,7 +2689,6 @@ pub mod tests { vec![ Component::AttributeInNoNamespace { local_name: DummyAtom::from("attr"), - local_name_lower: DummyAtom::from("attr"), operator: AttrSelectorOperator::DashMatch, value: DummyAtom::from("foo"), never_matches: false, @@ -2770,7 +2782,7 @@ pub mod tests { Selector::from_vec( vec![ Component::Negation( - vec![Component::ID(DummyAtom::from("provel"))].into_boxed_slice(), + vec![Component::ID(DummyAtom::from("provel"))].into_boxed_slice().into(), ), ], specificity(1, 0, 0), @@ -2789,7 +2801,7 @@ pub mod tests { name: DummyAtom::from("circle"), lower_name: DummyAtom::from("circle"), }), - ].into_boxed_slice(), + ].into_boxed_slice().into(), ), ], specificity(0, 0, 1), @@ -2803,7 +2815,7 @@ pub mod tests { Selector::from_vec( vec![ Component::Negation( - vec![Component::ExplicitUniversalType].into_boxed_slice(), + vec![Component::ExplicitUniversalType].into_boxed_slice().into(), ), ], specificity(0, 0, 0), @@ -2819,7 +2831,7 @@ pub mod tests { vec![ Component::ExplicitNoNamespace, Component::ExplicitUniversalType, - ].into_boxed_slice(), + ].into_boxed_slice().into(), ), ], specificity(0, 0, 0), @@ -2834,7 +2846,7 @@ pub mod tests { Selector::from_vec( vec![ Component::Negation( - vec![Component::ExplicitUniversalType].into_boxed_slice(), + vec![Component::ExplicitUniversalType].into_boxed_slice().into(), ), ], specificity(0, 0, 0), @@ -2850,7 +2862,7 @@ pub mod tests { vec![ Component::Namespace(DummyAtom("svg".into()), SVG.into()), Component::ExplicitUniversalType, - ].into_boxed_slice(), + ].into_boxed_slice().into(), ), ], specificity(0, 0, 0), diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index 2eabd9a0007a..7fb11e86f529 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -65,6 +65,7 @@ string_cache = { version = "0.7", optional = true } style_derive = {path = "../style_derive"} style_traits = {path = "../style_traits"} servo_url = {path = "../url", optional = true} +thin-slice = "0.1.0" time = "0.1" uluru = "0.2" unicode-bidi = "0.3" diff --git a/components/style/gecko/pseudo_element.rs b/components/style/gecko/pseudo_element.rs index df1b873a1b77..296b63398578 100644 --- a/components/style/gecko/pseudo_element.rs +++ b/components/style/gecko/pseudo_element.rs @@ -15,6 +15,7 @@ use properties::longhands::display::computed_value::T as Display; use selector_parser::{NonTSPseudoClass, PseudoElementCascadeType, SelectorImpl}; use std::fmt; use string_cache::Atom; +use thin_slice::ThinBoxedSlice; use values::serialize_atom_identifier; include!(concat!( diff --git a/components/style/gecko/pseudo_element_definition.mako.rs b/components/style/gecko/pseudo_element_definition.mako.rs index 216650c9295b..30f71a496d86 100644 --- a/components/style/gecko/pseudo_element_definition.mako.rs +++ b/components/style/gecko/pseudo_element_definition.mako.rs @@ -8,7 +8,7 @@ pub enum PseudoElement { % for pseudo in PSEUDOS: /// ${pseudo.value} % if pseudo.is_tree_pseudo_element(): - ${pseudo.capitalized()}(Box<[Atom]>), + ${pseudo.capitalized()}(ThinBoxedSlice), % else: ${pseudo.capitalized()}, % endif @@ -209,7 +209,7 @@ impl PseudoElement { % for pseudo in PSEUDOS: % if pseudo.is_tree_pseudo_element(): if atom == &atom!("${pseudo.value}") { - return Some(PseudoElement::${pseudo.capitalized()}(args)); + return Some(PseudoElement::${pseudo.capitalized()}(args.into())); } % endif % endfor @@ -256,7 +256,7 @@ impl PseudoElement { let tree_part = &name[10..]; % for pseudo in TREE_PSEUDOS: if tree_part.eq_ignore_ascii_case("${pseudo.value[11:]}") { - return Some(${pseudo_element_variant(pseudo, "args")}); + return Some(${pseudo_element_variant(pseudo, "args.into()")}); } % endfor None diff --git a/components/style/gecko/selector_parser.rs b/components/style/gecko/selector_parser.rs index 45f42acc09db..61e282c1dce0 100644 --- a/components/style/gecko/selector_parser.rs +++ b/components/style/gecko/selector_parser.rs @@ -13,11 +13,13 @@ use gecko_bindings::sugar::ownership::{HasBoxFFI, HasFFI, HasSimpleFFI}; use invalidation::element::document_state::InvalidationMatchingData; use selector_parser::{Direction, SelectorParser}; use selectors::SelectorList; -use selectors::parser::{self as selector_parser, Selector, SelectorParseErrorKind, Visit}; +use selectors::parser::{self as selector_parser, Selector}; +use selectors::parser::{SelectorParseErrorKind, Visit}; use selectors::visitor::SelectorVisitor; use std::fmt; use string_cache::{Atom, Namespace, WeakAtom, WeakNamespace}; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss as ToCss_}; +use thin_slice::ThinBoxedSlice; pub use gecko::pseudo_element::{PseudoElement, EAGER_PSEUDOS, EAGER_PSEUDO_COUNT, PSEUDO_COUNT}; pub use gecko::snapshot::SnapshotMap; @@ -34,7 +36,7 @@ bitflags! { } /// The type used for storing pseudo-class string arguments. -pub type PseudoClassStringArg = Box<[u16]>; +pub type PseudoClassStringArg = ThinBoxedSlice; macro_rules! pseudo_class_name { (bare: [$(($css:expr, $name:ident, $gecko_type:tt, $state:tt, $flags:tt),)*], @@ -56,7 +58,7 @@ macro_rules! pseudo_class_name { /// /// TODO(emilio): We disallow combinators and pseudos here, so we /// should use SimpleSelector instead - MozAny(Box<[Selector]>), + MozAny(ThinBoxedSlice>), /// The non-standard `:-moz-locale-dir` pseudo-class. MozLocaleDir(Box), } @@ -405,7 +407,7 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> { // convert to null terminated utf16 string // since that's what Gecko deals with let utf16: Vec = name.encode_utf16().chain(Some(0u16)).collect(); - NonTSPseudoClass::$s_name(utf16.into_boxed_slice()) + NonTSPseudoClass::$s_name(utf16.into_boxed_slice().into()) }, )* "-moz-locale-dir" => { NonTSPseudoClass::MozLocaleDir( @@ -422,7 +424,7 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> { selector_parser::parse_compound_selector_list( self, parser, - )? + )?.into() ) } _ => return Err(parser.new_custom_error( diff --git a/components/style/lib.rs b/components/style/lib.rs index a4de57228c49..519b61877597 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -93,6 +93,7 @@ extern crate string_cache; #[macro_use] extern crate style_derive; extern crate style_traits; +extern crate thin_slice; extern crate time; extern crate uluru; extern crate unicode_bidi;