From d717382c0c922e92165d9eead86c7b04e9ee4bc8 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sun, 8 Dec 2019 10:22:11 +0100 Subject: [PATCH 1/3] Disable in Layout 2020 an assertion that does not apply there --- components/style/values/specified/box.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index 7fd5a250e06..1fea857d3e8 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -454,10 +454,11 @@ impl ToCss for Display { where W: fmt::Write, { + #[cfg(any(feature = "servo-layout-2013", feature = "gecko"))] debug_assert_ne!( self.inside(), DisplayInside::Flow, - "`flow` never appears in `display` computed value" + "`flow` fears in `display` computed value" ); let outside = self.outside(); let inside = match self.inside() { From aade6030258ac644739cc3e7658c16f72fec5bd2 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sun, 8 Dec 2019 11:09:51 +0100 Subject: [PATCH 2/3] Support STYLO_THREADS=1 --- components/layout_thread_2020/lib.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 45202d51ad9..35b4c92c21b 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -1076,14 +1076,18 @@ impl LayoutThread { }; let rayon_pool = STYLE_THREAD_POOL.pool(); - let rayon_pool = rayon_pool.as_ref().unwrap(); + let rayon_pool = rayon_pool.as_ref(); let box_tree = if token.should_traverse() { - driver::traverse_dom(&traversal, token, Some(rayon_pool)); + driver::traverse_dom(&traversal, token, rayon_pool); let root_node = document.root_element().unwrap().as_node(); - let box_tree = - rayon_pool.install(|| BoxTreeRoot::construct(traversal.context(), root_node)); + let build_box_tree = || BoxTreeRoot::construct(traversal.context(), root_node); + let box_tree = if let Some(pool) = rayon_pool { + pool.install(build_box_tree) + } else { + build_box_tree() + }; Some(box_tree) } else { None @@ -1096,8 +1100,12 @@ impl LayoutThread { self.viewport_size.width.to_f32_px(), self.viewport_size.height.to_f32_px(), ); - let fragment_tree = - rayon_pool.install(|| box_tree.layout(&layout_context, viewport_size)); + let run_layout = || box_tree.layout(&layout_context, viewport_size); + let fragment_tree = if let Some(pool) = rayon_pool { + pool.install(run_layout) + } else { + run_layout() + }; *self.box_tree_root.borrow_mut() = Some(box_tree); *self.fragment_tree_root.borrow_mut() = Some(fragment_tree); } From c895e3d2363a868c3ac9b3dcb4f2a2a4fd1c6227 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Sun, 8 Dec 2019 15:00:40 +0100 Subject: [PATCH 3/3] Disable use of rayon with `--layout-threads 1` instead of panicking --- components/layout_2020/context.rs | 1 + components/layout_2020/flow/construct.rs | 54 ++++++++++++++---------- components/layout_2020/flow/mod.rs | 6 +-- components/layout_2020/flow/root.rs | 19 ++++++--- components/layout_2020/positioned.rs | 11 +++-- components/layout_thread_2020/lib.rs | 1 + 6 files changed, 56 insertions(+), 36 deletions(-) diff --git a/components/layout_2020/context.rs b/components/layout_2020/context.rs index 42106a76387..2caa66df068 100644 --- a/components/layout_2020/context.rs +++ b/components/layout_2020/context.rs @@ -11,6 +11,7 @@ use style::context::SharedStyleContext; pub struct LayoutContext<'a> { pub id: PipelineId, + pub use_rayon: bool, pub style_context: SharedStyleContext<'a>, pub font_cache_thread: Mutex, } diff --git a/components/layout_2020/flow/construct.rs b/components/layout_2020/flow/construct.rs index 896293831bc..4a397add2c3 100644 --- a/components/layout_2020/flow/construct.rs +++ b/components/layout_2020/flow/construct.rs @@ -168,12 +168,11 @@ impl BlockContainer { builder.end_ongoing_inline_formatting_context(); } - type Intermediate = IntermediateBlockLevelBox; - struct Target { + struct Accumulator { contains_floats: ContainsFloats, outer_content_sizes_of_children: ContentSizes, } - impl Default for Target { + impl Default for Accumulator { fn default() -> Self { Self { contains_floats: ContainsFloats::No, @@ -181,37 +180,46 @@ impl BlockContainer { } } } - let mut target = Target { + let mut acc = Accumulator { contains_floats: builder.contains_floats, outer_content_sizes_of_children: ContentSizes::zero(), }; - let iter = builder.block_level_boxes.into_par_iter(); - let iter = iter.mapfold_reduce_into( - &mut target, - |target, (intermediate, box_slot): (Intermediate<_>, BoxSlot<'_>)| { + let mapfold = + |acc: &mut Accumulator, + (intermediate, box_slot): (IntermediateBlockLevelBox<_>, BoxSlot<'_>)| { let (block_level_box, box_contains_floats) = intermediate.finish( context, - content_sizes - .if_requests_inline(|| &mut target.outer_content_sizes_of_children), + content_sizes.if_requests_inline(|| &mut acc.outer_content_sizes_of_children), ); - target.contains_floats |= box_contains_floats; + acc.contains_floats |= box_contains_floats; box_slot.set(LayoutBox::BlockLevel(block_level_box.clone())); block_level_box - }, - |left, right| { - left.contains_floats |= right.contains_floats; - if content_sizes.requests_inline() { - left.outer_content_sizes_of_children - .max_assign(&right.outer_content_sizes_of_children) - } - }, - ); - let container = BlockContainer::BlockLevelBoxes(iter.collect()); + }; + let block_level_boxes = if context.use_rayon { + builder + .block_level_boxes + .into_par_iter() + .mapfold_reduce_into(&mut acc, mapfold, |left, right| { + left.contains_floats |= right.contains_floats; + if content_sizes.requests_inline() { + left.outer_content_sizes_of_children + .max_assign(&right.outer_content_sizes_of_children) + } + }) + .collect() + } else { + builder + .block_level_boxes + .into_iter() + .map(|x| mapfold(&mut acc, x)) + .collect() + }; + let container = BlockContainer::BlockLevelBoxes(block_level_boxes); - let Target { + let Accumulator { contains_floats, outer_content_sizes_of_children, - } = target; + } = acc; let content_sizes = content_sizes.compute(|| outer_content_sizes_of_children); (container, contains_floats, content_sizes) } diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index ed578c825ef..3e45defb365 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -135,7 +135,7 @@ fn layout_block_level_children<'a>( containing_block: &ContainingBlock, tree_rank: usize, absolutely_positioned_fragments: &mut Vec>, - float_context: Option<&mut FloatContext>, + mut float_context: Option<&mut FloatContext>, collapsible_with_parent_start_margin: CollapsibleWithParentStartMargin, ) -> FlowLayout { fn place_block_level_fragment(fragment: &mut Fragment, placement_state: &mut PlacementState) { @@ -203,7 +203,7 @@ fn layout_block_level_children<'a>( current_block_direction_position: Length::zero(), }; let mut fragments: Vec<_>; - if let Some(float_context) = float_context { + if float_context.is_some() || !layout_context.use_rayon { // Because floats are involved, we do layout for this block formatting context // in tree order without parallelism. This enables mutable access // to a `FloatContext` that tracks every float encountered so far (again in tree order). @@ -216,7 +216,7 @@ fn layout_block_level_children<'a>( containing_block, tree_rank, absolutely_positioned_fragments, - Some(float_context), + float_context.as_mut().map(|c| &mut **c), ); place_block_level_fragment(&mut fragment, &mut placement_state); fragment diff --git a/components/layout_2020/flow/root.rs b/components/layout_2020/flow/root.rs index 2415d18e0ca..15dc6d404b6 100644 --- a/components/layout_2020/flow/root.rs +++ b/components/layout_2020/flow/root.rs @@ -12,7 +12,7 @@ use crate::formatting_contexts::IndependentFormattingContext; use crate::fragments::Fragment; use crate::geom; use crate::geom::flow_relative::Vec2; -use crate::positioned::AbsolutelyPositionedBox; +use crate::positioned::{AbsolutelyPositionedBox, AbsolutelyPositionedFragment}; use crate::replaced::ReplacedContent; use crate::sizing::ContentSizesRequest; use crate::style_ext::{Display, DisplayGeneratingBox, DisplayInside}; @@ -118,11 +118,18 @@ impl BoxTreeRoot { &mut absolutely_positioned_fragments, ); - independent_layout.fragments.par_extend( - absolutely_positioned_fragments - .par_iter() - .map(|a| a.layout(layout_context, &initial_containing_block)), - ); + let map = + |a: &AbsolutelyPositionedFragment| a.layout(layout_context, &initial_containing_block); + if layout_context.use_rayon { + independent_layout + .fragments + .par_extend(absolutely_positioned_fragments.par_iter().map(map)) + } else { + independent_layout + .fragments + .extend(absolutely_positioned_fragments.iter().map(map)) + } + FragmentTreeRoot(independent_layout.fragments) } } diff --git a/components/layout_2020/positioned.rs b/components/layout_2020/positioned.rs index 9459f3c3f84..b9c64ea54d3 100644 --- a/components/layout_2020/positioned.rs +++ b/components/layout_2020/positioned.rs @@ -139,11 +139,14 @@ impl<'a> AbsolutelyPositionedFragment<'a> { size: padding_rect.size.clone(), style, }; + let map = |a: &AbsolutelyPositionedFragment| a.layout(layout_context, &containing_block); + let children = if layout_context.use_rayon { + absolute.par_iter().map(map).collect() + } else { + absolute.iter().map(map).collect() + }; fragments.push(Fragment::Anonymous(AnonymousFragment { - children: absolute - .par_iter() - .map(|a| a.layout(layout_context, &containing_block)) - .collect(), + children, rect: padding_rect, mode: style.writing_mode, })) diff --git a/components/layout_thread_2020/lib.rs b/components/layout_thread_2020/lib.rs index 35b4c92c21b..bb9f614831c 100644 --- a/components/layout_thread_2020/lib.rs +++ b/components/layout_thread_2020/lib.rs @@ -568,6 +568,7 @@ impl LayoutThread { snapshot_map: snapshot_map, }, font_cache_thread: Mutex::new(self.font_cache_thread.clone()), + use_rayon: STYLE_THREAD_POOL.pool().is_some(), } }