From ebc5eb1b98e07c9d130bec81400f917b33927447 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 31 Dec 2015 11:05:17 -0800 Subject: [PATCH 1/8] Make parallel DOM traversal and style calculation operate on TNode instead of LayoutNode. --- components/layout/parallel.rs | 9 ++++----- components/layout/traversal.rs | 13 ++++++------- components/layout/wrapper.rs | 12 ++++++++++++ components/style/dom.rs | 8 ++++++++ 4 files changed, 30 insertions(+), 12 deletions(-) diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index fa52a7efdb2..a451747b410 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -15,14 +15,13 @@ use gfx::display_list::OpaqueNode; use profile_traits::time::{self, TimerMetadata, profile}; use std::mem; use std::sync::atomic::{AtomicIsize, Ordering}; -use style::dom::UnsafeNode; +use style::dom::{TNode, UnsafeNode}; use traversal::PostorderNodeMutTraversal; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes, BubbleISizes}; use traversal::{BuildDisplayList, ComputeAbsolutePositions}; use traversal::{DomTraversal, DomTraversalContext}; use util::opts; use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; -use wrapper::LayoutNode; const CHUNK_SIZE: usize = 64; @@ -236,7 +235,7 @@ impl<'a> ParallelPostorderFlowTraversal for BuildDisplayList<'a> {} #[inline(always)] fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, proxy: &mut WorkerProxy) - where N: LayoutNode<'ln>, T: DomTraversal<'ln, N> { + where N: TNode<'ln>, T: DomTraversal<'ln, N> { let shared_layout_context = proxy.user_data(); let layout_context = LayoutContext::new(shared_layout_context); let traversal_context = DomTraversalContext { @@ -294,7 +293,7 @@ fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, fn bottom_up_dom<'ln, N, T>(root: OpaqueNode, unsafe_node: UnsafeNode, proxy: &mut WorkerProxy) - where N: LayoutNode<'ln>, T: DomTraversal<'ln, N> { + where N: TNode<'ln>, T: DomTraversal<'ln, N> { let shared_layout_context = proxy.user_data(); let layout_context = LayoutContext::new(shared_layout_context); let traversal_context = DomTraversalContext { @@ -388,7 +387,7 @@ pub fn traverse_dom_preorder<'ln, N, T>( root: N, shared_layout_context: &SharedLayoutContext, queue: &mut WorkQueue) - where N: LayoutNode<'ln>, T: DomTraversal<'ln, N> { + where N: TNode<'ln>, T: DomTraversal<'ln, N> { run_queue_with_custom_work_data_type(queue, |queue| { queue.push(WorkUnit { fun: top_down_dom::, diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index b740e7dbcf3..a37d7301530 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -15,7 +15,7 @@ use selectors::bloom::BloomFilter; use std::cell::RefCell; use std::mem; use style::context::StyleContext; -use style::dom::{TRestyleDamage, UnsafeNode}; +use style::dom::{TNode, TRestyleDamage, UnsafeNode}; use style::matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use util::opts; use util::tid::tid; @@ -56,7 +56,7 @@ fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, root: OpaqueNode, layout_context: &LayoutContext) -> Box - where N: LayoutNode<'ln> { + where N: TNode<'ln> { STYLE_BLOOM.with(|style_bloom| { match (parent_node, style_bloom.borrow_mut().take()) { // Root node. Needs new bloom filter. @@ -102,7 +102,7 @@ fn put_task_local_bloom_filter(bf: Box, fn insert_ancestors_into_bloom_filter<'ln, N>(bf: &mut Box, mut n: N, root: OpaqueNode) - where N: LayoutNode<'ln> { + where N: TNode<'ln> { debug!("[{}] Inserting ancestors.", tid()); let mut ancestors = 0; loop { @@ -123,7 +123,7 @@ pub struct DomTraversalContext<'a> { pub root: OpaqueNode, } -pub trait DomTraversal<'ln, N: LayoutNode<'ln>> { +pub trait DomTraversal<'ln, N: TNode<'ln>> { fn process_preorder<'a>(context: &'a DomTraversalContext<'a>, node: N); fn process_postorder<'a>(context: &'a DomTraversalContext<'a>, node: N); } @@ -132,7 +132,7 @@ pub trait DomTraversal<'ln, N: LayoutNode<'ln>> { /// This is currently unused, but will be used shortly. #[allow(dead_code)] pub struct RecalcStyleOnly; -impl<'ln, N: LayoutNode<'ln>> DomTraversal<'ln, N> for RecalcStyleOnly { +impl<'ln, N: TNode<'ln>> DomTraversal<'ln, N> for RecalcStyleOnly { fn process_preorder<'a>(context: &'a DomTraversalContext<'a>, node: N) { recalc_style_at(context, node); } fn process_postorder<'a>(_: &'a DomTraversalContext<'a>, _: N) {} } @@ -153,7 +153,7 @@ pub trait PostorderNodeMutTraversal<'ln, ConcreteThreadSafeLayoutNode: ThreadSaf /// layout computation. This computes the styles applied to each node. #[inline] #[allow(unsafe_code)] -fn recalc_style_at<'a, 'ln, N: LayoutNode<'ln>> (context: &'a DomTraversalContext<'a>, node: N) { +fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a>, node: N) { // Initialize layout data. // // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML @@ -171,7 +171,6 @@ fn recalc_style_at<'a, 'ln, N: LayoutNode<'ln>> (context: &'a DomTraversalContex // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), // to force non-incremental reflow. if node.has_changed() { - let node = node.to_threadsafe(); node.unstyle(); } diff --git a/components/layout/wrapper.rs b/components/layout/wrapper.rs index 4a3ca347848..aa947937d0b 100644 --- a/components/layout/wrapper.rs +++ b/components/layout/wrapper.rs @@ -275,6 +275,14 @@ impl<'ln> TNode<'ln> for ServoLayoutNode<'ln> { self.node.next_sibling_ref().map(|node| self.new_with_this_lifetime(&node)) } } + + fn style(&self) -> Ref> { + Ref::map(self.borrow_data().unwrap(), |data| data.style.as_ref().unwrap()) + } + + fn unstyle(self) { + self.mutate_data().unwrap().style = None; + } } impl<'ln> LayoutNode<'ln> for ServoLayoutNode<'ln> { @@ -696,6 +704,8 @@ pub trait ThreadSafeLayoutNode<'ln> : Clone + Copy + Sized { /// Returns the style results for the given node. If CSS selector matching /// has not yet been performed, fails. + /// + /// Unlike the version on TNode, this handles pseudo-elements. #[inline] fn style(&self) -> Ref> { Ref::map(self.borrow_layout_data().unwrap(), |data| { @@ -709,6 +719,8 @@ pub trait ThreadSafeLayoutNode<'ln> : Clone + Copy + Sized { } /// Removes the style from this node. + /// + /// Unlike the version on TNode, this handles pseudo-elements. fn unstyle(self) { let mut data = self.mutate_layout_data().unwrap(); let style = diff --git a/components/style/dom.rs b/components/style/dom.rs index 40933b03a7e..f85484e0c70 100644 --- a/components/style/dom.rs +++ b/components/style/dom.rs @@ -156,6 +156,14 @@ pub trait TNode<'ln> : Sized + Copy + Clone { fn prev_sibling(&self) -> Option; fn next_sibling(&self) -> Option; + + + /// Returns the style results for the given node. If CSS selector matching + /// has not yet been performed, fails. + fn style(&self) -> Ref>; + + /// Removes the style from this node. + fn unstyle(self); } pub trait TDocument<'ld> : Sized + Copy + Clone { From bf1a7d243f443c272e92797ab34f56bfe349de4c Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Sun, 3 Jan 2016 19:18:34 -0800 Subject: [PATCH 2/8] Remove the dependency of parallel DOM traversal and style calculation on LayoutContext. --- components/layout/context.rs | 10 --- components/layout/parallel.rs | 62 ++++++-------- components/layout/sequential.rs | 27 +++--- components/layout/traversal.rs | 145 ++++++++++++++++++++++++-------- 4 files changed, 145 insertions(+), 99 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index a10fe334730..43c51147934 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -128,16 +128,6 @@ impl<'a> LayoutContext<'a> { self.cached_local_layout_context.font_context.borrow_mut() } - #[inline(always)] - pub fn applicable_declarations_cache(&self) -> RefMut { - self.local_context().applicable_declarations_cache.borrow_mut() - } - - #[inline(always)] - pub fn style_sharing_candidate_cache(&self) -> RefMut { - self.local_context().style_sharing_candidate_cache.borrow_mut() - } - pub fn get_or_request_image(&self, url: Url, use_placeholder: UsePlaceholder) -> Option> { // See if the image is already available diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index a451747b410..0aac7dc822e 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -19,7 +19,7 @@ use style::dom::{TNode, UnsafeNode}; use traversal::PostorderNodeMutTraversal; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes, BubbleISizes}; use traversal::{BuildDisplayList, ComputeAbsolutePositions}; -use traversal::{DomTraversal, DomTraversalContext}; +use traversal::DomTraversalContext; use util::opts; use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; @@ -233,15 +233,10 @@ impl<'a> ParallelPostorderFlowTraversal for BuildDisplayList<'a> {} /// A parallel top-down DOM traversal. #[inline(always)] -fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, - proxy: &mut WorkerProxy) - where N: TNode<'ln>, T: DomTraversal<'ln, N> { - let shared_layout_context = proxy.user_data(); - let layout_context = LayoutContext::new(shared_layout_context); - let traversal_context = DomTraversalContext { - layout_context: &layout_context, - root: unsafe_nodes.1, - }; +fn top_down_dom<'ln, N, C>(unsafe_nodes: UnsafeNodeList, + proxy: &mut WorkerProxy) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + let context = C::new(proxy.user_data(), unsafe_nodes.1); let mut discovered_child_nodes = Vec::new(); for unsafe_node in *unsafe_nodes.0 { @@ -249,7 +244,7 @@ fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, let node = unsafe { N::from_unsafe(&unsafe_node) }; // Perform the appropriate traversal. - T::process_preorder(&traversal_context, node); + context.process_preorder(node); let child_count = node.children_count(); @@ -267,13 +262,13 @@ fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, } } else { // If there were no more children, start walking back up. - bottom_up_dom::(unsafe_nodes.1, unsafe_node, proxy) + bottom_up_dom::(unsafe_nodes.1, unsafe_node, proxy) } } for chunk in discovered_child_nodes.chunks(CHUNK_SIZE) { proxy.push(WorkUnit { - fun: top_down_dom::, + fun: top_down_dom::, data: (box chunk.iter().cloned().collect(), unsafe_nodes.1), }); } @@ -290,24 +285,19 @@ fn top_down_dom<'ln, N, T>(unsafe_nodes: UnsafeNodeList, /// /// The only communication between siblings is that they both /// fetch-and-subtract the parent's children count. -fn bottom_up_dom<'ln, N, T>(root: OpaqueNode, +fn bottom_up_dom<'ln, N, C>(root: OpaqueNode, unsafe_node: UnsafeNode, - proxy: &mut WorkerProxy) - where N: TNode<'ln>, T: DomTraversal<'ln, N> { - let shared_layout_context = proxy.user_data(); - let layout_context = LayoutContext::new(shared_layout_context); - let traversal_context = DomTraversalContext { - layout_context: &layout_context, - root: root, - }; + proxy: &mut WorkerProxy) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + let context = C::new(proxy.user_data(), root); // Get a real layout node. let mut node = unsafe { N::from_unsafe(&unsafe_node) }; loop { // Perform the appropriate operation. - T::process_postorder(&traversal_context, node); + context.process_postorder(node); - let parent = match node.layout_parent_node(traversal_context.root) { + let parent = match node.layout_parent_node(root) { None => break, Some(parent) => parent, }; @@ -371,29 +361,29 @@ fn build_display_list(unsafe_flow: UnsafeFlow, build_display_list_traversal.run_parallel(unsafe_flow); } -fn run_queue_with_custom_work_data_type( - queue: &mut WorkQueue, +fn run_queue_with_custom_work_data_type( + queue: &mut WorkQueue, callback: F, - shared_layout_context: &SharedLayoutContext) - where To: 'static + Send, F: FnOnce(&mut WorkQueue) { - let queue: &mut WorkQueue = unsafe { + shared: &SharedContext) + where To: 'static + Send, F: FnOnce(&mut WorkQueue) { + let queue: &mut WorkQueue = unsafe { mem::transmute(queue) }; callback(queue); - queue.run(shared_layout_context); + queue.run(shared); } -pub fn traverse_dom_preorder<'ln, N, T>( +pub fn traverse_dom_preorder<'ln, N, C>( root: N, - shared_layout_context: &SharedLayoutContext, - queue: &mut WorkQueue) - where N: TNode<'ln>, T: DomTraversal<'ln, N> { + queue_data: &C::SharedContext, + queue: &mut WorkQueue) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { run_queue_with_custom_work_data_type(queue, |queue| { queue.push(WorkUnit { - fun: top_down_dom::, + fun: top_down_dom::, data: (box vec![root.to_unsafe()], root.opaque()), }); - }, shared_layout_context); + }, queue_data); } pub fn traverse_flow_tree_preorder( diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index 310560e2fa7..bafd47872d4 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -15,32 +15,27 @@ use generated_content::ResolveGeneratedContent; use traversal::PostorderNodeMutTraversal; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions}; -use traversal::{DomTraversal, DomTraversalContext}; +use traversal::DomTraversalContext; use util::opts; use wrapper::LayoutNode; -pub fn traverse_dom_preorder<'ln, N, T>(root: N, - shared_layout_context: &SharedLayoutContext) +pub fn traverse_dom_preorder<'ln, N, C>(root: N, + shared: &C::SharedContext) where N: LayoutNode<'ln>, - T: DomTraversal<'ln, N> { - fn doit<'a, 'ln, N, T>(context: &'a DomTraversalContext<'a>, node: N) - where N: LayoutNode<'ln>, T: DomTraversal<'ln, N> { - T::process_preorder(context, node); + C: DomTraversalContext<'ln, N> { + fn doit<'a, 'ln, N, C>(context: &'a C, node: N) + where N: LayoutNode<'ln>, C: DomTraversalContext<'ln, N> { + context.process_preorder(node); for kid in node.children() { - doit::(context, kid); + doit::(context, kid); } - T::process_postorder(context, node); + context.process_postorder(node); } - let layout_context = LayoutContext::new(shared_layout_context); - let traversal_context = DomTraversalContext { - layout_context: &layout_context, - root: root.opaque(), - }; - - doit::(&traversal_context, root); + let context = C::new(shared, root.opaque()); + doit::(&context, root); } pub fn resolve_generated_content(root: &mut FlowRef, shared_layout_context: &SharedLayoutContext) { diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index a37d7301530..05d4e6171b0 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -4,8 +4,10 @@ //! Traversals over the DOM and flow trees, running the layout computations. +#![allow(unsafe_code)] + use construct::FlowConstructor; -use context::LayoutContext; +use context::{LayoutContext, SharedLayoutContext}; use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; use flow::{self, Flow}; use gfx::display_list::OpaqueNode; @@ -14,7 +16,8 @@ use script::layout_interface::ReflowGoal; use selectors::bloom::BloomFilter; use std::cell::RefCell; use std::mem; -use style::context::StyleContext; +use std::rc::Rc; +use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; use style::dom::{TNode, TRestyleDamage, UnsafeNode}; use style::matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; use util::opts; @@ -54,7 +57,7 @@ thread_local!( /// it will be cleared and reused. fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, root: OpaqueNode, - layout_context: &LayoutContext) + context: &SharedStyleContext) -> Box where N: TNode<'ln> { STYLE_BLOOM.with(|style_bloom| { @@ -73,7 +76,7 @@ fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, // Found cached bloom filter. (Some(parent), Some((mut bloom_filter, old_node, old_generation))) => { if old_node == parent.to_unsafe() && - old_generation == layout_context.shared_context().generation { + old_generation == context.generation { // Hey, the cached parent is our parent! We can reuse the bloom filter. debug!("[{}] Parent matches (={}). Reusing bloom filter.", tid(), old_node.0); } else { @@ -90,11 +93,11 @@ fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, fn put_task_local_bloom_filter(bf: Box, unsafe_node: &UnsafeNode, - layout_context: &LayoutContext) { + context: &SharedStyleContext) { STYLE_BLOOM.with(move |style_bloom| { assert!(style_bloom.borrow().is_none(), "Putting into a never-taken task-local bloom filter"); - *style_bloom.borrow_mut() = Some((bf, *unsafe_node, layout_context.shared_context().generation)); + *style_bloom.borrow_mut() = Some((bf, *unsafe_node, context.generation)); }) } @@ -117,30 +120,98 @@ fn insert_ancestors_into_bloom_filter<'ln, N>(bf: &mut Box, debug!("[{}] Inserted {} ancestors.", tid(), ancestors); } -#[derive(Copy, Clone)] -pub struct DomTraversalContext<'a> { - pub layout_context: &'a LayoutContext<'a>, - pub root: OpaqueNode, -} - -pub trait DomTraversal<'ln, N: TNode<'ln>> { - fn process_preorder<'a>(context: &'a DomTraversalContext<'a>, node: N); - fn process_postorder<'a>(context: &'a DomTraversalContext<'a>, node: N); +pub trait DomTraversalContext<'ln, N: TNode<'ln>> { + type SharedContext: Sync + 'static; + fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self; + fn process_preorder(&self, node: N); + fn process_postorder(&self, node: N); } /// FIXME(bholley): I added this now to demonstrate the usefulness of the new design. /// This is currently unused, but will be used shortly. + #[allow(dead_code)] -pub struct RecalcStyleOnly; -impl<'ln, N: TNode<'ln>> DomTraversal<'ln, N> for RecalcStyleOnly { - fn process_preorder<'a>(context: &'a DomTraversalContext<'a>, node: N) { recalc_style_at(context, node); } - fn process_postorder<'a>(_: &'a DomTraversalContext<'a>, _: N) {} +pub struct StandaloneStyleContext<'a> { + pub shared: &'a SharedStyleContext, + cached_local_style_context: Rc, } -pub struct RecalcStyleAndConstructFlows; -impl<'ln, N: LayoutNode<'ln>> DomTraversal<'ln, N> for RecalcStyleAndConstructFlows { - fn process_preorder<'a>(context: &'a DomTraversalContext<'a>, node: N) { recalc_style_at(context, node); } - fn process_postorder<'a>(context: &'a DomTraversalContext<'a>, node: N) { construct_flows_at(context, node); } +impl<'a> StandaloneStyleContext<'a> { + pub fn new(_: &'a SharedStyleContext) -> Self { panic!("Not implemented") } +} + +impl<'a> StyleContext<'a> for StandaloneStyleContext<'a> { + fn shared_context(&self) -> &'a SharedStyleContext { + &self.shared + } + + fn local_context(&self) -> &LocalStyleContext { + &self.cached_local_style_context + } +} + +#[allow(dead_code)] +pub struct RecalcStyleOnly<'lc> { + context: StandaloneStyleContext<'lc>, + root: OpaqueNode, +} + +impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> { + type SharedContext = SharedStyleContext; + fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { + // See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is + // necessary. + let shared_lc: &'lc SharedStyleContext = unsafe { mem::transmute(shared) }; + RecalcStyleOnly { + context: StandaloneStyleContext::new(shared_lc), + root: root, + } + } + + fn process_preorder(&self, node: N) { recalc_style_at(&self.context, self.root, node); } + fn process_postorder(&self, _: N) {} +} + +pub struct RecalcStyleAndConstructFlows<'lc> { + context: LayoutContext<'lc>, + root: OpaqueNode, +} + +impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAndConstructFlows<'lc> { + type SharedContext = SharedLayoutContext; + fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { + // FIXME(bholley): This transmutation from &'a to &'lc is very unfortunate, but I haven't + // found a way to avoid it despite spending several days on it (and consulting Manishearth, + // brson, and nmatsakis). + // + // The crux of the problem is that parameterizing DomTraversalContext on the lifetime of + // the SharedContext doesn't work for a variety of reasons [1]. However, the code in + // parallel.rs needs to be able to use the DomTraversalContext trait (or something similar) + // to stack-allocate a struct (a generalized LayoutContext<'a>) that holds a borrowed + // SharedContext, which means that the struct needs to be parameterized on a lifetime. + // Given the aforementioned constraint, the only way to accomplish this is to avoid + // propagating the borrow lifetime from the struct to the trait, but that means that the + // new() method on the trait cannot require the lifetime of its argument to match the + // lifetime of the Self object it creates. + // + // This could be solved with an associated type with an unbound lifetime parameter, but + // that would require higher-kinded types, which don't exist yet and probably aren't coming + // for a while. + // + // So we transmute. :-( + // + // [1] For example, the WorkQueue type needs to be parameterized on the concrete type of + // DomTraversalContext::SharedContext, and the WorkQueue lifetime is similar to that of the + // LayoutTask, generally much longer than that of a given SharedLayoutContext borrow. + let shared_lc: &'lc SharedLayoutContext = unsafe { mem::transmute(shared) }; + RecalcStyleAndConstructFlows { + context: LayoutContext::new(shared_lc), + root: root, + } + } + + fn process_preorder(&self, node: N) { recalc_style_at(&self.context, self.root, node); } + fn process_postorder(&self, node: N) { construct_flows_at(&self.context, self.root, node); } } /// A bottom-up, parallelizable traversal. @@ -153,7 +224,7 @@ pub trait PostorderNodeMutTraversal<'ln, ConcreteThreadSafeLayoutNode: ThreadSaf /// layout computation. This computes the styles applied to each node. #[inline] #[allow(unsafe_code)] -fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a>, node: N) { +fn recalc_style_at<'a, 'ln, N: TNode<'ln>, C: StyleContext<'a>> (context: &'a C, root: OpaqueNode, node: N) { // Initialize layout data. // // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML @@ -161,10 +232,10 @@ fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a> node.initialize_data(); // Get the parent node. - let parent_opt = node.layout_parent_node(context.root); + let parent_opt = node.layout_parent_node(root); // Get the style bloom filter. - let mut bf = take_task_local_bloom_filter(parent_opt, context.root, context.layout_context); + let mut bf = take_task_local_bloom_filter(parent_opt, root, context.shared_context()); let nonincremental_layout = opts::get().nonincremental_layout; if nonincremental_layout || node.is_dirty() { @@ -176,7 +247,7 @@ fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a> // Check to see whether we can share a style with someone. let style_sharing_candidate_cache = - &mut context.layout_context.style_sharing_candidate_cache(); + &mut context.local_context().style_sharing_candidate_cache.borrow_mut(); let sharing_result = match node.as_element() { Some(element) => { @@ -196,7 +267,7 @@ fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a> let shareable_element = match node.as_element() { Some(element) => { // Perform the CSS selector matching. - let stylist = unsafe { &*context.layout_context.shared_context().stylist.0 }; + let stylist = unsafe { &*context.shared_context().stylist.0 }; if element.match_element(stylist, Some(&*bf), &mut applicable_declarations) { @@ -215,11 +286,11 @@ fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a> // Perform the CSS cascade. unsafe { - node.cascade_node(&context.layout_context.shared.style_context, + node.cascade_node(&context.shared_context(), parent_opt, &applicable_declarations, - &mut context.layout_context.applicable_declarations_cache(), - &context.layout_context.shared_context().new_animations_sender); + &mut context.local_context().applicable_declarations_cache.borrow_mut(), + &context.shared_context().new_animations_sender); } // Add ourselves to the LRU cache. @@ -242,13 +313,13 @@ fn recalc_style_at<'a, 'ln, N: TNode<'ln>> (context: &'a DomTraversalContext<'a> node.insert_into_bloom_filter(&mut *bf); // NB: flow construction updates the bloom filter on the way up. - put_task_local_bloom_filter(bf, &unsafe_layout_node, context.layout_context); + put_task_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); } /// The flow construction traversal, which builds flows for styled nodes. #[inline] #[allow(unsafe_code)] -fn construct_flows_at<'a, 'ln, N: LayoutNode<'ln>>(context: &'a DomTraversalContext<'a>, node: N) { +fn construct_flows_at<'a, 'ln, N: LayoutNode<'ln>>(context: &'a LayoutContext<'a>, root: OpaqueNode, node: N) { // Construct flows for this node. { let tnode = node.to_threadsafe(); @@ -256,7 +327,7 @@ fn construct_flows_at<'a, 'ln, N: LayoutNode<'ln>>(context: &'a DomTraversalCont // Always reconstruct if incremental layout is turned off. let nonincremental_layout = opts::get().nonincremental_layout; if nonincremental_layout || node.has_dirty_descendants() { - let mut flow_constructor = FlowConstructor::new(context.layout_context); + let mut flow_constructor = FlowConstructor::new(context); if nonincremental_layout || !flow_constructor.repair_if_possible(&tnode) { flow_constructor.process(&tnode); debug!("Constructed flow for {:x}: {:x}", @@ -285,9 +356,9 @@ fn construct_flows_at<'a, 'ln, N: LayoutNode<'ln>>(context: &'a DomTraversalCont }); assert_eq!(old_node, unsafe_layout_node); - assert_eq!(old_generation, context.layout_context.shared_context().generation); + assert_eq!(old_generation, context.shared_context().generation); - match node.layout_parent_node(context.root) { + match node.layout_parent_node(root) { None => { debug!("[{}] - {:X}, and deleting BF.", tid(), unsafe_layout_node.0); // If this is the reflow root, eat the task-local bloom filter. @@ -296,7 +367,7 @@ fn construct_flows_at<'a, 'ln, N: LayoutNode<'ln>>(context: &'a DomTraversalCont // Otherwise, put it back, but remove this node. node.remove_from_bloom_filter(&mut *bf); let unsafe_parent = parent.to_unsafe(); - put_task_local_bloom_filter(bf, &unsafe_parent, context.layout_context); + put_task_local_bloom_filter(bf, &unsafe_parent, &context.shared_context()); }, }; } From f9a02f0aba352736c372201a11d4c19ecc99dcf4 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 6 Jan 2016 18:36:14 -0800 Subject: [PATCH 3/8] Hoist the style parts of traversal.rs into style/. --- components/layout/parallel.rs | 5 +- components/layout/sequential.rs | 5 +- components/layout/traversal.rs | 254 +------------------------------ components/style/lib.rs | 1 + components/style/traversal.rs | 260 ++++++++++++++++++++++++++++++++ 5 files changed, 269 insertions(+), 256 deletions(-) create mode 100644 components/style/traversal.rs diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index 0aac7dc822e..3e63f4ae8bf 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -16,10 +16,9 @@ use profile_traits::time::{self, TimerMetadata, profile}; use std::mem; use std::sync::atomic::{AtomicIsize, Ordering}; use style::dom::{TNode, UnsafeNode}; -use traversal::PostorderNodeMutTraversal; +use style::traversal::DomTraversalContext; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes, BubbleISizes}; -use traversal::{BuildDisplayList, ComputeAbsolutePositions}; -use traversal::DomTraversalContext; +use traversal::{BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index bafd47872d4..98b8da8a9a9 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -12,10 +12,9 @@ use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtil use flow_ref::{self, FlowRef}; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; -use traversal::PostorderNodeMutTraversal; +use style::traversal::DomTraversalContext; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; -use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions}; -use traversal::DomTraversalContext; +use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; use wrapper::LayoutNode; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 05d4e6171b0..ac13a014c12 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -13,165 +13,15 @@ use flow::{self, Flow}; use gfx::display_list::OpaqueNode; use incremental::{BUBBLE_ISIZES, REFLOW, REFLOW_OUT_OF_FLOW, REPAINT, RestyleDamage}; use script::layout_interface::ReflowGoal; -use selectors::bloom::BloomFilter; -use std::cell::RefCell; use std::mem; -use std::rc::Rc; -use style::context::{LocalStyleContext, SharedStyleContext, StyleContext}; -use style::dom::{TNode, TRestyleDamage, UnsafeNode}; -use style::matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; +use style::context::StyleContext; +use style::matching::MatchMethods; +use style::traversal::{DomTraversalContext, STYLE_BLOOM}; +use style::traversal::{put_task_local_bloom_filter, recalc_style_at}; use util::opts; use util::tid::tid; use wrapper::{LayoutNode, ThreadSafeLayoutNode}; -/// Every time we do another layout, the old bloom filters are invalid. This is -/// detected by ticking a generation number every layout. -type Generation = u32; - -/// A pair of the bloom filter used for css selector matching, and the node to -/// which it applies. This is used to efficiently do `Descendant` selector -/// matches. Thanks to the bloom filter, we can avoid walking up the tree -/// looking for ancestors that aren't there in the majority of cases. -/// -/// As we walk down the DOM tree a task-local bloom filter is built of all the -/// CSS `SimpleSelector`s which are part of a `Descendant` compound selector -/// (i.e. paired with a `Descendant` combinator, in the `next` field of a -/// `CompoundSelector`. -/// -/// Before a `Descendant` selector match is tried, it's compared against the -/// bloom filter. If the bloom filter can exclude it, the selector is quickly -/// rejected. -/// -/// When done styling a node, all selectors previously inserted into the filter -/// are removed. -/// -/// Since a work-stealing queue is used for styling, sometimes, the bloom filter -/// will no longer be the for the parent of the node we're currently on. When -/// this happens, the task local bloom filter will be thrown away and rebuilt. -thread_local!( - static STYLE_BLOOM: RefCell, UnsafeNode, Generation)>> = RefCell::new(None)); - -/// Returns the task local bloom filter. -/// -/// If one does not exist, a new one will be made for you. If it is out of date, -/// it will be cleared and reused. -fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, - root: OpaqueNode, - context: &SharedStyleContext) - -> Box - where N: TNode<'ln> { - STYLE_BLOOM.with(|style_bloom| { - match (parent_node, style_bloom.borrow_mut().take()) { - // Root node. Needs new bloom filter. - (None, _ ) => { - debug!("[{}] No parent, but new bloom filter!", tid()); - box BloomFilter::new() - } - // No bloom filter for this thread yet. - (Some(parent), None) => { - let mut bloom_filter = box BloomFilter::new(); - insert_ancestors_into_bloom_filter(&mut bloom_filter, parent, root); - bloom_filter - } - // Found cached bloom filter. - (Some(parent), Some((mut bloom_filter, old_node, old_generation))) => { - if old_node == parent.to_unsafe() && - old_generation == context.generation { - // Hey, the cached parent is our parent! We can reuse the bloom filter. - debug!("[{}] Parent matches (={}). Reusing bloom filter.", tid(), old_node.0); - } else { - // Oh no. the cached parent is stale. I guess we need a new one. Reuse the existing - // allocation to avoid malloc churn. - bloom_filter.clear(); - insert_ancestors_into_bloom_filter(&mut bloom_filter, parent, root); - } - bloom_filter - }, - } - }) -} - -fn put_task_local_bloom_filter(bf: Box, - unsafe_node: &UnsafeNode, - context: &SharedStyleContext) { - STYLE_BLOOM.with(move |style_bloom| { - assert!(style_bloom.borrow().is_none(), - "Putting into a never-taken task-local bloom filter"); - *style_bloom.borrow_mut() = Some((bf, *unsafe_node, context.generation)); - }) -} - -/// "Ancestors" in this context is inclusive of ourselves. -fn insert_ancestors_into_bloom_filter<'ln, N>(bf: &mut Box, - mut n: N, - root: OpaqueNode) - where N: TNode<'ln> { - debug!("[{}] Inserting ancestors.", tid()); - let mut ancestors = 0; - loop { - ancestors += 1; - - n.insert_into_bloom_filter(&mut **bf); - n = match n.layout_parent_node(root) { - None => break, - Some(p) => p, - }; - } - debug!("[{}] Inserted {} ancestors.", tid(), ancestors); -} - -pub trait DomTraversalContext<'ln, N: TNode<'ln>> { - type SharedContext: Sync + 'static; - fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self; - fn process_preorder(&self, node: N); - fn process_postorder(&self, node: N); -} - -/// FIXME(bholley): I added this now to demonstrate the usefulness of the new design. -/// This is currently unused, but will be used shortly. - -#[allow(dead_code)] -pub struct StandaloneStyleContext<'a> { - pub shared: &'a SharedStyleContext, - cached_local_style_context: Rc, -} - -impl<'a> StandaloneStyleContext<'a> { - pub fn new(_: &'a SharedStyleContext) -> Self { panic!("Not implemented") } -} - -impl<'a> StyleContext<'a> for StandaloneStyleContext<'a> { - fn shared_context(&self) -> &'a SharedStyleContext { - &self.shared - } - - fn local_context(&self) -> &LocalStyleContext { - &self.cached_local_style_context - } -} - -#[allow(dead_code)] -pub struct RecalcStyleOnly<'lc> { - context: StandaloneStyleContext<'lc>, - root: OpaqueNode, -} - -impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> { - type SharedContext = SharedStyleContext; - fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { - // See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is - // necessary. - let shared_lc: &'lc SharedStyleContext = unsafe { mem::transmute(shared) }; - RecalcStyleOnly { - context: StandaloneStyleContext::new(shared_lc), - root: root, - } - } - - fn process_preorder(&self, node: N) { recalc_style_at(&self.context, self.root, node); } - fn process_postorder(&self, _: N) {} -} - pub struct RecalcStyleAndConstructFlows<'lc> { context: LayoutContext<'lc>, root: OpaqueNode, @@ -220,102 +70,6 @@ pub trait PostorderNodeMutTraversal<'ln, ConcreteThreadSafeLayoutNode: ThreadSaf fn process(&mut self, node: &ConcreteThreadSafeLayoutNode) -> bool; } -/// The recalc-style-for-node traversal, which styles each node and must run before -/// layout computation. This computes the styles applied to each node. -#[inline] -#[allow(unsafe_code)] -fn recalc_style_at<'a, 'ln, N: TNode<'ln>, C: StyleContext<'a>> (context: &'a C, root: OpaqueNode, node: N) { - // Initialize layout data. - // - // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML - // parser. - node.initialize_data(); - - // Get the parent node. - let parent_opt = node.layout_parent_node(root); - - // Get the style bloom filter. - let mut bf = take_task_local_bloom_filter(parent_opt, root, context.shared_context()); - - let nonincremental_layout = opts::get().nonincremental_layout; - if nonincremental_layout || node.is_dirty() { - // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), - // to force non-incremental reflow. - if node.has_changed() { - node.unstyle(); - } - - // Check to see whether we can share a style with someone. - let style_sharing_candidate_cache = - &mut context.local_context().style_sharing_candidate_cache.borrow_mut(); - - let sharing_result = match node.as_element() { - Some(element) => { - unsafe { - element.share_style_if_possible(style_sharing_candidate_cache, - parent_opt.clone()) - } - }, - None => StyleSharingResult::CannotShare, - }; - - // Otherwise, match and cascade selectors. - match sharing_result { - StyleSharingResult::CannotShare => { - let mut applicable_declarations = ApplicableDeclarations::new(); - - let shareable_element = match node.as_element() { - Some(element) => { - // Perform the CSS selector matching. - let stylist = unsafe { &*context.shared_context().stylist.0 }; - if element.match_element(stylist, - Some(&*bf), - &mut applicable_declarations) { - Some(element) - } else { - None - } - }, - None => { - if node.has_changed() { - node.set_restyle_damage(N::ConcreteRestyleDamage::rebuild_and_reflow()) - } - None - }, - }; - - // Perform the CSS cascade. - unsafe { - node.cascade_node(&context.shared_context(), - parent_opt, - &applicable_declarations, - &mut context.local_context().applicable_declarations_cache.borrow_mut(), - &context.shared_context().new_animations_sender); - } - - // Add ourselves to the LRU cache. - if let Some(element) = shareable_element { - style_sharing_candidate_cache.insert_if_possible(&element); - } - } - StyleSharingResult::StyleWasShared(index, damage) => { - style_sharing_candidate_cache.touch(index); - node.set_restyle_damage(damage); - } - } - } - - let unsafe_layout_node = node.to_unsafe(); - - // Before running the children, we need to insert our nodes into the bloom - // filter. - debug!("[{}] + {:X}", tid(), unsafe_layout_node.0); - node.insert_into_bloom_filter(&mut *bf); - - // NB: flow construction updates the bloom filter on the way up. - put_task_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); -} - /// The flow construction traversal, which builds flows for styled nodes. #[inline] #[allow(unsafe_code)] diff --git a/components/style/lib.rs b/components/style/lib.rs index e010a92f0b2..18c43638cfb 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -57,6 +57,7 @@ pub mod parser; pub mod restyle_hints; pub mod selector_matching; pub mod stylesheets; +pub mod traversal; #[macro_use] #[allow(non_camel_case_types)] pub mod values; diff --git a/components/style/traversal.rs b/components/style/traversal.rs new file mode 100644 index 00000000000..48bb8ce047b --- /dev/null +++ b/components/style/traversal.rs @@ -0,0 +1,260 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#![allow(unsafe_code)] + +use context::{LocalStyleContext, SharedStyleContext, StyleContext}; +use dom::{OpaqueNode, TNode, TRestyleDamage, UnsafeNode}; +use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; +use selectors::bloom::BloomFilter; +use std::cell::RefCell; +use std::mem; +use std::rc::Rc; +use util::opts; +use util::tid::tid; + +/// Every time we do another layout, the old bloom filters are invalid. This is +/// detected by ticking a generation number every layout. +pub type Generation = u32; + +/// A pair of the bloom filter used for css selector matching, and the node to +/// which it applies. This is used to efficiently do `Descendant` selector +/// matches. Thanks to the bloom filter, we can avoid walking up the tree +/// looking for ancestors that aren't there in the majority of cases. +/// +/// As we walk down the DOM tree a task-local bloom filter is built of all the +/// CSS `SimpleSelector`s which are part of a `Descendant` compound selector +/// (i.e. paired with a `Descendant` combinator, in the `next` field of a +/// `CompoundSelector`. +/// +/// Before a `Descendant` selector match is tried, it's compared against the +/// bloom filter. If the bloom filter can exclude it, the selector is quickly +/// rejected. +/// +/// When done styling a node, all selectors previously inserted into the filter +/// are removed. +/// +/// Since a work-stealing queue is used for styling, sometimes, the bloom filter +/// will no longer be the for the parent of the node we're currently on. When +/// this happens, the task local bloom filter will be thrown away and rebuilt. +thread_local!( + pub static STYLE_BLOOM: RefCell, UnsafeNode, Generation)>> = RefCell::new(None)); + +/// Returns the task local bloom filter. +/// +/// If one does not exist, a new one will be made for you. If it is out of date, +/// it will be cleared and reused. +fn take_task_local_bloom_filter<'ln, N>(parent_node: Option, + root: OpaqueNode, + context: &SharedStyleContext) + -> Box + where N: TNode<'ln> { + STYLE_BLOOM.with(|style_bloom| { + match (parent_node, style_bloom.borrow_mut().take()) { + // Root node. Needs new bloom filter. + (None, _ ) => { + debug!("[{}] No parent, but new bloom filter!", tid()); + box BloomFilter::new() + } + // No bloom filter for this thread yet. + (Some(parent), None) => { + let mut bloom_filter = box BloomFilter::new(); + insert_ancestors_into_bloom_filter(&mut bloom_filter, parent, root); + bloom_filter + } + // Found cached bloom filter. + (Some(parent), Some((mut bloom_filter, old_node, old_generation))) => { + if old_node == parent.to_unsafe() && + old_generation == context.generation { + // Hey, the cached parent is our parent! We can reuse the bloom filter. + debug!("[{}] Parent matches (={}). Reusing bloom filter.", tid(), old_node.0); + } else { + // Oh no. the cached parent is stale. I guess we need a new one. Reuse the existing + // allocation to avoid malloc churn. + bloom_filter.clear(); + insert_ancestors_into_bloom_filter(&mut bloom_filter, parent, root); + } + bloom_filter + }, + } + }) +} + +pub fn put_task_local_bloom_filter(bf: Box, + unsafe_node: &UnsafeNode, + context: &SharedStyleContext) { + STYLE_BLOOM.with(move |style_bloom| { + assert!(style_bloom.borrow().is_none(), + "Putting into a never-taken task-local bloom filter"); + *style_bloom.borrow_mut() = Some((bf, *unsafe_node, context.generation)); + }) +} + +/// "Ancestors" in this context is inclusive of ourselves. +fn insert_ancestors_into_bloom_filter<'ln, N>(bf: &mut Box, + mut n: N, + root: OpaqueNode) + where N: TNode<'ln> { + debug!("[{}] Inserting ancestors.", tid()); + let mut ancestors = 0; + loop { + ancestors += 1; + + n.insert_into_bloom_filter(&mut **bf); + n = match n.layout_parent_node(root) { + None => break, + Some(p) => p, + }; + } + debug!("[{}] Inserted {} ancestors.", tid(), ancestors); +} + +pub trait DomTraversalContext<'ln, N: TNode<'ln>> { + type SharedContext: Sync + 'static; + fn new<'a>(&'a Self::SharedContext, OpaqueNode) -> Self; + fn process_preorder(&self, node: N); + fn process_postorder(&self, node: N); +} + +/// FIXME(bholley): I added this now to demonstrate the usefulness of the new design. +/// This is currently unused, but will be used shortly. + +#[allow(dead_code)] +pub struct StandaloneStyleContext<'a> { + pub shared: &'a SharedStyleContext, + cached_local_style_context: Rc, +} + +impl<'a> StandaloneStyleContext<'a> { + pub fn new(_: &'a SharedStyleContext) -> Self { panic!("Not implemented") } +} + +impl<'a> StyleContext<'a> for StandaloneStyleContext<'a> { + fn shared_context(&self) -> &'a SharedStyleContext { + &self.shared + } + + fn local_context(&self) -> &LocalStyleContext { + &self.cached_local_style_context + } +} + +#[allow(dead_code)] +pub struct RecalcStyleOnly<'lc> { + context: StandaloneStyleContext<'lc>, + root: OpaqueNode, +} + +impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> { + type SharedContext = SharedStyleContext; + fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { + // See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is + // necessary. + let shared_lc: &'lc SharedStyleContext = unsafe { mem::transmute(shared) }; + RecalcStyleOnly { + context: StandaloneStyleContext::new(shared_lc), + root: root, + } + } + + fn process_preorder(&self, node: N) { recalc_style_at(&self.context, self.root, node); } + fn process_postorder(&self, _: N) {} +} + +/// The recalc-style-for-node traversal, which styles each node and must run before +/// layout computation. This computes the styles applied to each node. +#[inline] +#[allow(unsafe_code)] +pub fn recalc_style_at<'a, 'ln, N: TNode<'ln>, C: StyleContext<'a>> (context: &'a C, root: OpaqueNode, node: N) { + // Initialize layout data. + // + // FIXME(pcwalton): Stop allocating here. Ideally this should just be done by the HTML + // parser. + node.initialize_data(); + + // Get the parent node. + let parent_opt = node.layout_parent_node(root); + + // Get the style bloom filter. + let mut bf = take_task_local_bloom_filter(parent_opt, root, context.shared_context()); + + let nonincremental_layout = opts::get().nonincremental_layout; + if nonincremental_layout || node.is_dirty() { + // Remove existing CSS styles from nodes whose content has changed (e.g. text changed), + // to force non-incremental reflow. + if node.has_changed() { + node.unstyle(); + } + + // Check to see whether we can share a style with someone. + let style_sharing_candidate_cache = + &mut context.local_context().style_sharing_candidate_cache.borrow_mut(); + + let sharing_result = match node.as_element() { + Some(element) => { + unsafe { + element.share_style_if_possible(style_sharing_candidate_cache, + parent_opt.clone()) + } + }, + None => StyleSharingResult::CannotShare, + }; + + // Otherwise, match and cascade selectors. + match sharing_result { + StyleSharingResult::CannotShare => { + let mut applicable_declarations = ApplicableDeclarations::new(); + + let shareable_element = match node.as_element() { + Some(element) => { + // Perform the CSS selector matching. + let stylist = unsafe { &*context.shared_context().stylist.0 }; + if element.match_element(stylist, + Some(&*bf), + &mut applicable_declarations) { + Some(element) + } else { + None + } + }, + None => { + if node.has_changed() { + node.set_restyle_damage(N::ConcreteRestyleDamage::rebuild_and_reflow()) + } + None + }, + }; + + // Perform the CSS cascade. + unsafe { + node.cascade_node(&context.shared_context(), + parent_opt, + &applicable_declarations, + &mut context.local_context().applicable_declarations_cache.borrow_mut(), + &context.shared_context().new_animations_sender); + } + + // Add ourselves to the LRU cache. + if let Some(element) = shareable_element { + style_sharing_candidate_cache.insert_if_possible(&element); + } + } + StyleSharingResult::StyleWasShared(index, damage) => { + style_sharing_candidate_cache.touch(index); + node.set_restyle_damage(damage); + } + } + } + + let unsafe_layout_node = node.to_unsafe(); + + // Before running the children, we need to insert our nodes into the bloom + // filter. + debug!("[{}] + {:X}", tid(), unsafe_layout_node.0); + node.insert_into_bloom_filter(&mut *bf); + + // NB: flow construction updates the bloom filter on the way up. + put_task_local_bloom_filter(bf, &unsafe_layout_node, context.shared_context()); +} + From 60b3c66b28b5eaeef43f58945336fcc4b11f6c85 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 6 Jan 2016 19:02:25 -0800 Subject: [PATCH 4/8] Hoist the style parts of parallel.rs into style/. --- components/layout/layout_task.rs | 3 +- components/layout/parallel.rs | 132 +--------------------------- components/style/lib.rs | 1 + components/style/parallel.rs | 142 +++++++++++++++++++++++++++++++ 4 files changed, 148 insertions(+), 130 deletions(-) create mode 100644 components/style/parallel.rs diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 0015cd3fc33..1eac2e78fc1 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -38,7 +38,7 @@ use msg::ParseErrorReporter; use msg::compositor_msg::Epoch; use msg::constellation_msg::{ConstellationChan, Failure, PipelineId}; use net_traits::image_cache_task::{ImageCacheChan, ImageCacheResult, ImageCacheTask}; -use parallel::{self, WorkQueueData}; +use parallel; use profile_traits::mem::{self, Report, ReportKind, ReportsChan}; use profile_traits::time::{TimerMetadataFrameType, TimerMetadataReflowType}; use profile_traits::time::{self, TimerMetadata, profile}; @@ -67,6 +67,7 @@ use style::computed_values::{filter, mix_blend_mode}; use style::context::{SharedStyleContext, StylistWrapper}; use style::dom::{TDocument, TElement, TNode}; use style::media_queries::{Device, MediaType}; +use style::parallel::WorkQueueData; use style::selector_matching::{Stylist, USER_OR_USER_AGENT_STYLESHEETS}; use style::stylesheets::{CSSRuleIteratorExt, Stylesheet}; use traversal::RecalcStyleAndConstructFlows; diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index 3e63f4ae8bf..f5f2eb30856 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -11,26 +11,23 @@ use context::{LayoutContext, SharedLayoutContext}; use flow::{self, Flow, MutableFlowUtils, PostorderFlowTraversal, PreorderFlowTraversal}; use flow_ref::{self, FlowRef}; -use gfx::display_list::OpaqueNode; use profile_traits::time::{self, TimerMetadata, profile}; use std::mem; use std::sync::atomic::{AtomicIsize, Ordering}; use style::dom::{TNode, UnsafeNode}; -use style::traversal::DomTraversalContext; +use style::parallel::{CHUNK_SIZE, WorkQueueData}; +use style::parallel::{run_queue_with_custom_work_data_type}; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes, BubbleISizes}; use traversal::{BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; -const CHUNK_SIZE: usize = 64; - -pub struct WorkQueueData(usize, usize); +pub use style::parallel::traverse_dom_preorder; #[allow(dead_code)] fn static_assertion(node: UnsafeNode) { unsafe { let _: UnsafeFlow = ::std::intrinsics::transmute(node); - let _: UnsafeNodeList = ::std::intrinsics::transmute(node); } } @@ -53,18 +50,8 @@ pub fn borrowed_flow_to_unsafe_flow(flow: &Flow) -> UnsafeFlow { } } -pub type UnsafeNodeList = (Box>, OpaqueNode); - pub type UnsafeFlowList = (Box>, usize); -pub type ChunkedDomTraversalFunction = - extern "Rust" fn(UnsafeNodeList, - &mut WorkerProxy); - -pub type DomTraversalFunction = - extern "Rust" fn(OpaqueNode, UnsafeNode, - &mut WorkerProxy); - pub type ChunkedFlowTraversalFunction = extern "Rust" fn(UnsafeFlowList, &mut WorkerProxy); @@ -230,94 +217,6 @@ impl<'a> ParallelPreorderFlowTraversal for ComputeAbsolutePositions<'a> { impl<'a> ParallelPostorderFlowTraversal for BuildDisplayList<'a> {} -/// A parallel top-down DOM traversal. -#[inline(always)] -fn top_down_dom<'ln, N, C>(unsafe_nodes: UnsafeNodeList, - proxy: &mut WorkerProxy) - where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { - let context = C::new(proxy.user_data(), unsafe_nodes.1); - - let mut discovered_child_nodes = Vec::new(); - for unsafe_node in *unsafe_nodes.0 { - // Get a real layout node. - let node = unsafe { N::from_unsafe(&unsafe_node) }; - - // Perform the appropriate traversal. - context.process_preorder(node); - - let child_count = node.children_count(); - - // Reset the count of children. - { - let data = node.mutate_data().unwrap(); - data.parallel.children_count.store(child_count as isize, - 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. - bottom_up_dom::(unsafe_nodes.1, unsafe_node, proxy) - } - } - - for chunk in discovered_child_nodes.chunks(CHUNK_SIZE) { - proxy.push(WorkUnit { - fun: top_down_dom::, - data: (box chunk.iter().cloned().collect(), unsafe_nodes.1), - }); - } -} - -/// Process current node and potentially traverse its ancestors. -/// -/// If we are the last child that finished processing, recursively process -/// our parent. Else, stop. Also, stop at the root. -/// -/// Thus, if we start with all the leaves of a tree, we end up traversing -/// the whole tree bottom-up because each parent will be processed exactly -/// once (by the last child that finishes processing). -/// -/// The only communication between siblings is that they both -/// fetch-and-subtract the parent's children count. -fn bottom_up_dom<'ln, N, C>(root: OpaqueNode, - unsafe_node: UnsafeNode, - proxy: &mut WorkerProxy) - where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { - let context = C::new(proxy.user_data(), root); - - // Get a real layout node. - let mut node = unsafe { N::from_unsafe(&unsafe_node) }; - loop { - // Perform the appropriate operation. - context.process_postorder(node); - - let parent = match node.layout_parent_node(root) { - None => break, - Some(parent) => parent, - }; - - let parent_data = unsafe { - &*parent.borrow_data_unchecked().unwrap() - }; - - if parent_data - .parallel - .children_count - .fetch_sub(1, Ordering::Relaxed) != 1 { - // Get out of here and find another node to work on. - break - } - - // We were the last child of our parent. Construct flows for our parent. - node = parent; - } -} - fn assign_inline_sizes(unsafe_flows: UnsafeFlowList, proxy: &mut WorkerProxy) { let shared_layout_context = proxy.user_data(); @@ -360,31 +259,6 @@ fn build_display_list(unsafe_flow: UnsafeFlow, build_display_list_traversal.run_parallel(unsafe_flow); } -fn run_queue_with_custom_work_data_type( - queue: &mut WorkQueue, - callback: F, - shared: &SharedContext) - where To: 'static + Send, F: FnOnce(&mut WorkQueue) { - let queue: &mut WorkQueue = unsafe { - mem::transmute(queue) - }; - callback(queue); - queue.run(shared); -} - -pub fn traverse_dom_preorder<'ln, N, C>( - root: N, - queue_data: &C::SharedContext, - queue: &mut WorkQueue) - where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { - run_queue_with_custom_work_data_type(queue, |queue| { - queue.push(WorkUnit { - fun: top_down_dom::, - data: (box vec![root.to_unsafe()], root.opaque()), - }); - }, queue_data); -} - pub fn traverse_flow_tree_preorder( root: &mut FlowRef, profiler_metadata: Option, diff --git a/components/style/lib.rs b/components/style/lib.rs index 18c43638cfb..5623c5bd8ed 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -53,6 +53,7 @@ pub mod dom; pub mod font_face; pub mod matching; pub mod media_queries; +pub mod parallel; pub mod parser; pub mod restyle_hints; pub mod selector_matching; diff --git a/components/style/parallel.rs b/components/style/parallel.rs new file mode 100644 index 00000000000..de0ae680e3a --- /dev/null +++ b/components/style/parallel.rs @@ -0,0 +1,142 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! Implements parallel traversal over the DOM tree. +//! +//! This code is highly unsafe. Keep this file small and easy to audit. + +#![allow(unsafe_code)] + +use dom::{OpaqueNode, TNode, UnsafeNode}; +use std::mem; +use std::sync::atomic::Ordering; +use traversal::DomTraversalContext; +use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; + +#[allow(dead_code)] +fn static_assertion(node: UnsafeNode) { + unsafe { + let _: UnsafeNodeList = ::std::intrinsics::transmute(node); + } +} + +pub type UnsafeNodeList = (Box>, OpaqueNode); + +pub const CHUNK_SIZE: usize = 64; + +pub struct WorkQueueData(usize, usize); + +pub fn run_queue_with_custom_work_data_type( + queue: &mut WorkQueue, + callback: F, + shared: &SharedContext) + where To: 'static + Send, F: FnOnce(&mut WorkQueue) { + let queue: &mut WorkQueue = unsafe { + mem::transmute(queue) + }; + callback(queue); + queue.run(shared); +} + +pub fn traverse_dom_preorder<'ln, N, C>( + root: N, + queue_data: &C::SharedContext, + queue: &mut WorkQueue) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + run_queue_with_custom_work_data_type(queue, |queue| { + queue.push(WorkUnit { + fun: top_down_dom::, + data: (box vec![root.to_unsafe()], root.opaque()), + }); + }, queue_data); +} + +/// A parallel top-down DOM traversal. +#[inline(always)] +fn top_down_dom<'ln, N, C>(unsafe_nodes: UnsafeNodeList, + proxy: &mut WorkerProxy) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + let context = C::new(proxy.user_data(), unsafe_nodes.1); + + let mut discovered_child_nodes = Vec::new(); + for unsafe_node in *unsafe_nodes.0 { + // Get a real layout node. + let node = unsafe { N::from_unsafe(&unsafe_node) }; + + // Perform the appropriate traversal. + context.process_preorder(node); + + let child_count = node.children_count(); + + // Reset the count of children. + { + let data = node.mutate_data().unwrap(); + data.parallel.children_count.store(child_count as isize, + 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. + bottom_up_dom::(unsafe_nodes.1, unsafe_node, proxy) + } + } + + for chunk in discovered_child_nodes.chunks(CHUNK_SIZE) { + proxy.push(WorkUnit { + fun: top_down_dom::, + data: (box chunk.iter().cloned().collect(), unsafe_nodes.1), + }); + } +} + +/// Process current node and potentially traverse its ancestors. +/// +/// If we are the last child that finished processing, recursively process +/// our parent. Else, stop. Also, stop at the root. +/// +/// Thus, if we start with all the leaves of a tree, we end up traversing +/// the whole tree bottom-up because each parent will be processed exactly +/// once (by the last child that finishes processing). +/// +/// The only communication between siblings is that they both +/// fetch-and-subtract the parent's children count. +fn bottom_up_dom<'ln, N, C>(root: OpaqueNode, + unsafe_node: UnsafeNode, + proxy: &mut WorkerProxy) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + let context = C::new(proxy.user_data(), root); + + // Get a real layout node. + let mut node = unsafe { N::from_unsafe(&unsafe_node) }; + loop { + // Perform the appropriate operation. + context.process_postorder(node); + + let parent = match node.layout_parent_node(root) { + None => break, + Some(parent) => parent, + }; + + let parent_data = unsafe { + &*parent.borrow_data_unchecked().unwrap() + }; + + if parent_data + .parallel + .children_count + .fetch_sub(1, Ordering::Relaxed) != 1 { + // Get out of here and find another node to work on. + break + } + + // We were the last child of our parent. Construct flows for our parent. + node = parent; + } +} + From 57d2a0b0dbbe0592f0295a217e08d9d6e2ffaac3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 6 Jan 2016 19:08:03 -0800 Subject: [PATCH 5/8] Make sequential traversal operate on TNode instead of LayoutNode. I forgot to do this along with the parallel case. Ideally I'd merge this patch into that one, but then I'd need to rebase it over the LayoutContext patch, which would be a pain. --- components/layout/sequential.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index 98b8da8a9a9..ae236008121 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -12,18 +12,18 @@ use flow::{self, Flow, ImmutableFlowUtils, InorderFlowTraversal, MutableFlowUtil use flow_ref::{self, FlowRef}; use fragment::FragmentBorderBoxIterator; use generated_content::ResolveGeneratedContent; +use style::dom::TNode; use style::traversal::DomTraversalContext; use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; -use wrapper::LayoutNode; pub fn traverse_dom_preorder<'ln, N, C>(root: N, shared: &C::SharedContext) - where N: LayoutNode<'ln>, + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { fn doit<'a, 'ln, N, C>(context: &'a C, node: N) - where N: LayoutNode<'ln>, C: DomTraversalContext<'ln, N> { + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { context.process_preorder(node); for kid in node.children() { From 136c0938a25356578ac2e5d9ac66c477f7d579dd Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 6 Jan 2016 19:15:00 -0800 Subject: [PATCH 6/8] Hoist the style parts of sequential.rs into style/. --- components/layout/sequential.rs | 19 +------------------ components/style/lib.rs | 1 + components/style/sequential.rs | 28 ++++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 18 deletions(-) create mode 100644 components/style/sequential.rs diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index ae236008121..5fc123c30c7 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -18,24 +18,7 @@ use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; -pub fn traverse_dom_preorder<'ln, N, C>(root: N, - shared: &C::SharedContext) - where N: TNode<'ln>, - C: DomTraversalContext<'ln, N> { - fn doit<'a, 'ln, N, C>(context: &'a C, node: N) - where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { - context.process_preorder(node); - - for kid in node.children() { - doit::(context, kid); - } - - context.process_postorder(node); - } - - let context = C::new(shared, root.opaque()); - doit::(&context, root); -} +pub use style::sequential::traverse_dom_preorder; pub fn resolve_generated_content(root: &mut FlowRef, shared_layout_context: &SharedLayoutContext) { fn doit(flow: &mut Flow, level: u32, traversal: &mut ResolveGeneratedContent) { diff --git a/components/style/lib.rs b/components/style/lib.rs index 5623c5bd8ed..0f0dfca30cf 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -57,6 +57,7 @@ pub mod parallel; pub mod parser; pub mod restyle_hints; pub mod selector_matching; +pub mod sequential; pub mod stylesheets; pub mod traversal; #[macro_use] diff --git a/components/style/sequential.rs b/components/style/sequential.rs new file mode 100644 index 00000000000..b98d6e45029 --- /dev/null +++ b/components/style/sequential.rs @@ -0,0 +1,28 @@ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ + +//! Implements sequential traversal over the DOM tree. + +use dom::TNode; +use traversal::DomTraversalContext; + +pub fn traverse_dom_preorder<'ln, N, C>(root: N, + shared: &C::SharedContext) + where N: TNode<'ln>, + C: DomTraversalContext<'ln, N> { + fn doit<'a, 'ln, N, C>(context: &'a C, node: N) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { + context.process_preorder(node); + + for kid in node.children() { + doit::(context, kid); + } + + context.process_postorder(node); + } + + let context = C::new(shared, root.opaque()); + doit::(&context, root); +} + From 29987a67156107bcd784743d28901a178317a4b3 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 6 Jan 2016 19:18:12 -0800 Subject: [PATCH 7/8] Bonus Fix - Rename traverse_dom_preorder to traverse_dom. The incorrect naming here was bugging me - the dom traversal has both pre- and post-order processing steps. --- components/layout/layout_task.rs | 4 ++-- components/layout/parallel.rs | 2 +- components/layout/sequential.rs | 2 +- components/style/parallel.rs | 9 ++++----- components/style/sequential.rs | 8 ++++---- 5 files changed, 12 insertions(+), 13 deletions(-) diff --git a/components/layout/layout_task.rs b/components/layout/layout_task.rs index 1eac2e78fc1..7c39d61db22 100644 --- a/components/layout/layout_task.rs +++ b/components/layout/layout_task.rs @@ -1026,11 +1026,11 @@ impl LayoutTask { // Perform CSS selector matching and flow construction. match self.parallel_traversal { None => { - sequential::traverse_dom_preorder::( + sequential::traverse_dom::( node, &shared_layout_context); } Some(ref mut traversal) => { - parallel::traverse_dom_preorder::( + parallel::traverse_dom::( node, &shared_layout_context, traversal); } } diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index f5f2eb30856..4ea427b11ee 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -22,7 +22,7 @@ use traversal::{BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTrav use util::opts; use util::workqueue::{WorkQueue, WorkUnit, WorkerProxy}; -pub use style::parallel::traverse_dom_preorder; +pub use style::parallel::traverse_dom; #[allow(dead_code)] fn static_assertion(node: UnsafeNode) { diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index 5fc123c30c7..ad332f30fdd 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -18,7 +18,7 @@ use traversal::{AssignBSizesAndStoreOverflow, AssignISizes}; use traversal::{BubbleISizes, BuildDisplayList, ComputeAbsolutePositions, PostorderNodeMutTraversal}; use util::opts; -pub use style::sequential::traverse_dom_preorder; +pub use style::sequential::traverse_dom; pub fn resolve_generated_content(root: &mut FlowRef, shared_layout_context: &SharedLayoutContext) { fn doit(flow: &mut Flow, level: u32, traversal: &mut ResolveGeneratedContent) { diff --git a/components/style/parallel.rs b/components/style/parallel.rs index de0ae680e3a..cb8e8260a21 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -39,11 +39,10 @@ pub fn run_queue_with_custom_work_data_type( queue.run(shared); } -pub fn traverse_dom_preorder<'ln, N, C>( - root: N, - queue_data: &C::SharedContext, - queue: &mut WorkQueue) - where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { +pub fn traverse_dom<'ln, N, C>(root: N, + queue_data: &C::SharedContext, + queue: &mut WorkQueue) + where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { run_queue_with_custom_work_data_type(queue, |queue| { queue.push(WorkUnit { fun: top_down_dom::, diff --git a/components/style/sequential.rs b/components/style/sequential.rs index b98d6e45029..9dfbb0126c1 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -7,10 +7,10 @@ use dom::TNode; use traversal::DomTraversalContext; -pub fn traverse_dom_preorder<'ln, N, C>(root: N, - shared: &C::SharedContext) - where N: TNode<'ln>, - C: DomTraversalContext<'ln, N> { +pub fn traverse_dom<'ln, N, C>(root: N, + shared: &C::SharedContext) + where N: TNode<'ln>, + C: DomTraversalContext<'ln, N> { fn doit<'a, 'ln, N, C>(context: &'a C, node: N) where N: TNode<'ln>, C: DomTraversalContext<'ln, N> { context.process_preorder(node); From 3b33174163ad3c1b8cb1855c0713a9e2fdee2a40 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Thu, 7 Jan 2016 11:17:31 -0800 Subject: [PATCH 8/8] Review nits: Narrowly scoping unsafety, and expanding comment. --- components/layout/traversal.rs | 11 ++++++++--- components/style/traversal.rs | 3 +-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index ac13a014c12..c42ba491732 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 construct::FlowConstructor; use context::{LayoutContext, SharedLayoutContext}; use flow::{PostorderFlowTraversal, PreorderFlowTraversal}; @@ -29,6 +27,7 @@ pub struct RecalcStyleAndConstructFlows<'lc> { impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAndConstructFlows<'lc> { type SharedContext = SharedLayoutContext; + #[allow(unsafe_code)] fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { // FIXME(bholley): This transmutation from &'a to &'lc is very unfortunate, but I haven't // found a way to avoid it despite spending several days on it (and consulting Manishearth, @@ -48,7 +47,13 @@ impl<'lc, 'ln, N: LayoutNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleAn // that would require higher-kinded types, which don't exist yet and probably aren't coming // for a while. // - // So we transmute. :-( + // So we transmute. :-( This is safe because the DomTravesalContext is stack-allocated on + // the worker thread while processing a WorkUnit, whereas the borrowed SharedContext is + // live for the entire duration of the restyle. This really could _almost_ compile: all + // we'd need to do is change the signature to to |new<'a: 'lc>|, and everything would + // work great. But we can't do that, because that would cause a mismatch with the signature + // in the trait we're implementing, and we can't mention 'lc in that trait at all for the + // reasons described above. // // [1] For example, the WorkQueue type needs to be parameterized on the concrete type of // DomTraversalContext::SharedContext, and the WorkQueue lifetime is similar to that of the diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 48bb8ce047b..c3307aa2593 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -2,8 +2,6 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -#![allow(unsafe_code)] - use context::{LocalStyleContext, SharedStyleContext, StyleContext}; use dom::{OpaqueNode, TNode, TRestyleDamage, UnsafeNode}; use matching::{ApplicableDeclarations, ElementMatchMethods, MatchMethods, StyleSharingResult}; @@ -148,6 +146,7 @@ pub struct RecalcStyleOnly<'lc> { impl<'lc, 'ln, N: TNode<'ln>> DomTraversalContext<'ln, N> for RecalcStyleOnly<'lc> { type SharedContext = SharedStyleContext; + #[allow(unsafe_code)] fn new<'a>(shared: &'a Self::SharedContext, root: OpaqueNode) -> Self { // See the comment in RecalcStyleAndConstructFlows::new for an explanation of why this is // necessary.