From 3e2c44114c339e77b07a303bd02365f109003677 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 17 Jul 2015 11:40:40 +0200 Subject: [PATCH 1/6] Move the traversal traits into the traversal module. --- components/layout/construct.rs | 3 ++- components/layout/parallel.rs | 5 +++-- components/layout/sequential.rs | 4 ++-- components/layout/traversal.rs | 29 +++++++++++++++++++++++++++-- components/layout/wrapper.rs | 26 +------------------------- 5 files changed, 35 insertions(+), 32 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index cc67cc6c300..746ecef1e6b 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -40,7 +40,8 @@ use table_row::TableRowFlow; use table_rowgroup::TableRowGroupFlow; use table_wrapper::TableWrapperFlow; use text::TextRunScanner; -use wrapper::{PostorderNodeMutTraversal, PseudoElementType, ThreadSafeLayoutNode}; +use traversal::PostorderNodeMutTraversal; +use wrapper::{PseudoElementType, ThreadSafeLayoutNode}; use gfx::display_list::OpaqueNode; use script::dom::characterdata::CharacterDataTypeId; diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index 748f84d1a0d..6d00c4a4c44 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -12,12 +12,13 @@ use context::{LayoutContext, SharedLayoutContext}; use flow::{Flow, MutableFlowUtils, PreorderFlowTraversal, PostorderFlowTraversal}; use flow; use flow_ref::FlowRef; +use traversal::{PreorderDomTraversal, PostorderDomTraversal}; +use traversal::PostorderNodeMutTraversal; use traversal::{BubbleISizes, AssignISizes, AssignBSizesAndStoreOverflow}; use traversal::{ComputeAbsolutePositions, BuildDisplayList}; use traversal::{RecalcStyleForNode, ConstructFlows}; use wrapper::{layout_node_to_unsafe_layout_node, layout_node_from_unsafe_layout_node, LayoutNode}; -use wrapper::{PostorderNodeMutTraversal, UnsafeLayoutNode}; -use wrapper::{PreorderDomTraversal, PostorderDomTraversal}; +use wrapper::UnsafeLayoutNode; use profile_traits::time::{self, ProfilerMetadata, profile}; use std::mem; diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index 7ebd8a407e6..b4c8270e804 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -10,12 +10,12 @@ use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow_ref::FlowRef; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; +use traversal::{PreorderDomTraversal, PostorderDomTraversal}; +use traversal::PostorderNodeMutTraversal; use traversal::{BubbleISizes, RecalcStyleForNode, ConstructFlows}; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; use traversal::{ComputeAbsolutePositions, BuildDisplayList}; use wrapper::LayoutNode; -use wrapper::{PostorderNodeMutTraversal}; -use wrapper::{PreorderDomTraversal, PostorderDomTraversal}; use euclid::point::Point2D; use util::geometry::{Au, ZERO_POINT}; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 603245ecb88..c366d20206b 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -15,8 +15,7 @@ use flow::{PreorderFlowTraversal, PostorderFlowTraversal}; use incremental::{self, BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, RestyleDamage}; use script::layout_interface::ReflowGoal; use wrapper::{layout_node_to_unsafe_layout_node, LayoutNode}; -use wrapper::{PostorderNodeMutTraversal, ThreadSafeLayoutNode, UnsafeLayoutNode}; -use wrapper::{PreorderDomTraversal, PostorderDomTraversal}; +use wrapper::{ThreadSafeLayoutNode, UnsafeLayoutNode}; use selectors::bloom::BloomFilter; use selectors::Node; @@ -118,6 +117,32 @@ fn insert_ancestors_into_bloom_filter(bf: &mut Box, debug!("[{}] Inserted {} ancestors.", tid(), ancestors); } + +/// A top-down traversal. +pub trait PreorderDomTraversal { + /// The operation to perform. Return true to continue or false to stop. + fn process(&self, node: LayoutNode); +} + +/// A bottom-up traversal, with a optional in-order pass. +pub trait PostorderDomTraversal { + /// The operation to perform. Return true to continue or false to stop. + fn process(&self, node: LayoutNode); +} + +/// A bottom-up, parallelizable traversal. +pub trait PostorderNodeMutTraversal { + /// The operation to perform. Return true to continue or false to stop. + fn process<'a>(&'a mut self, node: &ThreadSafeLayoutNode<'a>) -> bool; + + /// Returns true if this node should be pruned. If this returns true, we skip the operation + /// entirely and do not process any descendant nodes. This is called *before* child nodes are + /// visited. The default implementation never prunes any nodes. + fn should_prune<'a>(&'a self, _node: &ThreadSafeLayoutNode<'a>) -> bool { + false + } +} + /// The recalc-style-for-node traversal, which styles each node and must run before /// layout computation. This computes the styles applied to each node. #[derive(Copy, Clone)] diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index dca07619751..89b45a83512 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -36,6 +36,7 @@ use css::node_style::StyledNode; use incremental::RestyleDamage; use data::{LayoutDataFlags, LayoutDataWrapper, PrivateLayoutData}; use opaque_node::OpaqueNodeMethods; +use traversal::PostorderNodeMutTraversal; use gfx::display_list::OpaqueNode; use script::dom::attr::AttrValue; @@ -1056,19 +1057,6 @@ impl<'le> ThreadSafeLayoutElement<'le> { } } -/// A bottom-up, parallelizable traversal. -pub trait PostorderNodeMutTraversal { - /// The operation to perform. Return true to continue or false to stop. - fn process<'a>(&'a mut self, node: &ThreadSafeLayoutNode<'a>) -> bool; - - /// Returns true if this node should be pruned. If this returns true, we skip the operation - /// entirely and do not process any descendant nodes. This is called *before* child nodes are - /// visited. The default implementation never prunes any nodes. - fn should_prune<'a>(&'a self, _node: &ThreadSafeLayoutNode<'a>) -> bool { - false - } -} - /// Opaque type stored in type-unsafe work queues for parallel layout. /// Must be transmutable to and from LayoutNode. pub type UnsafeLayoutNode = (usize, usize); @@ -1086,15 +1074,3 @@ pub unsafe fn layout_node_from_unsafe_layout_node(node: &UnsafeLayoutNode) -> La let (node, _) = *node; mem::transmute(node) } - -/// A top-down traversal. -pub trait PreorderDomTraversal { - /// The operation to perform. Return true to continue or false to stop. - fn process(&self, node: LayoutNode); -} - -/// A bottom-up traversal, with a optional in-order pass. -pub trait PostorderDomTraversal { - /// The operation to perform. Return true to continue or false to stop. - fn process(&self, node: LayoutNode); -} From 4b08cea663b823c704e51c9e9201dc8c9d1bd9be Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 17 Jul 2015 11:41:06 +0200 Subject: [PATCH 2/6] Scope the allowed unsafe code in traversal.rs. --- components/layout/traversal.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index c366d20206b..c60a01e1629 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -4,8 +4,6 @@ //! Traversals over the DOM and flow trees, running the layout computations. -#![allow(unsafe_code)] - use css::node_style::StyledNode; use css::matching::{ApplicableDeclarations, MatchMethods, StyleSharingResult}; use construct::FlowConstructor; @@ -152,6 +150,7 @@ pub struct RecalcStyleForNode<'a> { impl<'a> PreorderDomTraversal for RecalcStyleForNode<'a> { #[inline] + #[allow(unsafe_code)] fn process(&self, node: LayoutNode) { // Initialize layout data. // @@ -244,6 +243,7 @@ pub struct ConstructFlows<'a> { impl<'a> PostorderDomTraversal for ConstructFlows<'a> { #[inline] + #[allow(unsafe_code)] fn process(&self, node: LayoutNode) { // Construct flows for this node. { From c018863201e75a924e416a497c4fdb2dddaffc61 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 17 Jul 2015 11:41:57 +0200 Subject: [PATCH 3/6] Stop using Option in ThreadSafeLayoutNodeChildrenIterator::parent_node. --- components/layout/wrapper.rs | 50 +++++++++++++++--------------------- 1 file changed, 20 insertions(+), 30 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 89b45a83512..7247cffd818 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -684,7 +684,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> { pub fn children(&self) -> ThreadSafeLayoutNodeChildrenIterator<'ln> { ThreadSafeLayoutNodeChildrenIterator { current_node: self.first_child(), - parent_node: Some(self.clone()), + parent_node: self.clone(), } } @@ -990,7 +990,7 @@ impl<'ln> ThreadSafeLayoutNode<'ln> { pub struct ThreadSafeLayoutNodeChildrenIterator<'a> { current_node: Option>, - parent_node: Option>, + parent_node: ThreadSafeLayoutNode<'a>, } impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> { @@ -1004,36 +1004,26 @@ impl<'a> Iterator for ThreadSafeLayoutNodeChildrenIterator<'a> { return None } - match self.parent_node { - Some(ref parent_node) => { - self.current_node = if parent_node.pseudo == PseudoElementType::Normal { - self.current_node.clone().and_then(|node| { - unsafe { - node.next_sibling() - } - }) - } else { - None - }; - } - None => {} - } + self.current_node = if self.parent_node.pseudo == PseudoElementType::Normal { + self.current_node.clone().and_then(|node| { + unsafe { + node.next_sibling() + } + }) + } else { + None + }; } None => { - match self.parent_node { - Some(ref parent_node) => { - if parent_node.has_after_pseudo() { - let pseudo_after_node = if parent_node.pseudo == PseudoElementType::Normal { - let pseudo = PseudoElementType::After(parent_node.get_after_display()); - Some(parent_node.with_pseudo(pseudo)) - } else { - None - }; - self.current_node = pseudo_after_node; - return self.current_node.clone() - } - } - None => {} + if self.parent_node.has_after_pseudo() { + let pseudo_after_node = if self.parent_node.pseudo == PseudoElementType::Normal { + let pseudo = PseudoElementType::After(self.parent_node.get_after_display()); + Some(self.parent_node.with_pseudo(pseudo)) + } else { + None + }; + self.current_node = pseudo_after_node; + return self.current_node.clone() } } } From bb444df679346033afc42d9f2b6011b1bbcd5d16 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 17 Jul 2015 13:26:23 +0200 Subject: [PATCH 4/6] Remove unused ThreadSafeLayoutNode::traverse_postorder_mut. --- components/layout/wrapper.rs | 23 ----------------------- 1 file changed, 23 deletions(-) diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 7247cffd818..747b0d6cd12 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -36,7 +36,6 @@ use css::node_style::StyledNode; use incremental::RestyleDamage; use data::{LayoutDataFlags, LayoutDataWrapper, PrivateLayoutData}; use opaque_node::OpaqueNodeMethods; -use traversal::PostorderNodeMutTraversal; use gfx::display_list::OpaqueNode; use script::dom::attr::AttrValue; @@ -771,28 +770,6 @@ impl<'ln> ThreadSafeLayoutNode<'ln> { self.node.mutate_layout_data() } - /// Traverses the tree in postorder. - /// - /// TODO(pcwalton): Offer a parallel version with a compatible API. - pub fn traverse_postorder_mut(&mut self, traversal: &mut T) - -> bool { - if traversal.should_prune(self) { - return true - } - - let mut opt_kid = self.first_child(); - while let Some(mut kid) = opt_kid { - if !kid.traverse_postorder_mut(traversal) { - return false - } - unsafe { - opt_kid = kid.next_sibling() - } - } - - traversal.process(self) - } - pub fn is_ignorable_whitespace(&self) -> bool { unsafe { let text: LayoutJS = match TextCast::to_layout_js(self.get_jsmanaged()) { From 46b36242a37413d84089308a1339ecf28fc9a825 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Fri, 17 Jul 2015 14:30:33 +0200 Subject: [PATCH 5/6] Use Ref::map to make ThreadSafeLayoutNode::style safe. --- components/layout/construct.rs | 2 +- components/layout/css/node_style.rs | 18 ++++++++---------- components/layout/lib.rs | 1 + 3 files changed, 10 insertions(+), 11 deletions(-) diff --git a/components/layout/construct.rs b/components/layout/construct.rs index 746ecef1e6b..aad12f4e770 100644 --- a/components/layout/construct.rs +++ b/components/layout/construct.rs @@ -667,7 +667,7 @@ impl<'a> FlowConstructor<'a> { self.create_fragments_for_node_text_content(&mut initial_fragments, node, - node.style()); + &*node.style()); } self.build_flow_for_block_starting_with_fragments(flow, node, initial_fragments) diff --git a/components/layout/css/node_style.rs b/components/layout/css/node_style.rs index 5f513a19399..1a5c1a74933 100644 --- a/components/layout/css/node_style.rs +++ b/components/layout/css/node_style.rs @@ -7,16 +7,17 @@ use data::LayoutDataWrapper; use wrapper::{PseudoElementType, ThreadSafeLayoutNode}; -use std::mem; use style::properties::ComputedValues; + +use std::cell::Ref; use std::sync::Arc; /// Node mixin providing `style` method that returns a `NodeStyle` pub trait StyledNode { - fn get_style<'a>(&'a self, layout_data_ref: &'a LayoutDataWrapper) -> &'a Arc; + fn get_style<'a>(&self, layout_data_ref: &'a LayoutDataWrapper) -> &'a Arc; /// Returns the style results for the given node. If CSS selector matching has not yet been /// performed, fails. - fn style<'a>(&'a self) -> &'a Arc; + fn style<'a>(&'a self) -> Ref<'a, Arc>; /// Does this node have a computed style yet? fn has_style(&self) -> bool; /// Removes the style from this node. @@ -34,14 +35,11 @@ impl<'ln> StyledNode for ThreadSafeLayoutNode<'ln> { } #[inline] - #[allow(unsafe_code)] - fn style<'a>(&'a self) -> &'a Arc { - unsafe { - let layout_data_ref = self.borrow_layout_data(); + fn style<'a>(&'a self) -> Ref<'a, Arc> { + Ref::map(self.borrow_layout_data(), |layout_data_ref| { let layout_data = layout_data_ref.as_ref().expect("no layout data"); - mem::transmute::<&Arc, - &'a Arc>(self.get_style(&layout_data)) - } + self.get_style(layout_data) + }) } fn has_style(&self) -> bool { diff --git a/components/layout/lib.rs b/components/layout/lib.rs index f16272f0d22..9bb10cdd93c 100644 --- a/components/layout/lib.rs +++ b/components/layout/lib.rs @@ -5,6 +5,7 @@ #![feature(append)] #![feature(arc_unique)] #![feature(box_syntax)] +#![feature(cell_extras)] #![feature(custom_derive)] #![feature(filling_drop)] #![feature(hashmap_hasher)] From 5d36fbca291fcf80ead54a5917e9068fe71270c0 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Thu, 9 Jul 2015 13:03:18 +0200 Subject: [PATCH 6/6] Reduce the amount of code in the unsafe block in ParallelPostorderFlowTraversal::run_parallel. --- components/layout/parallel.rs | 60 ++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 29 deletions(-) diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index 6d00c4a4c44..3a122a8f88d 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -242,41 +242,43 @@ trait ParallelPostorderFlowTraversal : PostorderFlowTraversal { mut unsafe_flow: UnsafeFlow, _: &mut WorkerProxy) { loop { - unsafe { - // Get a real flow. - let flow: &mut FlowRef = mem::transmute(&mut unsafe_flow); + // Get a real flow. + let flow: &mut FlowRef = unsafe { + mem::transmute(&mut unsafe_flow) + }; - // Perform the appropriate traversal. - if self.should_process(&mut **flow) { - self.process(&mut **flow); - } + // Perform the appropriate traversal. + if self.should_process(&mut **flow) { + self.process(&mut **flow); + } - let base = flow::mut_base(&mut **flow); + let base = flow::mut_base(&mut **flow); - // Reset the count of children for the next layout traversal. - base.parallel.children_count.store(base.children.len() as isize, - Ordering::Relaxed); + // Reset the count of children for the next layout traversal. + base.parallel.children_count.store(base.children.len() as isize, + Ordering::Relaxed); - // Possibly enqueue the parent. - let mut unsafe_parent = base.parallel.parent; - if unsafe_parent == null_unsafe_flow() { - // We're done! - break - } + // Possibly enqueue the parent. + let mut unsafe_parent = base.parallel.parent; + if unsafe_parent == null_unsafe_flow() { + // We're done! + break + } - // No, we're not at the root yet. Then are we the last child - // of our parent to finish processing? If so, we can continue - // on with our parent; otherwise, we've gotta wait. - let parent: &mut FlowRef = mem::transmute(&mut unsafe_parent); - let parent_base = flow::mut_base(&mut **parent); - if parent_base.parallel.children_count.fetch_sub(1, Ordering::Relaxed) == 1 { - // We were the last child of our parent. Reflow our parent. - unsafe_flow = unsafe_parent - } else { - // Stop. - break - } + // No, we're not at the root yet. Then are we the last child + // of our parent to finish processing? If so, we can continue + // on with our parent; otherwise, we've gotta wait. + let parent: &mut FlowRef = unsafe { + mem::transmute(&mut unsafe_parent) + }; + let parent_base = flow::mut_base(&mut **parent); + if parent_base.parallel.children_count.fetch_sub(1, Ordering::Relaxed) == 1 { + // We were the last child of our parent. Reflow our parent. + unsafe_flow = unsafe_parent + } else { + // Stop. + break } } }