From 4f4c2a592279631d6313fcc42ef6f998cfba37cd Mon Sep 17 00:00:00 2001 From: Martin Robinson Date: Wed, 19 Jul 2023 09:01:55 +0200 Subject: [PATCH] Remove rayon_croissant and clean up `contains_floats` (#29960) Remove rayon_croissant and refactor the way that information about floats in flows bubbles up. This simplifies the code a good deal and lets us take advantage of some more optimized functions provided by rayon. This removes 2 crates from the dependency tree. In addition, this allows avoiding passing `contains_floats` up from every box tree construction function. This makes things simpler, but also opens up the possibility of passing more of these flags up in the future (such as `contains_counters`). --- Cargo.lock | 17 ---- components/layout_2020/Cargo.toml | 1 - components/layout_2020/flow/construct.rs | 119 ++++++++--------------- components/layout_2020/flow/inline.rs | 2 + components/layout_2020/flow/mod.rs | 23 +++++ components/layout_2020/flow/root.rs | 58 +++++------ components/layout_2020/positioned.rs | 32 +++--- 7 files changed, 108 insertions(+), 144 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 20bcaa2a47e..1909469a737 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3051,7 +3051,6 @@ dependencies = [ "quickcheck", "range", "rayon", - "rayon_croissant", "script_layout_interface", "script_traits", "serde", @@ -3688,12 +3687,6 @@ version = "1.1.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9de3eca27871df31c33b807f834b94ef7d000956f57aa25c5aed9c5f0aae8f6f" -[[package]] -name = "moite_moite" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "eeb5a94c61e12e2cfc16ee3e2b6eca8f126a43c888586626337544a7e824a1af" - [[package]] name = "mozangle" version = "0.3.5" @@ -4814,16 +4807,6 @@ dependencies = [ "num_cpus", ] -[[package]] -name = "rayon_croissant" -version = "0.2.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3e4aafda434bd10fec689858e2b1d713d0b784b1e60df3761ac8fa727d7e8e27" -dependencies = [ - "moite_moite", - "rayon", -] - [[package]] name = "redox_syscall" version = "0.2.13" diff --git a/components/layout_2020/Cargo.toml b/components/layout_2020/Cargo.toml index 64e46b3ff2f..ab98fcd9691 100644 --- a/components/layout_2020/Cargo.toml +++ b/components/layout_2020/Cargo.toml @@ -34,7 +34,6 @@ net_traits = { path = "../net_traits" } parking_lot = { workspace = true } range = { path = "../range" } rayon = { workspace = true } -rayon_croissant = "0.2.0" script_layout_interface = { path = "../script_layout_interface" } script_traits = { path = "../script_traits" } serde = { workspace = true } diff --git a/components/layout_2020/flow/construct.rs b/components/layout_2020/flow/construct.rs index 2989df6c7e5..370bf3ed134 100644 --- a/components/layout_2020/flow/construct.rs +++ b/components/layout_2020/flow/construct.rs @@ -13,7 +13,6 @@ use crate::formatting_contexts::IndependentFormattingContext; use crate::positioned::AbsolutelyPositionedBox; use crate::style_ext::{ComputedValuesExt, DisplayGeneratingBox, DisplayInside, DisplayOutside}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; -use rayon_croissant::ParallelIteratorExt; use servo_arc::Arc; use std::borrow::Cow; use std::convert::{TryFrom, TryInto}; @@ -34,18 +33,19 @@ impl BlockFormattingContext { where Node: NodeExt<'dom>, { - let (contents, contains_floats) = BlockContainer::construct( + let contents = BlockContainer::construct( context, info, contents, propagated_text_decoration_line, is_list_item, ); - let bfc = Self { + let contains_floats = contents.contains_floats(); + + Self { contents, - contains_floats: contains_floats == ContainsFloats::Yes, - }; - bfc + contains_floats, + } } pub fn construct_for_text_runs<'dom>( @@ -61,6 +61,7 @@ impl BlockFormattingContext { inline_level_boxes, text_decoration_line, has_first_formatted_line: true, + contains_floats: false, }; let contents = BlockContainer::InlineFormattingContext(ifc); let bfc = Self { @@ -163,9 +164,6 @@ struct BlockContainerBuilder<'dom, 'style, Node> { /// The style of the anonymous block boxes pushed to the list of block-level /// boxes, if any (see `end_ongoing_inline_formatting_context`). anonymous_style: Option>, - - /// Whether the resulting block container contains any float box. - contains_floats: ContainsFloats, } impl BlockContainer { @@ -175,7 +173,7 @@ impl BlockContainer { contents: NonReplacedContents, propagated_text_decoration_line: TextDecorationLine, is_list_item: bool, - ) -> (BlockContainer, ContainsFloats) + ) -> BlockContainer where Node: NodeExt<'dom>, { @@ -191,7 +189,6 @@ impl BlockContainer { ), ongoing_inline_boxes_stack: Vec::new(), anonymous_style: None, - contains_floats: ContainsFloats::No, }; if is_list_item { @@ -222,43 +219,28 @@ impl BlockContainer { .is_empty() { if builder.block_level_boxes.is_empty() { - let container = BlockContainer::InlineFormattingContext( + return BlockContainer::InlineFormattingContext( builder.ongoing_inline_formatting_context, ); - return (container, builder.contains_floats); } builder.end_ongoing_inline_formatting_context(); } - let mut contains_floats = builder.contains_floats; - let mapfold = |contains_floats: &mut ContainsFloats, creator: BlockLevelJob<'dom, _>| { - let (block_level_box, box_contains_floats) = creator.finish(context); - *contains_floats |= box_contains_floats; - block_level_box - }; let block_level_boxes = if context.use_rayon { builder .block_level_boxes .into_par_iter() - .mapfold_reduce_into( - &mut contains_floats, - mapfold, - || ContainsFloats::No, - |left, right| { - *left |= right; - }, - ) + .map(|block_level_job| block_level_job.finish(context)) .collect() } else { builder .block_level_boxes .into_iter() - .map(|x| mapfold(&mut contains_floats, x)) + .map(|block_level_job| block_level_job.finish(context)) .collect() }; - let container = BlockContainer::BlockLevelBoxes(block_level_boxes); - (container, contains_floats) + BlockContainer::BlockLevelBoxes(block_level_boxes) } } @@ -662,8 +644,6 @@ where contents: Contents, box_slot: BoxSlot<'dom>, ) { - self.contains_floats = ContainsFloats::Yes; - if !self.has_ongoing_inline_formatting_context() { let kind = BlockLevelCreator::OutOfFlowFloatBox { contents, @@ -681,6 +661,7 @@ where display_inside, contents, ))); + self.ongoing_inline_formatting_context.contains_floats = true; self.current_inline_level_boxes().push(box_.clone()); box_slot.set(LayoutBox::InlineLevel(box_)) } @@ -748,17 +729,18 @@ impl<'dom, Node> BlockLevelJob<'dom, Node> where Node: NodeExt<'dom>, { - fn finish(self, context: &LayoutContext) -> (ArcRefCell, ContainsFloats) { + fn finish(self, context: &LayoutContext) -> ArcRefCell { let info = &self.info; - let (block_level_box, contains_floats) = match self.kind { - BlockLevelCreator::SameFormattingContextBlock(contents) => { - let (contents, contains_floats) = contents.finish(context, info); - let block_level_box = ArcRefCell::new(BlockLevelBox::SameFormattingContextBlock { + let block_level_box = match self.kind { + BlockLevelCreator::SameFormattingContextBlock(intermediate_block_container) => { + let contents = intermediate_block_container.finish(context, info); + let contains_floats = contents.contains_floats(); + ArcRefCell::new(BlockLevelBox::SameFormattingContextBlock { base_fragment_info: info.into(), contents, style: Arc::clone(&info.style), - }); - (block_level_box, contains_floats) + contains_floats, + }) }, BlockLevelCreator::Independent { display_inside, @@ -772,35 +754,32 @@ where contents, propagated_text_decoration_line, ); - ( - ArcRefCell::new(BlockLevelBox::Independent(context)), - ContainsFloats::No, - ) + ArcRefCell::new(BlockLevelBox::Independent(context)) }, BlockLevelCreator::OutOfFlowAbsolutelyPositionedBox { display_inside, contents, - } => { - let block_level_box = ArcRefCell::new( - BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(ArcRefCell::new( - AbsolutelyPositionedBox::construct(context, info, display_inside, contents), - )), - ); - (block_level_box, ContainsFloats::No) - }, + } => ArcRefCell::new(BlockLevelBox::OutOfFlowAbsolutelyPositionedBox( + ArcRefCell::new(AbsolutelyPositionedBox::construct( + context, + info, + display_inside, + contents, + )), + )), BlockLevelCreator::OutOfFlowFloatBox { display_inside, contents, - } => { - let block_level_box = ArcRefCell::new(BlockLevelBox::OutOfFlowFloatBox( - FloatBox::construct(context, info, display_inside, contents), - )); - (block_level_box, ContainsFloats::Yes) - }, + } => ArcRefCell::new(BlockLevelBox::OutOfFlowFloatBox(FloatBox::construct( + context, + info, + display_inside, + contents, + ))), }; self.box_slot .set(LayoutBox::BlockLevel(block_level_box.clone())); - (block_level_box, contains_floats) + block_level_box } } @@ -809,7 +788,7 @@ impl IntermediateBlockContainer { self, context: &LayoutContext, info: &NodeAndStyleInfo, - ) -> (BlockContainer, ContainsFloats) + ) -> BlockContainer where Node: NodeExt<'dom>, { @@ -826,28 +805,8 @@ impl IntermediateBlockContainer { is_list_item, ), IntermediateBlockContainer::InlineFormattingContext(ifc) => { - // If that inline formatting context contained any float, those - // were already taken into account during the first phase of - // box construction. - ( - BlockContainer::InlineFormattingContext(ifc), - ContainsFloats::No, - ) + BlockContainer::InlineFormattingContext(ifc) }, } } } - -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(crate) enum ContainsFloats { - No, - Yes, -} - -impl std::ops::BitOrAssign for ContainsFloats { - fn bitor_assign(&mut self, other: Self) { - if other == ContainsFloats::Yes { - *self = ContainsFloats::Yes; - } - } -} diff --git a/components/layout_2020/flow/inline.rs b/components/layout_2020/flow/inline.rs index c274e615042..d88432c8cb7 100644 --- a/components/layout_2020/flow/inline.rs +++ b/components/layout_2020/flow/inline.rs @@ -39,6 +39,7 @@ pub(crate) struct InlineFormattingContext { // Whether this IFC contains the 1st formatted line of an element // https://www.w3.org/TR/css-pseudo-4/#first-formatted-line pub(super) has_first_formatted_line: bool, + pub(super) contains_floats: bool, } #[derive(Debug, Serialize)] @@ -172,6 +173,7 @@ impl InlineFormattingContext { inline_level_boxes: Default::default(), text_decoration_line, has_first_formatted_line, + contains_floats: false, } } diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index da57d42334e..d430eed74cd 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -49,6 +49,17 @@ pub(crate) enum BlockContainer { InlineFormattingContext(InlineFormattingContext), } +impl BlockContainer { + fn contains_floats(&self) -> bool { + match self { + BlockContainer::BlockLevelBoxes(boxes) => boxes + .iter() + .any(|block_level_box| block_level_box.borrow().contains_floats()), + BlockContainer::InlineFormattingContext { .. } => true, + } + } +} + #[derive(Debug, Serialize)] pub(crate) enum BlockLevelBox { SameFormattingContextBlock { @@ -56,6 +67,7 @@ pub(crate) enum BlockLevelBox { #[serde(skip_serializing)] style: Arc, contents: BlockContainer, + contains_floats: bool, }, OutOfFlowAbsolutelyPositionedBox(ArcRefCell), OutOfFlowFloatBox(FloatBox), @@ -63,6 +75,16 @@ pub(crate) enum BlockLevelBox { } impl BlockLevelBox { + fn contains_floats(&self) -> bool { + match self { + BlockLevelBox::SameFormattingContextBlock { + contains_floats, .. + } => *contains_floats, + BlockLevelBox::OutOfFlowFloatBox { .. } => true, + _ => false, + } + } + fn find_block_margin_collapsing_with_parent( &self, collected_margin: &mut CollapsedMargin, @@ -496,6 +518,7 @@ impl BlockLevelBox { base_fragment_info: tag, style, contents, + .. } => Fragment::Box(positioning_context.layout_maybe_position_relative_fragment( layout_context, containing_block, diff --git a/components/layout_2020/flow/root.rs b/components/layout_2020/flow/root.rs index 1262f7408aa..2a1603990d5 100644 --- a/components/layout_2020/flow/root.rs +++ b/components/layout_2020/flow/root.rs @@ -7,7 +7,6 @@ use crate::context::LayoutContext; use crate::dom::{LayoutBox, NodeExt}; use crate::dom_traversal::{iter_child_nodes, Contents, NodeAndStyleInfo}; use crate::flexbox::FlexLevelBox; -use crate::flow::construct::ContainsFloats; use crate::flow::float::FloatBox; use crate::flow::inline::InlineLevelBox; use crate::flow::{BlockContainer, BlockFormattingContext, BlockLevelBox}; @@ -45,15 +44,17 @@ impl BoxTree { where Node: 'dom + Copy + LayoutNode<'dom> + Send + Sync, { - let (contains_floats, boxes) = construct_for_root_element(&context, root_element); + let boxes = construct_for_root_element(&context, root_element); // Zero box for `:root { display: none }`, one for the root element otherwise. assert!(boxes.len() <= 1); + let contents = BlockContainer::BlockLevelBoxes(boxes); + let contains_floats = contents.contains_floats(); Self { root: BlockFormattingContext { - contains_floats: contains_floats == ContainsFloats::Yes, - contents: BlockContainer::BlockLevelBoxes(boxes), + contents, + contains_floats, }, canvas_background: CanvasBackground::for_root_element(context, root_element), } @@ -209,14 +210,14 @@ impl BoxTree { fn construct_for_root_element<'dom>( context: &LayoutContext, root_element: impl NodeExt<'dom>, -) -> (ContainsFloats, Vec>) { +) -> Vec> { let info = NodeAndStyleInfo::new(root_element, root_element.style(context)); let box_style = info.style.get_box(); let display_inside = match Display::from(box_style.display) { Display::None => { root_element.unset_all_boxes(); - return (ContainsFloats::No, Vec::new()); + return Vec::new(); }, Display::Contents => { // Unreachable because the style crate adjusts the computed values: @@ -230,41 +231,32 @@ fn construct_for_root_element<'dom>( let contents = ReplacedContent::for_element(root_element).map_or(Contents::OfElement, Contents::Replaced); - let (contains_floats, root_box) = if box_style.position.is_absolutely_positioned() { - ( - ContainsFloats::No, - BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(ArcRefCell::new( - AbsolutelyPositionedBox::construct(context, &info, display_inside, contents), - )), - ) + let root_box = if box_style.position.is_absolutely_positioned() { + BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(ArcRefCell::new( + AbsolutelyPositionedBox::construct(context, &info, display_inside, contents), + )) } else if box_style.float.is_floating() { - ( - ContainsFloats::Yes, - BlockLevelBox::OutOfFlowFloatBox(FloatBox::construct( - context, - &info, - display_inside, - contents, - )), - ) + BlockLevelBox::OutOfFlowFloatBox(FloatBox::construct( + context, + &info, + display_inside, + contents, + )) } else { let propagated_text_decoration_line = info.style.clone_text_decoration_line(); - ( - ContainsFloats::No, - BlockLevelBox::Independent(IndependentFormattingContext::construct( - context, - &info, - display_inside, - contents, - propagated_text_decoration_line, - )), - ) + BlockLevelBox::Independent(IndependentFormattingContext::construct( + context, + &info, + display_inside, + contents, + propagated_text_decoration_line, + )) }; let root_box = ArcRefCell::new(root_box); root_element .element_box_slot() .set(LayoutBox::BlockLevel(root_box.clone())); - (contains_floats, vec![root_box]) + vec![root_box] } impl BoxTree { diff --git a/components/layout_2020/positioned.rs b/components/layout_2020/positioned.rs index 65492a1afa3..fa94773965b 100644 --- a/components/layout_2020/positioned.rs +++ b/components/layout_2020/positioned.rs @@ -14,8 +14,8 @@ use crate::geom::flow_relative::{Rect, Sides, Vec2}; use crate::geom::{LengthOrAuto, LengthPercentageOrAuto}; use crate::style_ext::{ComputedValuesExt, DisplayInside}; use crate::{ContainingBlock, DefiniteContainingBlock}; -use rayon::iter::{IntoParallelRefMutIterator, ParallelExtend}; -use rayon_croissant::ParallelIteratorExt; +use rayon::iter::IntoParallelRefMutIterator; +use rayon::prelude::{IndexedParallelIterator, ParallelIterator}; use style::computed_values::position::T as Position; use style::properties::ComputedValues; use style::values::computed::{CSSPixelLength, Length}; @@ -354,21 +354,27 @@ impl HoistedAbsolutelyPositionedBox { containing_block: &DefiniteContainingBlock, ) { if layout_context.use_rayon { - fragments.par_extend(boxes.par_iter_mut().mapfold_reduce_into( - for_nearest_containing_block_for_all_descendants, - |for_nearest_containing_block_for_all_descendants, box_| { - let new_fragment = ArcRefCell::new(Fragment::Box(box_.layout( + let mut new_fragments = Vec::new(); + let mut new_hoisted_boxes = Vec::new(); + + boxes + .par_iter_mut() + .map(|hoisted_box| { + let mut new_hoisted_boxes: Vec = Vec::new(); + let new_fragment = ArcRefCell::new(Fragment::Box(hoisted_box.layout( layout_context, - for_nearest_containing_block_for_all_descendants, + &mut new_hoisted_boxes, containing_block, ))); - box_.fragment.borrow_mut().fragment = Some(new_fragment.clone()); - new_fragment - }, - Vec::new, - vec_append_owned, - )) + hoisted_box.fragment.borrow_mut().fragment = Some(new_fragment.clone()); + (new_fragment, new_hoisted_boxes) + }) + .unzip_into_vecs(&mut new_fragments, &mut new_hoisted_boxes); + + fragments.extend(new_fragments); + for_nearest_containing_block_for_all_descendants + .extend(new_hoisted_boxes.into_iter().flatten()); } else { fragments.extend(boxes.iter_mut().map(|box_| { let new_fragment = ArcRefCell::new(Fragment::Box(box_.layout(