From af89c74ab7fd174dbc9c17a0d4b97dc231e6108e Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 23 Jun 2017 13:41:35 -0700 Subject: [PATCH 1/2] Use a newtype within EagerPseudoValues. MozReview-Commit-ID: IIDxBJ8mqvJ --- components/style/context.rs | 2 +- components/style/data.rs | 42 ++++++++++++++++++++++++++++--------- 2 files changed, 33 insertions(+), 11 deletions(-) diff --git a/components/style/context.rs b/components/style/context.rs index 33ff5d61697..6a8f3535ce4 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -332,7 +332,7 @@ impl Clone for EagerPseudoCascadeInputs { impl EagerPseudoCascadeInputs { /// Construct inputs from previous cascade results, if any. fn new_from_style(styles: &EagerPseudoStyles) -> Self { - EagerPseudoCascadeInputs(styles.0.as_ref().map(|styles| { + EagerPseudoCascadeInputs(styles.as_array().map(|styles| { let mut inputs: [Option; EAGER_PSEUDO_COUNT] = Default::default(); for i in 0..EAGER_PSEUDO_COUNT { inputs[i] = styles[i].as_ref().map(|s| CascadeInputs::new_from_style(s)); diff --git a/components/style/data.rs b/components/style/data.rs index b50b2170191..104a2a83fad 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -13,6 +13,7 @@ use properties::longhands::display::computed_value as display; use rule_tree::StrongRuleNode; use selector_parser::{EAGER_PSEUDO_COUNT, PseudoElement, RestyleDamage}; use shared_lock::{Locked, StylesheetGuards}; +use std::ops::{Deref, DerefMut}; use stylearc::Arc; bitflags! { @@ -100,22 +101,35 @@ impl RestyleData { /// A list of styles for eagerly-cascaded pseudo-elements. /// Lazily-allocated. -#[derive(Debug)] -pub struct EagerPseudoStyles(pub Option>; EAGER_PSEUDO_COUNT]>>); +#[derive(Clone, Debug)] +pub struct EagerPseudoStyles(Option>); + +#[derive(Debug, Default)] +struct EagerPseudoArray(EagerPseudoArrayInner); +type EagerPseudoArrayInner = [Option>; EAGER_PSEUDO_COUNT]; + +impl Deref for EagerPseudoArray { + type Target = EagerPseudoArrayInner; + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl DerefMut for EagerPseudoArray { + fn deref_mut(&mut self) -> &mut Self::Target { + &mut self.0 + } +} // Manually implement `Clone` here because the derived impl of `Clone` for // array types assumes the value inside is `Copy`. -impl Clone for EagerPseudoStyles { +impl Clone for EagerPseudoArray { fn clone(&self) -> Self { - if self.0.is_none() { - return EagerPseudoStyles(None) - } - let self_values = self.0.as_ref().unwrap(); - let mut values: [Option>; EAGER_PSEUDO_COUNT] = Default::default(); + let mut clone = Self::default(); for i in 0..EAGER_PSEUDO_COUNT { - values[i] = self_values[i].clone(); + clone[i] = self.0[i].clone(); } - EagerPseudoStyles(Some(Box::new(values))) + clone } } @@ -125,6 +139,14 @@ impl EagerPseudoStyles { self.0.is_none() } + /// Grabs a reference to the list of styles, if they exist. + pub fn as_array(&self) -> Option<&EagerPseudoArrayInner> { + match self.0 { + None => None, + Some(ref x) => Some(&x.0), + } + } + /// Returns a reference to the style for a given eager pseudo, if it exists. pub fn get(&self, pseudo: &PseudoElement) -> Option<&Arc> { debug_assert!(pseudo.is_eager()); From eeb62edfeacd50911915ae188d53de477df3de4a Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 23 Jun 2017 13:43:15 -0700 Subject: [PATCH 2/2] Use an Arc for eager pseudo styles. MozReview-Commit-ID: BdYkXxYvUQ3 --- components/style/data.rs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index 104a2a83fad..7888445943d 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -99,10 +99,14 @@ impl RestyleData { } } -/// A list of styles for eagerly-cascaded pseudo-elements. -/// Lazily-allocated. +/// A lazily-allocated list of styles for eagerly-cascaded pseudo-elements. +/// +/// We use an Arc so that sharing these styles via the style sharing cache does +/// not require duplicate allocations. We leverage the copy-on-write semantics of +/// Arc::make_mut(), which is free (i.e. does not require atomic RMU operations) +/// in servo_arc. #[derive(Clone, Debug)] -pub struct EagerPseudoStyles(Option>); +pub struct EagerPseudoStyles(Option>); #[derive(Debug, Default)] struct EagerPseudoArray(EagerPseudoArrayInner); @@ -156,7 +160,10 @@ impl EagerPseudoStyles { /// Returns a mutable reference to the style for a given eager pseudo, if it exists. pub fn get_mut(&mut self, pseudo: &PseudoElement) -> Option<&mut Arc> { debug_assert!(pseudo.is_eager()); - self.0.as_mut().and_then(|p| p[pseudo.eager_index()].as_mut()) + match self.0 { + None => return None, + Some(ref mut arc) => Arc::make_mut(arc)[pseudo.eager_index()].as_mut(), + } } /// Returns true if the EagerPseudoStyles has the style for |pseudo|. @@ -167,9 +174,10 @@ impl EagerPseudoStyles { /// Sets the style for the eager pseudo. pub fn set(&mut self, pseudo: &PseudoElement, value: Arc) { if self.0.is_none() { - self.0 = Some(Box::new(Default::default())); + self.0 = Some(Arc::new(Default::default())); } - self.0.as_mut().unwrap()[pseudo.eager_index()] = Some(value); + let arr = Arc::make_mut(self.0.as_mut().unwrap()); + arr[pseudo.eager_index()] = Some(value); } /// Inserts a pseudo-element. The pseudo-element must not already exist. @@ -180,9 +188,9 @@ impl EagerPseudoStyles { /// Removes a pseudo-element style if it exists, and returns it. pub fn take(&mut self, pseudo: &PseudoElement) -> Option> { - let result = match self.0.as_mut() { + let result = match self.0 { None => return None, - Some(arr) => arr[pseudo.eager_index()].take(), + Some(ref mut arc) => Arc::make_mut(arc)[pseudo.eager_index()].take(), }; let empty = self.0.as_ref().unwrap().iter().all(|x| x.is_none()); if empty {