From c72fffa8f82800aa4203d4d01357cd22115028df Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Sat, 8 Oct 2016 18:08:01 -0700 Subject: [PATCH] Move children_to_process to layout. We don't need this for Gecko, and it's hard to implement in that case because there's nowhere obvious to put it (we don't plan to create TSDs for non-dirty nodes, and non-dirty nodes can have dirty children which require the children_to_process atomic). There are various solutions here, but punting is the easiest. We'll need to rethink this if/when we need to do a bottom-up traversal for Gecko. --- components/layout/construct.rs | 2 +- components/layout/data.rs | 17 ++++------- components/script/layout_wrapper.rs | 13 ++++++++ components/script_layout_interface/lib.rs | 36 +++++++++++++++++++++++ components/style/data.rs | 20 ------------- components/style/dom.rs | 8 +++++ components/style/gecko/wrapper.rs | 9 +++++- components/style/parallel.rs | 13 ++------ 8 files changed, 74 insertions(+), 44 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index ae77f4d7751..9722cc852d7 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -1418,7 +1418,7 @@ impl<'a, ConcreteThreadSafeLayoutNode: ThreadSafeLayoutNode> let result = { let mut style = node.style(self.style_context()); let mut data = node.mutate_layout_data().unwrap(); - let damage = data.restyle_damage; + let damage = data.base.restyle_damage; match *node.construction_result_mut(&mut *data) { ConstructionResult::None => true, diff --git a/components/layout/data.rs b/components/layout/data.rs index 69f8488e2e6..a9875a99876 100644 --- a/components/layout/data.rs +++ b/components/layout/data.rs @@ -3,19 +3,13 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use construct::ConstructionResult; -use script_layout_interface::restyle_damage::RestyleDamage; -use style::data::PersistentStyleData; +use script_layout_interface::PartialPersistentLayoutData; /// Data that layout associates with a node. pub struct PersistentLayoutData { - /// Data that the style system associates with a node. When the - /// style system is being used standalone, this is all that hangs - /// off the node. This must be first to permit the various - /// transmutations between PersistentStyleData and PersistentLayoutData. - pub style_data: PersistentStyleData, - - /// Description of how to account for recent style changes. - pub restyle_damage: RestyleDamage, + /// Data accessed by script_layout_interface. This must be first to allow + /// casting between PersistentLayoutData and PartialPersistentLayoutData. + pub base: PartialPersistentLayoutData, /// The current results of flow construction for this node. This is either a /// flow or a `ConstructionItem`. See comments in `construct.rs` for more @@ -38,8 +32,7 @@ impl PersistentLayoutData { /// Creates new layout data. pub fn new() -> PersistentLayoutData { PersistentLayoutData { - style_data: PersistentStyleData::new(), - restyle_damage: RestyleDamage::empty(), + base: PartialPersistentLayoutData::new(), flow_construction_result: ConstructionResult::None, before_flow_construction_result: ConstructionResult::None, after_flow_construction_result: ConstructionResult::None, diff --git a/components/script/layout_wrapper.rs b/components/script/layout_wrapper.rs index 91c2a29285d..efb3ca698ed 100644 --- a/components/script/layout_wrapper.rs +++ b/components/script/layout_wrapper.rs @@ -54,6 +54,7 @@ use std::fmt; use std::marker::PhantomData; use std::mem::transmute; use std::sync::Arc; +use std::sync::atomic::Ordering; use string_cache::{Atom, Namespace}; use style::atomic_refcell::{AtomicRef, AtomicRefCell, AtomicRefMut}; use style::attr::AttrValue; @@ -221,6 +222,18 @@ impl<'ln> TNode for ServoLayoutNode<'ln> { self.node.set_flag(CAN_BE_FRAGMENTED, value) } + fn store_children_to_process(&self, n: isize) { + let data = self.get_partial_layout_data().unwrap().borrow(); + data.parallel.children_to_process.store(n, Ordering::Relaxed); + } + + fn did_process_child(&self) -> isize { + let data = self.get_partial_layout_data().unwrap().borrow(); + let old_value = data.parallel.children_to_process.fetch_sub(1, Ordering::Relaxed); + debug_assert!(old_value >= 1); + old_value - 1 + } + fn borrow_data(&self) -> Option> { self.get_style_data().map(|d| d.borrow()) } diff --git a/components/script_layout_interface/lib.rs b/components/script_layout_interface/lib.rs index 18c4805a485..ee2a2a8b75f 100644 --- a/components/script_layout_interface/lib.rs +++ b/components/script_layout_interface/lib.rs @@ -52,12 +52,32 @@ use core::nonzero::NonZero; use ipc_channel::ipc::IpcSender; use libc::c_void; use restyle_damage::RestyleDamage; +use std::sync::atomic::AtomicIsize; use style::atomic_refcell::AtomicRefCell; use style::data::PersistentStyleData; pub struct PartialPersistentLayoutData { + /// Data that the style system associates with a node. When the + /// style system is being used standalone, this is all that hangs + /// off the node. This must be first to permit the various + /// transmutations between PersistentStyleData and PersistentLayoutData. pub style_data: PersistentStyleData, + + /// Description of how to account for recent style changes. pub restyle_damage: RestyleDamage, + + /// Information needed during parallel traversals. + pub parallel: DomParallelInfo, +} + +impl PartialPersistentLayoutData { + pub fn new() -> Self { + PartialPersistentLayoutData { + style_data: PersistentStyleData::new(), + restyle_damage: RestyleDamage::empty(), + parallel: DomParallelInfo::new(), + } + } } #[derive(Copy, Clone, HeapSizeOf)] @@ -70,6 +90,22 @@ pub struct OpaqueStyleAndLayoutData { #[allow(unsafe_code)] unsafe impl Send for OpaqueStyleAndLayoutData {} +/// Information that we need stored in each DOM node. +#[derive(HeapSizeOf)] +pub struct DomParallelInfo { + /// The number of children remaining to process during bottom-up traversal. + pub children_to_process: AtomicIsize, +} + +impl DomParallelInfo { + pub fn new() -> DomParallelInfo { + DomParallelInfo { + children_to_process: AtomicIsize::new(0), + } + } +} + + #[derive(Copy, Clone, PartialEq, Eq, Debug)] pub enum LayoutNodeType { Comment, diff --git a/components/style/data.rs b/components/style/data.rs index c1c87d5f96e..1fc24808d03 100644 --- a/components/style/data.rs +++ b/components/style/data.rs @@ -9,7 +9,6 @@ use selector_impl::PseudoElement; use std::collections::HashMap; use std::hash::BuildHasherDefault; use std::sync::Arc; -use std::sync::atomic::AtomicIsize; pub struct PersistentStyleData { /// The results of CSS styling for this node. @@ -18,9 +17,6 @@ pub struct PersistentStyleData { /// The results of CSS styling for each pseudo-element (if any). pub per_pseudo: HashMap, BuildHasherDefault<::fnv::FnvHasher>>, - - /// Information needed during parallel traversals. - pub parallel: DomParallelInfo, } impl PersistentStyleData { @@ -28,22 +24,6 @@ impl PersistentStyleData { PersistentStyleData { style: None, per_pseudo: HashMap::with_hasher(Default::default()), - parallel: DomParallelInfo::new(), - } - } -} - -/// Information that we need stored in each DOM node. -#[cfg_attr(feature = "servo", derive(HeapSizeOf))] -pub struct DomParallelInfo { - /// The number of children that still need work done. - pub children_to_process: AtomicIsize, -} - -impl DomParallelInfo { - pub fn new() -> DomParallelInfo { - DomParallelInfo { - children_to_process: AtomicIsize::new(0), } } } diff --git a/components/style/dom.rs b/components/style/dom.rs index 1dce89886ba..4e72e5745b6 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -139,6 +139,14 @@ pub trait TNode : Sized + Copy + Clone + NodeInfo { unsafe fn set_can_be_fragmented(&self, value: bool); + /// Atomically stores the number of children of this node that we will + /// need to process during bottom-up traversal. + fn store_children_to_process(&self, n: isize); + + /// Atomically notes that a child has been processed during bottom-up + /// traversal. Returns the number of children left to process. + fn did_process_child(&self) -> isize; + /// Borrows the style data immutably. Fails on a conflicting borrow. #[inline(always)] fn borrow_data(&self) -> Option>; diff --git a/components/style/gecko/wrapper.rs b/components/style/gecko/wrapper.rs index fd35c6c6c83..f2a7532883c 100644 --- a/components/style/gecko/wrapper.rs +++ b/components/style/gecko/wrapper.rs @@ -94,7 +94,6 @@ pub struct GeckoNode<'ln>(pub &'ln RawGeckoNode); impl<'ln> GeckoNode<'ln> { fn from_content(content: &'ln nsIContent) -> Self { - use std::mem; GeckoNode(&content._base) } @@ -302,6 +301,14 @@ impl<'ln> TNode for GeckoNode<'ln> { // Maybe this isn’t useful for Gecko? } + fn store_children_to_process(&self, _: isize) { + // This is only used for bottom-up traversal, and is thus a no-op for Gecko. + } + + fn did_process_child(&self) -> isize { + panic!("Atomic child count not implemented in Gecko"); + } + #[inline(always)] fn borrow_data(&self) -> Option> { self.get_node_data().as_ref().map(|d| d.0.borrow()) diff --git a/components/style/parallel.rs b/components/style/parallel.rs index b6ae6d49d66..d8dc89d43de 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -107,10 +107,7 @@ fn top_down_dom(unsafe_nodes: UnsafeNodeList, // Reset the count of children if we need to do a bottom-up traversal // after the top up. if context.needs_postorder_traversal() { - node.mutate_data().unwrap() - .parallel.children_to_process - .store(children_to_process, - Ordering::Relaxed); + node.store_children_to_process(children_to_process); // If there were no more children, start walking back up. if children_to_process == 0 { @@ -161,12 +158,8 @@ fn bottom_up_dom(root: OpaqueNode, Some(parent) => parent, }; - let parent_data = parent.borrow_data().unwrap(); - - if parent_data - .parallel - .children_to_process - .fetch_sub(1, Ordering::Relaxed) != 1 { + let remaining = parent.did_process_child(); + if remaining != 0 { // Get out of here and find another node to work on. break }