Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-03 20:50:07 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Massage the resolver code so it's cleaner and prevents the problem.

Browse source 
This way all the borrows stay in the sharing code, and prevents an extra borrow
on a cache hit, which is not a big deal but nice.
This commit is contained in:
Emilio Cobos Álvarez 2017-09-18 03:21:39 +02:00
parent 36fed07b1c
commit 891bc7d174
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
6 changed files with 119 additions and 119 deletions
Download patch file Download diff file Expand all files Collapse all files

11
components/style/data.rs
View file

@ -284,13 +284,18 @@ impl ElementData {
/// Returns this element's primary style as a resolved style to use for sharing. /// Returns this element's primary style as a resolved style to use for sharing.
pub fn share_primary_style(&self) -> PrimaryStyle { pub fn share_primary_style(&self) -> PrimaryStyle {
let primary_is_reused = self.flags.contains(PRIMARY_STYLE_REUSED_VIA_RULE_NODE); let reused_via_rule_node =
PrimaryStyle(ResolvedStyle::new(self.styles.primary().clone(), primary_is_reused)) self.flags.contains(PRIMARY_STYLE_REUSED_VIA_RULE_NODE);
PrimaryStyle {
style: ResolvedStyle(self.styles.primary().clone()),
reused_via_rule_node,
}
} }
/// Sets a new set of styles, returning the old ones. /// Sets a new set of styles, returning the old ones.
pub fn set_styles(&mut self, new_styles: ResolvedElementStyles) -> ElementStyles { pub fn set_styles(&mut self, new_styles: ResolvedElementStyles) -> ElementStyles {
if new_styles.primary.0.reused_via_rule_node { if new_styles.primary.reused_via_rule_node {
self.flags.insert(PRIMARY_STYLE_REUSED_VIA_RULE_NODE); self.flags.insert(PRIMARY_STYLE_REUSED_VIA_RULE_NODE);
} else { } else {
self.flags.remove(PRIMARY_STYLE_REUSED_VIA_RULE_NODE); self.flags.remove(PRIMARY_STYLE_REUSED_VIA_RULE_NODE);

4
components/style/matching.rs
View file

@ -158,7 +158,7 @@ trait PrivateMatchMethods: TElement {
StyleResolverForElement::new(*self, context, RuleInclusion::All, PseudoElementResolution::IfApplicable) StyleResolverForElement::new(*self, context, RuleInclusion::All, PseudoElementResolution::IfApplicable)
.cascade_style_and_visited_with_default_parents(inputs); .cascade_style_and_visited_with_default_parents(inputs);
Some(style.into()) Some(style.0)
} }
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]
@ -542,7 +542,7 @@ pub trait MatchMethods : TElement {
self.process_animations( self.process_animations(
context, context,
&mut data.styles.primary, &mut data.styles.primary,
&mut new_styles.primary.0.style, &mut new_styles.primary.style.0,
data.hint, data.hint,
important_rules_changed, important_rules_changed,
); );

69
components/style/sharing/mod.rs
View file

@ -82,7 +82,7 @@ use smallvec::SmallVec;
use std::marker::PhantomData; use std::marker::PhantomData;
use std::mem; use std::mem;
use std::ops::Deref; use std::ops::Deref;
use style_resolver::PrimaryStyle; use style_resolver::{PrimaryStyle, ResolvedElementStyles};
use stylist::Stylist; use stylist::Stylist;
mod checks; mod checks;
@ -380,7 +380,7 @@ impl<E: TElement> StyleSharingTarget<E> {
pub fn share_style_if_possible( pub fn share_style_if_possible(
&mut self, &mut self,
context: &mut StyleContext<E>, context: &mut StyleContext<E>,
) -> Option<E> { ) -> Option<ResolvedElementStyles> {
let cache = &mut context.thread_local.sharing_cache; let cache = &mut context.thread_local.sharing_cache;
let shared_context = &context.shared; let shared_context = &context.shared;
let selector_flags_map = &mut context.thread_local.selector_flags; let selector_flags_map = &mut context.thread_local.selector_flags;
@ -443,25 +443,25 @@ impl<E: TElement> SharingCache<E> {
self.entries.insert(StyleSharingCandidate { element, validation_data }); self.entries.insert(StyleSharingCandidate { element, validation_data });
} }
fn lookup<F>(&mut self, mut is_match: F) -> Option<E> fn lookup<F, R>(&mut self, mut test_one: F) -> Option<R>
where where
F: FnMut(&mut StyleSharingCandidate<E>) -> bool F: FnMut(&mut StyleSharingCandidate<E>) -> Option<R>
{ {
let mut index = None; let mut result = None;
for (i, candidate) in self.entries.iter_mut() { for (i, candidate) in self.entries.iter_mut() {
if is_match(candidate) { if let Some(r) = test_one(candidate) {
index = Some(i); result = Some((i, r));
break; break;
} }
}; };
match index { match result {
None => None, None => None,
Some(i) => { Some((i, r)) => {
self.entries.touch(i); self.entries.touch(i);
let front = self.entries.front_mut().unwrap(); let front = self.entries.front_mut().unwrap();
debug_assert!(is_match(front)); debug_assert!(test_one(front).is_some());
Some(front.element) Some(r)
} }
} }
} }
@ -620,7 +620,7 @@ impl<E: TElement> StyleSharingCache<E> {
selector_flags_map: &mut SelectorFlagsMap<E>, selector_flags_map: &mut SelectorFlagsMap<E>,
bloom_filter: &StyleBloom<E>, bloom_filter: &StyleBloom<E>,
target: &mut StyleSharingTarget<E>, target: &mut StyleSharingTarget<E>,
) -> Option<E> { ) -> Option<ResolvedElementStyles> {
if shared_context.options.disable_style_sharing_cache { if shared_context.options.disable_style_sharing_cache {
debug!("{:?} Cannot share style: style sharing cache disabled", debug!("{:?} Cannot share style: style sharing cache disabled",
target.element); target.element);
@ -655,42 +655,42 @@ impl<E: TElement> StyleSharingCache<E> {
shared: &SharedStyleContext, shared: &SharedStyleContext,
bloom: &StyleBloom<E>, bloom: &StyleBloom<E>,
selector_flags_map: &mut SelectorFlagsMap<E> selector_flags_map: &mut SelectorFlagsMap<E>
) -> bool { ) -> Option<ResolvedElementStyles> {
// Check that we have the same parent, or at least that the parents // Check that we have the same parent, or at least that the parents
// share styles and permit sharing across their children. The latter // share styles and permit sharing across their children. The latter
// check allows us to share style between cousins if the parents // check allows us to share style between cousins if the parents
// shared style. // shared style.
if !checks::parents_allow_sharing(target, candidate) { if !checks::parents_allow_sharing(target, candidate) {
trace!("Miss: Parent"); trace!("Miss: Parent");
return false; return None;
} }
if target.is_native_anonymous() { if target.is_native_anonymous() {
debug_assert!(!candidate.element.is_native_anonymous(), debug_assert!(!candidate.element.is_native_anonymous(),
"Why inserting NAC into the cache?"); "Why inserting NAC into the cache?");
trace!("Miss: Native Anonymous Content"); trace!("Miss: Native Anonymous Content");
return false; return None;
} }
if *target.get_local_name() != *candidate.element.get_local_name() { if *target.get_local_name() != *candidate.element.get_local_name() {
trace!("Miss: Local Name"); trace!("Miss: Local Name");
return false; return None;
} }
if *target.get_namespace() != *candidate.element.get_namespace() { if *target.get_namespace() != *candidate.element.get_namespace() {
trace!("Miss: Namespace"); trace!("Miss: Namespace");
return false; return None;
} }
if target.is_link() != candidate.element.is_link() { if target.is_link() != candidate.element.is_link() {
trace!("Miss: Link"); trace!("Miss: Link");
return false; return None;
} }
if target.matches_user_and_author_rules() != if target.matches_user_and_author_rules() !=
candidate.element.matches_user_and_author_rules() { candidate.element.matches_user_and_author_rules() {
trace!("Miss: User and Author Rules"); trace!("Miss: User and Author Rules");
return false; return None;
} }
// We do not ignore visited state here, because Gecko // We do not ignore visited state here, because Gecko
@ -698,7 +698,7 @@ impl<E: TElement> StyleSharingCache<E> {
// so these contexts cannot be shared // so these contexts cannot be shared
if target.element.get_state() != candidate.get_state() { if target.element.get_state() != candidate.get_state() {
trace!("Miss: User and Author State"); trace!("Miss: User and Author State");
return false; return None;
} }
let element_id = target.element.get_id(); let element_id = target.element.get_id();
@ -708,38 +708,38 @@ impl<E: TElement> StyleSharingCache<E> {
if checks::may_have_rules_for_ids(shared, element_id.as_ref(), if checks::may_have_rules_for_ids(shared, element_id.as_ref(),
candidate_id.as_ref()) { candidate_id.as_ref()) {
trace!("Miss: ID Attr"); trace!("Miss: ID Attr");
return false; return None;
} }
} }
if !checks::have_same_style_attribute(target, candidate) { if !checks::have_same_style_attribute(target, candidate) {
trace!("Miss: Style Attr"); trace!("Miss: Style Attr");
return false; return None;
} }
if !checks::have_same_class(target, candidate) { if !checks::have_same_class(target, candidate) {
trace!("Miss: Class"); trace!("Miss: Class");
return false; return None;
} }
if !checks::have_same_presentational_hints(target, candidate) { if !checks::have_same_presentational_hints(target, candidate) {
trace!("Miss: Pres Hints"); trace!("Miss: Pres Hints");
return false; return None;
} }
if !checks::revalidate(target, candidate, shared, bloom, if !checks::revalidate(target, candidate, shared, bloom,
selector_flags_map) { selector_flags_map) {
trace!("Miss: Revalidation"); trace!("Miss: Revalidation");
return false; return None;
} }
debug_assert!(target.has_current_styles_for_traversal( debug_assert!(target.has_current_styles_for_traversal(
&candidate.element.borrow_data().unwrap(), &candidate.element.borrow_data().unwrap(),
shared.traversal_flags, shared.traversal_flags,
)); ));
debug!("Sharing allowed between {:?} and {:?}", target.element, candidate.element);
true debug!("Sharing allowed between {:?} and {:?}", target.element, candidate.element);
Some(candidate.element.borrow_data().unwrap().share_styles())
} }
/// Attempts to find an element in the cache with the given primary rule node and parent. /// Attempts to find an element in the cache with the given primary rule node and parent.
@ -749,21 +749,18 @@ impl<E: TElement> StyleSharingCache<E> {
rules: &StrongRuleNode, rules: &StrongRuleNode,
visited_rules: Option<&StrongRuleNode>, visited_rules: Option<&StrongRuleNode>,
target: E, target: E,
) -> Option<E> { ) -> Option<PrimaryStyle> {
self.cache_mut().lookup(|candidate| { self.cache_mut().lookup(|candidate| {
if candidate.element == target {
return false;
}
if !candidate.parent_style_identity().eq(inherited) { if !candidate.parent_style_identity().eq(inherited) {
return false; return None;
} }
let data = candidate.element.borrow_data().unwrap(); let data = candidate.element.borrow_data().unwrap();
let style = data.styles.primary(); let style = data.styles.primary();
if style.rules.as_ref() != Some(&rules) { if style.rules.as_ref() != Some(&rules) {
return false; return None;
} }
if style.visited_rules() != visited_rules { if style.visited_rules() != visited_rules {
return false; return None;
} }
// Rule nodes and styles are computed independent of the element's // Rule nodes and styles are computed independent of the element's
@ -778,10 +775,10 @@ impl<E: TElement> StyleSharingCache<E> {
// requirements of visited, assuming we get a cache hit on only one // requirements of visited, assuming we get a cache hit on only one
// of unvisited vs. visited. // of unvisited vs. visited.
if target.is_visited_link() != candidate.element.is_visited_link() { if target.is_visited_link() != candidate.element.is_visited_link() {
return false; return None;
} }
true Some(data.share_primary_style())
}) })
} }
} }

140
components/style/style_resolver.rs
View file

@ -50,15 +50,16 @@ struct MatchingResults {
} }
/// A style returned from the resolver machinery. /// A style returned from the resolver machinery.
pub struct ResolvedStyle { pub struct ResolvedStyle(pub Arc<ComputedValues>);
/// The style itself.
pub style: Arc<ComputedValues>,
/// Whether the style was reused from another element via the rule node.
pub reused_via_rule_node: bool,
}
/// The primary style of an element or an element-backed pseudo-element. /// The primary style of an element or an element-backed pseudo-element.
pub struct PrimaryStyle(pub ResolvedStyle); pub struct PrimaryStyle {
/// The style itself.
pub style: ResolvedStyle,
/// Whether the style was reused from another element via the rule node (see
/// `StyleSharingCache::lookup_by_rules`).
pub reused_via_rule_node: bool,
}
/// A set of style returned from the resolver machinery. /// A set of style returned from the resolver machinery.
pub struct ResolvedElementStyles { pub struct ResolvedElementStyles {
@ -68,30 +69,17 @@ pub struct ResolvedElementStyles {
pub pseudos: EagerPseudoStyles, pub pseudos: EagerPseudoStyles,
} }
impl ResolvedStyle {
/// Creates a new ResolvedStyle.
pub fn new(style: Arc<ComputedValues>, reused_via_rule_node: bool) -> Self {
ResolvedStyle { style, reused_via_rule_node }
}
}
impl PrimaryStyle { impl PrimaryStyle {
/// Convenience accessor for the style. /// Convenience accessor for the style.
pub fn style(&self) -> &ComputedValues { pub fn style(&self) -> &ComputedValues {
&*self.0.style &*self.style.0
}
}
impl From<ResolvedStyle> for Arc<ComputedValues> {
fn from(r: ResolvedStyle) -> Arc<ComputedValues> {
r.style
} }
} }
impl From<ResolvedElementStyles> for ElementStyles { impl From<ResolvedElementStyles> for ElementStyles {
fn from(r: ResolvedElementStyles) -> ElementStyles { fn from(r: ResolvedElementStyles) -> ElementStyles {
ElementStyles { ElementStyles {
primary: Some(r.primary.0.into()), primary: Some(r.primary.style.0),
pseudos: r.pseudos, pseudos: r.pseudos,
} }
} }
@ -182,17 +170,54 @@ where
None None
}; };
PrimaryStyle( self.cascade_primary_style(
self.cascade_style_and_visited( CascadeInputs {
CascadeInputs { rules: Some(primary_results.rule_node),
rules: Some(primary_results.rule_node), visited_rules,
visited_rules, },
}, parent_style,
layout_parent_style,
)
}
fn cascade_primary_style(
&mut self,
inputs: CascadeInputs,
parent_style: Option<&ComputedValues>,
layout_parent_style: Option<&ComputedValues>,
) -> PrimaryStyle {
// Before doing the cascade, check the sharing cache and see if we can
// reuse the style via rule node identity.
let may_reuse =
!self.element.is_native_anonymous() &&
parent_style.is_some() &&
inputs.rules.is_some();
if may_reuse {
let cached = self.context.thread_local.sharing_cache.lookup_by_rules(
parent_style.unwrap(),
inputs.rules.as_ref().unwrap(),
inputs.visited_rules.as_ref(),
self.element,
);
if let Some(mut primary_style) = cached {
self.context.thread_local.statistics.styles_reused += 1;
primary_style.reused_via_rule_node |= true;
return primary_style;
}
}
// No style to reuse. Cascade the style, starting with visited style
// if necessary.
PrimaryStyle {
style: self.cascade_style_and_visited(
inputs,
parent_style, parent_style,
layout_parent_style, layout_parent_style,
/* pseudo = */ None, /* pseudo = */ None,
) ),
) reused_via_rule_node: false,
}
} }
/// Resolve the style of a given element, and all its eager pseudo-elements. /// Resolve the style of a given element, and all its eager pseudo-elements.
@ -222,10 +247,10 @@ where
if let Some(style) = pseudo_style { if let Some(style) = pseudo_style {
if !matches!(self.pseudo_resolution, PseudoElementResolution::Force) && if !matches!(self.pseudo_resolution, PseudoElementResolution::Force) &&
eager_pseudo_is_definitely_not_generated(&pseudo, &style) { eager_pseudo_is_definitely_not_generated(&pseudo, &style.0) {
return; return;
} }
pseudo_styles.set(&pseudo, style); pseudo_styles.set(&pseudo, style.0);
} }
}) })
} }
@ -266,27 +291,6 @@ where
layout_parent_style: Option<&ComputedValues>, layout_parent_style: Option<&ComputedValues>,
pseudo: Option<&PseudoElement>, pseudo: Option<&PseudoElement>,
) -> ResolvedStyle { ) -> ResolvedStyle {
// Before doing the cascade, check the sharing cache and see if we can reuse
// the style via rule node identity.
let may_reuse = pseudo.is_none() && !self.element.is_native_anonymous() &&
parent_style.is_some() && inputs.rules.is_some();
if may_reuse {
let cached = self.context.thread_local.sharing_cache.lookup_by_rules(
parent_style.unwrap(),
inputs.rules.as_ref().unwrap(),
inputs.visited_rules.as_ref(),
self.element,
);
if let Some(el) = cached {
self.context.thread_local.statistics.styles_reused += 1;
let mut style = el.borrow_data().unwrap().share_primary_style().0;
style.reused_via_rule_node = true;
return style;
}
}
// No style to reuse. Cascade the style, starting with visited style
// if necessary.
let mut style_if_visited = None; let mut style_if_visited = None;
if parent_style.map_or(false, |s| s.get_visited_style().is_some()) || if parent_style.map_or(false, |s| s.get_visited_style().is_some()) ||
inputs.visited_rules.is_some() { inputs.visited_rules.is_some() {
@ -300,17 +304,16 @@ where
)); ));
} }
ResolvedStyle { ResolvedStyle(
style: self.cascade_style( self.cascade_style(
inputs.rules.as_ref(), inputs.rules.as_ref(),
style_if_visited, style_if_visited,
parent_style, parent_style,
layout_parent_style, layout_parent_style,
CascadeVisitedMode::Unvisited, CascadeVisitedMode::Unvisited,
pseudo, pseudo,
), )
reused_via_rule_node: false, )
}
} }
/// Cascade the element and pseudo-element styles with the default parents. /// Cascade the element and pseudo-element styles with the default parents.
@ -319,13 +322,10 @@ where
inputs: ElementCascadeInputs, inputs: ElementCascadeInputs,
) -> ResolvedElementStyles { ) -> ResolvedElementStyles {
with_default_parent_styles(self.element, move |parent_style, layout_parent_style| { with_default_parent_styles(self.element, move |parent_style, layout_parent_style| {
let primary_style = PrimaryStyle( let primary_style = self.cascade_primary_style(
self.cascade_style_and_visited( inputs.primary,
inputs.primary, parent_style,
parent_style, layout_parent_style,
layout_parent_style,
/* pseudo = */ None,
)
); );
let mut pseudo_styles = EagerPseudoStyles::default(); let mut pseudo_styles = EagerPseudoStyles::default();
@ -344,17 +344,17 @@ where
let style = let style =
self.cascade_style_and_visited( self.cascade_style_and_visited(
inputs, inputs,
Some(&*primary_style.style()), Some(primary_style.style()),
layout_parent_style_for_pseudo, layout_parent_style_for_pseudo,
Some(&pseudo), Some(&pseudo),
); );
if !matches!(self.pseudo_resolution, PseudoElementResolution::Force) && if !matches!(self.pseudo_resolution, PseudoElementResolution::Force) &&
eager_pseudo_is_definitely_not_generated(&pseudo, &style.style) { eager_pseudo_is_definitely_not_generated(&pseudo, &style.0) {
continue; continue;
} }
pseudo_styles.set(&pseudo, style.style); pseudo_styles.set(&pseudo, style.0);
} }
} }
} }
@ -371,7 +371,7 @@ where
pseudo: &PseudoElement, pseudo: &PseudoElement,
originating_element_style: &PrimaryStyle, originating_element_style: &PrimaryStyle,
layout_parent_style: Option<&ComputedValues>, layout_parent_style: Option<&ComputedValues>,
) -> Option<Arc<ComputedValues>> { ) -> Option<ResolvedStyle> {
let rules = self.match_pseudo( let rules = self.match_pseudo(
originating_element_style.style(), originating_element_style.style(),
pseudo, pseudo,
@ -399,7 +399,7 @@ where
Some(originating_element_style.style()), Some(originating_element_style.style()),
layout_parent_style, layout_parent_style,
Some(pseudo), Some(pseudo),
).style) ))
} }
fn match_primary( fn match_primary(

8
components/style/traversal.rs
View file

@ -421,7 +421,7 @@ where
let is_display_contents = primary_style.style().is_display_contents(); let is_display_contents = primary_style.style().is_display_contents();
style = Some(primary_style.0.into()); style = Some(primary_style.style.0);
if !is_display_contents { if !is_display_contents {
layout_parent_style = style.clone(); layout_parent_style = style.clone();
} }
@ -659,9 +659,9 @@ where
// Now that our bloom filter is set up, try the style sharing // Now that our bloom filter is set up, try the style sharing
// cache. // cache.
match target.share_style_if_possible(context) { match target.share_style_if_possible(context) {
Some(shareable_element) => { Some(shared_styles) => {
context.thread_local.statistics.styles_shared += 1; context.thread_local.statistics.styles_shared += 1;
shareable_element.borrow_data().unwrap().share_styles() shared_styles
} }
None => { None => {
context.thread_local.statistics.elements_matched += 1; context.thread_local.statistics.elements_matched += 1;
@ -738,7 +738,7 @@ where
// number of eventual styles, but would potentially miss out on various // number of eventual styles, but would potentially miss out on various
// opportunities for skipping selector matching, which could hurt // opportunities for skipping selector matching, which could hurt
// performance. // performance.
if !new_styles.primary.0.reused_via_rule_node { if !new_styles.primary.reused_via_rule_node {
context.thread_local.sharing_cache.insert_if_possible( context.thread_local.sharing_cache.insert_if_possible(
&element, &element,
&new_styles.primary, &new_styles.primary,

6
ports/geckolib/glue.rs
View file

@ -731,11 +731,9 @@ pub extern "C" fn Servo_StyleSet_GetBaseComputedValuesForElement(raw_data: RawSe
}; };
// Actually `PseudoElementResolution` doesn't matter. // Actually `PseudoElementResolution` doesn't matter.
let style: Arc<ComputedValues> = StyleResolverForElement::new(element, &mut context, RuleInclusion::All, PseudoElementResolution::IfApplicable)
StyleResolverForElement::new(element, &mut context, RuleInclusion::All, PseudoElementResolution::IfApplicable)
.cascade_style_and_visited_with_default_parents(inputs) .cascade_style_and_visited_with_default_parents(inputs)
.into(); .0.into()
style.into()
} }
#[no_mangle] #[no_mangle]