style: Simplify selector flag setting now that flag setting is atomic

These bits are write-only, actually, and we don't even need to read
them.

Differential Revision: https://phabricator.services.mozilla.com/D141888
This commit is contained in:
Emilio Cobos Álvarez 2023-06-18 13:06:43 +02:00 committed by Martin Robinson
parent 46978b2543
commit a2a4ec6ffb
8 changed files with 29 additions and 174 deletions

View file

@ -27,8 +27,8 @@ use crate::traversal_flags::TraversalFlags;
use app_units::Au;
use euclid::default::Size2D;
use euclid::Scale;
#[cfg(feature = "servo")]
use fxhash::FxHashMap;
use selectors::matching::ElementSelectorFlags;
use selectors::NthIndexCache;
#[cfg(feature = "gecko")]
use servo_arc::Arc;
@ -41,7 +41,6 @@ use style_traits::DevicePixel;
#[cfg(feature = "servo")]
use style_traits::SpeculativePainter;
use time;
use uluru::LRUCache;
pub use selectors::matching::QuirksMode;
@ -517,65 +516,6 @@ impl<E: TElement> SequentialTask<E> {
}
}
type CacheItem<E> = (SendElement<E>, ElementSelectorFlags);
/// Map from Elements to ElementSelectorFlags. Used to defer applying selector
/// flags until after the traversal.
pub struct SelectorFlagsMap<E: TElement> {
/// The hashmap storing the flags to apply.
map: FxHashMap<SendElement<E>, ElementSelectorFlags>,
/// An LRU cache to avoid hashmap lookups, which can be slow if the map
/// gets big.
cache: LRUCache<CacheItem<E>, { 4 + 1 }>,
}
#[cfg(debug_assertions)]
impl<E: TElement> Drop for SelectorFlagsMap<E> {
fn drop(&mut self) {
debug_assert!(self.map.is_empty());
}
}
impl<E: TElement> SelectorFlagsMap<E> {
/// Creates a new empty SelectorFlagsMap.
pub fn new() -> Self {
SelectorFlagsMap {
map: FxHashMap::default(),
cache: LRUCache::default(),
}
}
/// Inserts some flags into the map for a given element.
pub fn insert_flags(&mut self, element: E, flags: ElementSelectorFlags) {
let el = unsafe { SendElement::new(element) };
// Check the cache. If the flags have already been noted, we're done.
if let Some(item) = self.cache.find(|x| x.0 == el) {
if !item.1.contains(flags) {
item.1.insert(flags);
self.map.get_mut(&el).unwrap().insert(flags);
}
return;
}
let f = self.map.entry(el).or_insert(ElementSelectorFlags::empty());
*f |= flags;
self.cache
.insert((unsafe { SendElement::new(element) }, *f))
}
/// Applies the flags. Must be called on the main thread.
fn apply_flags(&mut self) {
debug_assert_eq!(thread_state::get(), ThreadState::LAYOUT);
self.cache.clear();
for (el, flags) in self.map.drain() {
unsafe {
el.set_selector_flags(flags);
}
}
}
}
/// A list of SequentialTasks that get executed on Drop.
pub struct SequentialTaskList<E>(Vec<SequentialTask<E>>)
where
@ -697,11 +637,6 @@ pub struct ThreadLocalStyleContext<E: TElement> {
/// filter, to ensure they're dropped before we execute the tasks, which
/// could create another ThreadLocalStyleContext for style computation.
pub tasks: SequentialTaskList<E>,
/// ElementSelectorFlags that need to be applied after the traversal is
/// complete. This map is used in cases where the matching algorithm needs
/// to set flags on elements it doesn't have exclusive access to (i.e. other
/// than the current element).
pub selector_flags: SelectorFlagsMap<E>,
/// Statistics about the traversal.
pub statistics: PerThreadTraversalStatistics,
/// A checker used to ensure that parallel.rs does not recurse indefinitely
@ -719,7 +654,6 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
rule_cache: RuleCache::new(),
bloom_filter: StyleBloom::new(),
tasks: SequentialTaskList(Vec::new()),
selector_flags: SelectorFlagsMap::new(),
statistics: PerThreadTraversalStatistics::default(),
stack_limit_checker: StackLimitChecker::new(
(STYLE_THREAD_STACK_SIZE_KB - STACK_SAFETY_MARGIN_KB) * 1024,
@ -729,15 +663,6 @@ impl<E: TElement> ThreadLocalStyleContext<E> {
}
}
impl<E: TElement> Drop for ThreadLocalStyleContext<E> {
fn drop(&mut self) {
debug_assert_eq!(thread_state::get(), ThreadState::LAYOUT);
// Apply any slow selector flags that need to be set on parents.
self.selector_flags.apply_flags();
}
}
/// A `StyleContext` is just a simple container for a immutable reference to a
/// shared style context, and a mutable reference to a local one.
pub struct StyleContext<'a, E: TElement + 'a> {

View file

@ -738,14 +738,26 @@ pub trait TElement:
/// matched on this element and therefore what kind of work may need to
/// be performed when DOM state changes.
///
/// This is unsafe, like all the flag-setting methods, because it's only safe
/// to call with exclusive access to the element. When setting flags on the
/// parent during parallel traversal, we use SequentialTask to queue up the
/// set to run after the threads join.
unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags);
/// You probably don't want to use this directly and want to use
/// apply_selector_flags, since that sets flags on the parent as needed.
fn set_selector_flags(&self, flags: ElementSelectorFlags);
/// Returns true if the element has all the specified selector flags.
fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool;
/// Applies selector flags to an element and its parent as needed.
fn apply_selector_flags(&self, flags: ElementSelectorFlags) {
// Handle flags that apply to the element.
let self_flags = flags.for_self();
if !self_flags.is_empty() {
self.set_selector_flags(self_flags);
}
// Handle flags that apply to the parent.
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = self.parent_element() {
p.set_selector_flags(parent_flags);
}
}
}
/// In Gecko, element has a flag that represents the element may have
/// any type of animations or not to bail out animation stuff early.

View file

@ -1393,16 +1393,11 @@ impl<'le> TElement for GeckoElement<'le> {
self.is_root_of_native_anonymous_subtree()
}
unsafe fn set_selector_flags(&self, flags: ElementSelectorFlags) {
fn set_selector_flags(&self, flags: ElementSelectorFlags) {
debug_assert!(!flags.is_empty());
self.set_flags(selector_flags_to_node_flags(flags));
}
fn has_selector_flags(&self, flags: ElementSelectorFlags) -> bool {
let node_flags = selector_flags_to_node_flags(flags);
(self.flags() & node_flags) == node_flags
}
#[inline]
fn may_have_animations(&self) -> bool {
if let Some(pseudo) = self.implemented_pseudo_element() {

View file

@ -8,7 +8,7 @@
#![deny(missing_docs)]
use crate::computed_value_flags::ComputedValueFlags;
use crate::context::{CascadeInputs, ElementCascadeInputs, QuirksMode, SelectorFlagsMap};
use crate::context::{CascadeInputs, ElementCascadeInputs, QuirksMode};
use crate::context::{SharedStyleContext, StyleContext};
use crate::data::{ElementData, ElementStyles};
use crate::dom::TElement;
@ -26,7 +26,6 @@ use crate::style_resolver::{PseudoElementResolution, StyleResolverForElement};
use crate::stylesheets::layer_rule::LayerOrder;
use crate::stylist::RuleInclusion;
use crate::traversal_flags::TraversalFlags;
use selectors::matching::ElementSelectorFlags;
use servo_arc::{Arc, ArcBorrow};
/// Represents the result of comparing an element's old and new style.
@ -1007,51 +1006,6 @@ pub trait MatchMethods: TElement {
cascade_requirement
}
/// Applies selector flags to an element, deferring mutations of the parent
/// until after the traversal.
///
/// TODO(emilio): This is somewhat inefficient, because it doesn't take
/// advantage of us knowing that the traversal is sequential.
fn apply_selector_flags(
&self,
map: &mut SelectorFlagsMap<Self>,
element: &Self,
flags: ElementSelectorFlags,
) {
// Handle flags that apply to the element.
let self_flags = flags.for_self();
if !self_flags.is_empty() {
if element == self {
// If this is the element we're styling, we have exclusive
// access to the element, and thus it's fine inserting them,
// even from the worker.
unsafe {
element.set_selector_flags(self_flags);
}
} else {
// Otherwise, this element is an ancestor of the current element
// we're styling, and thus multiple children could write to it
// if we did from here.
//
// Instead, we can read them, and post them if necessary as a
// sequential task in order for them to be processed later.
if !element.has_selector_flags(self_flags) {
map.insert_flags(*element, self_flags);
}
}
}
// Handle flags that apply to the parent.
let parent_flags = flags.for_parent();
if !parent_flags.is_empty() {
if let Some(p) = element.parent_element() {
if !p.has_selector_flags(parent_flags) {
map.insert_flags(p, parent_flags);
}
}
}
}
/// Updates the rule nodes without re-running selector matching, using just
/// the rule tree.
///

View file

@ -7,7 +7,7 @@
//! elements can indeed share the same style.
use crate::bloom::StyleBloom;
use crate::context::{SelectorFlagsMap, SharedStyleContext};
use crate::context::SharedStyleContext;
use crate::dom::TElement;
use crate::sharing::{StyleSharingCandidate, StyleSharingTarget};
use selectors::NthIndexCache;
@ -120,7 +120,6 @@ pub fn revalidate<E>(
shared_context: &SharedStyleContext,
bloom: &StyleBloom<E>,
nth_index_cache: &mut NthIndexCache,
selector_flags_map: &mut SelectorFlagsMap<E>,
) -> bool
where
E: TElement,
@ -128,7 +127,7 @@ where
let stylist = &shared_context.stylist;
let for_element =
target.revalidation_match_results(stylist, bloom, nth_index_cache, selector_flags_map);
target.revalidation_match_results(stylist, bloom, nth_index_cache);
let for_candidate = candidate.revalidation_match_results(stylist, bloom, nth_index_cache);

View file

@ -66,9 +66,8 @@
use crate::applicable_declarations::ApplicableDeclarationBlock;
use crate::bloom::StyleBloom;
use crate::context::{SelectorFlagsMap, SharedStyleContext, StyleContext};
use crate::context::{SharedStyleContext, StyleContext};
use crate::dom::{SendElement, TElement};
use crate::matching::MatchMethods;
use crate::properties::ComputedValues;
use crate::rule_tree::StrongRuleNode;
use crate::style_resolver::{PrimaryStyle, ResolvedElementStyles};
@ -384,7 +383,6 @@ impl<E: TElement> StyleSharingTarget<E> {
stylist: &Stylist,
bloom: &StyleBloom<E>,
nth_index_cache: &mut NthIndexCache,
selector_flags_map: &mut SelectorFlagsMap<E>,
) -> &SmallBitVec {
// It's important to set the selector flags. Otherwise, if we succeed in
// sharing the style, we may not set the slow selector flags for the
@ -401,9 +399,8 @@ impl<E: TElement> StyleSharingTarget<E> {
// The style sharing cache will get a hit for the second span. When the
// child span is subsequently removed from the DOM, missing selector
// flags would cause us to miss the restyle on the second span.
let element = self.element;
let mut set_selector_flags = |el: &E, flags: ElementSelectorFlags| {
element.apply_selector_flags(selector_flags_map, el, flags);
el.apply_selector_flags(flags);
};
self.validation_data.revalidation_match_results(
@ -423,7 +420,6 @@ impl<E: TElement> StyleSharingTarget<E> {
) -> Option<ResolvedElementStyles> {
let cache = &mut context.thread_local.sharing_cache;
let shared_context = &context.shared;
let selector_flags_map = &mut context.thread_local.selector_flags;
let bloom_filter = &context.thread_local.bloom_filter;
let nth_index_cache = &mut context.thread_local.nth_index_cache;
@ -443,7 +439,6 @@ impl<E: TElement> StyleSharingTarget<E> {
cache.share_style_if_possible(
shared_context,
selector_flags_map,
bloom_filter,
nth_index_cache,
self,
@ -667,7 +662,6 @@ impl<E: TElement> StyleSharingCache<E> {
fn share_style_if_possible(
&mut self,
shared_context: &SharedStyleContext,
selector_flags_map: &mut SelectorFlagsMap<E>,
bloom_filter: &StyleBloom<E>,
nth_index_cache: &mut NthIndexCache,
target: &mut StyleSharingTarget<E>,
@ -700,7 +694,6 @@ impl<E: TElement> StyleSharingCache<E> {
&shared_context,
bloom_filter,
nth_index_cache,
selector_flags_map,
shared_context,
)
})
@ -712,7 +705,6 @@ impl<E: TElement> StyleSharingCache<E> {
shared: &SharedStyleContext,
bloom: &StyleBloom<E>,
nth_index_cache: &mut NthIndexCache,
selector_flags_map: &mut SelectorFlagsMap<E>,
shared_context: &SharedStyleContext,
) -> Option<ResolvedElementStyles> {
debug_assert!(!target.is_in_native_anonymous_subtree());
@ -817,7 +809,6 @@ impl<E: TElement> StyleSharingCache<E> {
shared,
bloom,
nth_index_cache,
selector_flags_map,
) {
trace!("Miss: Revalidation");
return None;

View file

@ -451,7 +451,6 @@ where
);
let mut applicable_declarations = ApplicableDeclarationList::new();
let map = &mut self.context.thread_local.selector_flags;
let bloom_filter = self.context.thread_local.bloom_filter.filter();
let nth_index_cache = &mut self.context.thread_local.nth_index_cache;
let mut matching_context = MatchingContext::new_for_visited(
@ -465,9 +464,8 @@ where
let stylist = &self.context.shared.stylist;
let implemented_pseudo = self.element.implemented_pseudo_element();
{
let resolving_element = self.element;
let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
resolving_element.apply_selector_flags(map, element, flags);
element.apply_selector_flags(flags);
};
// Compute the primary rule node.
@ -542,10 +540,8 @@ where
self.context.shared.quirks_mode(),
);
let map = &mut self.context.thread_local.selector_flags;
let resolving_element = self.element;
let mut set_selector_flags = |element: &E, flags: ElementSelectorFlags| {
resolving_element.apply_selector_flags(map, element, flags);
element.apply_selector_flags(flags);
};
// NB: We handle animation rules for ::before and ::after when

View file

@ -39,7 +39,6 @@ use crate::stylesheets::{
CssRule, EffectiveRulesIterator, Origin, OriginSet, PageRule, PerOrigin, PerOriginIter,
};
use crate::stylesheets::{StyleRule, StylesheetContents, StylesheetInDocument};
use crate::thread_state::{self, ThreadState};
use crate::AllocErr;
use crate::{Atom, LocalName, Namespace, ShrinkIfNeeded, WeakAtom};
use fxhash::FxHashMap;
@ -1088,23 +1087,7 @@ impl Stylist {
return;
}
// Gecko calls this from sequential mode, so we can directly apply
// the flags.
debug_assert_eq!(thread_state::get(), ThreadState::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);
}
}
}
element.apply_selector_flags(flags);
};
let mut declarations = ApplicableDeclarationList::new();