From b72d5a39d84e5fcd6e2edf11bf04e1cf1c59b5d6 Mon Sep 17 00:00:00 2001 From: Aliaksandr Kalenik Date: Fri, 31 Jul 2026 04:00:12 +0200 Subject: [PATCH] LibWeb: Hand layout a Rust-owned style container instead of a mirror Layout read style through a per-node side table in the node arena that mirrored all 23 group payload pointers, copied out of ComputedValues on every style application. The mirror was 184 bytes per node of duplicated pointers whose validity rested on the rewrite-on-set protocol alone: nothing owned the payloads on the Rust side, and the generation-keyed side table existed only to keep stale copies from being read after slot reuse. Every built ComputedValues now lazily creates a style container on first request: a Rust-owned refcounted allocation, using the same header-before-payload scheme as the group payloads themselves, holding the group pointer array and retaining every group. NodeData.style points at the container's array, which the layout side reads in place as the payload array, so style application shrinks to publishing one pointer and the side table, its generation keying, and the per-set 23-pointer copy are deleted. The container is created after group adoption has settled (adopt_identical_group_payloads now asserts it runs first) and released with its ComputedValues, making payload ownership explicit where the mirror only had it by protocol. It also gives the style a single identity usable for future retention from paintables and for input-keyed layout caching. --- Libraries/LibWeb/CSS/ComputedProperties.cpp | 2 + Libraries/LibWeb/CSS/ComputedValues.cpp | 13 ++- Libraries/LibWeb/CSS/ComputedValues.h | 4 +- Libraries/LibWeb/Layout/Node.cpp | 12 +-- Libraries/LibWeb/Layout/Node.h | 2 +- .../LibWeb/Rust/src/css/computed_values.rs | 88 +++++++++++++++++++ .../Rust/src/layout/formatting_context.rs | 16 ++-- .../Rust/src/layout/layout_node_arena.rs | 60 +++---------- Libraries/LibWeb/Rust/src/layout/node_data.rs | 7 +- 9 files changed, 127 insertions(+), 77 deletions(-) diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index 0a0e6f0055c96..25230a9e33daf 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -79,6 +79,8 @@ ComputedValues::ComputedValues() ComputedValues::~ComputedValues() { + if (m_style_container) + ComputedValuesFFI::rust_style_container_unref(m_style_container, to_underlying(StyleGroupIndex::Count)); --s_statistics.live_instance_count; } diff --git a/Libraries/LibWeb/CSS/ComputedValues.cpp b/Libraries/LibWeb/CSS/ComputedValues.cpp index d6679a69ff893..a8772749fcd87 100644 --- a/Libraries/LibWeb/CSS/ComputedValues.cpp +++ b/Libraries/LibWeb/CSS/ComputedValues.cpp @@ -850,6 +850,7 @@ bool ComputedValues::FontValues::operator==(FontValues const& other) const bool ComputedValues::adopt_identical_group_payloads(ComputedValues const& previous) const { + VERIFY(!m_style_container); bool all_shared = true; auto adopt = [&](StyleStructRef const& mine, StyleStructRef const& theirs) { if (mine.ptr_equals(theirs)) @@ -943,11 +944,15 @@ void const* ComputedValues::style_group_payload(StyleGroupIndex group) const VERIFY_NOT_REACHED(); } -void ComputedValues::fill_style_group_payloads(Span payloads) const +void const* ComputedValues::style_container() const { - VERIFY(payloads.size() == to_underlying(StyleGroupIndex::Count)); - for (size_t index = 0; index < payloads.size(); ++index) - payloads[index] = style_group_payload(static_cast(index)); + if (!m_style_container) { + Array groups; + for (size_t index = 0; index < groups.size(); ++index) + groups[index] = style_group_payload(static_cast(index)); + m_style_container = ComputedValuesFFI::rust_style_container_create(groups.data(), groups.size()); + } + return m_style_container; } NonnullRefPtr ComputedValues::create(ComputedProperties const& computed_style, DOM::Document const& document, StyleScope const& style_scope, ColorResolutionContext color_resolution_context, ComputedValues const* inherit_parent) diff --git a/Libraries/LibWeb/CSS/ComputedValues.h b/Libraries/LibWeb/CSS/ComputedValues.h index 4e4ffb18fdb05..52514ff46b110 100644 --- a/Libraries/LibWeb/CSS/ComputedValues.h +++ b/Libraries/LibWeb/CSS/ComputedValues.h @@ -1074,8 +1074,7 @@ class WEB_API ComputedValues final : public RefCounted { // pointer is borrowed from this immutable ComputedValues instance. void const* style_group_payload(StyleGroupIndex) const; - // Fills one payload pointer per StyleGroupIndex, in enum order. - void fill_style_group_payloads(Span) const; + void const* style_container() const; // Calls back with (name, shared_with_parent, is_default) for every style value group, // for introspecting how well group sharing is working (see internals.styleGroupSharingInfo()). @@ -2089,6 +2088,7 @@ class WEB_API ComputedValues final : public RefCounted { }; NonInheritedValues m_noninherited; + mutable void const* m_style_container { nullptr }; AK::FixedBitmap m_property_important { false }; AK::FixedBitmap m_property_inherited { false }; HashMap> m_inheritance_dependent_specified_values; diff --git a/Libraries/LibWeb/Layout/Node.cpp b/Libraries/LibWeb/Layout/Node.cpp index 52d7ad93f8047..7abaa478f4d80 100644 --- a/Libraries/LibWeb/Layout/Node.cpp +++ b/Libraries/LibWeb/Layout/Node.cpp @@ -650,7 +650,7 @@ NodeWithStyle::NodeWithStyle(DOM::Document& document, DOM::Node* node, NonnullRe { set_flag(RustFFI::NodeFlag::HasStyle, true); set_flag(RustFFI::NodeFlag::IsBody, node && node == document.body()); - mirror_computed_values_to_node_data(); + publish_style_container_to_node_data(); synchronize_table_span_data(); } @@ -956,7 +956,7 @@ void NodeWithStyle::set_computed_values(NonnullRefPtr { VERIFY(!layout_pass_currently_running()); m_computed_values = move(computed_values); - mirror_computed_values_to_node_data(); + publish_style_container_to_node_data(); for (auto* child = first_child_ptr(); child; child = child->next_sibling_ptr()) { if (auto* text_child = as_if(*child)) @@ -964,13 +964,9 @@ void NodeWithStyle::set_computed_values(NonnullRefPtr } } -void NodeWithStyle::mirror_computed_values_to_node_data() +void NodeWithStyle::publish_style_container_to_node_data() { - node_data().style = m_computed_values.ptr(); - - RustFFI::FfiStylePayloads style_payloads {}; - m_computed_values->fill_style_group_payloads({ style_payloads.groups, array_size(style_payloads.groups) }); - RustFFI::layout_arena_set_style_payloads(arena_handle(), slot_id(this), &style_payloads); + node_data().style = m_computed_values->style_container(); } void NodeWithStyle::synchronize_table_span_data() diff --git a/Libraries/LibWeb/Layout/Node.h b/Libraries/LibWeb/Layout/Node.h index e2d79dfb6aebd..9f352716353a6 100644 --- a/Libraries/LibWeb/Layout/Node.h +++ b/Libraries/LibWeb/Layout/Node.h @@ -447,7 +447,7 @@ class WEB_API NodeWithStyle : public Node { void reset_table_box_computed_values_used_by_wrapper_to_init_values(); void propagate_non_inherit_values(CSS::ComputedValues::Builder&) const; void propagate_style_to_anonymous_wrappers(); - void mirror_computed_values_to_node_data(); + void publish_style_container_to_node_data(); void rebuild_image_observers(); diff --git a/Libraries/LibWeb/Rust/src/css/computed_values.rs b/Libraries/LibWeb/Rust/src/css/computed_values.rs index ac427b8ce134c..dd2b66e405530 100644 --- a/Libraries/LibWeb/Rust/src/css/computed_values.rs +++ b/Libraries/LibWeb/Rust/src/css/computed_values.rs @@ -566,6 +566,94 @@ pub unsafe extern "C" fn rust_style_group_free(group_index: usize, payload: *mut }); } +fn release_group_payload(group_index: usize, payload: *const c_void) { + let table = vtable(group_index); + let refcount = refcount_of(payload, payload_align(table)); + if refcount.load(Ordering::Relaxed) == STYLE_GROUP_STATIC_REFCOUNT { + return; + } + if refcount.fetch_sub(1, Ordering::AcqRel) == 1 { + // SAFETY: The count reached zero, so this reference was the last one. + unsafe { + destruct(table, payload.cast_mut()); + let allocation = (payload as *mut u8).sub(header_size(payload_align(table))); + dealloc(allocation, allocation_layout(table)); + } + } +} + +fn style_container_header_size() -> usize { + header_size(align_of::<*const c_void>()) +} + +fn style_container_allocation_layout(group_count: usize) -> Layout { + Layout::from_size_align( + style_container_header_size() + group_count * size_of::<*const c_void>(), + align_of::(), + ) + .expect("style container layout overflow") +} + +/// Allocates the style container for one built ComputedValues: a Rust-owned +/// refcounted `[ ArcHeader | group payload pointer array ]` allocation that +/// retains every group. The returned pointer addresses the pointer array, so +/// the layout side reads it in place as the node's style payload array. +/// +/// # Safety +/// `groups` must point at `group_count` valid group payload pointers, in +/// style group index order, covering every registered group. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn rust_style_container_create( + groups: *const *const c_void, + group_count: usize, +) -> *const c_void { + abort_on_panic(|| unsafe { + let registered_count = REGISTRY + .get() + .expect("style groups used before registration") + .vtables + .len(); + assert_eq!(group_count, registered_count, "style container must cover every group"); + let allocation = alloc(style_container_allocation_layout(group_count)); + if allocation.is_null() { + std::process::abort(); + } + (*(allocation as *mut AtomicUsize)).store(1, Ordering::Relaxed); + let array = allocation.add(style_container_header_size()) as *mut *const c_void; + for group_index in 0..group_count { + let payload = *groups.add(group_index); + assert!(!payload.is_null(), "style container group payload is null"); + retain_group_payload(group_index, payload); + array.add(group_index).write(payload); + } + array as *const c_void + }) +} + +/// Releases one reference to a style container, releasing its group payloads +/// and freeing the allocation when the count reaches zero. +/// +/// # Safety +/// `container` must be a pointer returned by rust_style_container_create with +/// an outstanding reference, and `group_count` must match its creation. +#[unsafe(no_mangle)] +pub unsafe extern "C" fn rust_style_container_unref(container: *const c_void, group_count: usize) { + abort_on_panic(|| unsafe { + let refcount = &*(container as *const u8) + .sub(style_container_header_size()) + .cast::(); + if refcount.fetch_sub(1, Ordering::AcqRel) != 1 { + return; + } + let array = container as *const *const c_void; + for group_index in 0..group_count { + release_group_payload(group_index, *array.add(group_index)); + } + let allocation = (container as *mut u8).sub(style_container_header_size()); + dealloc(allocation, style_container_allocation_layout(group_count)); + }); +} + /// Compares two payloads of the same style group for value equality, letting /// C++ group structs that inherit a Rust-native payload layout reuse the Rust /// field-wise equality instead of hand-writing a second one. diff --git a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs index 78f2936218d89..2dc25e7bc86a5 100644 --- a/Libraries/LibWeb/Rust/src/layout/formatting_context.rs +++ b/Libraries/LibWeb/Rust/src/layout/formatting_context.rs @@ -2888,18 +2888,20 @@ impl FfiLayoutFcCallbacks { let payloads = self .arena() .style_payloads(node) - .expect("styled node payloads must be mirrored to the arena before layout"); - // SAFETY: The document arena outlives the layout pass, and the mirror - // is only rewritten between passes: set_computed_values verifies no - // pass is running and no layout node is created mid-pass. + .expect("styled node must publish its style container before layout"); + // SAFETY: The node's ComputedValues keep the style container alive + // for the pass, and the container is only replaced between passes: + // set_computed_values verifies no pass is running and no layout node + // is created mid-pass. unsafe { &*std::ptr::from_ref(payloads) } } pub(crate) fn style_reader_if_styled(&self, node: Node) -> Option> { let payloads = self.arena().style_payloads(node)?; - // SAFETY: The document arena outlives the layout pass, and the mirror - // is only rewritten between passes: set_computed_values verifies no - // pass is running and no layout node is created mid-pass. + // SAFETY: The node's ComputedValues keep the style container alive + // for the pass, and the container is only replaced between passes: + // set_computed_values verifies no pass is running and no layout node + // is created mid-pass. Some(StyleReader::new(unsafe { &*std::ptr::from_ref(payloads) })) } diff --git a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs index f5daa03a7e45e..58e86d80c6c8a 100644 --- a/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs +++ b/Libraries/LibWeb/Rust/src/layout/layout_node_arena.rs @@ -144,16 +144,6 @@ struct TextContentSlot { content: Option>, } -/// Mirror of the node's ComputedValues group payload pointers, rewritten by -/// C++ on every style application so layout can read style without a per-pass -/// FFI round trip. Generation 0 never matches a live slot, so a matching -/// generation alone means the mirror was written for this slot incarnation. -#[derive(Default)] -struct StylePayloadsSlot { - generation: u8, - payloads: FfiStylePayloads, -} - #[derive(Clone, Copy, PartialEq)] pub(crate) struct TextChunkCacheKey { pub(crate) should_wrap_lines: bool, @@ -219,7 +209,6 @@ pub(crate) struct LayoutNodeArena { intrinsic_size_caches: RefCell>, saved_abspos_layout_inputs: RefCell>, text_contents: Vec, - style_payloads: Vec, text_chunk_caches: RefCell>, owner_thread: thread::ThreadId, } @@ -236,7 +225,6 @@ impl LayoutNodeArena { intrinsic_size_caches: RefCell::new(Vec::new()), saved_abspos_layout_inputs: RefCell::new(Vec::new()), text_contents: Vec::new(), - style_payloads: Vec::new(), text_chunk_caches: RefCell::new(Vec::new()), owner_thread: thread::current().id(), } @@ -327,9 +315,6 @@ impl LayoutNodeArena { if let Some(slot) = self.text_contents.get_mut(index as usize) { *slot = TextContentSlot::default(); } - if let Some(slot) = self.style_payloads.get_mut(index as usize) { - *slot = StylePayloadsSlot::default(); - } if let Some(slot) = self.text_chunk_caches.get_mut().get_mut(index as usize) { *slot = TextChunkCacheSlot::default(); } @@ -693,25 +678,18 @@ impl LayoutNodeArena { .and_then(|slot| slot.content.as_deref()) } - pub(crate) fn set_style_payloads(&mut self, id: NodeSlotId, payloads: FfiStylePayloads) { - self.assert_owner_thread(); - self.data(id); - let index = id.slot_index() as usize; - if self.style_payloads.len() <= index { - self.style_payloads.resize_with(index + 1, StylePayloadsSlot::default); - } - self.style_payloads[index] = StylePayloadsSlot { - generation: id.generation(), - payloads, - }; - } - + /// The node's group payload pointer array, read in place from the + /// Rust-owned style container that NodeData.style addresses. The node's + /// retained immutable ComputedValues owns the container, and the pointer + /// is only replaced between passes, so the array stays valid for as long + /// as the node occupies its arena slot. pub(crate) fn style_payloads(&self, id: NodeSlotId) -> Option<&FfiStylePayloads> { - assert!(!id.is_invalid(), "invalid layout node arena slot ID"); - self.style_payloads - .get(id.slot_index() as usize) - .filter(|slot| slot.generation == id.generation()) - .map(|slot| &slot.payloads) + // SAFETY: data() generation-checks the slot and returns an + // initialized NodeData. + let style = unsafe { (&raw const (*self.data(id)).style).read() }; + // SAFETY: A non-null style pointer addresses the container's group + // pointer array, which FfiStylePayloads mirrors exactly. + (!style.is_null()).then(|| unsafe { &*style.cast::() }) } pub(crate) fn text_chunks( @@ -888,22 +866,6 @@ pub unsafe extern "C" fn layout_arena_set_text_content( }); } -#[unsafe(no_mangle)] -pub unsafe extern "C" fn layout_arena_set_style_payloads( - arena: *mut c_void, - id: NodeSlotId, - payloads: *const FfiStylePayloads, -) { - abort_on_panic(|| { - assert!(!arena.is_null(), "layout node arena handle is null"); - assert!(!payloads.is_null(), "style payload snapshot pointer is null"); - // SAFETY: The C++ caller passes a live payload snapshot for the - // duration of this synchronous call, and the C++ wrapper keeps the - // arena alive while serializing all access on the document thread. - unsafe { (&mut *arena.cast::()).set_style_payloads(id, *payloads) }; - }); -} - #[unsafe(no_mangle)] pub unsafe extern "C" fn layout_arena_transfer_saved_abspos_layout_inputs( arena: *mut c_void, diff --git a/Libraries/LibWeb/Rust/src/layout/node_data.rs b/Libraries/LibWeb/Rust/src/layout/node_data.rs index a5c0f3b8787ec..02c8e67e3b0db 100644 --- a/Libraries/LibWeb/Rust/src/layout/node_data.rs +++ b/Libraries/LibWeb/Rust/src/layout/node_data.rs @@ -10,14 +10,9 @@ pub const INVALID_NODE_SLOT_INDEX: u32 = u32::MAX; pub const GENERATED_FOR_MARKER: u8 = 6; // The full C++ StyleGroupIndex space; LayoutRustBridge.cpp static-asserts the -// count so the payload mirror and the registered group indices line up. +// count so the style container array and the registered group indices line up. pub const STYLE_GROUP_COUNT: usize = 23; -/// Borrowed pointers to every `ComputedValues` group payload, mirrored into -/// the layout node arena at style application. The node's retained immutable -/// `ComputedValues` keeps every payload alive, and the mirror is rewritten -/// whenever that style is replaced, so the pointers stay valid for as long as -/// the node occupies its arena slot. #[derive(Clone, Copy)] #[repr(C)] pub struct FfiStylePayloads {