From 91026444701cfd68658beb21fbf446f6ed8723e6 Mon Sep 17 00:00:00 2001 From: Oriol Brufau Date: Wed, 13 Nov 2024 10:56:02 +0100 Subject: [PATCH] Use a RwLock to cache inline_content_sizes() (#34232) In order to support size keywords in block layout, we may need to call `inline_content_sizes()` in order to compute the min/max-content sizes. But this required a mutable reference in order the update the cache, and in various places we already had mutable references. So this switches the cache into a RwLock to avoid needing mutable refs. Note OnceCell wouldn't work because it's not thread-safe. Signed-off-by: Oriol Brufau --- components/layout_2020/flexbox/construct.rs | 2 +- components/layout_2020/flexbox/layout.rs | 63 +++++++++---------- components/layout_2020/flow/float.rs | 2 +- components/layout_2020/flow/inline/mod.rs | 12 ++-- components/layout_2020/flow/mod.rs | 14 ++--- components/layout_2020/formatting_contexts.rs | 39 +++++++----- components/layout_2020/positioned.rs | 4 +- components/layout_2020/table/construct.rs | 4 +- components/layout_2020/table/layout.rs | 10 +-- 9 files changed, 76 insertions(+), 74 deletions(-) diff --git a/components/layout_2020/flexbox/construct.rs b/components/layout_2020/flexbox/construct.rs index c4c5ff57d60..260e5266b82 100644 --- a/components/layout_2020/flexbox/construct.rs +++ b/components/layout_2020/flexbox/construct.rs @@ -171,7 +171,7 @@ where let non_replaced = NonReplacedFormattingContext { base_fragment_info: info.into(), style: info.style.clone(), - content_sizes_result: None, + content_sizes_result: Default::default(), contents: NonReplacedFormattingContextContents::Flow( block_formatting_context, ), diff --git a/components/layout_2020/flexbox/layout.rs b/components/layout_2020/flexbox/layout.rs index a968926cdca..11bfedb3255 100644 --- a/components/layout_2020/flexbox/layout.rs +++ b/components/layout_2020/flexbox/layout.rs @@ -6,10 +6,10 @@ use std::cell::Cell; use std::cmp::Ordering; use app_units::Au; -use atomic_refcell::AtomicRefMut; +use atomic_refcell::AtomicRef; use itertools::izip; use rayon::iter::{ - IndexedParallelIterator, IntoParallelRefMutIterator, ParallelDrainRange, ParallelIterator, + IndexedParallelIterator, IntoParallelRefIterator, ParallelDrainRange, ParallelIterator, }; use style::computed_values::position::T as Position; use style::logical_geometry::Direction; @@ -56,7 +56,7 @@ struct FlexContext<'a> { /// A flex item with some intermediate results struct FlexItem<'a> { - box_: &'a mut FlexItemBox, + box_: &'a FlexItemBox, content_box_size: FlexRelativeVec2, content_min_size: FlexRelativeVec2, content_max_size: FlexRelativeVec2>, @@ -399,7 +399,7 @@ impl FlexContainer { ) )] pub fn inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, constraint_space: &ConstraintSpace, ) -> InlineContentSizesResult { @@ -416,7 +416,7 @@ impl FlexContainer { } fn cross_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, containing_block_for_children: &IndefiniteContainingBlock, ) -> InlineContentSizesResult { @@ -429,13 +429,13 @@ impl FlexContainer { let mut sizes = ContentSizes::zero(); let mut depends_on_block_constraints = false; for kid in self.children.iter() { - let kid = &mut *kid.borrow_mut(); + let kid = &*kid.borrow(); match kid { FlexLevelBox::FlexItem(item) => { // TODO: For the max-content size we should distribute items into // columns, and sum the column sizes and gaps. // TODO: Use the proper automatic minimum size. - let ifc = &mut item.independent_formatting_context; + let ifc = &item.independent_formatting_context; let result = ifc.outer_inline_content_sizes( layout_context, containing_block_for_children, @@ -476,7 +476,7 @@ impl FlexContainer { let container_is_horizontal = self.style.writing_mode.is_horizontal(); for kid in self.children.iter() { - let kid = &mut *kid.borrow_mut(); + let kid = &*kid.borrow(); match kid { FlexLevelBox::FlexItem(item) => { sum_of_flex_grow_factors += item.style().get_position().flex_grow.0; @@ -664,13 +664,13 @@ impl FlexContainer { .children .iter() .map(|arcrefcell| { - let borrowed = arcrefcell.borrow_mut(); + let borrowed = arcrefcell.borrow(); match &*borrowed { FlexLevelBox::OutOfFlowAbsolutelyPositionedBox(absolutely_positioned) => { FlexContent::AbsolutelyPositionedBox(absolutely_positioned.clone()) }, FlexLevelBox::FlexItem(_) => { - let item = AtomicRefMut::map(borrowed, |child| match child { + let item = AtomicRef::map(borrowed, |child| match child { FlexLevelBox::FlexItem(item) => item, _ => unreachable!(), }); @@ -681,7 +681,7 @@ impl FlexContainer { }) .collect::>(); - let flex_item_boxes = flex_items.iter_mut().map(|child| &mut **child); + let flex_item_boxes = flex_items.iter().map(|child| &**child); let flex_items = flex_item_boxes .map(|flex_item_box| FlexItem::new(&flex_context, flex_item_box)) .collect::>(); @@ -1088,7 +1088,7 @@ fn allocate_free_cross_space_for_flex_line( } impl<'a> FlexItem<'a> { - fn new(flex_context: &FlexContext, box_: &'a mut FlexItemBox) -> Self { + fn new(flex_context: &FlexContext, box_: &'a FlexItemBox) -> Self { let containing_block = flex_context.containing_block; let parent_writing_mode = containing_block.style.writing_mode; let item_writing_mode = box_.style().writing_mode; @@ -1321,7 +1321,7 @@ struct InitialFlexLineLayout<'a> { impl InitialFlexLineLayout<'_> { fn new<'items>( flex_context: &FlexContext, - mut items: Vec>, + items: Vec>, outer_hypothetical_main_sizes_sum: Au, container_main_size: Au, main_gap: Au, @@ -1335,7 +1335,7 @@ impl InitialFlexLineLayout<'_> { // https://drafts.csswg.org/css-flexbox/#algo-cross-item let layout_results = items - .par_iter_mut() + .par_iter() .zip(&item_used_main_sizes) .map(|(item, used_main_size)| { item.layout(*used_main_size, flex_context, None, None) @@ -1833,7 +1833,7 @@ impl FlexItem<'_> { /// > size and the given available space, treating `auto` as `fit-content`. #[allow(clippy::too_many_arguments)] fn layout( - &mut self, + &self, used_main_size: Au, flex_context: &FlexContext, used_cross_size_override: Option, @@ -2205,7 +2205,7 @@ impl FlexItem<'_> { impl FlexItemBox { fn main_content_size_info<'a>( - &mut self, + &self, layout_context: &LayoutContext, containing_block: &IndefiniteContainingBlock, container_is_horizontal: bool, @@ -2214,7 +2214,7 @@ impl FlexItemBox { ) -> FlexItemBoxInlineContentSizesInfo { let flex_axis = config.flex_axis; let main_start_cross_start = config.main_start_cross_start_sides_are; - let style = self.style().clone(); + let style = &self.style(); let item_writing_mode = style.writing_mode; let item_is_horizontal = item_writing_mode.is_horizontal(); let cross_axis_is_item_block_axis = @@ -2241,7 +2241,7 @@ impl FlexItemBox { cross: padding_border.cross, } + margin_auto_is_zero.sum_by_axis(); let item_with_auto_cross_size_stretches_to_container_size = - config.item_with_auto_cross_size_stretches_to_container_size(&style, &margin); + config.item_with_auto_cross_size_stretches_to_container_size(style, &margin); let automatic_min_size = self.automatic_min_size( layout_context, containing_block, @@ -2274,7 +2274,7 @@ impl FlexItemBox { block: content_min_box_size.block.auto_is(|| automatic_min_size), } }; - let block_content_size_callback = |item: &mut FlexItemBox| { + let block_content_size_callback = |item: &FlexItemBox| { item.layout_for_block_content_size( flex_context_getter(), &pbm, @@ -2434,7 +2434,7 @@ impl FlexItemBox { /// This is an implementation of . #[allow(clippy::too_many_arguments)] fn automatic_min_size( - &mut self, + &self, layout_context: &LayoutContext, containing_block: &IndefiniteContainingBlock, cross_axis_is_item_block_axis: bool, @@ -2443,14 +2443,11 @@ impl FlexItemBox { max_size: FlexRelativeVec2>, pbm_auto_is_zero: &FlexRelativeVec2, auto_cross_size_stretches_to_container_size: bool, - block_content_size_callback: impl FnOnce(&mut FlexItemBox) -> Au, + block_content_size_callback: impl FnOnce(&FlexItemBox) -> Au, ) -> Au { // FIXME(stshine): Consider more situations when auto min size is not needed. - if self - .independent_formatting_context - .style() - .establishes_scroll_container() - { + let style = &self.independent_formatting_context.style(); + if style.establishes_scroll_container() { return Au::zero(); } @@ -2503,7 +2500,7 @@ impl FlexItemBox { // > preferred aspect ratio, by any definite minimum and maximum cross sizes converted through the // > aspect ratio. let main_content_size = if cross_axis_is_item_block_axis { - let writing_mode = self.independent_formatting_context.style().writing_mode; + let writing_mode = style.writing_mode; let constraint_space = ConstraintSpace::new(cross_size, writing_mode); self.independent_formatting_context .inline_content_sizes(layout_context, &constraint_space, containing_block) @@ -2540,7 +2537,7 @@ impl FlexItemBox { /// #[allow(clippy::too_many_arguments)] fn flex_base_size( - &mut self, + &self, layout_context: &LayoutContext, containing_block: &IndefiniteContainingBlock, container_definite_inner_size: FlexRelativeVec2>, @@ -2551,9 +2548,9 @@ impl FlexItemBox { padding_border_sums: FlexRelativeVec2, pbm_auto_is_zero: &FlexRelativeVec2, item_with_auto_cross_size_stretches_to_container_size: bool, - block_content_size_callback: impl FnOnce(&mut FlexItemBox) -> Au, + block_content_size_callback: impl FnOnce(&FlexItemBox) -> Au, ) -> (Au, bool) { - let flex_item = &mut self.independent_formatting_context; + let flex_item = &self.independent_formatting_context; let style = flex_item.style(); let used_flex_basis = match &style.get_position().flex_basis { @@ -2655,7 +2652,7 @@ impl FlexItemBox { let flex_basis = if cross_axis_is_item_block_axis { // The main axis is the inline axis, so we can get the content size from the normal // preferred widths calculation. - let writing_mode = flex_item.style().writing_mode; + let writing_mode = style.writing_mode; let constraint_space = ConstraintSpace::new( SizeConstraint::new( content_box_size.cross.non_auto(), @@ -2696,7 +2693,7 @@ impl FlexItemBox { ) )] fn layout_for_block_content_size( - &mut self, + &self, flex_context: &FlexContext, padding_border_margin: &PaddingBorderMargin, mut content_box_size: LogicalVec2, @@ -2711,7 +2708,7 @@ impl FlexItemBox { .collects_for_nearest_positioned_ancestor(), ); - match &mut self.independent_formatting_context { + match &self.independent_formatting_context { IndependentFormattingContext::Replaced(replaced) => { content_box_size.inline = content_box_size.inline.map(|v| v.max(Au::zero())); if intrinsic_sizing_mode == IntrinsicSizingMode::Size { diff --git a/components/layout_2020/flow/float.rs b/components/layout_2020/flow/float.rs index bf2c468e07d..307cbfc68cc 100644 --- a/components/layout_2020/flow/float.rs +++ b/components/layout_2020/flow/float.rs @@ -891,7 +891,7 @@ impl FloatBox { /// the float containing block formatting context. A later step adjusts the position /// to be relative to the containing block. pub fn layout( - &mut self, + &self, layout_context: &LayoutContext, positioning_context: &mut PositioningContext, containing_block: &ContainingBlock, diff --git a/components/layout_2020/flow/inline/mod.rs b/components/layout_2020/flow/inline/mod.rs index 13714235b0b..b9f97f62622 100644 --- a/components/layout_2020/flow/inline/mod.rs +++ b/components/layout_2020/flow/inline/mod.rs @@ -1656,7 +1656,7 @@ impl InlineFormattingContext { } for item in self.inline_items.iter() { - let item = &mut *item.borrow_mut(); + let item = &*item.borrow(); // Any new box should flush a pending hard line break. if !matches!(item, InlineItem::EndInlineBox) { @@ -1684,7 +1684,7 @@ impl InlineFormattingContext { }, )); }, - InlineItem::OutOfFlowFloatBox(ref mut float_box) => { + InlineItem::OutOfFlowFloatBox(ref float_box) => { float_box.layout_into_line_items(&mut layout); }, } @@ -1915,7 +1915,7 @@ impl InlineContainerState { impl IndependentFormattingContext { fn layout_into_line_items( - &mut self, + &self, layout: &mut InlineFormattingContextLayout, offset_in_text: usize, bidi_level: Level, @@ -2071,7 +2071,7 @@ impl IndependentFormattingContext { } impl FloatBox { - fn layout_into_line_items(&mut self, layout: &mut InlineFormattingContextLayout) { + fn layout_into_line_items(&self, layout: &mut InlineFormattingContextLayout) { let fragment = self.layout( layout.layout_context, layout.positioning_context, @@ -2211,7 +2211,7 @@ impl<'layout_data> ContentSizesComputation<'layout_data> { inline_formatting_context: &InlineFormattingContext, ) -> InlineContentSizesResult { for inline_item in inline_formatting_context.inline_items.iter() { - self.process_item(&mut inline_item.borrow_mut(), inline_formatting_context); + self.process_item(&inline_item.borrow(), inline_formatting_context); } self.forced_line_break(); @@ -2223,7 +2223,7 @@ impl<'layout_data> ContentSizesComputation<'layout_data> { fn process_item( &mut self, - inline_item: &mut InlineItem, + inline_item: &InlineItem, inline_formatting_context: &InlineFormattingContext, ) { match inline_item { diff --git a/components/layout_2020/flow/mod.rs b/components/layout_2020/flow/mod.rs index 2a444b28fed..fdc632ac021 100644 --- a/components/layout_2020/flow/mod.rs +++ b/components/layout_2020/flow/mod.rs @@ -370,10 +370,10 @@ fn calculate_inline_content_size_for_block_level_boxes( containing_block: &IndefiniteContainingBlock, ) -> InlineContentSizesResult { let get_box_info = |box_: &ArcRefCell| { - match &mut *box_.borrow_mut() { + match &*box_.borrow() { BlockLevelBox::OutOfFlowAbsolutelyPositionedBox(_) | BlockLevelBox::OutsideMarker { .. } => None, - BlockLevelBox::OutOfFlowFloatBox(ref mut float_box) => { + BlockLevelBox::OutOfFlowFloatBox(ref float_box) => { let inline_content_sizes_result = float_box.contents.outer_inline_content_sizes( layout_context, containing_block, @@ -404,7 +404,7 @@ fn calculate_inline_content_size_for_block_level_boxes( // Instead, we treat it like an independent block with 'clear: both'. Some((inline_content_sizes_result, Float::None, Clear::Both)) }, - BlockLevelBox::Independent(ref mut independent) => { + BlockLevelBox::Independent(ref independent) => { let inline_content_sizes_result = independent.outer_inline_content_sizes( layout_context, containing_block, @@ -606,7 +606,7 @@ fn layout_block_level_children_in_parallel( .map(|child_box| { let mut child_positioning_context = PositioningContext::new_for_subtree(collects_for_nearest_positioned_ancestor); - let fragment = child_box.borrow_mut().layout( + let fragment = child_box.borrow().layout( layout_context, &mut child_positioning_context, containing_block, @@ -646,7 +646,7 @@ fn layout_block_level_children_sequentially( .iter() .map(|child_box| { let positioning_context_length_before_layout = positioning_context.len(); - let mut fragment = child_box.borrow_mut().layout( + let mut fragment = child_box.borrow().layout( layout_context, positioning_context, containing_block, @@ -670,7 +670,7 @@ fn layout_block_level_children_sequentially( impl BlockLevelBox { fn layout( - &mut self, + &self, layout_context: &LayoutContext, positioning_context: &mut PositioningContext, containing_block: &ContainingBlock, @@ -1997,7 +1997,7 @@ fn block_size_is_zero_or_intrinsic(size: &StyleSize, containing_block: &Containi impl IndependentFormattingContext { pub(crate) fn layout_float_or_atomic_inline( - &mut self, + &self, layout_context: &LayoutContext, child_positioning_context: &mut PositioningContext, containing_block: &ContainingBlock, diff --git a/components/layout_2020/formatting_contexts.rs b/components/layout_2020/formatting_contexts.rs index 6dd214b1580..8b638b028f7 100644 --- a/components/layout_2020/formatting_contexts.rs +++ b/components/layout_2020/formatting_contexts.rs @@ -2,6 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::sync::RwLock; + use app_units::Au; use serde::Serialize; use servo_arc::Arc; @@ -38,7 +40,7 @@ pub(crate) struct NonReplacedFormattingContext { #[serde(skip_serializing)] pub style: Arc, /// If it was requested during construction - pub content_sizes_result: Option<(SizeConstraint, InlineContentSizesResult)>, + pub content_sizes_result: RwLock>, pub contents: NonReplacedFormattingContextContents, } @@ -157,7 +159,7 @@ impl IndependentFormattingContext { Self::NonReplaced(NonReplacedFormattingContext { style: Arc::clone(&node_and_style_info.style), base_fragment_info, - content_sizes_result: None, + content_sizes_result: RwLock::default(), contents, }) }, @@ -188,7 +190,7 @@ impl IndependentFormattingContext { } pub(crate) fn inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, constraint_space: &ConstraintSpace, containing_block: &IndefiniteContainingBlock, @@ -206,7 +208,7 @@ impl IndependentFormattingContext { } pub(crate) fn outer_inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, containing_block: &IndefiniteContainingBlock, auto_minimum: &LogicalVec2, @@ -276,30 +278,33 @@ impl NonReplacedFormattingContext { } pub(crate) fn inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, constraint_space: &ConstraintSpace, ) -> InlineContentSizesResult { - if let Some((previous_cb_block_size, result)) = self.content_sizes_result { + if let Ok(Some((previous_cb_block_size, result))) = + self.content_sizes_result.read().as_deref() + { if !result.depends_on_block_constraints || - previous_cb_block_size == constraint_space.block_size + *previous_cb_block_size == constraint_space.block_size { - return result; + return *result; } // TODO: Should we keep multiple caches for various block sizes? } - self.content_sizes_result - .insert(( - constraint_space.block_size, - self.contents - .inline_content_sizes(layout_context, constraint_space), - )) - .1 + let writer = self.content_sizes_result.write(); + let result = self + .contents + .inline_content_sizes(layout_context, constraint_space); + if let Ok(mut cache) = writer { + *cache = Some((constraint_space.block_size, result)); + } + result } pub(crate) fn outer_inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, containing_block: &IndefiniteContainingBlock, auto_minimum: &LogicalVec2, @@ -317,7 +322,7 @@ impl NonReplacedFormattingContext { impl NonReplacedFormattingContextContents { pub(crate) fn inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, constraint_space: &ConstraintSpace, ) -> InlineContentSizesResult { diff --git a/components/layout_2020/positioned.rs b/components/layout_2020/positioned.rs index f53c4142d20..0db018d2055 100644 --- a/components/layout_2020/positioned.rs +++ b/components/layout_2020/positioned.rs @@ -449,8 +449,8 @@ impl HoistedAbsolutelyPositionedBox { let cbis = containing_block.size.inline; let cbbs = containing_block.size.block; let containing_block_writing_mode = containing_block.style.writing_mode; - let mut absolutely_positioned_box = self.absolutely_positioned_box.borrow_mut(); - let context = &mut absolutely_positioned_box.context; + let absolutely_positioned_box = self.absolutely_positioned_box.borrow(); + let context = &absolutely_positioned_box.context; let style = context.style().clone(); let containing_block = &containing_block.into(); let pbm = style.padding_border_margin(containing_block); diff --git a/components/layout_2020/table/construct.rs b/components/layout_2020/table/construct.rs index 089c8843f1e..ca0aa3a92d4 100644 --- a/components/layout_2020/table/construct.rs +++ b/components/layout_2020/table/construct.rs @@ -137,7 +137,7 @@ impl Table { IndependentFormattingContext::NonReplaced(NonReplacedFormattingContext { base_fragment_info: (&anonymous_info).into(), style: grid_and_wrapper_style, - content_sizes_result: None, + content_sizes_result: Default::default(), contents: NonReplacedFormattingContextContents::Table(table), }) } @@ -858,7 +858,7 @@ where context: ArcRefCell::new(NonReplacedFormattingContext { style: info.style.clone(), base_fragment_info: info.into(), - content_sizes_result: None, + content_sizes_result: Default::default(), contents, }), }; diff --git a/components/layout_2020/table/layout.rs b/components/layout_2020/table/layout.rs index da49cc00015..12519849c21 100644 --- a/components/layout_2020/table/layout.rs +++ b/components/layout_2020/table/layout.rs @@ -772,7 +772,7 @@ impl<'a> TableLayout<'a> { } /// Compute CAPMIN: - fn compute_caption_minimum_inline_size(&mut self, layout_context: &LayoutContext) -> Au { + fn compute_caption_minimum_inline_size(&self, layout_context: &LayoutContext) -> Au { let containing_block = IndefiniteContainingBlock { size: LogicalVec2 { inline: AuOrAuto::Auto, @@ -784,7 +784,7 @@ impl<'a> TableLayout<'a> { .captions .iter() .map(|caption| { - let mut context = caption.context.borrow_mut(); + let context = caption.context.borrow(); context .outer_inline_content_sizes( layout_context, @@ -1605,7 +1605,7 @@ impl<'a> TableLayout<'a> { } fn layout_caption( - &mut self, + &self, caption: &TableCaption, table_pbm: &PaddingBorderMargin, layout_context: &LayoutContext, @@ -2186,7 +2186,7 @@ impl<'a> TableLayout<'a> { } fn make_fragments_for_columns_and_column_groups( - &mut self, + &self, dimensions: &TableAndTrackDimensions, fragments: &mut Vec, ) { @@ -2630,7 +2630,7 @@ impl Table { ) )] pub(crate) fn inline_content_sizes( - &mut self, + &self, layout_context: &LayoutContext, constraint_space: &ConstraintSpace, ) -> InlineContentSizesResult {