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

Bug 1345950: stylo: Fix slow selector flags. r=bholley

Browse source 
MozReview-Commit-ID: QwuSxulNG0
Signed-off-by: Emilio Cobos Álvarez <emilio@crisal.io>
This commit is contained in:
Emilio Cobos Álvarez 2017-03-21 12:58:28 +01:00
parent f90f7d6824
commit a9b133bcbb
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
9 changed files with 288 additions and 189 deletions
Download patch file Download diff file Expand all files Collapse all files

23
components/style/gecko/wrapper.rs
View file

@ -663,10 +663,13 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
}
}
fn match_non_ts_pseudo_class(&self,
pseudo_class: &NonTSPseudoClass,
relations: &mut StyleRelations,
flags: &mut ElementSelectorFlags) -> bool {
fn match_non_ts_pseudo_class<F>(&self,
pseudo_class: &NonTSPseudoClass,
relations: &mut StyleRelations,
flags_setter: &mut F)
-> bool
where F: FnMut(&Self, ElementSelectorFlags),
{
match *pseudo_class {
// https://github.com/servo/servo/issues/8718
NonTSPseudoClass::AnyLink => unsafe { Gecko_IsLink(self.0) },
@ -704,7 +707,13 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
Gecko_MatchesElement(pseudo_class.to_gecko_pseudoclasstype().unwrap(), self.0)
},
NonTSPseudoClass::MozAny(ref sels) => {
sels.iter().any(|s| matches_complex_selector(s, self, None, relations, flags))
sels.iter().any(|s| {
matches_complex_selector(s,
self,
None,
relations,
flags_setter)
})
}
NonTSPseudoClass::MozSystemMetric(ref s) |
NonTSPseudoClass::MozLocaleDir(ref s) |
@ -718,7 +727,7 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
pseudo_class.to_gecko_pseudoclasstype().unwrap(),
s.as_ptr(), &mut set_slow_selector);
if set_slow_selector {
*flags |= HAS_SLOW_SELECTOR;
flags_setter(self, HAS_SLOW_SELECTOR);
}
matches
}
@ -862,7 +871,7 @@ impl<'le> ElementExt for GeckoElement<'le> {
fn is_link(&self) -> bool {
self.match_non_ts_pseudo_class(&NonTSPseudoClass::AnyLink,
&mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty())
&mut |_, _| {})
}
#[inline]

96
components/style/matching.rs
View file

@ -18,7 +18,7 @@ use dom::{AnimationRules, SendElement, TElement, TNode};
use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
use properties::longhands::display::computed_value as display;
use restyle_hints::{RESTYLE_STYLE_ATTRIBUTE, RestyleHint};
use rule_tree::{CascadeLevel, StrongRuleNode};
use rule_tree::{CascadeLevel, RuleTree, StrongRuleNode};
use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl};
use selectors::MatchAttr;
use selectors::bloom::BloomFilter;
@ -750,12 +750,12 @@ trait PrivateMatchMethods: TElement {
}
}
fn compute_rule_node<E: TElement>(context: &StyleContext<E>,
fn compute_rule_node<E: TElement>(rule_tree: &RuleTree,
applicable_declarations: &mut Vec<ApplicableDeclarationBlock>)
-> StrongRuleNode
{
let rules = applicable_declarations.drain(..).map(|d| (d.source, d.level));
let rule_node = context.shared.stylist.rule_tree.insert_ordered_rules(rules);
let rule_node = rule_tree.insert_ordered_rules(rules);
rule_node
}
@ -775,20 +775,75 @@ pub trait MatchMethods : TElement {
let stylist = &context.shared.stylist;
let style_attribute = self.style_attribute();
let animation_rules = self.get_animation_rules(None);
let mut flags = ElementSelectorFlags::empty();
let mut rule_nodes_changed = false;
// TODO(emilio): This is somewhat inefficient, because of a variety of
// reasons:
//
// * It doesn't coalesce flags.
// * It doesn't look at flags already sent in a task for the main
// thread to process.
// * It doesn't take advantage of us knowing that the traversal is
// sequential.
//
// I suspect (need to measure!) that we don't use to set flags on
// a lot of different elements, but we could end up posting the same
// flag over and over with this approach.
//
// If the number of elements is low, perhaps a small cache with the
// flags already sent would be appropriate.
//
// The sequential task business for this is kind of sad :(.
//
// Anyway, let's do the obvious thing for now.
let tasks = &mut context.thread_local.tasks;
let mut set_selector_flags = |element: &Self, flags: ElementSelectorFlags| {
// Apply the selector flags.
let self_flags = flags.for_self();
if !self_flags.is_empty() {
if element == self {
unsafe { element.set_selector_flags(self_flags); }
} else {
if !element.has_selector_flags(self_flags) {
let task =
SequentialTask::set_selector_flags(element.clone(),
self_flags);
tasks.push(task);
}
}
}
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = element.parent_element() {
// Avoid the overhead of the SequentialTask if the flags are
// already set.
if !p.has_selector_flags(parent_flags) {
let task = SequentialTask::set_selector_flags(p, parent_flags);
tasks.push(task);
}
}
}
};
// Borrow the stuff we need here so the borrow checker doesn't get mad
// at us later in the closure.
let guards = &context.shared.guards;
let rule_tree = &context.shared.stylist.rule_tree;
let bloom_filter = context.thread_local.bloom_filter.filter();
// Compute the primary rule node.
let mut primary_relations =
stylist.push_applicable_declarations(self,
Some(context.thread_local.bloom_filter.filter()),
Some(bloom_filter),
style_attribute,
animation_rules,
None,
&context.shared.guards,
guards,
&mut applicable_declarations,
&mut flags);
let primary_rule_node = compute_rule_node(context, &mut applicable_declarations);
&mut set_selector_flags);
let primary_rule_node =
compute_rule_node::<Self>(rule_tree, &mut applicable_declarations);
if !data.has_styles() {
data.set_styles(ElementStyles::new(ComputedStyle::new_partial(primary_rule_node)));
rule_nodes_changed = true;
@ -808,15 +863,16 @@ pub trait MatchMethods : TElement {
AnimationRules(None, None)
};
stylist.push_applicable_declarations(self,
Some(context.thread_local.bloom_filter.filter()),
Some(bloom_filter),
None, pseudo_animation_rules,
Some(&pseudo),
&context.shared.guards,
&guards,
&mut applicable_declarations,
&mut flags);
&mut set_selector_flags);
if !applicable_declarations.is_empty() {
let new_rules = compute_rule_node(context, &mut applicable_declarations);
let new_rules =
compute_rule_node::<Self>(rule_tree, &mut applicable_declarations);
match per_pseudo.entry(pseudo) {
Entry::Occupied(mut e) => {
if e.get().rules != new_rules {
@ -848,22 +904,6 @@ pub trait MatchMethods : TElement {
primary_relations |= AFFECTED_BY_PSEUDO_ELEMENTS;
}
// Apply the selector flags.
let self_flags = flags.for_self();
if !self_flags.is_empty() {
unsafe { self.set_selector_flags(self_flags); }
}
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = self.parent_element() {
// Avoid the overhead of the SequentialTask if the flags are already set.
if !p.has_selector_flags(parent_flags) {
let task = SequentialTask::set_selector_flags(p, parent_flags);
context.thread_local.tasks.push(task);
}
}
}
MatchResults {
primary_relations: Some(primary_relations),
rule_nodes_changed: rule_nodes_changed,

32
components/style/restyle_hints.rs
View file

@ -269,17 +269,25 @@ impl<'a, E> MatchAttr for ElementWrapper<'a, E>
impl<'a, E> Element for ElementWrapper<'a, E>
where E: TElement,
{
fn match_non_ts_pseudo_class(&self,
pseudo_class: &NonTSPseudoClass,
relations: &mut StyleRelations,
flags: &mut ElementSelectorFlags) -> bool {
fn match_non_ts_pseudo_class<F>(&self,
pseudo_class: &NonTSPseudoClass,
relations: &mut StyleRelations,
_: &mut F)
-> bool
where F: FnMut(&Self, ElementSelectorFlags),
{
let flag = SelectorImpl::pseudo_class_state_flag(pseudo_class);
if flag == ElementState::empty() {
self.element.match_non_ts_pseudo_class(pseudo_class, relations, flags)
} else {
match self.snapshot.and_then(|s| s.state()) {
Some(snapshot_state) => snapshot_state.contains(flag),
_ => self.element.match_non_ts_pseudo_class(pseudo_class, relations, flags)
if flag.is_empty() {
return self.element.match_non_ts_pseudo_class(pseudo_class,
relations,
&mut |_, _| {})
}
match self.snapshot.and_then(|s| s.state()) {
Some(snapshot_state) => snapshot_state.contains(flag),
None => {
self.element.match_non_ts_pseudo_class(pseudo_class,
relations,
&mut |_, _| {})
}
}
}
@ -581,11 +589,11 @@ impl DependencySet {
let matched_then =
matches_complex_selector(&dep.selector, snapshot, None,
&mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
let matches_now =
matches_complex_selector(&dep.selector, element, None,
&mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
if matched_then != matches_now {
hint.insert(dep.hint);
}

4
components/style/servo/selector_parser.rs
View file

@ -14,7 +14,7 @@ use restyle_hints::ElementSnapshot;
use selector_parser::{ElementExt, PseudoElementCascadeType, SelectorParser};
use selector_parser::{attr_equals_selector_is_shareable, attr_exists_selector_is_shareable};
use selectors::{Element, MatchAttrGeneric};
use selectors::matching::{ElementSelectorFlags, StyleRelations};
use selectors::matching::StyleRelations;
use selectors::parser::{AttrSelector, SelectorMethods};
use std::borrow::Cow;
use std::fmt;
@ -459,7 +459,7 @@ impl<E: Element<Impl=SelectorImpl> + Debug> ElementExt for E {
fn is_link(&self) -> bool {
self.match_non_ts_pseudo_class(&NonTSPseudoClass::AnyLink,
&mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty())
&mut |_, _| {})
}
#[inline]

135
components/style/stylist.rs
View file

@ -398,7 +398,31 @@ impl Stylist {
let mut declarations = vec![];
let mut flags = ElementSelectorFlags::empty();
// Apply the selector flags. We should be in sequential mode
// already, so we can directly apply the parent flags.
let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
if cfg!(feature = "servo") {
// Servo calls this function from the worker, but only for internal
// pseudos, so we should never generate selector flags here.
unreachable!("internal pseudo generated slow selector flags?");
}
// Gecko calls this from sequential mode, so we can directly apply
// the flags.
debug_assert!(thread_state::get() == thread_state::LAYOUT);
let self_flags = flags.for_self();
if !self_flags.is_empty() {
unsafe { element.set_selector_flags(self_flags); }
}
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = element.parent_element() {
unsafe { p.set_selector_flags(parent_flags); }
}
}
};
self.push_applicable_declarations(element,
None,
None,
@ -406,7 +430,7 @@ impl Stylist {
Some(pseudo),
guards,
&mut declarations,
&mut flags);
&mut set_selector_flags);
let rule_node =
self.rule_tree.insert_ordered_rules(
@ -426,28 +450,6 @@ impl Stylist {
&StdoutErrorReporter,
CascadeFlags::empty());
// Apply the selector flags. We should be in sequential mode already,
// so we can directly apply the parent flags.
if cfg!(feature = "servo") {
// Servo calls this function from the worker, but only for internal
// pseudos, so we should never generate selector flags here.
debug_assert!(flags.is_empty());
} else {
// Gecko calls this from sequential mode, so we can directly apply
// the flags.
debug_assert!(thread_state::get() == thread_state::LAYOUT);
let self_flags = flags.for_self();
if !self_flags.is_empty() {
unsafe { element.set_selector_flags(self_flags); }
}
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = element.parent_element() {
unsafe { p.set_selector_flags(parent_flags); }
}
}
}
Some(ComputedStyle::new(rule_node, Arc::new(computed)))
}
@ -537,7 +539,7 @@ impl Stylist {
///
/// The returned `StyleRelations` indicate hints about which kind of rules
/// have matched.
pub fn push_applicable_declarations<E, V>(
pub fn push_applicable_declarations<E, V, F>(
&self,
element: &E,
parent_bf: Option<&BloomFilter>,
@ -546,11 +548,13 @@ impl Stylist {
pseudo_element: Option<&PseudoElement>,
guards: &StylesheetGuards,
applicable_declarations: &mut V,
flags: &mut ElementSelectorFlags) -> StyleRelations
flags_setter: &mut F)
-> StyleRelations
where E: TElement +
fmt::Debug +
PresentationalHintsSynthetizer,
V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock>
V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock>,
F: FnMut(&E, ElementSelectorFlags),
{
debug_assert!(!self.is_device_dirty);
debug_assert!(style_attribute.is_none() || pseudo_element.is_none(),
@ -573,7 +577,7 @@ impl Stylist {
guards.ua_or_user,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::UANormal);
debug!("UA normal: {:?}", relations);
@ -598,7 +602,7 @@ impl Stylist {
guards.ua_or_user,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::UserNormal);
debug!("user normal: {:?}", relations);
map.author.get_all_matching_rules(element,
@ -606,7 +610,7 @@ impl Stylist {
guards.author,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::AuthorNormal);
debug!("author normal: {:?}", relations);
@ -641,7 +645,7 @@ impl Stylist {
guards.author,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::AuthorImportant);
debug!("author important: {:?}", relations);
@ -665,7 +669,7 @@ impl Stylist {
guards.ua_or_user,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::UserImportant);
debug!("user important: {:?}", relations);
@ -679,7 +683,7 @@ impl Stylist {
guards.ua_or_user,
applicable_declarations,
&mut relations,
flags,
flags_setter,
CascadeLevel::UAImportant);
debug!("UA important: {:?}", relations);
@ -735,11 +739,11 @@ impl Stylist {
let element_matches =
matches_complex_selector(&selector.complex_selector, element,
None, &mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
let candidate_matches =
matches_complex_selector(&selector.complex_selector, candidate,
None, &mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
if element_matches != candidate_matches {
return false;
@ -774,12 +778,12 @@ impl Stylist {
let element_matches =
matches_complex_selector(&selector.complex_selector, element,
None, &mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
let candidate_matches =
matches_complex_selector(&selector.complex_selector, candidate,
None, &mut StyleRelations::empty(),
&mut ElementSelectorFlags::empty());
&mut |_, _| {});
if element_matches != candidate_matches {
debug!("match_same_sibling_affecting_rules: Failure due to {:?}",
@ -912,16 +916,17 @@ impl SelectorMap {
///
/// Extract matching rules as per element's ID, classes, tag name, etc..
/// Sort the Rules at the end to maintain cascading order.
pub fn get_all_matching_rules<E, V>(&self,
element: &E,
parent_bf: Option<&BloomFilter>,
guard: &SharedRwLockReadGuard,
matching_rules_list: &mut V,
relations: &mut StyleRelations,
flags: &mut ElementSelectorFlags,
cascade_level: CascadeLevel)
pub fn get_all_matching_rules<E, V, F>(&self,
element: &E,
parent_bf: Option<&BloomFilter>,
guard: &SharedRwLockReadGuard,
matching_rules_list: &mut V,
relations: &mut StyleRelations,
flags_setter: &mut F,
cascade_level: CascadeLevel)
where E: Element<Impl=SelectorImpl>,
V: VecLike<ApplicableDeclarationBlock>
V: VecLike<ApplicableDeclarationBlock>,
F: FnMut(&E, ElementSelectorFlags),
{
if self.empty {
return
@ -937,7 +942,7 @@ impl SelectorMap {
guard,
matching_rules_list,
relations,
flags,
flags_setter,
cascade_level)
}
@ -949,7 +954,7 @@ impl SelectorMap {
guard,
matching_rules_list,
relations,
flags,
flags_setter,
cascade_level);
});
@ -965,7 +970,7 @@ impl SelectorMap {
guard,
matching_rules_list,
relations,
flags,
flags_setter,
cascade_level);
SelectorMap::get_matching_rules(element,
@ -974,7 +979,7 @@ impl SelectorMap {
guard,
matching_rules_list,
relations,
flags,
flags_setter,
cascade_level);
// Sort only the rules we just added.
@ -1027,7 +1032,7 @@ impl SelectorMap {
matching_rules_list
}
fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector>(
fn get_matching_rules_from_hash<E, Str, BorrowedStr: ?Sized, Vector, F>(
element: &E,
parent_bf: Option<&BloomFilter>,
hash: &FnvHashMap<Str, Vec<Rule>>,
@ -1035,12 +1040,13 @@ impl SelectorMap {
guard: &SharedRwLockReadGuard,
matching_rules: &mut Vector,
relations: &mut StyleRelations,
flags: &mut ElementSelectorFlags,
flags_setter: &mut F,
cascade_level: CascadeLevel)
where E: Element<Impl=SelectorImpl>,
Str: Borrow<BorrowedStr> + Eq + Hash,
BorrowedStr: Eq + Hash,
Vector: VecLike<ApplicableDeclarationBlock>
Vector: VecLike<ApplicableDeclarationBlock>,
F: FnMut(&E, ElementSelectorFlags),
{
if let Some(rules) = hash.get(key) {
SelectorMap::get_matching_rules(element,
@ -1049,22 +1055,23 @@ impl SelectorMap {
guard,
matching_rules,
relations,
flags,
flags_setter,
cascade_level)
}
}
/// Adds rules in `rules` that match `element` to the `matching_rules` list.
fn get_matching_rules<E, V>(element: &E,
parent_bf: Option<&BloomFilter>,
rules: &[Rule],
guard: &SharedRwLockReadGuard,
matching_rules: &mut V,
relations: &mut StyleRelations,
flags: &mut ElementSelectorFlags,
cascade_level: CascadeLevel)
fn get_matching_rules<E, V, F>(element: &E,
parent_bf: Option<&BloomFilter>,
rules: &[Rule],
guard: &SharedRwLockReadGuard,
matching_rules: &mut V,
relations: &mut StyleRelations,
flags_setter: &mut F,
cascade_level: CascadeLevel)
where E: Element<Impl=SelectorImpl>,
V: VecLike<ApplicableDeclarationBlock>
V: VecLike<ApplicableDeclarationBlock>,
F: FnMut(&E, ElementSelectorFlags),
{
for rule in rules.iter() {
let style_rule = rule.style_rule.read_with(guard);
@ -1076,7 +1083,7 @@ impl SelectorMap {
};
if any_declaration_for_importance &&
matches_complex_selector(&*rule.selector, element, parent_bf,
relations, flags) {
relations, flags_setter) {
matching_rules.push(
rule.to_applicable_declaration_block(cascade_level));
}