From a3020419d9eace898727f5b778e661b61a1bb8df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Fri, 22 Jul 2016 16:48:08 -0700 Subject: [PATCH] stylo: Don't traverse the whole dom every restyle, propagate the dirty flag down the DOM. This commit adds hooks to the Servo style traversal to avoid traversing all the DOM for every restyle. Additionally it changes the behavior of the dirty flag to be propagated top down, to prevent extra overhead when an element is dirtied. This commit doesn't aim to change the behavior on Servo just yet, since Servo might rely on a full bottom up reconstruction of the flows. I'll need to double check and implement that separately. --- components/style/data.rs | 4 ++-- components/style/parallel.rs | 32 ++++++++++++++++++++------------ components/style/sequential.rs | 19 ++++++++++++++----- components/style/traversal.rs | 28 ++++++++++++++-------------- ports/geckolib/traversal.rs | 16 ++++++++++++++++ 5 files changed, 66 insertions(+), 33 deletions(-) diff --git a/components/style/data.rs b/components/style/data.rs index 906848998fb..44920c72c98 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -37,13 +37,13 @@ impl PrivateStyleData { #[cfg_attr(feature = "servo", derive(HeapSizeOf))] pub struct DomParallelInfo { /// The number of children that still need work done. - pub children_count: AtomicIsize, + pub children_to_process: AtomicIsize, } impl DomParallelInfo { pub fn new() -> DomParallelInfo { DomParallelInfo { - children_count: AtomicIsize::new(0), + children_to_process: AtomicIsize::new(0), } } } diff --git a/components/style/parallel.rs b/components/style/parallel.rs index 0bc840f7492..889f7f0a15b 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -63,25 +63,34 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, // Get a real layout node. let node = unsafe { N::from_unsafe(&unsafe_node) }; + if !context.should_process(node) { + continue; + } + // Perform the appropriate traversal. context.process_preorder(node); - let child_count = node.children_count(); + // Possibly enqueue the children. + let mut children_to_process = 0isize; + for kid in node.children() { + context.pre_process_child_hook(node, kid); + if context.should_process(kid) { + children_to_process += 1; + discovered_child_nodes.push(kid.to_unsafe()) + } + } // Reset the count of children. { let data = node.mutate_data().unwrap(); - data.parallel.children_count.store(child_count as isize, - Ordering::Relaxed); + data.parallel.children_to_process + .store(children_to_process, + Ordering::Relaxed); } - // Possibly enqueue the children. - if child_count != 0 { - for kid in node.children() { - discovered_child_nodes.push(kid.to_unsafe()) - } - } else { - // If there were no more children, start walking back up. + + // If there were no more children, start walking back up. + if children_to_process == 0 { bottom_up_dom::(unsafe_nodes.1, unsafe_node, proxy) } } @@ -128,7 +137,7 @@ fn bottom_up_dom(root: OpaqueNode, if parent_data .parallel - .children_count + .children_to_process .fetch_sub(1, Ordering::Relaxed) != 1 { // Get out of here and find another node to work on. break @@ -138,4 +147,3 @@ fn bottom_up_dom(root: OpaqueNode, node = parent; } } - diff --git a/components/style/sequential.rs b/components/style/sequential.rs index a5a9b519e57..56b8a563d27 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -9,20 +9,29 @@ use traversal::DomTraversalContext; pub fn traverse_dom(root: N, shared: &C::SharedContext) - where N: TNode, - C: DomTraversalContext { + where N: TNode, + C: DomTraversalContext +{ fn doit<'a, N, C>(context: &'a C, node: N) - where N: TNode, C: DomTraversalContext { + where N: TNode, + C: DomTraversalContext + { + debug_assert!(context.should_process(node)); context.process_preorder(node); for kid in node.children() { - doit::(context, kid); + context.pre_process_child_hook(node, kid); + if context.should_process(node) { + doit::(context, kid); + } } context.process_postorder(node); } let context = C::new(shared, root.opaque()); - doit::(&context, root); + if context.should_process(root) { + doit::(&context, root); + } } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 76f8539c72f..e4b1d8f11de 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -147,6 +147,12 @@ pub trait DomTraversalContext { fn process_preorder(&self, node: N); /// Process `node` on the way up, after its children have been processed. fn process_postorder(&self, node: N); + + /// Returns if the node should be processed by the preorder traversal. + fn should_process(&self, _node: N) -> bool { true } + + /// Do an action over the child before pushing him to the work queue. + fn pre_process_child_hook(&self, _parent: N, _kid: N) {} } /// Calculates the style for a single node. @@ -245,21 +251,15 @@ pub fn recalc_style_at<'a, N, C>(context: &'a C, put_thread_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); // Mark the node as DIRTY_ON_VIEWPORT_SIZE_CHANGE is it uses viewport percentage units. - match node.as_element() { - Some(element) => { - match *element.style_attribute() { - Some(ref property_declaration_block) => { - if property_declaration_block.declarations().any(|d| d.0.has_viewport_percentage()) { - unsafe { - node.set_dirty_on_viewport_size_changed(); - } - node.set_descendants_dirty_on_viewport_size_changed(); - } - }, - None => {} + if let Some(element) = node.as_element() { + if let Some(ref property_declaration_block) = *element.style_attribute() { + if property_declaration_block.declarations().any(|d| d.0.has_viewport_percentage()) { + unsafe { + node.set_dirty_on_viewport_size_changed(); + } + node.set_descendants_dirty_on_viewport_size_changed(); } - }, - None => {} + } } } diff --git a/ports/geckolib/traversal.rs b/ports/geckolib/traversal.rs index 87b63e83e15..6a3667e0b96 100644 --- a/ports/geckolib/traversal.rs +++ b/ports/geckolib/traversal.rs @@ -6,6 +6,7 @@ use context::StandaloneStyleContext; use std::mem; use style::context::SharedStyleContext; use style::dom::OpaqueNode; +use style::dom::TNode; use style::traversal::{DomTraversalContext, recalc_style_at}; use wrapper::GeckoNode; @@ -35,5 +36,20 @@ impl<'lc, 'ln> DomTraversalContext> for RecalcStyleOnly<'lc> { recalc_style_at(&self.context, self.root, node); } + fn should_process(&self, node: GeckoNode<'ln>) -> bool { + node.is_dirty() || node.has_dirty_descendants() + } + fn process_postorder(&self, _: GeckoNode<'ln>) {} + + fn pre_process_child_hook(&self, parent: GeckoNode<'ln>, kid: GeckoNode<'ln>) { + // NOTE: At this point is completely safe to modify either the parent or + // the child, since we have exclusive access to them. + if parent.is_dirty() { + unsafe { + kid.set_dirty(true); + parent.set_dirty_descendants(true); + } + } + } }