From 5c66815a2164358a3de5183a7e841df29ff64bde Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 16 Dec 2016 11:47:28 -0800 Subject: [PATCH 1/4] Upgrade to rayon 0.6. jpeg-decoder still needs updating. Until then we'll just have two copies. --- Cargo.lock | 20 ++++++++++++++++---- components/layout/Cargo.toml | 2 +- components/layout_thread/Cargo.toml | 2 +- components/style/Cargo.toml | 2 +- tests/unit/style/Cargo.toml | 2 +- 5 files changed, 20 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 790738373b6..d5a17c3e2ed 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1315,7 +1315,7 @@ dependencies = [ "plugins 0.0.1", "profile_traits 0.0.1", "range 0.0.1", - "rayon 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "script_layout_interface 0.0.1", "script_traits 0.0.1", "selectors 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -1361,7 +1361,7 @@ dependencies = [ "parking_lot 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", "plugins 0.0.1", "profile_traits 0.0.1", - "rayon 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "script 0.0.1", "script_layout_interface 0.0.1", "script_traits 0.0.1", @@ -2190,6 +2190,17 @@ dependencies = [ "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", ] +[[package]] +name = "rayon" +version = "0.6.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +dependencies = [ + "deque 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", + "libc 0.2.17 (registry+https://github.com/rust-lang/crates.io-index)", + "num_cpus 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", +] + [[package]] name = "ref_filter_map" version = "1.0.1" @@ -2744,7 +2755,7 @@ dependencies = [ "plugins 0.0.1", "quickersort 2.2.0 (registry+https://github.com/rust-lang/crates.io-index)", "rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)", - "rayon 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.76 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "selectors 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", @@ -2771,7 +2782,7 @@ dependencies = [ "matches 0.1.2 (registry+https://github.com/rust-lang/crates.io-index)", "owning_ref 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)", "parking_lot 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)", - "rayon 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)", + "rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)", "rustc-serialize 0.3.19 (registry+https://github.com/rust-lang/crates.io-index)", "selectors 0.15.0 (registry+https://github.com/rust-lang/crates.io-index)", "servo_atoms 0.0.1", @@ -3481,6 +3492,7 @@ dependencies = [ "checksum quote 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)" = "ea1e0c9bc6bfb0a60d539aab6e338207c1a5456e62f5bd5375132cee119aa4b3" "checksum rand 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)" = "2791d88c6defac799c3f20d74f094ca33b9332612d9aef9078519c82e4fe04a5" "checksum rayon 0.5.0 (registry+https://github.com/rust-lang/crates.io-index)" = "3b6a6e05e0e6b703e9f2ad266eb63f3712e693a17a2702b95a23de14ce8defa9" +"checksum rayon 0.6.0 (registry+https://github.com/rust-lang/crates.io-index)" = "50c575b58c2b109e2fbc181820cbe177474f35610ff9e357dc75f6bac854ffbf" "checksum ref_filter_map 1.0.1 (registry+https://github.com/rust-lang/crates.io-index)" = "2b5ceb840e4009da4841ed22a15eb49f64fdd00a2138945c5beacf506b2fb5ed" "checksum ref_slice 1.1.0 (registry+https://github.com/rust-lang/crates.io-index)" = "546bb4aa91c85f232732cc5b3c8097ea97ae9a77304f9ab4df8b203ff7672dad" "checksum regex 0.1.76 (registry+https://github.com/rust-lang/crates.io-index)" = "63b49f873f36ddc838d773972511e5fed2ef7350885af07d58e2f48ce8073dcd" diff --git a/components/layout/Cargo.toml b/components/layout/Cargo.toml index 9196f559afa..d3da21bf1a6 100644 --- a/components/layout/Cargo.toml +++ b/components/layout/Cargo.toml @@ -31,7 +31,7 @@ parking_lot = "0.3.3" plugins = {path = "../plugins"} profile_traits = {path = "../profile_traits"} range = {path = "../range"} -rayon = "0.5" +rayon = "0.6" script_layout_interface = {path = "../script_layout_interface"} script_traits = {path = "../script_traits"} selectors = "0.15" diff --git a/components/layout_thread/Cargo.toml b/components/layout_thread/Cargo.toml index b145d71ea44..d99a52061fb 100644 --- a/components/layout_thread/Cargo.toml +++ b/components/layout_thread/Cargo.toml @@ -27,7 +27,7 @@ net_traits = {path = "../net_traits"} parking_lot = {version = "0.3.3", features = ["nightly"]} plugins = {path = "../plugins"} profile_traits = {path = "../profile_traits"} -rayon = "0.5" +rayon = "0.6" script = {path = "../script"} script_layout_interface = {path = "../script_layout_interface"} script_traits = {path = "../script_traits"} diff --git a/components/style/Cargo.toml b/components/style/Cargo.toml index d2c9dbb32af..32f359fa407 100644 --- a/components/style/Cargo.toml +++ b/components/style/Cargo.toml @@ -44,7 +44,7 @@ parking_lot = "0.3.3" phf = "0.7.20" quickersort = "2.0.0" rand = "0.3" -rayon = "0.5" +rayon = "0.6" rustc-serialize = "0.3" selectors = "0.15" serde = {version = "0.8", optional = true} diff --git a/tests/unit/style/Cargo.toml b/tests/unit/style/Cargo.toml index 27962330e9b..69e5c3e9991 100644 --- a/tests/unit/style/Cargo.toml +++ b/tests/unit/style/Cargo.toml @@ -20,7 +20,7 @@ html5ever-atoms = "0.1" matches = "0.1" owning_ref = "0.2.2" parking_lot = "0.3" -rayon = "0.5" +rayon = "0.6" rustc-serialize = "0.3" selectors = "0.15" servo_atoms = {path = "../../../components/atoms"} From 8f7f62f8104bf576c38f7cfa1fedfaf2c51c0049 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Wed, 21 Dec 2016 11:09:52 -0800 Subject: [PATCH 2/4] Allow duplicate versions of rayon until https://github.com/kaksmet/jpeg-decoder/pull/60 lands. --- servo-tidy.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/servo-tidy.toml b/servo-tidy.toml index 755d0282b32..545f97acddd 100644 --- a/servo-tidy.toml +++ b/servo-tidy.toml @@ -11,7 +11,7 @@ lint-scripts = [ [ignore] # Ignored packages with duplicated versions -packages = ["bitflags", "lazy_static", "semver"] +packages = ["bitflags", "lazy_static", "semver", "rayon"] # Files that are ignored for all tidy and lint checks. files = [ # Generated and upstream code combined with our own. Could use cleanup From c5f01fe3b89c2a381fb891e9728fa2951b87a747 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 16 Dec 2016 11:50:11 -0800 Subject: [PATCH 3/4] Introduce and use Scoped TLS. It turns out that it's problematic to embed ThreadLocalStyleContext within LayoutContext, because parameterizing the former on TElement (which we do in the next patch) infects all the traversal stuff with the trait parameters, which we don't really want. In general, it probably makes sense to use separate scoped TLS types for the separate DOM and Flow tree passes, so we can add a different ScopedTLS type for the Flow pass if we ever need it. We also reorder the |scope| and |shared| parameters in parallel.rs, because it aligns more with the order in style/parallel.rs. I did this when I was adding a TLS parameter to all these functions, which I realized we don't need for now. --- components/layout/context.rs | 71 +++++++++++----------- components/layout/parallel.rs | 37 ++++++------ components/layout/query.rs | 7 +-- components/layout/sequential.rs | 8 +-- components/layout/traversal.rs | 31 +++++----- components/layout_thread/lib.rs | 10 ++-- components/style/context.rs | 4 +- components/style/gecko/context.rs | 27 --------- components/style/gecko/mod.rs | 1 - components/style/gecko/traversal.rs | 21 +++---- components/style/lib.rs | 1 + components/style/parallel.rs | 93 +++++++++++++++-------------- components/style/scoped_tls.rs | 51 ++++++++++++++++ components/style/sequential.rs | 25 ++++---- components/style/traversal.rs | 13 ++-- ports/geckolib/glue.rs | 17 ++---- 16 files changed, 212 insertions(+), 205 deletions(-) delete mode 100644 components/style/gecko/context.rs create mode 100644 components/style/scoped_tls.rs diff --git a/components/layout/context.rs b/components/layout/context.rs index 9497d736b47..cc20fa4ef0c 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -19,7 +19,6 @@ use net_traits::image_cache_thread::{ImageOrMetadataAvailable, UsePlaceholder}; use parking_lot::RwLock; use servo_config::opts; use servo_url::ServoUrl; -use std::borrow::Borrow; use std::cell::{RefCell, RefMut}; use std::collections::HashMap; use std::hash::BuildHasherDefault; @@ -27,59 +26,64 @@ use std::rc::Rc; use std::sync::{Arc, Mutex}; use style::context::{SharedStyleContext, ThreadLocalStyleContext}; -pub struct ThreadLocalLayoutContext { +/// TLS data scoped to the traversal. +pub struct ScopedThreadLocalLayoutContext { pub style_context: ThreadLocalStyleContext, +} + +impl ScopedThreadLocalLayoutContext { + pub fn new(shared: &SharedLayoutContext) -> Self { + ScopedThreadLocalLayoutContext { + style_context: ThreadLocalStyleContext::new(&shared.style_context), + } + } +} + +/// TLS data that persists across traversals. +pub struct PersistentThreadLocalLayoutContext { + // FontContext uses Rc all over the place and so isn't Send, which means we + // can't use ScopedTLS for it. There's also no reason to scope it to the + // traversal, and performance is probably better if we don't. pub font_context: RefCell, } -impl ThreadLocalLayoutContext { +impl PersistentThreadLocalLayoutContext { pub fn new(shared: &SharedLayoutContext) -> Rc { let font_cache_thread = shared.font_cache_thread.lock().unwrap().clone(); - let local_style_data = shared.style_context.local_context_creation_data.lock().unwrap(); - - Rc::new(ThreadLocalLayoutContext { - style_context: ThreadLocalStyleContext::new(&local_style_data), + Rc::new(PersistentThreadLocalLayoutContext { font_context: RefCell::new(FontContext::new(font_cache_thread)), }) } } -impl Borrow for ThreadLocalLayoutContext { - fn borrow(&self) -> &ThreadLocalStyleContext { - &self.style_context - } -} - -impl HeapSizeOf for ThreadLocalLayoutContext { - // FIXME(njn): measure other fields eventually. +impl HeapSizeOf for PersistentThreadLocalLayoutContext { fn heap_size_of_children(&self) -> usize { self.font_context.heap_size_of_children() } } -thread_local!(static LOCAL_CONTEXT_KEY: RefCell>> = RefCell::new(None)); +thread_local!(static LOCAL_CONTEXT_KEY: RefCell>> = RefCell::new(None)); -pub fn heap_size_of_local_context() -> usize { - LOCAL_CONTEXT_KEY.with(|r| { - r.borrow().clone().map_or(0, |context| context.heap_size_of_children()) - }) -} - -// Keep this implementation in sync with the one in ports/geckolib/traversal.rs. -pub fn create_or_get_local_context(shared: &SharedLayoutContext) - -> Rc { +fn create_or_get_persistent_context(shared: &SharedLayoutContext) + -> Rc { LOCAL_CONTEXT_KEY.with(|r| { let mut r = r.borrow_mut(); if let Some(context) = r.clone() { context } else { - let context = ThreadLocalLayoutContext::new(shared); + let context = PersistentThreadLocalLayoutContext::new(shared); *r = Some(context.clone()); context } }) } +pub fn heap_size_of_persistent_local_context() -> usize { + LOCAL_CONTEXT_KEY.with(|r| { + r.borrow().clone().map_or(0, |context| context.heap_size_of_children()) + }) +} + /// Layout information shared among all workers. This must be thread-safe. pub struct SharedLayoutContext { /// Bits shared by the layout and style system. @@ -100,24 +104,17 @@ pub struct SharedLayoutContext { BuildHasherDefault>>>, } -impl Borrow for SharedLayoutContext { - fn borrow(&self) -> &SharedStyleContext { - &self.style_context - } -} - pub struct LayoutContext<'a> { pub shared: &'a SharedLayoutContext, - pub thread_local: &'a ThreadLocalLayoutContext, + pub persistent: Rc, } impl<'a> LayoutContext<'a> { - pub fn new(shared: &'a SharedLayoutContext, - thread_local: &'a ThreadLocalLayoutContext) -> Self + pub fn new(shared: &'a SharedLayoutContext) -> Self { LayoutContext { shared: shared, - thread_local: thread_local, + persistent: create_or_get_persistent_context(shared), } } } @@ -138,7 +135,7 @@ impl<'a> LayoutContext<'a> { #[inline(always)] pub fn font_context(&self) -> RefMut { - self.thread_local.font_context.borrow_mut() + self.persistent.font_context.borrow_mut() } } diff --git a/components/layout/parallel.rs b/components/layout/parallel.rs index 505afa7897f..a51f585eb3c 100644 --- a/components/layout/parallel.rs +++ b/components/layout/parallel.rs @@ -8,8 +8,7 @@ #![allow(unsafe_code)] -use context::{LayoutContext, SharedLayoutContext, ThreadLocalLayoutContext}; -use context::create_or_get_local_context; +use context::{LayoutContext, SharedLayoutContext}; use flow::{self, Flow, MutableFlowUtils, PostorderFlowTraversal, PreorderFlowTraversal}; use flow_ref::FlowRef; use profile_traits::time::{self, TimerMetadata, profile}; @@ -51,7 +50,7 @@ pub fn borrowed_flow_to_unsafe_flow(flow: &Flow) -> UnsafeFlow { } pub type ChunkedFlowTraversalFunction<'scope> = - extern "Rust" fn(Box<[UnsafeFlow]>, &'scope SharedLayoutContext, &rayon::Scope<'scope>); + extern "Rust" fn(Box<[UnsafeFlow]>, &rayon::Scope<'scope>, &'scope SharedLayoutContext); pub type FlowTraversalFunction = extern "Rust" fn(UnsafeFlow, &LayoutContext); @@ -133,23 +132,22 @@ trait ParallelPostorderFlowTraversal : PostorderFlowTraversal { trait ParallelPreorderFlowTraversal : PreorderFlowTraversal { fn run_parallel<'scope>(&self, unsafe_flows: &[UnsafeFlow], - layout_context: &'scope SharedLayoutContext, - scope: &rayon::Scope<'scope>); + scope: &rayon::Scope<'scope>, + shared: &'scope SharedLayoutContext); fn should_record_thread_ids(&self) -> bool; #[inline(always)] fn run_parallel_helper<'scope>(&self, unsafe_flows: &[UnsafeFlow], - shared: &'scope SharedLayoutContext, scope: &rayon::Scope<'scope>, + shared: &'scope SharedLayoutContext, top_down_func: ChunkedFlowTraversalFunction<'scope>, bottom_up_func: FlowTraversalFunction) { - let tlc = create_or_get_local_context(shared); - let context = LayoutContext::new(&shared, &*tlc); - let mut discovered_child_flows = vec![]; + let context = LayoutContext::new(&shared); + for unsafe_flow in unsafe_flows { let mut had_children = false; unsafe { @@ -187,7 +185,7 @@ trait ParallelPreorderFlowTraversal : PreorderFlowTraversal { let nodes = chunk.iter().cloned().collect::>().into_boxed_slice(); scope.spawn(move |scope| { - top_down_func(nodes, shared, scope); + top_down_func(nodes, scope, shared); }); } } @@ -196,12 +194,12 @@ trait ParallelPreorderFlowTraversal : PreorderFlowTraversal { impl<'a> ParallelPreorderFlowTraversal for AssignISizes<'a> { fn run_parallel<'scope>(&self, unsafe_flows: &[UnsafeFlow], - layout_context: &'scope SharedLayoutContext, - scope: &rayon::Scope<'scope>) + scope: &rayon::Scope<'scope>, + shared: &'scope SharedLayoutContext) { self.run_parallel_helper(unsafe_flows, - layout_context, scope, + shared, assign_inline_sizes, assign_block_sizes_and_store_overflow) } @@ -214,12 +212,12 @@ impl<'a> ParallelPreorderFlowTraversal for AssignISizes<'a> { impl<'a> ParallelPostorderFlowTraversal for AssignBSizes<'a> {} fn assign_inline_sizes<'scope>(unsafe_flows: Box<[UnsafeFlow]>, - shared_layout_context: &'scope SharedLayoutContext, - scope: &rayon::Scope<'scope>) { + scope: &rayon::Scope<'scope>, + shared: &'scope SharedLayoutContext) { let assign_inline_sizes_traversal = AssignISizes { - shared_context: &shared_layout_context.style_context, + shared_context: &shared.style_context, }; - assign_inline_sizes_traversal.run_parallel(&unsafe_flows, shared_layout_context, scope) + assign_inline_sizes_traversal.run_parallel(&unsafe_flows, scope, shared) } fn assign_block_sizes_and_store_overflow( @@ -238,8 +236,7 @@ pub fn traverse_flow_tree_preorder( shared: &SharedLayoutContext, queue: &rayon::ThreadPool) { if opts::get().bubble_inline_sizes_separately { - let tlc = ThreadLocalLayoutContext::new(shared); - let context = LayoutContext::new(shared, &*tlc); + let context = LayoutContext::new(shared); let bubble_inline_sizes = BubbleISizes { layout_context: &context }; root.traverse_postorder(&bubble_inline_sizes); } @@ -250,7 +247,7 @@ pub fn traverse_flow_tree_preorder( rayon::scope(move |scope| { profile(time::ProfilerCategory::LayoutParallelWarmup, profiler_metadata, time_profiler_chan, move || { - assign_inline_sizes(nodes, &shared, scope); + assign_inline_sizes(nodes, scope, &shared); }); }); }); diff --git a/components/layout/query.rs b/components/layout/query.rs index b6eb7cfeb39..0a44a9b1b49 100644 --- a/components/layout/query.rs +++ b/components/layout/query.rs @@ -6,8 +6,7 @@ use app_units::Au; use construct::ConstructionResult; -use context::SharedLayoutContext; -use context::create_or_get_local_context; +use context::{ScopedThreadLocalLayoutContext, SharedLayoutContext}; use euclid::point::Point2D; use euclid::rect::Rect; use euclid::size::Size2D; @@ -646,10 +645,10 @@ pub fn process_resolved_style_request<'a, N>(shared: &SharedLayoutContext, // we'd need a mechanism to prevent detect when it's stale (since we don't // traverse display:none subtrees during restyle). let display_none_root = if element.get_data().is_none() { - let tlc = create_or_get_local_context(shared); + let mut tlc = ScopedThreadLocalLayoutContext::new(shared); let context = StyleContext { shared: &shared.style_context, - thread_local: &tlc.style_context, + thread_local: &mut tlc.style_context, }; Some(style_element_in_display_none_subtree(&context, element, diff --git a/components/layout/sequential.rs b/components/layout/sequential.rs index e5f62fe1cea..b8c62aaafb0 100644 --- a/components/layout/sequential.rs +++ b/components/layout/sequential.rs @@ -5,7 +5,7 @@ //! Implements sequential traversals over the DOM and flow trees. use app_units::Au; -use context::{LayoutContext, SharedLayoutContext, ThreadLocalLayoutContext}; +use context::{LayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use euclid::point::Point2D; use floats::SpeculatedFloatPlacement; @@ -34,8 +34,7 @@ pub fn resolve_generated_content(root: &mut Flow, shared: &SharedLayoutContext) } } - let tlc = ThreadLocalLayoutContext::new(shared); - let layout_context = LayoutContext::new(shared, &*tlc); + let layout_context = LayoutContext::new(shared); let mut traversal = ResolveGeneratedContent::new(&layout_context); doit(root, 0, &mut traversal) } @@ -58,8 +57,7 @@ pub fn traverse_flow_tree_preorder(root: &mut Flow, } } - let tlc = ThreadLocalLayoutContext::new(shared); - let layout_context = LayoutContext::new(shared, &*tlc); + let layout_context = LayoutContext::new(shared); if opts::get().bubble_inline_sizes_separately { let bubble_inline_sizes = BubbleISizes { layout_context: &layout_context }; diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index 43ef8e14fda..c94dccab2ff 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -5,15 +5,13 @@ //! Traversals over the DOM and flow trees, running the layout computations. use construct::FlowConstructor; -use context::{LayoutContext, SharedLayoutContext, ThreadLocalLayoutContext}; -use context::create_or_get_local_context; +use context::{LayoutContext, ScopedThreadLocalLayoutContext, SharedLayoutContext}; use display_list_builder::DisplayListBuildState; use flow::{self, PreorderFlowTraversal}; use flow::{CAN_BE_FRAGMENTED, Flow, ImmutableFlowUtils, PostorderFlowTraversal}; use gfx::display_list::OpaqueNode; use script_layout_interface::wrapper_traits::{LayoutNode, ThreadSafeLayoutNode}; use servo_config::opts; -use std::rc::Rc; use style::atomic_refcell::AtomicRefCell; use style::context::{SharedStyleContext, StyleContext}; use style::data::ElementData; @@ -56,9 +54,10 @@ impl DomTraversal for RecalcStyleAndConstructFlows where N: LayoutNode + TNode, N::ConcreteElement: TElement { - type ThreadLocalContext = ThreadLocalLayoutContext; + type ThreadLocalContext = ScopedThreadLocalLayoutContext; - fn process_preorder(&self, node: N, traversal_data: &mut PerLevelTraversalData) { + fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData, + thread_local: &mut Self::ThreadLocalContext, node: N) { // FIXME(pcwalton): Stop allocating here. Ideally this should just be // done by the HTML parser. node.initialize_data(); @@ -66,19 +65,17 @@ impl DomTraversal for RecalcStyleAndConstructFlows if !node.is_text_node() { let el = node.as_element().unwrap(); let mut data = el.mutate_data().unwrap(); - let tlc = create_or_get_local_context(&self.shared); - let context = StyleContext { + let mut context = StyleContext { shared: &self.shared.style_context, - thread_local: &tlc.style_context, + thread_local: &mut thread_local.style_context, }; - recalc_style_at(self, traversal_data, &context, el, &mut data); + recalc_style_at(self, traversal_data, &mut context, el, &mut data); } } - fn process_postorder(&self, node: N) { - let tlc = create_or_get_local_context(&self.shared); - let context = LayoutContext::new(&self.shared, &*tlc); - construct_flows_at(&context, self.root, node); + fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: N) { + let context = LayoutContext::new(&self.shared); + construct_flows_at(&context, thread_local, self.root, node); } fn text_node_needs_traversal(node: N) -> bool { @@ -103,8 +100,8 @@ impl DomTraversal for RecalcStyleAndConstructFlows &self.shared.style_context } - fn create_or_get_thread_local_context(&self) -> Rc { - create_or_get_local_context(&self.shared) + fn create_thread_local_context(&self) -> Self::ThreadLocalContext { + ScopedThreadLocalLayoutContext::new(&self.shared) } } @@ -117,7 +114,9 @@ pub trait PostorderNodeMutTraversal(context: &LayoutContext<'a>, root: OpaqueNode, node: N) +fn construct_flows_at<'a, N>(context: &LayoutContext<'a>, + _thread_local: &ScopedThreadLocalLayoutContext, + root: OpaqueNode, node: N) where N: LayoutNode, { debug!("construct_flows_at: {:?}", node); diff --git a/components/layout_thread/lib.rs b/components/layout_thread/lib.rs index d2bdf3a8f35..524b325dcec 100644 --- a/components/layout_thread/lib.rs +++ b/components/layout_thread/lib.rs @@ -63,7 +63,8 @@ use ipc_channel::ipc::{self, IpcReceiver, IpcSender}; use ipc_channel::router::ROUTER; use layout::animation; use layout::construct::ConstructionResult; -use layout::context::{LayoutContext, SharedLayoutContext, ThreadLocalLayoutContext, heap_size_of_local_context}; +use layout::context::{LayoutContext, SharedLayoutContext}; +use layout::context::heap_size_of_persistent_local_context; use layout::display_list_builder::ToGfxColor; use layout::flow::{self, Flow, ImmutableFlowUtils, MutableFlowUtils, MutableOwnedFlowUtils}; use layout::flow_ref::FlowRef; @@ -723,11 +724,11 @@ impl LayoutThread { size: stylist.heap_size_of_children(), }); - // The LayoutThread has a context in TLS... + // The LayoutThread has data in Persistent TLS... reports.push(Report { path: path![formatted_url, "layout-thread", "local-context"], kind: ReportKind::ExplicitJemallocHeapSize, - size: heap_size_of_local_context(), + size: heap_size_of_persistent_local_context(), }); reports_chan.send(reports); @@ -1447,8 +1448,7 @@ impl LayoutThread { self.profiler_metadata(), self.time_profiler_chan.clone(), || { - let tlc = ThreadLocalLayoutContext::new(&shared); - let context = LayoutContext::new(&shared, &*tlc); + let context = LayoutContext::new(&shared); sequential::store_overflow(&context, FlowRef::deref_mut(&mut root_flow) as &mut Flow); }); diff --git a/components/style/context.rs b/components/style/context.rs index ffe9668bd32..e8e2ee1cc13 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -84,10 +84,10 @@ pub struct ThreadLocalStyleContext { } impl ThreadLocalStyleContext { - pub fn new(local_context_creation_data: &ThreadLocalStyleContextCreationInfo) -> Self { + pub fn new(shared: &SharedStyleContext) -> Self { ThreadLocalStyleContext { style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()), - new_animations_sender: local_context_creation_data.new_animations_sender.clone(), + new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(), } } } diff --git a/components/style/gecko/context.rs b/components/style/gecko/context.rs deleted file mode 100644 index c7ba65dce61..00000000000 --- a/components/style/gecko/context.rs +++ /dev/null @@ -1,27 +0,0 @@ -/* 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/. */ - -use context::{SharedStyleContext, ThreadLocalStyleContext}; -use std::cell::RefCell; -use std::rc::Rc; - -thread_local!(static LOCAL_CONTEXT_KEY: RefCell>> = RefCell::new(None)); - -// Keep this implementation in sync with the one in components/layout/context.rs. -pub fn create_or_get_local_context(shared: &SharedStyleContext) -> Rc { - LOCAL_CONTEXT_KEY.with(|r| { - let mut r = r.borrow_mut(); - if let Some(context) = r.clone() { - context - } else { - let context = Rc::new(ThreadLocalStyleContext::new(&shared.local_context_creation_data.lock().unwrap())); - *r = Some(context.clone()); - context - } - }) -} - -pub fn clear_local_context() { - LOCAL_CONTEXT_KEY.with(|r| *r.borrow_mut() = None); -} diff --git a/components/style/gecko/mod.rs b/components/style/gecko/mod.rs index 2e1e653f88e..7f227f154dc 100644 --- a/components/style/gecko/mod.rs +++ b/components/style/gecko/mod.rs @@ -3,7 +3,6 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -pub mod context; pub mod data; pub mod restyle_damage; pub mod snapshot; diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 005da9620d0..8d7ff1c9731 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -6,9 +6,8 @@ use atomic_refcell::AtomicRefCell; use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext}; use data::ElementData; use dom::{NodeInfo, TNode}; -use gecko::context::create_or_get_local_context; use gecko::wrapper::{GeckoElement, GeckoNode}; -use std::rc::Rc; use traversal::{DomTraversal, PerLevelTraversalData, recalc_style_at}; +use traversal::{DomTraversal, PerLevelTraversalData, recalc_style_at}; pub struct RecalcStyleOnly { shared: SharedStyleContext, @@ -25,20 +24,22 @@ impl RecalcStyleOnly { impl<'ln> DomTraversal> for RecalcStyleOnly { type ThreadLocalContext = ThreadLocalStyleContext; - fn process_preorder(&self, node: GeckoNode<'ln>, traversal_data: &mut PerLevelTraversalData) { + fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData, + thread_local: &mut ThreadLocalStyleContext, + node: GeckoNode<'ln>) + { if node.is_element() { let el = node.as_element().unwrap(); let mut data = unsafe { el.ensure_data() }.borrow_mut(); - let tlc = self.create_or_get_thread_local_context(); - let context = StyleContext { + let mut context = StyleContext { shared: &self.shared, - thread_local: &*tlc, + thread_local: thread_local, }; - recalc_style_at(self, traversal_data, &context, el, &mut data); + recalc_style_at(self, traversal_data, &mut context, el, &mut data); } } - fn process_postorder(&self, _: GeckoNode<'ln>) { + fn process_postorder(&self, _: &mut ThreadLocalStyleContext, _: GeckoNode<'ln>) { unreachable!(); } @@ -57,7 +58,7 @@ impl<'ln> DomTraversal> for RecalcStyleOnly { &self.shared } - fn create_or_get_thread_local_context(&self) -> Rc { - create_or_get_local_context(&self.shared) + fn create_thread_local_context(&self) -> ThreadLocalStyleContext { + ThreadLocalStyleContext::new(&self.shared) } } diff --git a/components/style/lib.rs b/components/style/lib.rs index 64b40b9e96e..3481a03c4fe 100644 --- a/components/style/lib.rs +++ b/components/style/lib.rs @@ -115,6 +115,7 @@ pub mod parallel; pub mod parser; pub mod restyle_hints; pub mod rule_tree; +pub mod scoped_tls; pub mod selector_parser; pub mod stylist; #[cfg(feature = "servo")] #[allow(unsafe_code)] pub mod servo; diff --git a/components/style/parallel.rs b/components/style/parallel.rs index 5fa27f8ac96..80652bfa6b8 100644 --- a/components/style/parallel.rs +++ b/components/style/parallel.rs @@ -8,8 +8,8 @@ use dom::{OpaqueNode, TElement, TNode, UnsafeNode}; use rayon; +use scoped_tls::ScopedTLS; use servo_config::opts; -use std::borrow::Borrow; use std::sync::atomic::Ordering; use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken}; use traversal::{STYLE_SHARING_CACHE_HITS, STYLE_SHARING_CACHE_MISSES}; @@ -45,14 +45,15 @@ pub fn traverse_dom(traversal: &D, (vec![root.as_node().to_unsafe()], known_root_dom_depth) }; - let data = PerLevelTraversalData { + let traversal_data = PerLevelTraversalData { current_dom_depth: depth, }; - + let tls = ScopedTLS::::new(queue); let root = root.as_node().opaque(); + queue.install(|| { rayon::scope(|scope| { - traverse_nodes(nodes, root, data, scope, traversal); + traverse_nodes(nodes, root, traversal_data, scope, traversal, &tls); }); }); @@ -71,57 +72,60 @@ pub fn traverse_dom(traversal: &D, #[allow(unsafe_code)] fn top_down_dom<'a, 'scope, N, D>(unsafe_nodes: &'a [UnsafeNode], root: OpaqueNode, - mut data: PerLevelTraversalData, + mut traversal_data: PerLevelTraversalData, scope: &'a rayon::Scope<'scope>, - traversal: &'scope D) + traversal: &'scope D, + tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>) where N: TNode, D: DomTraversal, { let mut discovered_child_nodes = vec![]; - for unsafe_node in unsafe_nodes { - // Get a real layout node. - let node = unsafe { N::from_unsafe(&unsafe_node) }; + { + // Scope the borrow of the TLS so that the borrow is dropped before + // potentially traversing a child on this thread. + let mut tlc = tls.ensure(|| traversal.create_thread_local_context()); - // Perform the appropriate traversal. - let mut children_to_process = 0isize; - traversal.process_preorder(node, &mut data); - if let Some(el) = node.as_element() { - D::traverse_children(el, |kid| { - children_to_process += 1; - discovered_child_nodes.push(kid.to_unsafe()) - }); - } + for unsafe_node in unsafe_nodes { + // Get a real layout node. + let node = unsafe { N::from_unsafe(&unsafe_node) }; - // Reset the count of children if we need to do a bottom-up traversal - // after the top up. - if D::needs_postorder_traversal() { - if children_to_process == 0 { - // If there were no more children, start walking back up. - bottom_up_dom(root, *unsafe_node, traversal) - } else { - // Otherwise record the number of children to process when the - // time comes. - node.as_element().unwrap().store_children_to_process(children_to_process); + // Perform the appropriate traversal. + let mut children_to_process = 0isize; + traversal.process_preorder(&mut traversal_data, &mut *tlc, node); + if let Some(el) = node.as_element() { + D::traverse_children(el, |kid| { + children_to_process += 1; + discovered_child_nodes.push(kid.to_unsafe()) + }); + } + + // Reset the count of children if we need to do a bottom-up traversal + // after the top up. + if D::needs_postorder_traversal() { + if children_to_process == 0 { + // If there were no more children, start walking back up. + bottom_up_dom(traversal, &mut *tlc, root, *unsafe_node) + } else { + // Otherwise record the number of children to process when the + // time comes. + node.as_element().unwrap().store_children_to_process(children_to_process); + } } } } - // NB: In parallel traversal mode we have to purge the LRU cache in order to - // be able to access it without races. - let tlc = traversal.create_or_get_thread_local_context(); - (*tlc).borrow().style_sharing_candidate_cache.borrow_mut().clear(); - - if let Some(ref mut depth) = data.current_dom_depth { + if let Some(ref mut depth) = traversal_data.current_dom_depth { *depth += 1; } - traverse_nodes(discovered_child_nodes, root, data, scope, traversal); + traverse_nodes(discovered_child_nodes, root, traversal_data, scope, traversal, tls); } fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec, root: OpaqueNode, - data: PerLevelTraversalData, + traversal_data: PerLevelTraversalData, scope: &'a rayon::Scope<'scope>, - traversal: &'scope D) + traversal: &'scope D, + tls: &'scope ScopedTLS<'scope, D::ThreadLocalContext>) where N: TNode, D: DomTraversal, { @@ -133,17 +137,17 @@ fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec, root: OpaqueNode, // we're only pushing one work unit. if nodes.len() <= CHUNK_SIZE { let nodes = nodes.into_boxed_slice(); - top_down_dom(&nodes, root, data, scope, traversal); + top_down_dom(&nodes, root, traversal_data, scope, traversal, tls); return; } // General case. for chunk in nodes.chunks(CHUNK_SIZE) { let nodes = chunk.iter().cloned().collect::>().into_boxed_slice(); - let data = data.clone(); + let traversal_data = traversal_data.clone(); scope.spawn(move |scope| { let nodes = nodes; - top_down_dom(&nodes, root, data, scope, traversal) + top_down_dom(&nodes, root, traversal_data, scope, traversal, tls) }) } } @@ -160,9 +164,10 @@ fn traverse_nodes<'a, 'scope, N, D>(nodes: Vec, root: OpaqueNode, /// The only communication between siblings is that they both /// fetch-and-subtract the parent's children count. #[allow(unsafe_code)] -fn bottom_up_dom(root: OpaqueNode, - unsafe_node: UnsafeNode, - traversal: &D) +fn bottom_up_dom(traversal: &D, + thread_local: &mut D::ThreadLocalContext, + root: OpaqueNode, + unsafe_node: UnsafeNode) where N: TNode, D: DomTraversal { @@ -170,7 +175,7 @@ fn bottom_up_dom(root: OpaqueNode, let mut node = unsafe { N::from_unsafe(&unsafe_node) }; loop { // Perform the appropriate operation. - traversal.process_postorder(node); + traversal.process_postorder(thread_local, node); if node.opaque() == root { break; diff --git a/components/style/scoped_tls.rs b/components/style/scoped_tls.rs new file mode 100644 index 00000000000..1f695aaa4c9 --- /dev/null +++ b/components/style/scoped_tls.rs @@ -0,0 +1,51 @@ +/* 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 rayon; +use std::cell::{Ref, RefCell, RefMut}; + +/// Stack-scoped thread-local storage for rayon thread pools. + +pub struct ScopedTLS<'a, T: Send> { + pool: &'a rayon::ThreadPool, + slots: Box<[RefCell>]>, +} + +unsafe impl<'a, T: Send> Sync for ScopedTLS<'a, T> {} + +impl<'a, T: Send> ScopedTLS<'a, T> { + pub fn new(p: &'a rayon::ThreadPool) -> Self { + let count = p.num_threads(); + let mut v = Vec::with_capacity(count); + for _ in 0..count { + v.push(RefCell::new(None)); + } + + ScopedTLS { + pool: p, + slots: v.into_boxed_slice(), + } + } + + pub fn borrow(&self) -> Ref> { + let idx = self.pool.current_thread_index().unwrap(); + self.slots[idx].borrow() + } + + pub fn borrow_mut(&self) -> RefMut> { + let idx = self.pool.current_thread_index().unwrap(); + self.slots[idx].borrow_mut() + } + + pub fn ensure T>(&self, f: F) -> RefMut { + let mut opt = self.borrow_mut(); + if opt.is_none() { + *opt = Some(f()); + } + + RefMut::map(opt, |x| x.as_mut().unwrap()) + } +} diff --git a/components/style/sequential.rs b/components/style/sequential.rs index 1b8d602a0f4..b47d95a8517 100644 --- a/components/style/sequential.rs +++ b/components/style/sequential.rs @@ -5,7 +5,6 @@ //! Implements sequential traversal over the DOM tree. use dom::{TElement, TNode}; -use std::borrow::Borrow; use traversal::{DomTraversal, PerLevelTraversalData, PreTraverseToken}; pub fn traverse_dom(traversal: &D, @@ -16,43 +15,41 @@ pub fn traverse_dom(traversal: &D, { debug_assert!(token.should_traverse()); - fn doit(traversal: &D, node: N, data: &mut PerLevelTraversalData) + fn doit(traversal: &D, traversal_data: &mut PerLevelTraversalData, + thread_local: &mut D::ThreadLocalContext, node: N) where N: TNode, D: DomTraversal { - traversal.process_preorder(node, data); + traversal.process_preorder(traversal_data, thread_local, node); if let Some(el) = node.as_element() { - if let Some(ref mut depth) = data.current_dom_depth { + if let Some(ref mut depth) = traversal_data.current_dom_depth { *depth += 1; } - D::traverse_children(el, |kid| doit(traversal, kid, data)); + D::traverse_children(el, |kid| doit(traversal, traversal_data, thread_local, kid)); - if let Some(ref mut depth) = data.current_dom_depth { + if let Some(ref mut depth) = traversal_data.current_dom_depth { *depth -= 1; } } if D::needs_postorder_traversal() { - traversal.process_postorder(node); + traversal.process_postorder(thread_local, node); } } - let mut data = PerLevelTraversalData { + let mut traversal_data = PerLevelTraversalData { current_dom_depth: None, }; + let mut tlc = traversal.create_thread_local_context(); if token.traverse_unstyled_children_only() { for kid in root.as_node().children() { if kid.as_element().map_or(false, |el| el.get_data().is_none()) { - doit(traversal, kid, &mut data); + doit(traversal, &mut traversal_data, &mut tlc, kid); } } } else { - doit(traversal, root.as_node(), &mut data); + doit(traversal, &mut traversal_data, &mut tlc, root.as_node()); } - - // Clear the local LRU cache since we store stateful elements inside. - let tlc = traversal.create_or_get_thread_local_context(); - (*tlc).borrow().style_sharing_candidate_cache.borrow_mut().clear(); } diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 4a44619cef4..177c6a59ddc 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -6,7 +6,7 @@ use atomic_refcell::{AtomicRefCell, AtomicRefMut}; use bloom::StyleBloom; -use context::{SharedStyleContext, StyleContext, ThreadLocalStyleContext}; +use context::{SharedStyleContext, StyleContext}; use data::{ElementData, StoredRestyleHint}; use dom::{OpaqueNode, TElement, TNode}; use matching::{MatchMethods, StyleSharingResult}; @@ -15,10 +15,8 @@ use selector_parser::RestyleDamage; use selectors::Element; use selectors::matching::StyleRelations; use servo_config::opts; -use std::borrow::Borrow; use std::cell::RefCell; use std::mem; -use std::rc::Rc; use std::sync::atomic::{AtomicUsize, ATOMIC_USIZE_INIT, Ordering}; use stylist::Stylist; @@ -128,15 +126,16 @@ impl LogBehavior { } pub trait DomTraversal : Sync { - type ThreadLocalContext: Borrow; + type ThreadLocalContext: Send; /// Process `node` on the way down, before its children have been processed. - fn process_preorder(&self, node: N, data: &mut PerLevelTraversalData); + fn process_preorder(&self, data: &mut PerLevelTraversalData, + thread_local: &mut Self::ThreadLocalContext, node: N); /// Process `node` on the way up, after its children have been processed. /// /// This is only executed if `needs_postorder_traversal` returns true. - fn process_postorder(&self, node: N); + fn process_postorder(&self, thread_local: &mut Self::ThreadLocalContext, node: N); /// Boolean that specifies whether a bottom up traversal should be /// performed. @@ -321,7 +320,7 @@ pub trait DomTraversal : Sync { fn shared_context(&self) -> &SharedStyleContext; - fn create_or_get_thread_local_context(&self) -> Rc; + fn create_thread_local_context(&self) -> Self::ThreadLocalContext; } /// Determines the amount of relations where we're going to share style. diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 4d47ec460a4..470fa15dcfa 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -17,11 +17,11 @@ use std::ptr; use std::sync::{Arc, Mutex}; use style::arc_ptr_eq; use style::atomic_refcell::AtomicRefMut; -use style::context::{ThreadLocalStyleContextCreationInfo, QuirksMode, ReflowGoal, SharedStyleContext, StyleContext}; +use style::context::{QuirksMode, ReflowGoal, SharedStyleContext, StyleContext}; +use style::context::{ThreadLocalStyleContext, ThreadLocalStyleContextCreationInfo}; use style::data::{ElementData, RestyleData}; use style::dom::{ShowSubtreeData, TElement, TNode}; use style::error_reporting::StdoutErrorReporter; -use style::gecko::context::clear_local_context; use style::gecko::data::{NUM_THREADS, PerDocumentStyleData, PerDocumentStyleDataImpl}; use style::gecko::restyle_damage::GeckoRestyleDamage; use style::gecko::selector_parser::{SelectorImpl, PseudoElement}; @@ -88,12 +88,6 @@ pub extern "C" fn Servo_Initialize() -> () { pub extern "C" fn Servo_Shutdown() -> () { // Destroy our default computed values. unsafe { ComputedValues::shutdown(); } - - // In general, ThreadLocalStyleContexts will get destroyed when the worker thread - // is joined and the TLS is dropped. However, under some configurations we - // may do sequential style computation on the main thread, so we need to be - // sure to clear the main thread TLS entry as well. - clear_local_context(); } fn create_shared_context(mut per_doc_data: &mut AtomicRefMut) -> SharedStyleContext { @@ -875,22 +869,19 @@ pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed, let mut per_doc_data = PerDocumentStyleData::from_ffi(raw_data).borrow_mut(); let shared_style_context = create_shared_context(&mut per_doc_data); let traversal = RecalcStyleOnly::new(shared_style_context); - let tlc = traversal.create_or_get_thread_local_context(); let mut traversal_data = PerLevelTraversalData { current_dom_depth: None, }; + let mut tlc = ThreadLocalStyleContext::new(traversal.shared_context()); let context = StyleContext { shared: traversal.shared_context(), - thread_local: &*tlc, + thread_local: &mut tlc, }; recalc_style_at(&traversal, &mut traversal_data, &context, element, &mut data); - // We don't want to keep any cached style around after this one-off style resolution. - tlc.style_sharing_candidate_cache.borrow_mut().clear(); - // The element was either unstyled or needed restyle. If it was unstyled, it may have // additional unstyled children that subsequent traversals won't find now that the style // on this element is up-to-date. Mark dirty descendants in that case. From 946e7fb7c34afd51ba332180387a10d88c878585 Mon Sep 17 00:00:00 2001 From: Bobby Holley Date: Fri, 16 Dec 2016 17:42:46 -0800 Subject: [PATCH 4/4] Stop using UnsafeNode in the StyleSharingCandidateCache. --- components/layout/context.rs | 7 +-- components/layout/traversal.rs | 4 +- components/style/context.rs | 15 +++--- components/style/gecko/traversal.rs | 8 ++-- components/style/matching.rs | 73 ++++++++++++++--------------- components/style/traversal.rs | 19 ++++---- ports/geckolib/glue.rs | 4 +- 7 files changed, 63 insertions(+), 67 deletions(-) diff --git a/components/layout/context.rs b/components/layout/context.rs index cc20fa4ef0c..e2d1fa7831c 100644 --- a/components/layout/context.rs +++ b/components/layout/context.rs @@ -25,13 +25,14 @@ use std::hash::BuildHasherDefault; use std::rc::Rc; use std::sync::{Arc, Mutex}; use style::context::{SharedStyleContext, ThreadLocalStyleContext}; +use style::dom::TElement; /// TLS data scoped to the traversal. -pub struct ScopedThreadLocalLayoutContext { - pub style_context: ThreadLocalStyleContext, +pub struct ScopedThreadLocalLayoutContext { + pub style_context: ThreadLocalStyleContext, } -impl ScopedThreadLocalLayoutContext { +impl ScopedThreadLocalLayoutContext { pub fn new(shared: &SharedLayoutContext) -> Self { ScopedThreadLocalLayoutContext { style_context: ThreadLocalStyleContext::new(&shared.style_context), diff --git a/components/layout/traversal.rs b/components/layout/traversal.rs index c94dccab2ff..be4525cd17a 100644 --- a/components/layout/traversal.rs +++ b/components/layout/traversal.rs @@ -54,7 +54,7 @@ impl DomTraversal for RecalcStyleAndConstructFlows where N: LayoutNode + TNode, N::ConcreteElement: TElement { - type ThreadLocalContext = ScopedThreadLocalLayoutContext; + type ThreadLocalContext = ScopedThreadLocalLayoutContext; fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData, thread_local: &mut Self::ThreadLocalContext, node: N) { @@ -115,7 +115,7 @@ pub trait PostorderNodeMutTraversal(context: &LayoutContext<'a>, - _thread_local: &ScopedThreadLocalLayoutContext, + _thread_local: &ScopedThreadLocalLayoutContext, root: OpaqueNode, node: N) where N: LayoutNode, { diff --git a/components/style/context.rs b/components/style/context.rs index e8e2ee1cc13..a1695bb7e8e 100644 --- a/components/style/context.rs +++ b/components/style/context.rs @@ -6,12 +6,11 @@ use animation::Animation; use app_units::Au; -use dom::OpaqueNode; +use dom::{OpaqueNode, TElement}; use error_reporting::ParseErrorReporter; use euclid::Size2D; use matching::StyleSharingCandidateCache; use parking_lot::RwLock; -use std::cell::RefCell; use std::collections::HashMap; use std::sync::{Arc, Mutex}; use std::sync::mpsc::Sender; @@ -76,25 +75,25 @@ pub struct SharedStyleContext { pub quirks_mode: QuirksMode, } -pub struct ThreadLocalStyleContext { - pub style_sharing_candidate_cache: RefCell, +pub struct ThreadLocalStyleContext { + pub style_sharing_candidate_cache: StyleSharingCandidateCache, /// A channel on which new animations that have been triggered by style /// recalculation can be sent. pub new_animations_sender: Sender, } -impl ThreadLocalStyleContext { +impl ThreadLocalStyleContext { pub fn new(shared: &SharedStyleContext) -> Self { ThreadLocalStyleContext { - style_sharing_candidate_cache: RefCell::new(StyleSharingCandidateCache::new()), + style_sharing_candidate_cache: StyleSharingCandidateCache::new(), new_animations_sender: shared.local_context_creation_data.lock().unwrap().new_animations_sender.clone(), } } } -pub struct StyleContext<'a> { +pub struct StyleContext<'a, E: TElement + 'a> { pub shared: &'a SharedStyleContext, - pub thread_local: &'a ThreadLocalStyleContext, + pub thread_local: &'a mut ThreadLocalStyleContext, } /// Why we're doing reflow. diff --git a/components/style/gecko/traversal.rs b/components/style/gecko/traversal.rs index 8d7ff1c9731..7f83fbe5426 100644 --- a/components/style/gecko/traversal.rs +++ b/components/style/gecko/traversal.rs @@ -22,10 +22,10 @@ impl RecalcStyleOnly { } impl<'ln> DomTraversal> for RecalcStyleOnly { - type ThreadLocalContext = ThreadLocalStyleContext; + type ThreadLocalContext = ThreadLocalStyleContext>; fn process_preorder(&self, traversal_data: &mut PerLevelTraversalData, - thread_local: &mut ThreadLocalStyleContext, + thread_local: &mut Self::ThreadLocalContext, node: GeckoNode<'ln>) { if node.is_element() { @@ -39,7 +39,7 @@ impl<'ln> DomTraversal> for RecalcStyleOnly { } } - fn process_postorder(&self, _: &mut ThreadLocalStyleContext, _: GeckoNode<'ln>) { + fn process_postorder(&self, _: &mut Self::ThreadLocalContext, _: GeckoNode<'ln>) { unreachable!(); } @@ -58,7 +58,7 @@ impl<'ln> DomTraversal> for RecalcStyleOnly { &self.shared } - fn create_thread_local_context(&self) -> ThreadLocalStyleContext { + fn create_thread_local_context(&self) -> Self::ThreadLocalContext { ThreadLocalStyleContext::new(&self.shared) } } diff --git a/components/style/matching.rs b/components/style/matching.rs index 23b832e2587..a1580e404b4 100644 --- a/components/style/matching.rs +++ b/components/style/matching.rs @@ -13,7 +13,7 @@ use cache::LRUCache; use cascade_info::CascadeInfo; use context::{SharedStyleContext, StyleContext}; use data::{ComputedStyle, ElementData, ElementStyles, PseudoStyles}; -use dom::{TElement, TNode, UnsafeNode}; +use dom::{TElement, TNode}; use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade}; use properties::longhands::display::computed_value as display; use rule_tree::StrongRuleNode; @@ -65,22 +65,29 @@ impl MatchResults { } } +// TElement isn't Send because we want to be careful and explicit about our +// parallel traversal, but we need the candidates to be Send so that we can stick +// them in ScopedTLS. +#[derive(Debug, PartialEq)] +struct SendElement(pub E); +unsafe impl Send for SendElement {} + /// Information regarding a candidate. /// /// TODO: We can stick a lot more info here. #[derive(Debug)] -struct StyleSharingCandidate { - /// The node, guaranteed to be an element. - node: UnsafeNode, +struct StyleSharingCandidate { + /// The element. + element: SendElement, /// The cached common style affecting attribute info. common_style_affecting_attributes: Option, /// the cached class names. class_attributes: Option>, } -impl PartialEq for StyleSharingCandidate { +impl PartialEq> for StyleSharingCandidate { fn eq(&self, other: &Self) -> bool { - self.node == other.node && + self.element == other.element && self.common_style_affecting_attributes == other.common_style_affecting_attributes } } @@ -90,13 +97,8 @@ impl PartialEq for StyleSharingCandidate { /// /// Note that this cache is flushed every time we steal work from the queue, so /// storing nodes here temporarily is safe. -/// -/// NB: We store UnsafeNode's, but this is not unsafe. It's a shame being -/// generic over elements is unfeasible (you can make compile style without much -/// difficulty, but good luck with layout and all the types with assoc. -/// lifetimes). -pub struct StyleSharingCandidateCache { - cache: LRUCache, +pub struct StyleSharingCandidateCache { + cache: LRUCache, ()>, } #[derive(Clone, Debug)] @@ -117,7 +119,7 @@ pub enum CacheMiss { } fn element_matches_candidate(element: &E, - candidate: &mut StyleSharingCandidate, + candidate: &mut StyleSharingCandidate, candidate_element: &E, shared_context: &SharedStyleContext) -> Result { @@ -193,7 +195,7 @@ fn element_matches_candidate(element: &E, } fn have_same_common_style_affecting_attributes(element: &E, - candidate: &mut StyleSharingCandidate, + candidate: &mut StyleSharingCandidate, candidate_element: &E) -> bool { if candidate.common_style_affecting_attributes.is_none() { candidate.common_style_affecting_attributes = @@ -272,7 +274,7 @@ pub fn rare_style_affecting_attributes() -> [LocalName; 3] { } fn have_same_class(element: &E, - candidate: &mut StyleSharingCandidate, + candidate: &mut StyleSharingCandidate, candidate_element: &E) -> bool { // XXX Efficiency here, I'm only validating ideas. let mut element_class_attributes = vec![]; @@ -304,21 +306,21 @@ fn match_same_sibling_affecting_rules(element: &E, static STYLE_SHARING_CANDIDATE_CACHE_SIZE: usize = 8; -impl StyleSharingCandidateCache { +impl StyleSharingCandidateCache { pub fn new() -> Self { StyleSharingCandidateCache { cache: LRUCache::new(STYLE_SHARING_CANDIDATE_CACHE_SIZE), } } - fn iter_mut(&mut self) -> IterMut<(StyleSharingCandidate, ())> { + fn iter_mut(&mut self) -> IterMut<(StyleSharingCandidate, ())> { self.cache.iter_mut() } - pub fn insert_if_possible(&mut self, - element: &E, - style: &Arc, - relations: StyleRelations) { + pub fn insert_if_possible(&mut self, + element: &E, + style: &Arc, + relations: StyleRelations) { use traversal::relations_are_shareable; let parent = match element.parent_element() { @@ -348,10 +350,10 @@ impl StyleSharingCandidateCache { } debug!("Inserting into cache: {:?} with parent {:?}", - element.as_node().to_unsafe(), parent.as_node().to_unsafe()); + element, parent); self.cache.insert(StyleSharingCandidate { - node: element.as_node().to_unsafe(), + element: SendElement(*element), common_style_affecting_attributes: None, class_attributes: None, }, ()); @@ -392,7 +394,7 @@ trait PrivateMatchMethods: TElement { /// Note that animations only apply to nodes or ::before or ::after /// pseudo-elements. fn cascade_node_pseudo_element<'a>(&self, - context: &StyleContext, + context: &StyleContext, parent_style: Option<&Arc>, old_style: Option<&Arc>, rule_node: &StrongRuleNode, @@ -497,20 +499,17 @@ trait PrivateMatchMethods: TElement { fn share_style_with_candidate_if_possible(&self, shared_context: &SharedStyleContext, - candidate: &mut StyleSharingCandidate) + candidate: &mut StyleSharingCandidate) -> Result { - let candidate_element = unsafe { - Self::ConcreteNode::from_unsafe(&candidate.node).as_element().unwrap() - }; - + let candidate_element = candidate.element.0; element_matches_candidate(self, candidate, &candidate_element, shared_context) } } -fn compute_rule_node(context: &StyleContext, - applicable_declarations: &mut Vec) - -> StrongRuleNode +fn compute_rule_node(context: &StyleContext, + applicable_declarations: &mut Vec) + -> StrongRuleNode { let rules = applicable_declarations.drain(..).map(|d| (d.source, d.importance)); let rule_node = context.shared.stylist.rule_tree.insert_ordered_rules(rules); @@ -520,7 +519,7 @@ fn compute_rule_node(context: &StyleContext, impl PrivateMatchMethods for E {} pub trait MatchMethods : TElement { - fn match_element(&self, context: &StyleContext, parent_bf: Option<&BloomFilter>) + fn match_element(&self, context: &StyleContext, parent_bf: Option<&BloomFilter>) -> MatchResults { let mut applicable_declarations: Vec = Vec::with_capacity(16); @@ -569,7 +568,7 @@ pub trait MatchMethods : TElement { /// guarantee that at the type system level yet. unsafe fn share_style_if_possible(&self, style_sharing_candidate_cache: - &mut StyleSharingCandidateCache, + &mut StyleSharingCandidateCache, shared_context: &SharedStyleContext, data: &mut AtomicRefMut) -> StyleSharingResult { @@ -718,7 +717,7 @@ pub trait MatchMethods : TElement { } unsafe fn cascade_node(&self, - context: &StyleContext, + context: &StyleContext, mut data: &mut AtomicRefMut, parent: Option, primary_rule_node: StrongRuleNode, @@ -795,7 +794,7 @@ pub trait MatchMethods : TElement { mut old_pseudos: Option<&mut PseudoStyles>, new_primary: &Arc, new_pseudos: &mut PseudoStyles, - context: &StyleContext, + context: &StyleContext, mut pseudo_rule_nodes: PseudoRuleNodes, possibly_expired_animations: &mut Vec) -> RestyleDamage diff --git a/components/style/traversal.rs b/components/style/traversal.rs index 177c6a59ddc..f6fd16429a9 100644 --- a/components/style/traversal.rs +++ b/components/style/traversal.rs @@ -336,7 +336,7 @@ pub fn relations_are_shareable(relations: &StyleRelations) -> bool { /// Handles lazy resolution of style in display:none subtrees. See the comment /// at the callsite in query.rs. -pub fn style_element_in_display_none_subtree(context: &StyleContext, +pub fn style_element_in_display_none_subtree(context: &StyleContext, element: E, init_data: &F) -> E where E: TElement, F: Fn(E), @@ -375,7 +375,7 @@ pub fn style_element_in_display_none_subtree(context: &StyleContext, #[allow(unsafe_code)] pub fn recalc_style_at(traversal: &D, traversal_data: &mut PerLevelTraversalData, - context: &StyleContext, + context: &mut StyleContext, element: E, mut data: &mut AtomicRefMut) where E: TElement, @@ -422,7 +422,7 @@ pub fn recalc_style_at(traversal: &D, // since we have separate bits for each now. fn compute_style(_traversal: &D, traversal_data: &mut PerLevelTraversalData, - context: &StyleContext, + context: &mut StyleContext, element: E, mut data: &mut AtomicRefMut) -> bool where E: TElement, @@ -444,13 +444,10 @@ fn compute_style(_traversal: &D, bf.assert_complete(element); // Check to see whether we can share a style with someone. - let mut style_sharing_candidate_cache = - context.thread_local.style_sharing_candidate_cache.borrow_mut(); - let sharing_result = if element.parent_element().is_none() { StyleSharingResult::CannotShare } else { - unsafe { element.share_style_if_possible(&mut style_sharing_candidate_cache, + unsafe { element.share_style_if_possible(&mut context.thread_local.style_sharing_candidate_cache, shared_context, &mut data) } }; @@ -485,16 +482,16 @@ fn compute_style(_traversal: &D, // Add ourselves to the LRU cache. if let Some(element) = shareable_element { - style_sharing_candidate_cache.insert_if_possible(&element, - &data.styles().primary.values, - relations); + context.thread_local + .style_sharing_candidate_cache + .insert_if_possible(&element, &data.styles().primary.values, relations); } } StyleSharingResult::StyleWasShared(index) => { if opts::get().style_sharing_stats { STYLE_SHARING_CACHE_HITS.fetch_add(1, Ordering::Relaxed); } - style_sharing_candidate_cache.touch(index); + context.thread_local.style_sharing_candidate_cache.touch(index); } } diff --git a/ports/geckolib/glue.rs b/ports/geckolib/glue.rs index 470fa15dcfa..bab3a36f470 100644 --- a/ports/geckolib/glue.rs +++ b/ports/geckolib/glue.rs @@ -875,12 +875,12 @@ pub extern "C" fn Servo_ResolveStyle(element: RawGeckoElementBorrowed, }; let mut tlc = ThreadLocalStyleContext::new(traversal.shared_context()); - let context = StyleContext { + let mut context = StyleContext { shared: traversal.shared_context(), thread_local: &mut tlc, }; - recalc_style_at(&traversal, &mut traversal_data, &context, element, &mut data); + recalc_style_at(&traversal, &mut traversal_data, &mut context, element, &mut data); // The element was either unstyled or needed restyle. If it was unstyled, it may have // additional unstyled children that subsequent traversals won't find now that the style