Skip to content

Commit

Permalink
style: Simplify dependency visitor, avoid tracking dependencies of ne…
Browse files Browse the repository at this point in the history
…sted complex selectors separately.
  • Loading branch information
emilio committed Apr 12, 2017
1 parent 568fa4c commit 9e33cd5
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 76 deletions.
2 changes: 1 addition & 1 deletion components/selectors/parser.rs
Expand Up @@ -145,7 +145,7 @@ impl<Impl: SelectorImpl> SelectorMethods for Selector<Impl> {
}
}

impl<Impl: SelectorImpl> SelectorMethods for Arc<ComplexSelector<Impl>> {
impl<Impl: SelectorImpl> SelectorMethods for ComplexSelector<Impl> {
type Impl = Impl;

fn visit<V>(&self, visitor: &mut V) -> bool
Expand Down
2 changes: 1 addition & 1 deletion components/selectors/visitor.rs
Expand Up @@ -34,7 +34,7 @@ pub trait SelectorVisitor {
/// Gets the combinator to the right of the selector, or `None` if the
/// selector is the leftmost one.
fn visit_complex_selector(&mut self,
_: &Arc<ComplexSelector<Self::Impl>>,
_: &ComplexSelector<Self::Impl>,
_combinator_to_right: Option<Combinator>)
-> bool {
true
Expand Down
128 changes: 61 additions & 67 deletions components/style/restyle_hints.rs
Expand Up @@ -421,6 +421,8 @@ fn needs_cache_revalidation(sel: &SimpleSelector<SelectorImpl>) -> bool {
SimpleSelector::FirstOfType |
SimpleSelector::LastOfType |
SimpleSelector::OnlyOfType => true,
// FIXME(emilio): This sets the "revalidation" flag for :any, which is
// probably expensive given we use it a lot in UA sheets.
SimpleSelector::NonTSPseudoClass(ref p) => p.state_flag().is_empty(),
_ => false,
}
Expand Down Expand Up @@ -491,85 +493,31 @@ struct Dependency {
/// of them is sensitive to attribute or state changes.
struct SensitivitiesVisitor {
sensitivities: Sensitivities,
hint: RestyleHint,
needs_revalidation: bool,
}

impl SelectorVisitor for SensitivitiesVisitor {
type Impl = SelectorImpl;

fn visit_simple_selector(&mut self, s: &SimpleSelector<SelectorImpl>) -> bool {
self.sensitivities.states.insert(selector_to_state(s));

if !self.sensitivities.attrs {
self.sensitivities.attrs = is_attr_selector(s);
}

fn visit_complex_selector(&mut self,
_: &ComplexSelector<SelectorImpl>,
combinator: Option<Combinator>) -> bool {
self.hint |= combinator_to_restyle_hint(combinator);
self.needs_revalidation |= self.hint.contains(RESTYLE_LATER_SIBLINGS);
true
}
}

/// A visitor struct that collects information for a given selector.
///
/// This is the struct responsible of adding dependencies for a given complex
/// selector and all the selectors to its left.
///
/// This uses a `SensitivitiesVisitor` internally to collect all the
/// dependencies inside the given complex selector.
pub struct SelectorDependencyVisitor<'a> {
dependency_set: &'a mut DependencySet,
needs_cache_revalidation: bool,
}

impl<'a> SelectorDependencyVisitor<'a> {
/// Create a new `SelectorDependencyVisitor`.
pub fn new(dependency_set: &'a mut DependencySet) -> Self {
SelectorDependencyVisitor {
dependency_set: dependency_set,
needs_cache_revalidation: false,
}
}

/// Returns whether this visitor has encountered a simple selector that needs
/// cache revalidation.
pub fn needs_cache_revalidation(&self) -> bool {
self.needs_cache_revalidation
}
}

impl<'a> SelectorVisitor for SelectorDependencyVisitor<'a> {
type Impl = SelectorImpl;

fn visit_simple_selector(&mut self, s: &SimpleSelector<SelectorImpl>) -> bool {
if !self.needs_cache_revalidation {
self.needs_cache_revalidation = needs_cache_revalidation(s);
}

true
}

fn visit_complex_selector(&mut self,
selector: &Arc<ComplexSelector<SelectorImpl>>,
combinator: Option<Combinator>)
-> bool
{
let mut sensitivity_visitor = SensitivitiesVisitor {
sensitivities: Sensitivities::new(),
};
self.sensitivities.states.insert(selector_to_state(s));

for s in &selector.compound_selector {
s.visit(&mut sensitivity_visitor);
if !self.sensitivities.attrs {
self.sensitivities.attrs = is_attr_selector(s);
self.needs_revalidation = true;
}

let hint = combinator_to_restyle_hint(combinator);

self.needs_cache_revalidation |= sensitivity_visitor.sensitivities.attrs;
self.needs_cache_revalidation |= hint.intersects(RESTYLE_LATER_SIBLINGS);

if !sensitivity_visitor.sensitivities.is_empty() {
self.dependency_set.add_dependency(Dependency {
selector: selector.clone(),
hint: hint,
sensitivities: sensitivity_visitor.sensitivities,
});
if !self.needs_revalidation {
self.needs_revalidation = needs_cache_revalidation(s);
}

true
Expand Down Expand Up @@ -606,6 +554,52 @@ impl DependencySet {
}
}

/// Adds a selector to this `DependencySet`, and returns whether it may need
/// cache revalidation, that is, whether two siblings of the same "shape"
/// may have different style due to this selector.
pub fn note_selector(&mut self,
selector: &Arc<ComplexSelector<SelectorImpl>>)
-> bool
{
let mut combinator = None;
let mut current = selector;

let mut needs_revalidation = false;

loop {
let mut sensitivities_visitor = SensitivitiesVisitor {
sensitivities: Sensitivities::new(),
hint: RestyleHint::empty(),
needs_revalidation: false,
};

for ss in &current.compound_selector {
ss.visit(&mut sensitivities_visitor);
}

sensitivities_visitor.hint |= combinator_to_restyle_hint(combinator);
needs_revalidation |= sensitivities_visitor.needs_revalidation;

if !sensitivities_visitor.sensitivities.is_empty() {
self.add_dependency(Dependency {
sensitivities: sensitivities_visitor.sensitivities,
hint: sensitivities_visitor.hint,
selector: current.clone(),
})
}

match current.next {
Some((ref next, next_combinator)) => {
combinator = Some(next_combinator);
current = next;
}
None => break,
}
}

needs_revalidation
}

/// Create an empty `DependencySet`.
pub fn new() -> Self {
DependencySet {
Expand Down
11 changes: 4 additions & 7 deletions components/style/stylist.rs
Expand Up @@ -19,7 +19,7 @@ use properties::{self, CascadeFlags, ComputedValues};
#[cfg(feature = "servo")]
use properties::INHERIT_ALL;
use properties::PropertyDeclarationBlock;
use restyle_hints::{RestyleHint, DependencySet, SelectorDependencyVisitor};
use restyle_hints::{RestyleHint, DependencySet};
use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode, StyleSource};
use selector_parser::{SelectorImpl, PseudoElement, Snapshot};
use selectors::Element;
Expand All @@ -28,7 +28,6 @@ use selectors::matching::{AFFECTED_BY_ANIMATIONS, AFFECTED_BY_TRANSITIONS};
use selectors::matching::{AFFECTED_BY_STYLE_ATTRIBUTE, AFFECTED_BY_PRESENTATIONAL_HINTS};
use selectors::matching::{ElementSelectorFlags, StyleRelations, matches_complex_selector};
use selectors::parser::{Selector, SimpleSelector, LocalName as LocalNameSelector, ComplexSelector};
use selectors::parser::SelectorMethods;
use shared_lock::{Locked, SharedRwLockReadGuard, StylesheetGuards};
use sink::Push;
use smallvec::VecLike;
Expand Down Expand Up @@ -294,11 +293,9 @@ impl Stylist {
self.rules_source_order += 1;

for selector in &style_rule.selectors.0 {
let mut visitor =
SelectorDependencyVisitor::new(&mut self.state_deps);
selector.visit(&mut visitor);

if visitor.needs_cache_revalidation() {
let needs_cache_revalidation =
self.state_deps.note_selector(&selector.complex_selector);
if needs_cache_revalidation {
self.selectors_for_cache_revalidation.push(selector.clone());
}
}
Expand Down

0 comments on commit 9e33cd5

Please sign in to comment.