From 7933b24c9c02d83d97c4c06c39aa04b90f08131c Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 12 Aug 2026 10:41:09 +0100 Subject: [PATCH 1/2] LibWeb: Reuse the captured live subgrid record in scratch measurement subgrid_item_contributions_to_track_sizing already binds the subgrid's live record before building its scratch measurement run, but re-fetched it from the parent run's records afterwards. Use the captured handle for the two remaining reads so nothing consults the parent table while the scratch run shadows the subgrid's slot. --- Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs index cbdf27a14013a..b7223f2d99a0f 100644 --- a/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs @@ -2498,8 +2498,8 @@ impl GridFormattingContext { }; let mut context = GridFormattingContext::new(&scratch_run, Some(self)); let mut available = self.available_space.unwrap(); - if !axis.is_column() && self.used(subgrid).has_definite_inline_size() { - available.inline_size = AvailableSize::definite(self.used(subgrid).content_inline_size.get()); + if !axis.is_column() && live.has_definite_inline_size() { + available.inline_size = AvailableSize::definite(live.content_inline_size.get()); } let input = LayoutInput::new(available, self.track_sizing_constraints(), ParticipationInParentFormattingContext::Item); context.reset_for_run(input); From 1fd6241592594db6f60d8d4f46396c71ffaf2b1e Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Wed, 12 Aug 2026 11:05:51 +0100 Subject: [PATCH 2/2] LibWeb: Replace per-run record maps with a slot-indexed arena table When used-values records moved from the pass-global PagedStore into run-scoped ownership, lookups regressed from an indexed page-table load to a per-run HashMap: a RefCell borrow, a SipHash of the slot index, a bucket probe, and an Rc clone, adding up to roughly a fifth of WebContent's layout profile on table- and flex-heavy documents. Restore indexed lookups without giving up run scoping by storing the records in one slot-indexed side table in the layout node arena, sized with the slot space and stamped with a per-run nonce: a read becomes an indexed load and a compare. Runs strictly nest on the call stack, so a run that registers a slot another live run owns displaces that entry into its undo list and restores it on drop, which keeps the ownership semantics (and the unowned-read panics) exactly as before. Record reads drop from ~57ns to ~9ns and registrations halve, which translates to 23-31% faster layout passes on documents dominated by large tables, deep block nesting, or many flex/grid items. --- .../Rust/src/layout/formatting_context.rs | 8 +- .../src/layout/grid_formatting_context.rs | 2 +- .../Rust/src/layout/layout_node_arena.rs | 79 +++++++++++++++++++ .../LibWeb/Rust/src/layout/run_records.rs | 52 +++++++++--- .../LibWeb/Rust/src/layout/sizing_context.rs | 2 +- 5 files changed, 128 insertions(+), 15 deletions(-) diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 06ab894e92e9c..28a0819feb078 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -1649,7 +1649,7 @@ fn execute_formatting_context_run( let root_used = std::rc::Rc::new(root_cells.materialize_record()); let run = FormattingContextRun { purpose, - records: std::rc::Rc::new(RunRecords::new(box_, root_used)), + records: std::rc::Rc::new(RunRecords::new(callbacks.arena, box_, root_used)), box_, layout_mode, callbacks, @@ -2039,7 +2039,7 @@ pub unsafe extern "C" fn rust_layout_run_root_layout( percentage_basis_block_size: Some(viewport_block_size), ..crate::layout::ContainingBlockConstraints::default() }; - let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(root)); + let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(callbacks.arena, root)); let viewport_used = entry_records.create_used_values(&callbacks, root, root_constraints); let entry_fragments = std::rc::Rc::new(RunFragmentBuilder::new_entry_accumulator(root)); let entry_run = FormattingContextRun { @@ -2142,7 +2142,7 @@ pub unsafe extern "C" fn rust_layout_compute_subtree_layout( let callbacks = unsafe { *callbacks }; let sink = unsafe { &*sink }; - let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(root)); + let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(callbacks.arena, root)); let root_used = used_values_from_paintable(&callbacks, root, paintable_to_replace) .expect("partial relayout root must have committed geometry"); entry_records.register(root, root_used.clone()); @@ -2242,7 +2242,7 @@ pub unsafe extern "C" fn rust_layout_replay_saved_abspos_layout( let containing_block = callbacks.containing_block(box_); assert!(!containing_block.is_invalid()); let entry_fragments = std::rc::Rc::new(RunFragmentBuilder::new_entry_accumulator(containing_block)); - let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(containing_block)); + let entry_records = std::rc::Rc::new(RunRecords::new_unrooted(callbacks.arena, containing_block)); let run = crate::layout::FormattingContextRun { purpose: LayoutPurpose::Commit, records: entry_records.clone(), diff --git a/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs index b7223f2d99a0f..b19ead1ec2e01 100644 --- a/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/grid_formatting_context.rs @@ -2488,7 +2488,7 @@ impl GridFormattingContext { scratch_root.has_definite_block_size.set(live.has_definite_block_size.get()); let scratch_run = FormattingContextRun { purpose: LayoutPurpose::Measurement, - records: std::rc::Rc::new(RunRecords::new(subgrid.box_, scratch_root)), + records: std::rc::Rc::new(RunRecords::new(self.callbacks.arena, subgrid.box_, scratch_root)), box_: subgrid.box_, layout_mode: LayoutMode::IntrinsicSizing, callbacks: self.callbacks, diff --git a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs index 98533f5a04802..692583d7ce8a6 100644 --- a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs +++ b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs @@ -9,11 +9,14 @@ use crate::layout::AbsposLayoutInputs; use crate::layout::AvailableSize; use crate::layout::CssPixels; use crate::layout::FfiReplacedContentFacts; +use crate::layout::UsedValues; use crate::layout::node_data::{FfiStylePayloads, MAX_NODE_SLOT_COUNT, NodeData, NodeFlag, NodeSlotId}; +use std::cell::Cell; use std::cell::RefCell; use std::collections::HashMap; use std::ffi::c_void; use std::hash::{Hash, Hasher}; +use std::rc::Rc; use std::thread; pub(crate) const SLOTS_PER_CHUNK: usize = 256; @@ -173,6 +176,12 @@ struct TextChunkCacheSlot { entry: Option>, } +#[derive(Default)] +struct RunRecordSlot { + nonce: u64, // 0 = vacant + record: Option>, +} + // NodeData is sized to one cache line; the aligned chunk keeps every densely-strided slot // line-aligned, and per-slot bookkeeping lives in a parallel array so it stays that way. #[repr(align(64))] @@ -218,6 +227,8 @@ pub(crate) struct LayoutNodeArena { text_chunk_caches: RefCell>, replaced_content_facts: Vec, raw_table_column_spans: HashMap, + run_used_records: RefCell>, + next_run_nonce: Cell, owner_thread: thread::ThreadId, } @@ -236,6 +247,8 @@ impl LayoutNodeArena { text_chunk_caches: RefCell::new(Vec::new()), replaced_content_facts: Vec::new(), raw_table_column_spans: HashMap::new(), + run_used_records: RefCell::new(Vec::new()), + next_run_nonce: Cell::new(1), owner_thread: thread::current().id(), } } @@ -267,6 +280,9 @@ impl LayoutNodeArena { self.chunks.push(chunk); } self.slot_metadata.push(SlotMetadata::default()); + // Grown with the slot space up front: nearly every slot gets a run + // record each layout pass, so register() never has to resize. + self.run_used_records.get_mut().push(RunRecordSlot::default()); self.next_index = self .next_index .checked_add(1) @@ -331,6 +347,15 @@ impl LayoutNodeArena { if let Some(slot) = self.replaced_content_facts.get_mut(index as usize) { *slot = ReplacedContentFactsSlot::default(); } + // free() never interleaves with a layout pass (C++ is blocked on the + // synchronous FFI entry), so a live record here means a run leaked. + if let Some(slot) = self.run_used_records.get_mut().get_mut(index as usize) { + debug_assert!( + slot.record.is_none(), + "layout node arena freed a slot with a live run record" + ); + *slot = RunRecordSlot::default(); + } self.raw_table_column_spans.remove(&id); *self.data_mut(index) = NodeData::default(); @@ -791,6 +816,60 @@ impl LayoutNodeArena { unsafe { std::slice::from_raw_parts(entry.chunks.as_ptr(), entry.chunks.len()) } } + pub(crate) fn allocate_run_nonce(&self) -> u64 { + let nonce = self.next_run_nonce.get(); + self.next_run_nonce + .set(nonce.checked_add(1).expect("layout run nonce space exhausted")); + nonce + } + + pub(crate) fn run_record(&self, slot_index: u32, run_nonce: u64) -> Option> { + let records = self.run_used_records.borrow(); + let slot = records.get(slot_index as usize)?; + if slot.nonce != run_nonce { + return None; + } + slot.record.clone() + } + + pub(crate) fn replace_run_record( + &self, + slot_index: u32, + run_nonce: u64, + record: Rc, + ) -> Option<(u64, Rc)> { + let mut records = self.run_used_records.borrow_mut(); + let slot = records + .get_mut(slot_index as usize) + .expect("registered layout run record slot must exist"); + let previous = std::mem::replace( + slot, + RunRecordSlot { + nonce: run_nonce, + record: Some(record), + }, + ); + previous.record.map(|record| (previous.nonce, record)) + } + + pub(crate) fn restore_run_record(&self, slot_index: u32, run_nonce: u64, previous: Option<(u64, Rc)>) { + let mut records = self.run_used_records.borrow_mut(); + let slot = records + .get_mut(slot_index as usize) + .expect("restored layout run record slot must exist"); + debug_assert_eq!( + slot.nonce, run_nonce, + "layout run records were not restored in LIFO order" + ); + *slot = match previous { + Some((nonce, record)) => RunRecordSlot { + nonce, + record: Some(record), + }, + None => RunRecordSlot::default(), + }; + } + pub(crate) unsafe fn from_handle<'a>(arena: *mut c_void) -> &'a Self { assert!(!arena.is_null(), "layout node arena handle is null"); // SAFETY: Layout passes borrow the document's arena synchronously, diff --git a/Libraries/LibWeb/Rust/src/layout/run_records.rs b/Libraries/LibWeb/Rust/src/layout/run_records.rs index 82fbbc49a2916..35991c2409fa2 100644 --- a/Libraries/LibWeb/Rust/src/layout/run_records.rs +++ b/Libraries/LibWeb/Rust/src/layout/run_records.rs @@ -4,33 +4,57 @@ * SPDX-License-Identifier: BSD-2-Clause */ +/// The per-run registry of UsedValues records, backed by the slot-indexed side +/// table in the layout node arena. Registering a slot that a surrounding run +/// owns displaces that run's entry, and dropping the RunRecords restores it. +/// That is sound only because runs strictly nest on the call stack, so an +/// Rc must never escape its run. pub(crate) struct RunRecords { root: Node, - map: RefCell>>, + arena: *mut c_void, + nonce: u64, + undo: RefCell>, +} + +struct UndoEntry { + slot_index: u32, + previous: Option<(u64, std::rc::Rc)>, } impl RunRecords { - pub(crate) fn new(root: Node, root_used: std::rc::Rc) -> Self { - let records = Self::new_unrooted(root); + pub(crate) fn new(arena: *mut c_void, root: Node, root_used: std::rc::Rc) -> Self { + let records = Self::new_unrooted(arena, root); records.register(root, root_used); records } - pub(crate) fn new_unrooted(root: Node) -> Self { + pub(crate) fn new_unrooted(arena: *mut c_void, root: Node) -> Self { + // SAFETY: Layout passes borrow the document's arena synchronously, and + // the document keeps it alive for the duration of the pass. + let nonce = unsafe { LayoutNodeArena::from_handle(arena) }.allocate_run_nonce(); Self { root, - map: RefCell::new(HashMap::new()), + arena, + nonce, + undo: RefCell::new(Vec::new()), } } + fn arena(&self) -> &LayoutNodeArena { + // SAFETY: See new_unrooted(). + unsafe { LayoutNodeArena::from_handle(self.arena) } + } + pub(crate) fn register(&self, node: Node, used: std::rc::Rc) { - let previous = self.map.borrow_mut().insert(node.slot_index(), used); + let slot_index = node.slot_index(); + let previous = self.arena().replace_run_record(slot_index, self.nonce, used); assert!( - previous.is_none(), + previous.as_ref().is_none_or(|(nonce, _)| *nonce != self.nonce), "slot {} registered twice in the run rooted at slot {}", - node.slot_index(), + slot_index, self.root.slot_index() ); + self.undo.borrow_mut().push(UndoEntry { slot_index, previous }); } pub(crate) fn create_used_values( @@ -57,6 +81,16 @@ impl RunRecords { } pub(crate) fn used_values_if_owned(&self, node: Node) -> Option> { - self.map.borrow().get(&node.slot_index()).cloned() + self.arena().run_record(node.slot_index(), self.nonce) + } +} + +impl Drop for RunRecords { + fn drop(&mut self) { + let undo = std::mem::take(self.undo.get_mut()); + let arena = self.arena(); + for entry in undo.into_iter().rev() { + arena.restore_run_record(entry.slot_index, self.nonce, entry.previous); + } } } diff --git a/Libraries/LibWeb/Rust/src/layout/sizing_context.rs b/Libraries/LibWeb/Rust/src/layout/sizing_context.rs index 0635cef0afe54..44b953a9d7dc0 100644 --- a/Libraries/LibWeb/Rust/src/layout/sizing_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/sizing_context.rs @@ -1831,7 +1831,7 @@ impl SizingContext { let table_run = crate::layout::FormattingContextRun { purpose: LayoutPurpose::Measurement, - records: std::rc::Rc::new(RunRecords::new(table_box, table_used.clone())), + records: std::rc::Rc::new(RunRecords::new(measurement.callbacks().arena, table_box, table_used.clone())), box_: table_box, layout_mode: LayoutMode::IntrinsicSizing, callbacks: *measurement.callbacks(),