From 3d3a9e1a1d285c69c801e55f52bed81aa473f3fb Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 11:49:21 +0200 Subject: [PATCH 01/17] remove Clone impl from DomPosition This prepares a change in the structure to a link/cut tree. We want clear ownership rules, where each DynamicDomSlot owns an internal node, the clones are only handles for it, but should not keep the slot alive. --- Cargo.lock | 1 + packages/yew/src/app_handle.rs | 8 +- packages/yew/src/dom_bundle/bcomp.rs | 18 ++--- packages/yew/src/dom_bundle/blist.rs | 4 +- packages/yew/src/dom_bundle/bnode.rs | 4 +- packages/yew/src/dom_bundle/braw.rs | 11 +-- packages/yew/src/dom_bundle/bsuspense.rs | 4 +- packages/yew/src/dom_bundle/btag/mod.rs | 22 +++--- packages/yew/src/dom_bundle/btext.rs | 9 +-- packages/yew/src/dom_bundle/mod.rs | 4 +- packages/yew/src/dom_bundle/position.rs | 81 +++++++++++++++++++- packages/yew/src/dom_bundle/traits.rs | 4 +- packages/yew/src/html/component/lifecycle.rs | 8 +- packages/yew/src/html/component/scope.rs | 23 +++--- packages/yew/src/virtual_dom/vcomp.rs | 18 ++--- 15 files changed, 143 insertions(+), 76 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 61fdaceef1f..ee5ce6392b1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5229,6 +5229,7 @@ dependencies = [ name = "yew-router-macro" version = "0.20.0" dependencies = [ + "matchit 0.9.2", "proc-macro2", "quote", "rustversion", diff --git a/packages/yew/src/app_handle.rs b/packages/yew/src/app_handle.rs index 94356a2c1df..5f805020e83 100644 --- a/packages/yew/src/app_handle.rs +++ b/packages/yew/src/app_handle.rs @@ -87,7 +87,7 @@ fn clear_element(host: &Element) { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::Fragment; + use crate::dom_bundle::{Fragment, SlotBulletin}; impl AppHandle where @@ -106,17 +106,13 @@ mod feat_hydration { let mut fragment = Fragment::collect_children(&host); let hosting_root = BSubtree::create_root(&host); - let mut previous_next_sibling = None; app.scope.hydrate_in_place( hosting_root, host.clone(), &mut fragment, Rc::clone(&props), - &mut previous_next_sibling, + &mut SlotBulletin::new(), ); - if let Some(previous_next_sibling) = previous_next_sibling { - previous_next_sibling.reassign(DomSlot::at_end()); - } // We remove all remaining nodes, this mimics the clear_element behaviour in // mount_with_props. diff --git a/packages/yew/src/dom_bundle/bcomp.rs b/packages/yew/src/dom_bundle/bcomp.rs index a28dbb15c44..c653b03fc14 100644 --- a/packages/yew/src/dom_bundle/bcomp.rs +++ b/packages/yew/src/dom_bundle/bcomp.rs @@ -6,7 +6,7 @@ use std::fmt; use web_sys::Element; -use super::{BNode, BSubtree, DomSlot, DynamicDomSlot, Reconcilable, ReconcileTarget}; +use super::{BNode, BSubtree, DomSlot, Reconcilable, ReconcileTarget}; use crate::html::{AnyScope, Scoped}; use crate::virtual_dom::{Key, VComp}; @@ -16,7 +16,7 @@ pub(super) struct BComp { scope: Box, /// An internal [`DomSlot`] passed around to track this components position. This /// will dynamically adjust when a lifecycle changes the render state of this component. - own_position: DynamicDomSlot, + own_position: DomSlot, key: Option, } @@ -43,7 +43,7 @@ impl ReconcileTarget for BComp { fn shift(&self, next_parent: &Element, slot: DomSlot) -> DomSlot { self.scope.shift_node(next_parent.clone(), slot); - self.own_position.to_position() + self.own_position.clone() } } @@ -64,14 +64,14 @@ impl Reconcilable for VComp { .. } = self; - let (scope, internal_ref) = mountable.mount(root, parent_scope, parent.to_owned(), slot); + let (scope, own_position) = mountable.mount(root, parent_scope, parent.to_owned(), slot); ( - internal_ref.to_position(), + own_position.clone(), BComp { type_id, scope, - own_position: internal_ref, + own_position, key, }, ) @@ -106,14 +106,14 @@ impl Reconcilable for VComp { bcomp.key = key; mountable.reuse(bcomp.scope.borrow(), slot); - bcomp.own_position.to_position() + bcomp.own_position.clone() } } #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; impl Hydratable for VComp { fn hydrate( @@ -122,7 +122,7 @@ mod feat_hydration { parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let VComp { type_id, diff --git a/packages/yew/src/dom_bundle/blist.rs b/packages/yew/src/dom_bundle/blist.rs index 05d99dde83b..92b71e2aa5a 100644 --- a/packages/yew/src/dom_bundle/blist.rs +++ b/packages/yew/src/dom_bundle/blist.rs @@ -516,7 +516,7 @@ impl Reconcilable for VList { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; impl Hydratable for VList { fn hydrate( @@ -525,7 +525,7 @@ mod feat_hydration { parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let (key, fully_keyed, vchildren) = self.split_for_blist(); diff --git a/packages/yew/src/dom_bundle/bnode.rs b/packages/yew/src/dom_bundle/bnode.rs index 39bc39add3d..fdfe33d6f43 100644 --- a/packages/yew/src/dom_bundle/bnode.rs +++ b/packages/yew/src/dom_bundle/bnode.rs @@ -267,7 +267,7 @@ impl fmt::Debug for BNode { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; impl Hydratable for VNode { fn hydrate( @@ -276,7 +276,7 @@ mod feat_hydration { parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { match self { VNode::VTag(vtag) => RcExt::unwrap_or_clone(vtag) diff --git a/packages/yew/src/dom_bundle/braw.rs b/packages/yew/src/dom_bundle/braw.rs index abf520060bc..0b82489e4ae 100644 --- a/packages/yew/src/dom_bundle/braw.rs +++ b/packages/yew/src/dom_bundle/braw.rs @@ -146,7 +146,7 @@ impl Reconcilable for VRaw { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; use crate::virtual_dom::Collectable; impl Hydratable for VRaw { @@ -156,17 +156,14 @@ mod feat_hydration { _parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let collectable = Collectable::Raw; let fallback_fragment = Fragment::collect_between(fragment, &collectable, parent); let first_child = fallback_fragment.iter().next().cloned(); - if let (Some(first_child), prev_next_sibling) = (&first_child, prev_next_sibling) { - if let Some(prev_next_sibling) = prev_next_sibling { - prev_next_sibling.reassign(DomSlot::at(first_child.clone())); - } - *prev_next_sibling = None; + if let Some(first_child) = &first_child { + prev_next_sibling.write_at_node(first_child.clone()); } let Self { html } = self; diff --git a/packages/yew/src/dom_bundle/bsuspense.rs b/packages/yew/src/dom_bundle/bsuspense.rs index f5e639303fa..be79a222954 100644 --- a/packages/yew/src/dom_bundle/bsuspense.rs +++ b/packages/yew/src/dom_bundle/bsuspense.rs @@ -224,7 +224,7 @@ impl Reconcilable for VSuspense { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; use crate::virtual_dom::Collectable; impl Hydratable for VSuspense { @@ -234,7 +234,7 @@ mod feat_hydration { parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - previous_next_sibling: &mut Option, + previous_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let detached_parent = document() .create_element("div") diff --git a/packages/yew/src/dom_bundle/btag/mod.rs b/packages/yew/src/dom_bundle/btag/mod.rs index 3b822409e7b..f9c269db031 100644 --- a/packages/yew/src/dom_bundle/btag/mod.rs +++ b/packages/yew/src/dom_bundle/btag/mod.rs @@ -352,7 +352,7 @@ mod feat_hydration { use web_sys::Node; use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable, node_type_str}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin, node_type_str}; impl Hydratable for VTag { fn hydrate( @@ -361,7 +361,7 @@ mod feat_hydration { parent_scope: &AnyScope, _parent: &Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let tag_name = self.tag().to_owned(); @@ -429,12 +429,13 @@ mod feat_hydration { } VTagInner::Other { children, tag } => { let mut nodes = Fragment::collect_children(&el); - let mut prev_next_child = None; - let child_bundle = - children.hydrate(root, parent_scope, &el, &mut nodes, &mut prev_next_child); - if let Some(prev_next_child) = prev_next_child { - prev_next_child.reassign(DomSlot::at_end()); - } + let child_bundle = children.hydrate( + root, + parent_scope, + &el, + &mut nodes, + &mut SlotBulletin::new(), + ); nodes.trim_start_text_nodes(); @@ -445,10 +446,7 @@ mod feat_hydration { }; node_ref.set(Some((*el).clone())); - if let Some(prev_next_sibling) = prev_next_sibling { - prev_next_sibling.reassign(DomSlot::at((*el).clone())); - } - *prev_next_sibling = None; + prev_next_sibling.write_at_node((*el).clone()); BTag { inner, diff --git a/packages/yew/src/dom_bundle/btext.rs b/packages/yew/src/dom_bundle/btext.rs index fa5fd882016..a1eba090d0e 100644 --- a/packages/yew/src/dom_bundle/btext.rs +++ b/packages/yew/src/dom_bundle/btext.rs @@ -94,7 +94,7 @@ mod feat_hydration { use web_sys::Node; use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment, Hydratable}; + use crate::dom_bundle::{Fragment, Hydratable, SlotBulletin}; impl Hydratable for VText { fn hydrate( @@ -103,7 +103,7 @@ mod feat_hydration { _parent_scope: &AnyScope, parent: &Element, fragment: &mut Fragment, - previous_next_sibling: &mut Option, + previous_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle { let create_at = |next_sibling: Option, text: AttrValue| { // If there are multiple text nodes placed back-to-back in SSR, it may be parsed as @@ -140,10 +140,7 @@ mod feat_hydration { } _ => create_at(fragment.sibling_at_end().cloned(), self.text), }; - if let Some(previous_next_sibling) = previous_next_sibling { - previous_next_sibling.reassign(DomSlot::at(btext.text_node.clone().into())); - } - *previous_next_sibling = None; + previous_next_sibling.write_at_node(btext.text_node.clone().into()); btext } } diff --git a/packages/yew/src/dom_bundle/mod.rs b/packages/yew/src/dom_bundle/mod.rs index aa82d211838..b64690fb833 100644 --- a/packages/yew/src/dom_bundle/mod.rs +++ b/packages/yew/src/dom_bundle/mod.rs @@ -32,6 +32,8 @@ use braw::BRaw; use bsuspense::BSuspense; use btag::{BTag, Registry}; use btext::BText; +#[cfg(feature = "hydration")] +pub(crate) use position::SlotBulletin; pub(crate) use position::{DomSlot, DynamicDomSlot}; use subtree_root::EventDescriptor; pub use subtree_root::{BSubtree, set_event_bubbling}; @@ -94,7 +96,7 @@ mod feat_hydration { parent: &Element, fragment: &mut Fragment, node: VNode, - previous_next_sibling: &mut Option, + previous_next_sibling: &mut SlotBulletin<'_>, ) -> Self { let bundle = node.hydrate(root, parent_scope, parent, fragment, previous_next_sibling); Self(bundle) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index 8c64b91f048..e14060c74a4 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -14,15 +14,22 @@ pub(crate) struct DomSlot { variant: DomSlotVariant, } -#[derive(Clone)] enum DomSlotVariant { Node(Option), Chained(DynamicDomSlot), } +impl Clone for DomSlotVariant { + fn clone(&self) -> Self { + match self { + Self::Node(node) => Self::Node(node.clone()), + Self::Chained(slot) => Self::Chained(slot.clone_to_follower()), + } + } +} + /// A dynamic dom slot can be reassigned. This change is also seen by the [`DomSlot`] from /// [`Self::to_position`] before the reassignment took place. -#[derive(Clone)] pub(crate) struct DynamicDomSlot { target: Rc>, } @@ -186,7 +193,19 @@ impl DynamicDomSlot { /// slots are equivalent to each other and point to the same position. pub fn to_position(&self) -> DomSlot { DomSlot { - variant: DomSlotVariant::Chained(self.clone()), + variant: DomSlotVariant::Chained(self.clone_to_follower()), + } + } + + /// There can only be one owner of a dynamic dom slot. Reassigning a dom slot is only allowed + /// while that owner is still alive. All other accesses (e.g. through DomSlot) are followers + /// and should only read the value, but never write to it. + /// This does not imply that access is always serialized! Followers are allowed to write at any + /// point without prior synchronization, as long as they ensure that the owner is still alive. + fn clone_to_follower(&self) -> Self { + // TODO: the return value could be a different type + Self { + target: self.target.clone(), } } @@ -228,6 +247,62 @@ impl DynamicDomSlot { } } +#[cfg(feature = "hydration")] +mod feat_hydration { + use std::marker::PhantomData; + + use web_sys::Node; + + use super::{DomSlot, DynamicDomSlot}; + + pub struct SlotBulletin<'tree> { + prev_next_sibling: Option, + _owner: PhantomData<&'tree mut DynamicDomSlot>, + } + impl<'tree> SlotBulletin<'tree> { + pub fn start(slot: &'tree mut DynamicDomSlot) -> Self { + // We take a follower, but we are sure the owner is alive + Self { + prev_next_sibling: Some(slot.clone_to_follower()), + _owner: PhantomData, + } + } + + pub fn new() -> Self { + Self { + prev_next_sibling: None, + _owner: PhantomData, + } + } + + fn write(&mut self, pos: DomSlot) { + if let Some(slot) = &mut self.prev_next_sibling { + slot.reassign(pos); + } + } + + pub fn write_at_node(&mut self, node: Node) { + self.write(DomSlot::at(node)); + self.prev_next_sibling = None; + } + + // This method does not track that `inner_next_sibling` (which is the owner) lives for + // lifetime of this call. This must be done by the caller, which puts it somewhere in + // its component state + pub fn write_at_comp(&mut self, slot: DomSlot, inner_next_sibling: &DynamicDomSlot) { + self.write(slot); + self.prev_next_sibling = Some(inner_next_sibling.clone_to_follower()); + } + } + impl Drop for SlotBulletin<'_> { + fn drop(&mut self) { + self.write(DomSlot::at_end()) + } + } +} +#[cfg(feature = "hydration")] +pub use feat_hydration::SlotBulletin; + #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] #[cfg(test)] mod layout_tests { diff --git a/packages/yew/src/dom_bundle/traits.rs b/packages/yew/src/dom_bundle/traits.rs index 629b8604de2..a07e6c3c79d 100644 --- a/packages/yew/src/dom_bundle/traits.rs +++ b/packages/yew/src/dom_bundle/traits.rs @@ -100,7 +100,7 @@ pub(super) trait Reconcilable { #[cfg(feature = "hydration")] mod feat_hydration { use super::*; - use crate::dom_bundle::{DynamicDomSlot, Fragment}; + use crate::dom_bundle::{Fragment, SlotBulletin}; pub(in crate::dom_bundle) trait Hydratable: Reconcilable { /// hydrates current tree. @@ -121,7 +121,7 @@ mod feat_hydration { // `Node.insertAfter`) Hence, we pass an optional argument to inform of the // new hydrated node's position. This should end up assigning the same // position that would have been returned from `Self::attach` on creation. - prev_next_sibling: &mut Option, + prev_next_sibling: &mut SlotBulletin<'_>, ) -> Self::Bundle; } } diff --git a/packages/yew/src/html/component/lifecycle.rs b/packages/yew/src/html/component/lifecycle.rs index 4452cbbc57f..f2ed846a255 100644 --- a/packages/yew/src/html/component/lifecycle.rs +++ b/packages/yew/src/html/component/lifecycle.rs @@ -8,11 +8,11 @@ use web_sys::Element; use super::BaseComponent; use super::scope::{AnyScope, Scope}; -#[cfg(feature = "hydration")] -use crate::dom_bundle::Fragment; #[cfg(feature = "csr")] use crate::dom_bundle::{BSubtree, Bundle, DomSlot, DynamicDomSlot}; #[cfg(feature = "hydration")] +use crate::dom_bundle::{Fragment, SlotBulletin}; +#[cfg(feature = "hydration")] use crate::html::RenderMode; use crate::html::{Html, RenderError}; use crate::scheduler::{self, Runnable, Shared}; @@ -545,6 +545,7 @@ impl ComponentState { // We schedule a "first" render to run immediately after hydration. // Most notably, only this render will trigger the "rendered" callback, hence we // want to prioritize this. + scheduler::push_component_priority_render( self.comp_id, Box::new(RenderRunner { @@ -559,7 +560,8 @@ impl ComponentState { parent, fragment, new_vdom, - &mut Some(own_slot.clone()), + // own_slot is alive for the call + &mut SlotBulletin::start(own_slot), ); // We trim all text nodes before checking as it's likely these are whitespaces. diff --git a/packages/yew/src/html/component/scope.rs b/packages/yew/src/html/component/scope.rs index 4a4681f385e..4490718f716 100644 --- a/packages/yew/src/html/component/scope.rs +++ b/packages/yew/src/html/component/scope.rs @@ -546,11 +546,11 @@ mod feat_csr { parent: Element, slot: DomSlot, props: Rc, - ) -> DynamicDomSlot { + ) -> DomSlot { let bundle = Bundle::new(); let sibling_slot = DynamicDomSlot::new(slot); let own_slot = DynamicDomSlot::new(sibling_slot.to_position()); - let shared_slot = own_slot.clone(); + let shared_slot = own_slot.to_position(); let state = ComponentRenderState::Render { bundle, @@ -654,7 +654,7 @@ mod feat_hydration { use web_sys::{Element, HtmlScriptElement}; use super::*; - use crate::dom_bundle::{BSubtree, DomSlot, DynamicDomSlot, Fragment}; + use crate::dom_bundle::{BSubtree, DomSlot, DynamicDomSlot, Fragment, SlotBulletin}; use crate::html::component::lifecycle::{ComponentRenderState, CreateRunner, RenderRunner}; use crate::scheduler; use crate::virtual_dom::Collectable; @@ -677,8 +677,8 @@ mod feat_hydration { parent: Element, fragment: &mut Fragment, props: Rc, - prev_next_sibling: &mut Option, - ) -> DynamicDomSlot { + prev_next_sibling: &mut SlotBulletin<'_>, + ) -> DomSlot { // This is very helpful to see which component is failing during hydration // which means this component may not having a stable layout / differs between // client-side and server-side. @@ -705,16 +705,15 @@ mod feat_hydration { _ => None, }; + let sibling_slot = DynamicDomSlot::new_debug_trapped(); let own_slot = match fragment.front().cloned() { Some(first_node) => DynamicDomSlot::new(DomSlot::at(first_node)), - None => DynamicDomSlot::new(DomSlot::at_end()), + None => DynamicDomSlot::new(sibling_slot.to_position()), }; - let shared_slot = own_slot.clone(); - let sibling_slot = DynamicDomSlot::new_debug_trapped(); - if let Some(prev_next_sibling) = prev_next_sibling { - prev_next_sibling.reassign(shared_slot.to_position()); - } - *prev_next_sibling = Some(sibling_slot.clone()); + // sibling must be reassignable, but only for this call hierarchy. Hence the owner is + // alive when that write in the sibling component happens. + prev_next_sibling.write_at_comp(own_slot.to_position(), &sibling_slot); + let shared_slot = own_slot.to_position(); let state = ComponentRenderState::Hydration { parent, root, diff --git a/packages/yew/src/virtual_dom/vcomp.rs b/packages/yew/src/virtual_dom/vcomp.rs index 5087cc36c7a..ca082625e06 100644 --- a/packages/yew/src/virtual_dom/vcomp.rs +++ b/packages/yew/src/virtual_dom/vcomp.rs @@ -10,10 +10,10 @@ use futures::future::{FutureExt, LocalBoxFuture}; use web_sys::Element; use super::Key; -#[cfg(feature = "hydration")] -use crate::dom_bundle::Fragment; #[cfg(feature = "csr")] -use crate::dom_bundle::{BSubtree, DomSlot, DynamicDomSlot}; +use crate::dom_bundle::{BSubtree, DomSlot}; +#[cfg(feature = "hydration")] +use crate::dom_bundle::{Fragment, SlotBulletin}; use crate::html::BaseComponent; #[cfg(feature = "csr")] use crate::html::Scoped; @@ -65,7 +65,7 @@ pub(crate) trait Mountable { parent_scope: &AnyScope, parent: Element, slot: DomSlot, - ) -> (Box, DynamicDomSlot); + ) -> (Box, DomSlot); #[cfg(feature = "csr")] fn reuse(self: Box, scope: &dyn Scoped, slot: DomSlot); @@ -86,8 +86,8 @@ pub(crate) trait Mountable { parent_scope: &AnyScope, parent: Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, - ) -> (Box, DynamicDomSlot); + prev_next_sibling: &mut SlotBulletin<'_>, + ) -> (Box, DomSlot); } pub(crate) struct PropsWrapper { @@ -126,7 +126,7 @@ impl Mountable for PropsWrapper { parent_scope: &AnyScope, parent: Element, slot: DomSlot, - ) -> (Box, DynamicDomSlot) { + ) -> (Box, DomSlot) { let scope: Scope = Scope::new(Some(parent_scope.clone())); let own_slot = scope.mount_in_place(root.clone(), parent, slot, self.props); @@ -164,8 +164,8 @@ impl Mountable for PropsWrapper { parent_scope: &AnyScope, parent: Element, fragment: &mut Fragment, - prev_next_sibling: &mut Option, - ) -> (Box, DynamicDomSlot) { + prev_next_sibling: &mut SlotBulletin<'_>, + ) -> (Box, DomSlot) { let scope: Scope = Scope::new(Some(parent_scope.clone())); let own_slot = scope.hydrate_in_place(root, parent, fragment, self.props, prev_next_sibling); From 0de095b58c1033ee190f0a1fd0295760da806ef8 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 12:06:02 +0200 Subject: [PATCH 02/17] update wasm opt --- examples/async_clock/Trunk.toml | 2 +- examples/boids/Trunk.toml | 2 +- examples/communication_child_to_parent/Trunk.toml | 2 +- examples/communication_grandchild_with_grandparent/Trunk.toml | 2 +- examples/communication_grandparent_to_grandchild/Trunk.toml | 2 +- examples/communication_parent_to_child/Trunk.toml | 2 +- examples/contexts/Trunk.toml | 2 +- examples/counter/Trunk.toml | 2 +- examples/counter_functional/Trunk.toml | 2 +- examples/dyn_create_destroy_apps/Trunk.toml | 2 +- examples/file_upload/Trunk.toml | 2 +- examples/function_delayed_input/Trunk.toml | 2 +- examples/function_memory_game/Trunk.toml | 2 +- examples/function_router/Trunk.toml | 2 +- examples/function_todomvc/Trunk.toml | 2 +- examples/futures/Trunk.toml | 2 +- examples/game_of_life/Trunk.toml | 2 +- examples/immutable/Trunk.toml | 2 +- examples/inner_html/Trunk.toml | 2 +- examples/js_callback/Trunk.toml | 2 +- examples/keyed_list/Trunk.toml | 2 +- examples/mount_point/Trunk.toml | 2 +- examples/nested_list/Trunk.toml | 2 +- examples/node_refs/Trunk.toml | 2 +- examples/password_strength/Trunk.toml | 2 +- examples/portals/Trunk.toml | 2 +- examples/router/Trunk.toml | 2 +- examples/suspense/Trunk.toml | 2 +- examples/timer/Trunk.toml | 2 +- examples/timer_functional/Trunk.toml | 2 +- examples/todomvc/Trunk.toml | 2 +- examples/two_apps/Trunk.toml | 2 +- examples/web_worker_fib/Trunk.toml | 2 +- examples/web_worker_prime/Trunk.toml | 2 +- examples/webgl/Trunk.toml | 2 +- 35 files changed, 35 insertions(+), 35 deletions(-) diff --git a/examples/async_clock/Trunk.toml b/examples/async_clock/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/async_clock/Trunk.toml +++ b/examples/async_clock/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/boids/Trunk.toml b/examples/boids/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/boids/Trunk.toml +++ b/examples/boids/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/communication_child_to_parent/Trunk.toml b/examples/communication_child_to_parent/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/communication_child_to_parent/Trunk.toml +++ b/examples/communication_child_to_parent/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/communication_grandchild_with_grandparent/Trunk.toml b/examples/communication_grandchild_with_grandparent/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/communication_grandchild_with_grandparent/Trunk.toml +++ b/examples/communication_grandchild_with_grandparent/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/communication_grandparent_to_grandchild/Trunk.toml b/examples/communication_grandparent_to_grandchild/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/communication_grandparent_to_grandchild/Trunk.toml +++ b/examples/communication_grandparent_to_grandchild/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/communication_parent_to_child/Trunk.toml b/examples/communication_parent_to_child/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/communication_parent_to_child/Trunk.toml +++ b/examples/communication_parent_to_child/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/contexts/Trunk.toml b/examples/contexts/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/contexts/Trunk.toml +++ b/examples/contexts/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/counter/Trunk.toml b/examples/counter/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/counter/Trunk.toml +++ b/examples/counter/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/counter_functional/Trunk.toml b/examples/counter_functional/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/counter_functional/Trunk.toml +++ b/examples/counter_functional/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/dyn_create_destroy_apps/Trunk.toml b/examples/dyn_create_destroy_apps/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/dyn_create_destroy_apps/Trunk.toml +++ b/examples/dyn_create_destroy_apps/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/file_upload/Trunk.toml b/examples/file_upload/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/file_upload/Trunk.toml +++ b/examples/file_upload/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/function_delayed_input/Trunk.toml b/examples/function_delayed_input/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/function_delayed_input/Trunk.toml +++ b/examples/function_delayed_input/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/function_memory_game/Trunk.toml b/examples/function_memory_game/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/function_memory_game/Trunk.toml +++ b/examples/function_memory_game/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/function_router/Trunk.toml b/examples/function_router/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/function_router/Trunk.toml +++ b/examples/function_router/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/function_todomvc/Trunk.toml b/examples/function_todomvc/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/function_todomvc/Trunk.toml +++ b/examples/function_todomvc/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/futures/Trunk.toml b/examples/futures/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/futures/Trunk.toml +++ b/examples/futures/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/game_of_life/Trunk.toml b/examples/game_of_life/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/game_of_life/Trunk.toml +++ b/examples/game_of_life/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/immutable/Trunk.toml b/examples/immutable/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/immutable/Trunk.toml +++ b/examples/immutable/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/inner_html/Trunk.toml b/examples/inner_html/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/inner_html/Trunk.toml +++ b/examples/inner_html/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/js_callback/Trunk.toml b/examples/js_callback/Trunk.toml index c18aef7c57c..806f328106e 100644 --- a/examples/js_callback/Trunk.toml +++ b/examples/js_callback/Trunk.toml @@ -1,5 +1,5 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" [[hooks]] stage = "pre_build" diff --git a/examples/keyed_list/Trunk.toml b/examples/keyed_list/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/keyed_list/Trunk.toml +++ b/examples/keyed_list/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/mount_point/Trunk.toml b/examples/mount_point/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/mount_point/Trunk.toml +++ b/examples/mount_point/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/nested_list/Trunk.toml b/examples/nested_list/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/nested_list/Trunk.toml +++ b/examples/nested_list/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/node_refs/Trunk.toml b/examples/node_refs/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/node_refs/Trunk.toml +++ b/examples/node_refs/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/password_strength/Trunk.toml b/examples/password_strength/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/password_strength/Trunk.toml +++ b/examples/password_strength/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/portals/Trunk.toml b/examples/portals/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/portals/Trunk.toml +++ b/examples/portals/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/router/Trunk.toml b/examples/router/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/router/Trunk.toml +++ b/examples/router/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/suspense/Trunk.toml b/examples/suspense/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/suspense/Trunk.toml +++ b/examples/suspense/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/timer/Trunk.toml b/examples/timer/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/timer/Trunk.toml +++ b/examples/timer/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/timer_functional/Trunk.toml b/examples/timer_functional/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/timer_functional/Trunk.toml +++ b/examples/timer_functional/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/todomvc/Trunk.toml b/examples/todomvc/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/todomvc/Trunk.toml +++ b/examples/todomvc/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/two_apps/Trunk.toml b/examples/two_apps/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/two_apps/Trunk.toml +++ b/examples/two_apps/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/web_worker_fib/Trunk.toml b/examples/web_worker_fib/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/web_worker_fib/Trunk.toml +++ b/examples/web_worker_fib/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/web_worker_prime/Trunk.toml b/examples/web_worker_prime/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/web_worker_prime/Trunk.toml +++ b/examples/web_worker_prime/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" diff --git a/examples/webgl/Trunk.toml b/examples/webgl/Trunk.toml index f6ad36c4520..783e0ba4fd5 100644 --- a/examples/webgl/Trunk.toml +++ b/examples/webgl/Trunk.toml @@ -1,2 +1,2 @@ [tools] -wasm_opt = "version_129" +wasm_opt = "version_131" From 141801218f95cc1577759d1ea0f986b44ba8ca78 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 12:13:07 +0200 Subject: [PATCH 03/17] fix strict lint in timer --- tools/build-examples/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build-examples/src/main.rs b/tools/build-examples/src/main.rs index d9e55ddc12a..68da747fd7d 100644 --- a/tools/build-examples/src/main.rs +++ b/tools/build-examples/src/main.rs @@ -60,7 +60,7 @@ fn main() -> ExitCode { println!("::endgroup::"); if !sample_success { - eprintln!("::error ::{example} failed to build"); + println!("::error::{example} failed to build"); failure = true; } } From 0f4c65d6330d876f4e8e2b3bd382881bfe0c0ae9 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 12:21:27 +0200 Subject: [PATCH 04/17] fix another example: futures --- tools/build-examples/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/build-examples/src/main.rs b/tools/build-examples/src/main.rs index 68da747fd7d..abb3aa2f513 100644 --- a/tools/build-examples/src/main.rs +++ b/tools/build-examples/src/main.rs @@ -60,7 +60,7 @@ fn main() -> ExitCode { println!("::endgroup::"); if !sample_success { - println!("::error::{example} failed to build"); + println!("::error ::{example} failed to build"); failure = true; } } From 7e990140100868e8cb36d35c0224c36492ad502a Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 12:33:39 +0200 Subject: [PATCH 05/17] temp save --- packages/yew/src/dom_bundle/position.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index e14060c74a4..32fcf919b39 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -1,10 +1,13 @@ //! Structs for keeping track where in the DOM a node belongs use std::cell::RefCell; +use std::marker::PhantomData; use std::rc::Rc; use web_sys::{Element, Node}; +type PhantomNotSendNorSync = PhantomData<*const u8>; + /// A position in the list of children of an implicit parent [`Element`]. /// /// This can either be in front of a `DomSlot::at(next_sibling)`, at the end of the list with @@ -301,7 +304,7 @@ mod feat_hydration { } } #[cfg(feature = "hydration")] -pub use feat_hydration::SlotBulletin; +pub(crate) use feat_hydration::SlotBulletin; #[cfg(all(target_arch = "wasm32", not(target_os = "wasi")))] #[cfg(test)] From 2c925c6dfec56ef97fa977f684d08927d2c92611 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 13:42:13 +0200 Subject: [PATCH 06/17] implement simple slab based link tree --- packages/yew/src/dom_bundle/position.rs | 201 +++++++++++++++++------- 1 file changed, 141 insertions(+), 60 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index 32fcf919b39..a4ed2e5a817 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -2,8 +2,8 @@ use std::cell::RefCell; use std::marker::PhantomData; -use std::rc::Rc; +use slab::Slab; use web_sys::{Element, Node}; type PhantomNotSendNorSync = PhantomData<*const u8>; @@ -17,43 +17,85 @@ pub(crate) struct DomSlot { variant: DomSlotVariant, } +impl std::fmt::Debug for DomSlot { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.with_next_sibling(|n| { + let formatted_node = match n { + None => None, + Some(n) if trap_impl::is_trap(n) => Some("".to_string()), + Some(n) => Some(crate::utils::print_node(n)), + }; + write!(f, "DomSlot {{ next_sibling: {formatted_node:?} }}") + }) + } +} + +#[derive(Clone)] enum DomSlotVariant { Node(Option), - Chained(DynamicDomSlot), + Chained(DynamicDomSlotHandle), +} + +struct Link { + parent: DomSlot, + /// counts the owner + the number of links in DYNAMIC_SLOTS that refer to this link + /// does NOT count the number of handles + ref_count: usize, + has_owner: bool, } -impl Clone for DomSlotVariant { - fn clone(&self) -> Self { - match self { - Self::Node(node) => Self::Node(node.clone()), - Self::Chained(slot) => Self::Chained(slot.clone_to_follower()), +impl Link { + fn new(parent: DomSlot) -> Self { + Self { + parent, + ref_count: 1, + has_owner: true, } } + + fn dec_owner(&mut self) -> bool { + debug_assert!(self.has_owner, "must have an owner"); + self.has_owner = false; + self.dec_ref() + } + + fn dec_ref(&mut self) -> bool { + debug_assert!(self.ref_count > 0, "must have refs"); + self.ref_count -= 1; + self.ref_count == 0 + } + + fn add_ref(&mut self) { + debug_assert!(self.ref_count > 0, "no revives"); + self.ref_count += 1; + } +} + +thread_local! { + static DYNAMIC_SLOTS: RefCell> = RefCell::new(Slab::new()); } +type LinkId = usize; // Dictated by slab + /// A dynamic dom slot can be reassigned. This change is also seen by the [`DomSlot`] from /// [`Self::to_position`] before the reassignment took place. pub(crate) struct DynamicDomSlot { - target: Rc>, + link: LinkId, + // The link is tied to this specific thread and can't be accessed elsewhere + _phantom: PhantomNotSendNorSync, } -impl std::fmt::Debug for DomSlot { +impl std::fmt::Debug for DynamicDomSlot { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - self.with_next_sibling(|n| { - let formatted_node = match n { - None => None, - Some(n) if trap_impl::is_trap(n) => Some("".to_string()), - Some(n) => Some(crate::utils::print_node(n)), - }; - write!(f, "DomSlot {{ next_sibling: {formatted_node:?} }}") - }) + write!(f, "#{}", self.link) } } -impl std::fmt::Debug for DynamicDomSlot { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{:#?}", *self.target.borrow()) - } +#[derive(Clone)] +struct DynamicDomSlotHandle { + link: LinkId, + // The link is tied to this specific thread and can't be accessed elsewhere + _phantom: PhantomNotSendNorSync, } mod trap_impl { @@ -167,8 +209,10 @@ impl DynamicDomSlot { /// Create a dynamic dom slot that initially represents ("targets") the same slot as the /// argument. pub fn new(initial_position: DomSlot) -> Self { + let link = DYNAMIC_SLOTS.with_borrow_mut(|slots| slots.insert(Link::new(initial_position))); Self { - target: Rc::new(RefCell::new(initial_position)), + link, + _phantom: PhantomData, } } @@ -188,8 +232,7 @@ impl DynamicDomSlot { /// Change the [`DomSlot`] that is targeted. Subsequently, this will behave as if `self` was /// created from the passed DomSlot in the first place. pub fn reassign(&self, next_position: DomSlot) { - // TODO: is not defensive against accidental reference loops - *self.target.borrow_mut() = next_position; + self.clone_to_follower().reassign_unchecked(next_position); } /// Get a [`DomSlot`] that gets automatically updated when `self` gets reassigned. All such @@ -205,48 +248,86 @@ impl DynamicDomSlot { /// and should only read the value, but never write to it. /// This does not imply that access is always serialized! Followers are allowed to write at any /// point without prior synchronization, as long as they ensure that the owner is still alive. - fn clone_to_follower(&self) -> Self { - // TODO: the return value could be a different type - Self { - target: self.target.clone(), + fn clone_to_follower(&self) -> DynamicDomSlotHandle { + DynamicDomSlotHandle { + link: self.link, + _phantom: self._phantom, } } +} + +fn remove_link(slots: &mut Slab, link: LinkId) { + let mut link = link; + loop { + let removed = slots.remove(link); + let DomSlotVariant::Chained(handle) = removed.parent.variant else { + break; + }; + if !slots.get_mut(handle.link).unwrap().dec_ref() { + break; + } + link = handle.link; + } + // from time to time, clean up memory in the slab + const ALLOWED_SLACK: usize = 1024 * 1024 * 1024 / size_of::(); + if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { + slots.shrink_to_fit(); + } +} + +impl Drop for DynamicDomSlot { + fn drop(&mut self) { + DYNAMIC_SLOTS.with_borrow_mut(|slots| { + if slots.get_mut(self.link).unwrap().dec_owner() { + remove_link(slots, self.link); + } + }); + } +} + +impl DynamicDomSlotHandle { + /// Reassign through a handle. This is only valid if the owning [DynamicDomSlot] is still alive. + fn reassign_unchecked(&self, next_position: DomSlot) { + // TODO: is not defensive against accidental reference loops + DYNAMIC_SLOTS.with_borrow_mut(|slots| { + let old_parent = slots.get_mut(self.link).unwrap().parent.clone(); + match (&old_parent.variant, &next_position.variant) { + (DomSlotVariant::Node(_), DomSlotVariant::Node(_)) => {} + (DomSlotVariant::Node(_), DomSlotVariant::Chained(new_parent)) => { + slots.get_mut(new_parent.link).unwrap().add_ref(); + } + (DomSlotVariant::Chained(old_parent), DomSlotVariant::Node(_)) => { + if slots.get_mut(old_parent.link).unwrap().dec_ref() { + remove_link(slots, old_parent.link); + } + } + (DomSlotVariant::Chained(old_parent), DomSlotVariant::Chained(new_parent)) => { + if old_parent.link == new_parent.link { + return; + } + slots.get_mut(new_parent.link).unwrap().add_ref(); + if slots.get_mut(old_parent.link).unwrap().dec_ref() { + remove_link(slots, old_parent.link); + } + } + } + slots.get_mut(self.link).unwrap().parent = next_position; + }); + } fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { // We use an iterative approach to traverse a possible long chain of references. // See issue #3043 for why a recursive call is impossible for large lists in vdom. - // - // TODO: there could be some data structure that performs better here. E.g. a balanced tree - // with parent pointers come to mind, but they are a bit fiddly to implement in rust - // - // We traverse via raw pointers to avoid Rc refcount overhead (clone + drop) per hop, then - // clone the terminal next-sibling out of the chain before invoking `f`. Invoking `f` with - // no borrow held and no reliance on chain structure keeps the traversal sound: `f` runs - // arbitrary code (panic drop glue, `gloo::console::error`, tracing subscribers) that - // could, in principle, reassign a link in the chain and drop the last strong reference - // to the RefCell we would otherwise still borrow from. - // - // SAFETY: All RefCells visited by the loop remain live while we dereference them: - // - `self.target` (Rc) is alive because `self` is borrowed - // - Each DomSlot::Chained(DynamicDomSlot { target }) in the chain holds a strong Rc to the - // next RefCell, so all links are transitively kept alive - // - Yew is single-threaded and the loop body does not run user code, so no mutable borrow - // (e.g. from reassign()) can occur on any RefCell in the chain during traversal - // - Each RefCell::borrow() is dropped before advancing to the next hop - let node: Option = { - let mut ptr: *const RefCell = Rc::as_ptr(&self.target); + DYNAMIC_SLOTS.with_borrow(|slots| { + let mut link = self.link; loop { - let cell = unsafe { &*ptr }; - let slot_ref = cell.borrow(); - match &slot_ref.variant { - DomSlotVariant::Node(n) => break n.clone(), - DomSlotVariant::Chained(chain) => { - ptr = Rc::as_ptr(&chain.target); - } + match &slots.get(link).unwrap().parent.variant { + // NOTE: This leaves the slots borrowed. We can't re-enter mutably! + DomSlotVariant::Node(node) => return f(node.as_ref()), + DomSlotVariant::Chained(handle) => link = handle.link, } } - }; - f(node.as_ref()) + }) } } @@ -256,10 +337,10 @@ mod feat_hydration { use web_sys::Node; - use super::{DomSlot, DynamicDomSlot}; + use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle}; pub struct SlotBulletin<'tree> { - prev_next_sibling: Option, + prev_next_sibling: Option, _owner: PhantomData<&'tree mut DynamicDomSlot>, } impl<'tree> SlotBulletin<'tree> { @@ -280,7 +361,7 @@ mod feat_hydration { fn write(&mut self, pos: DomSlot) { if let Some(slot) = &mut self.prev_next_sibling { - slot.reassign(pos); + slot.reassign_unchecked(pos); } } From 1a1d193321a7e65de0f63c3f5180a63f98b0c1e5 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 17:05:48 +0200 Subject: [PATCH 07/17] fix re-entrancy and basic refcounting --- packages/yew/src/dom_bundle/bcomp.rs | 10 +++++----- packages/yew/src/dom_bundle/position.rs | 16 +++++++++++----- packages/yew/tests/use_state.rs | 2 -- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/yew/src/dom_bundle/bcomp.rs b/packages/yew/src/dom_bundle/bcomp.rs index c653b03fc14..f8cf509add4 100644 --- a/packages/yew/src/dom_bundle/bcomp.rs +++ b/packages/yew/src/dom_bundle/bcomp.rs @@ -207,19 +207,19 @@ mod tests { #[test] fn set_properties_to_component() { - html! { + let _ = html! { }; - html! { + let _ = html! { }; - html! { + let _ = html! { }; - html! { + let _ = html! { }; @@ -228,7 +228,7 @@ mod tests { field_2: 1, }; - html! { + let _ = html! { }; } diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index a4ed2e5a817..5a60a647a37 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -209,7 +209,12 @@ impl DynamicDomSlot { /// Create a dynamic dom slot that initially represents ("targets") the same slot as the /// argument. pub fn new(initial_position: DomSlot) -> Self { - let link = DYNAMIC_SLOTS.with_borrow_mut(|slots| slots.insert(Link::new(initial_position))); + let link = DYNAMIC_SLOTS.with_borrow_mut(|slots| { + if let DomSlotVariant::Chained(parent) = &initial_position.variant { + slots.get_mut(parent.link).unwrap().add_ref(); + } + slots.insert(Link::new(initial_position)) + }); Self { link, _phantom: PhantomData, @@ -318,16 +323,17 @@ impl DynamicDomSlotHandle { fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { // We use an iterative approach to traverse a possible long chain of references. // See issue #3043 for why a recursive call is impossible for large lists in vdom. - DYNAMIC_SLOTS.with_borrow(|slots| { + let node = DYNAMIC_SLOTS.with_borrow(|slots| { let mut link = self.link; loop { match &slots.get(link).unwrap().parent.variant { - // NOTE: This leaves the slots borrowed. We can't re-enter mutably! - DomSlotVariant::Node(node) => return f(node.as_ref()), + // NOTE: We clone to drop the borrow and let f re-enter this method + DomSlotVariant::Node(node) => break node.clone(), DomSlotVariant::Chained(handle) => link = handle.link, } } - }) + }); + f(node.as_ref()) } } diff --git a/packages/yew/tests/use_state.rs b/packages/yew/tests/use_state.rs index 1dbe17ce423..7593bc7fad0 100644 --- a/packages/yew/tests/use_state.rs +++ b/packages/yew/tests/use_state.rs @@ -2,8 +2,6 @@ mod common; -use std::rc::Rc; - use common::obtain_result; use wasm_bindgen_test::*; use yew::prelude::*; From 69549c34242f7502abe46837f3e6f703c3b3e3b1 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 17:07:33 +0200 Subject: [PATCH 08/17] make static initializer const ty clippy --- packages/yew/src/dom_bundle/position.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index 5a60a647a37..fd223307b54 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -72,7 +72,7 @@ impl Link { } thread_local! { - static DYNAMIC_SLOTS: RefCell> = RefCell::new(Slab::new()); + static DYNAMIC_SLOTS: RefCell> = const { RefCell::new(Slab::new()) }; } type LinkId = usize; // Dictated by slab From 73933a8c298d729e42a4b7f33104fdac9c440ac0 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 18:38:37 +0200 Subject: [PATCH 09/17] reserve trap spot up front --- packages/yew/src/dom_bundle/position.rs | 131 +++++++++++++----------- 1 file changed, 71 insertions(+), 60 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index fd223307b54..efa70a1963c 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -71,8 +71,16 @@ impl Link { } } +const RESERVED_TRAP_SLOT: usize = 0; thread_local! { - static DYNAMIC_SLOTS: RefCell> = const { RefCell::new(Slab::new()) }; + static DYNAMIC_SLOTS: RefCell> = { + let mut slots = Slab::new(); + let mut fake_trap_link = Link::new(DomSlot::at_end()); + fake_trap_link.add_ref(); // ensure this is never collected + fake_trap_link.dec_owner(); // has no owner though, so never written to + assert_eq!(slots.insert(fake_trap_link), RESERVED_TRAP_SLOT); + RefCell::new(slots) + }; } type LinkId = usize; // Dictated by slab @@ -87,7 +95,10 @@ pub(crate) struct DynamicDomSlot { impl std::fmt::Debug for DynamicDomSlot { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "#{}", self.link) + DYNAMIC_SLOTS.with(|slots| match slots.try_borrow() { + Ok(slots) => write!(f, "<{:?}>", slots[self.link].parent), + Err(_) => write!(f, "#{}", self.link), + }) } } @@ -100,35 +111,30 @@ struct DynamicDomSlotHandle { mod trap_impl { use super::Node; - #[cfg(debug_assertions)] + #[cfg(all(debug_assertions, feature = "hydration"))] thread_local! { // A special marker element that should not be referenced - static TRAP: Node = gloo::utils::document().create_element("div").unwrap().into(); + static TRAP: Node = { + use super::{DYNAMIC_SLOTS, DomSlot, RESERVED_TRAP_SLOT}; + let node: Node = gloo::utils::document().create_element("div").unwrap().into(); + DYNAMIC_SLOTS.with_borrow_mut(|slots| slots[RESERVED_TRAP_SLOT].parent = DomSlot::at(node.clone())); + node + }; } - /// Get a "trap" node, or None if compiled without debug_assertions - #[cfg(feature = "hydration")] - pub fn get_trap_node() -> Option { - #[cfg(debug_assertions)] + #[inline] + pub fn with_trap_ref(f: impl FnOnce(Option<&Node>) -> R) -> R { + #[cfg(all(debug_assertions, feature = "hydration"))] { - TRAP.with(|trap| Some(trap.clone())) + TRAP.with(|trap| f(Some(trap))) } - #[cfg(not(debug_assertions))] + #[cfg(not(all(debug_assertions, feature = "hydration")))] { - None + f(None) } } #[inline] pub fn is_trap(node: &Node) -> bool { - #[cfg(debug_assertions)] - { - TRAP.with(|trap| node == trap) - } - #[cfg(not(debug_assertions))] - { - // When not running with debug_assertions, there is no trap node - let _ = node; - false - } + with_trap_ref(|trap| trap == Some(node)) } } @@ -149,13 +155,6 @@ impl DomSlot { } } - /// A new "placeholder" [DomSlot] that should not be used to insert nodes - #[inline] - #[cfg(feature = "hydration")] - pub fn new_debug_trapped() -> Self { - Self::create(trap_impl::get_trap_node()) - } - /// Get the [Node] that comes just after the position, or `None` if this denotes the position at /// the end fn with_next_sibling_check_trap(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { @@ -211,7 +210,7 @@ impl DynamicDomSlot { pub fn new(initial_position: DomSlot) -> Self { let link = DYNAMIC_SLOTS.with_borrow_mut(|slots| { if let DomSlotVariant::Chained(parent) = &initial_position.variant { - slots.get_mut(parent.link).unwrap().add_ref(); + slots[parent.link].add_ref(); } slots.insert(Link::new(initial_position)) }); @@ -221,19 +220,6 @@ impl DynamicDomSlot { } } - #[cfg(feature = "hydration")] - pub fn new_debug_trapped() -> Self { - Self::new(DomSlot::new_debug_trapped()) - } - - /// Move out of self, leaving behind a trapped slot. `self` should not be used afterwards. - /// Used during the transition from a hydrating to a rendered component to move state between - /// enum variants. - #[cfg(feature = "hydration")] - pub fn take(&mut self) -> Self { - std::mem::replace(self, Self::new(DomSlot::new_debug_trapped())) - } - /// Change the [`DomSlot`] that is targeted. Subsequently, this will behave as if `self` was /// created from the passed DomSlot in the first place. pub fn reassign(&self, next_position: DomSlot) { @@ -261,14 +247,22 @@ impl DynamicDomSlot { } } -fn remove_link(slots: &mut Slab, link: LinkId) { +fn remove_link(slots: &mut Slab, link: LinkId, owner: bool) { + let was_last = if owner { + slots[link].dec_owner() + } else { + slots[link].dec_ref() + }; + if !was_last { + return; + } let mut link = link; loop { let removed = slots.remove(link); let DomSlotVariant::Chained(handle) = removed.parent.variant else { break; }; - if !slots.get_mut(handle.link).unwrap().dec_ref() { + if !slots[handle.link].dec_ref() { break; } link = handle.link; @@ -283,9 +277,7 @@ fn remove_link(slots: &mut Slab, link: LinkId) { impl Drop for DynamicDomSlot { fn drop(&mut self) { DYNAMIC_SLOTS.with_borrow_mut(|slots| { - if slots.get_mut(self.link).unwrap().dec_owner() { - remove_link(slots, self.link); - } + remove_link(slots, self.link, true); }); } } @@ -295,28 +287,24 @@ impl DynamicDomSlotHandle { fn reassign_unchecked(&self, next_position: DomSlot) { // TODO: is not defensive against accidental reference loops DYNAMIC_SLOTS.with_borrow_mut(|slots| { - let old_parent = slots.get_mut(self.link).unwrap().parent.clone(); + let old_parent = slots[self.link].parent.clone(); match (&old_parent.variant, &next_position.variant) { (DomSlotVariant::Node(_), DomSlotVariant::Node(_)) => {} (DomSlotVariant::Node(_), DomSlotVariant::Chained(new_parent)) => { - slots.get_mut(new_parent.link).unwrap().add_ref(); + slots[new_parent.link].add_ref(); } (DomSlotVariant::Chained(old_parent), DomSlotVariant::Node(_)) => { - if slots.get_mut(old_parent.link).unwrap().dec_ref() { - remove_link(slots, old_parent.link); - } + remove_link(slots, old_parent.link, false); } (DomSlotVariant::Chained(old_parent), DomSlotVariant::Chained(new_parent)) => { if old_parent.link == new_parent.link { return; } - slots.get_mut(new_parent.link).unwrap().add_ref(); - if slots.get_mut(old_parent.link).unwrap().dec_ref() { - remove_link(slots, old_parent.link); - } + slots[new_parent.link].add_ref(); + remove_link(slots, old_parent.link, false); } } - slots.get_mut(self.link).unwrap().parent = next_position; + slots[self.link].parent = next_position; }); } @@ -326,7 +314,7 @@ impl DynamicDomSlotHandle { let node = DYNAMIC_SLOTS.with_borrow(|slots| { let mut link = self.link; loop { - match &slots.get(link).unwrap().parent.variant { + match &slots[link].parent.variant { // NOTE: We clone to drop the borrow and let f re-enter this method DomSlotVariant::Node(node) => break node.clone(), DomSlotVariant::Chained(handle) => link = handle.link, @@ -343,7 +331,30 @@ mod feat_hydration { use web_sys::Node; - use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle}; + use super::{ + DomSlot, DomSlotVariant, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_SLOT, + }; + + impl DynamicDomSlot { + pub fn new_debug_trapped() -> Self { + // make sure the trap is initialized before we return + let _ = super::trap_impl::with_trap_ref(|_| ()); + let trap_handle = DomSlot { + variant: DomSlotVariant::Chained(DynamicDomSlotHandle { + link: RESERVED_TRAP_SLOT, + _phantom: PhantomData, + }), + }; + Self::new(trap_handle) + } + + /// Move out of self, leaving behind a trapped slot. `self` should not be used afterwards. + /// Used during the transition from a hydrating to a rendered component to move state + /// between enum variants. + pub fn take(&mut self) -> Self { + std::mem::replace(self, Self::new_debug_trapped()) + } + } pub struct SlotBulletin<'tree> { prev_next_sibling: Option, @@ -471,7 +482,7 @@ mod layout_tests { fn debug_printing() { // basic tests that these don't panic. We don't enforce any specific format. println!("At end: {:?}", DomSlot::at_end()); - println!("Trapped: {:?}", DomSlot::new_debug_trapped()); + println!("Trapped: {:?}", DynamicDomSlot::new_debug_trapped()); println!( "At element: {:?}", DomSlot::at(document().create_element("p").unwrap().into()) From f335249d225f8d355c51fd1658ae3d5ced5af0a6 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Wed, 22 Jul 2026 18:57:39 +0200 Subject: [PATCH 10/17] only reserve trap slot if debug_assertions are on --- packages/yew/src/dom_bundle/position.rs | 84 +++++++++++++------------ 1 file changed, 45 insertions(+), 39 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index efa70a1963c..73d197dc695 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -53,13 +53,11 @@ impl Link { } } - fn dec_owner(&mut self) -> bool { - debug_assert!(self.has_owner, "must have an owner"); - self.has_owner = false; - self.dec_ref() - } - - fn dec_ref(&mut self) -> bool { + fn dec_ref(&mut self, owner: bool) -> bool { + if owner { + debug_assert!(self.has_owner, "must have an owner"); + self.has_owner = false; + } debug_assert!(self.ref_count > 0, "must have refs"); self.ref_count -= 1; self.ref_count == 0 @@ -75,10 +73,14 @@ const RESERVED_TRAP_SLOT: usize = 0; thread_local! { static DYNAMIC_SLOTS: RefCell> = { let mut slots = Slab::new(); - let mut fake_trap_link = Link::new(DomSlot::at_end()); - fake_trap_link.add_ref(); // ensure this is never collected - fake_trap_link.dec_owner(); // has no owner though, so never written to - assert_eq!(slots.insert(fake_trap_link), RESERVED_TRAP_SLOT); + trap_impl::with_trap_ref(|trap| { + if let Some(trap) = trap { + let mut fake_trap_link = Link::new(DomSlot::at(trap.clone())); + fake_trap_link.add_ref(); // ensure this is never collected + fake_trap_link.dec_ref(true); // has no owner though, so never written to + assert_eq!(slots.insert(fake_trap_link), RESERVED_TRAP_SLOT); + } + }); RefCell::new(slots) }; } @@ -114,12 +116,7 @@ mod trap_impl { #[cfg(all(debug_assertions, feature = "hydration"))] thread_local! { // A special marker element that should not be referenced - static TRAP: Node = { - use super::{DYNAMIC_SLOTS, DomSlot, RESERVED_TRAP_SLOT}; - let node: Node = gloo::utils::document().create_element("div").unwrap().into(); - DYNAMIC_SLOTS.with_borrow_mut(|slots| slots[RESERVED_TRAP_SLOT].parent = DomSlot::at(node.clone())); - node - }; + static TRAP: Node = gloo::utils::document().create_element("div").unwrap().into(); } #[inline] pub fn with_trap_ref(f: impl FnOnce(Option<&Node>) -> R) -> R { @@ -229,9 +226,7 @@ impl DynamicDomSlot { /// Get a [`DomSlot`] that gets automatically updated when `self` gets reassigned. All such /// slots are equivalent to each other and point to the same position. pub fn to_position(&self) -> DomSlot { - DomSlot { - variant: DomSlotVariant::Chained(self.clone_to_follower()), - } + self.clone_to_follower().into_position() } /// There can only be one owner of a dynamic dom slot. Reassigning a dom slot is only allowed @@ -248,12 +243,7 @@ impl DynamicDomSlot { } fn remove_link(slots: &mut Slab, link: LinkId, owner: bool) { - let was_last = if owner { - slots[link].dec_owner() - } else { - slots[link].dec_ref() - }; - if !was_last { + if !slots[link].dec_ref(owner) { return; } let mut link = link; @@ -262,12 +252,18 @@ fn remove_link(slots: &mut Slab, link: LinkId, owner: bool) { let DomSlotVariant::Chained(handle) = removed.parent.variant else { break; }; - if !slots[handle.link].dec_ref() { + if !slots[handle.link].dec_ref(false) { break; } link = handle.link; } // from time to time, clean up memory in the slab + // TODO: this needs more analysis under amortized runtime costs and a clever potential + // definition. shrink_to_fit will first check if there are any vacant slots at "the end". If + // there are, it will then do a full pass over empty and filled slots. The problem is that + // "the end" is not easily available from the public API. We know it's somewhere between + // len() and capacity(), and also past the link(s) we just removed. But we can't check the + // internal entries.len(). const ALLOWED_SLACK: usize = 1024 * 1024 * 1024 / size_of::(); if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { slots.shrink_to_fit(); @@ -283,6 +279,12 @@ impl Drop for DynamicDomSlot { } impl DynamicDomSlotHandle { + fn into_position(self) -> DomSlot { + DomSlot { + variant: DomSlotVariant::Chained(self), + } + } + /// Reassign through a handle. This is only valid if the owning [DynamicDomSlot] is still alive. fn reassign_unchecked(&self, next_position: DomSlot) { // TODO: is not defensive against accidental reference loops @@ -331,21 +333,25 @@ mod feat_hydration { use web_sys::Node; - use super::{ - DomSlot, DomSlotVariant, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_SLOT, - }; + use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_SLOT}; - impl DynamicDomSlot { - pub fn new_debug_trapped() -> Self { - // make sure the trap is initialized before we return - let _ = super::trap_impl::with_trap_ref(|_| ()); - let trap_handle = DomSlot { - variant: DomSlotVariant::Chained(DynamicDomSlotHandle { + fn trapped_position() -> DomSlot { + super::trap_impl::with_trap_ref(|trap| match trap { + Some(_) => { + // this handle exists only if we have a trap node + let fake_handle = DynamicDomSlotHandle { link: RESERVED_TRAP_SLOT, _phantom: PhantomData, - }), - }; - Self::new(trap_handle) + }; + fake_handle.into_position() + } + None => DomSlot::at_end(), + }) + } + + impl DynamicDomSlot { + pub fn new_debug_trapped() -> Self { + Self::new(trapped_position()) } /// Move out of self, leaving behind a trapped slot. `self` should not be used afterwards. From 195ce925a72542fd925ce976269cde4ef8ac59a8 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 23 Jul 2026 13:30:07 +0200 Subject: [PATCH 11/17] extract forest impl into own file --- packages/yew/src/dom_bundle/position.rs | 151 +++------------- .../yew/src/dom_bundle/position/forest.rs | 168 ++++++++++++++++++ 2 files changed, 193 insertions(+), 126 deletions(-) create mode 100644 packages/yew/src/dom_bundle/position/forest.rs diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index 73d197dc695..9f4b5d89c93 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -3,7 +3,6 @@ use std::cell::RefCell; use std::marker::PhantomData; -use slab::Slab; use web_sys::{Element, Node}; type PhantomNotSendNorSync = PhantomData<*const u8>; @@ -36,57 +35,28 @@ enum DomSlotVariant { Chained(DynamicDomSlotHandle), } -struct Link { - parent: DomSlot, - /// counts the owner + the number of links in DYNAMIC_SLOTS that refer to this link - /// does NOT count the number of handles - ref_count: usize, - has_owner: bool, -} - -impl Link { - fn new(parent: DomSlot) -> Self { - Self { - parent, - ref_count: 1, - has_owner: true, - } - } - - fn dec_ref(&mut self, owner: bool) -> bool { - if owner { - debug_assert!(self.has_owner, "must have an owner"); - self.has_owner = false; - } - debug_assert!(self.ref_count > 0, "must have refs"); - self.ref_count -= 1; - self.ref_count == 0 - } - - fn add_ref(&mut self) { - debug_assert!(self.ref_count > 0, "no revives"); - self.ref_count += 1; - } -} +mod forest; +use forest::{LinkForest, LinkId}; -const RESERVED_TRAP_SLOT: usize = 0; +// This handle is only valid when trap nodes are active +const RESERVED_TRAP_HANDLE: DynamicDomSlotHandle = DynamicDomSlotHandle { + link: 0, + _phantom: PhantomData, +}; thread_local! { - static DYNAMIC_SLOTS: RefCell> = { - let mut slots = Slab::new(); + static LINK_FOREST: RefCell = { + let mut slots = LinkForest::default(); trap_impl::with_trap_ref(|trap| { if let Some(trap) = trap { - let mut fake_trap_link = Link::new(DomSlot::at(trap.clone())); - fake_trap_link.add_ref(); // ensure this is never collected - fake_trap_link.dec_ref(true); // has no owner though, so never written to - assert_eq!(slots.insert(fake_trap_link), RESERVED_TRAP_SLOT); + let trap_link = slots.insert(DomSlot::at(trap.clone())); + assert_eq!(trap_link, RESERVED_TRAP_HANDLE.link); + slots.leak(trap_link); } }); RefCell::new(slots) }; } -type LinkId = usize; // Dictated by slab - /// A dynamic dom slot can be reassigned. This change is also seen by the [`DomSlot`] from /// [`Self::to_position`] before the reassignment took place. pub(crate) struct DynamicDomSlot { @@ -97,10 +67,7 @@ pub(crate) struct DynamicDomSlot { impl std::fmt::Debug for DynamicDomSlot { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - DYNAMIC_SLOTS.with(|slots| match slots.try_borrow() { - Ok(slots) => write!(f, "<{:?}>", slots[self.link].parent), - Err(_) => write!(f, "#{}", self.link), - }) + write!(f, "#{} -> {:?}", self.link, self.to_position()) } } @@ -205,12 +172,7 @@ impl DynamicDomSlot { /// Create a dynamic dom slot that initially represents ("targets") the same slot as the /// argument. pub fn new(initial_position: DomSlot) -> Self { - let link = DYNAMIC_SLOTS.with_borrow_mut(|slots| { - if let DomSlotVariant::Chained(parent) = &initial_position.variant { - slots[parent.link].add_ref(); - } - slots.insert(Link::new(initial_position)) - }); + let link = LINK_FOREST.with_borrow_mut(|slots| slots.insert(initial_position)); Self { link, _phantom: PhantomData, @@ -220,13 +182,13 @@ impl DynamicDomSlot { /// Change the [`DomSlot`] that is targeted. Subsequently, this will behave as if `self` was /// created from the passed DomSlot in the first place. pub fn reassign(&self, next_position: DomSlot) { - self.clone_to_follower().reassign_unchecked(next_position); + self.clone_to_handle().reassign_unchecked(next_position); } /// Get a [`DomSlot`] that gets automatically updated when `self` gets reassigned. All such /// slots are equivalent to each other and point to the same position. pub fn to_position(&self) -> DomSlot { - self.clone_to_follower().into_position() + self.clone_to_handle().into_position() } /// There can only be one owner of a dynamic dom slot. Reassigning a dom slot is only allowed @@ -234,7 +196,7 @@ impl DynamicDomSlot { /// and should only read the value, but never write to it. /// This does not imply that access is always serialized! Followers are allowed to write at any /// point without prior synchronization, as long as they ensure that the owner is still alive. - fn clone_to_follower(&self) -> DynamicDomSlotHandle { + fn clone_to_handle(&self) -> DynamicDomSlotHandle { DynamicDomSlotHandle { link: self.link, _phantom: self._phantom, @@ -242,39 +204,9 @@ impl DynamicDomSlot { } } -fn remove_link(slots: &mut Slab, link: LinkId, owner: bool) { - if !slots[link].dec_ref(owner) { - return; - } - let mut link = link; - loop { - let removed = slots.remove(link); - let DomSlotVariant::Chained(handle) = removed.parent.variant else { - break; - }; - if !slots[handle.link].dec_ref(false) { - break; - } - link = handle.link; - } - // from time to time, clean up memory in the slab - // TODO: this needs more analysis under amortized runtime costs and a clever potential - // definition. shrink_to_fit will first check if there are any vacant slots at "the end". If - // there are, it will then do a full pass over empty and filled slots. The problem is that - // "the end" is not easily available from the public API. We know it's somewhere between - // len() and capacity(), and also past the link(s) we just removed. But we can't check the - // internal entries.len(). - const ALLOWED_SLACK: usize = 1024 * 1024 * 1024 / size_of::(); - if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { - slots.shrink_to_fit(); - } -} - impl Drop for DynamicDomSlot { fn drop(&mut self) { - DYNAMIC_SLOTS.with_borrow_mut(|slots| { - remove_link(slots, self.link, true); - }); + LINK_FOREST.with_borrow_mut(|links| links.remove(self.link)); } } @@ -288,41 +220,15 @@ impl DynamicDomSlotHandle { /// Reassign through a handle. This is only valid if the owning [DynamicDomSlot] is still alive. fn reassign_unchecked(&self, next_position: DomSlot) { // TODO: is not defensive against accidental reference loops - DYNAMIC_SLOTS.with_borrow_mut(|slots| { - let old_parent = slots[self.link].parent.clone(); - match (&old_parent.variant, &next_position.variant) { - (DomSlotVariant::Node(_), DomSlotVariant::Node(_)) => {} - (DomSlotVariant::Node(_), DomSlotVariant::Chained(new_parent)) => { - slots[new_parent.link].add_ref(); - } - (DomSlotVariant::Chained(old_parent), DomSlotVariant::Node(_)) => { - remove_link(slots, old_parent.link, false); - } - (DomSlotVariant::Chained(old_parent), DomSlotVariant::Chained(new_parent)) => { - if old_parent.link == new_parent.link { - return; - } - slots[new_parent.link].add_ref(); - remove_link(slots, old_parent.link, false); - } - } - slots[self.link].parent = next_position; + LINK_FOREST.with_borrow_mut(|forest| { + forest.reassign(self.link, next_position); }); } fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { // We use an iterative approach to traverse a possible long chain of references. // See issue #3043 for why a recursive call is impossible for large lists in vdom. - let node = DYNAMIC_SLOTS.with_borrow(|slots| { - let mut link = self.link; - loop { - match &slots[link].parent.variant { - // NOTE: We clone to drop the borrow and let f re-enter this method - DomSlotVariant::Node(node) => break node.clone(), - DomSlotVariant::Chained(handle) => link = handle.link, - } - } - }); + let node = LINK_FOREST.with_borrow(|forest| forest.find_root(self.link).clone()); f(node.as_ref()) } } @@ -333,18 +239,11 @@ mod feat_hydration { use web_sys::Node; - use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_SLOT}; + use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_HANDLE}; fn trapped_position() -> DomSlot { super::trap_impl::with_trap_ref(|trap| match trap { - Some(_) => { - // this handle exists only if we have a trap node - let fake_handle = DynamicDomSlotHandle { - link: RESERVED_TRAP_SLOT, - _phantom: PhantomData, - }; - fake_handle.into_position() - } + Some(_) => RESERVED_TRAP_HANDLE.into_position(), None => DomSlot::at_end(), }) } @@ -370,7 +269,7 @@ mod feat_hydration { pub fn start(slot: &'tree mut DynamicDomSlot) -> Self { // We take a follower, but we are sure the owner is alive Self { - prev_next_sibling: Some(slot.clone_to_follower()), + prev_next_sibling: Some(slot.clone_to_handle()), _owner: PhantomData, } } @@ -398,7 +297,7 @@ mod feat_hydration { // its component state pub fn write_at_comp(&mut self, slot: DomSlot, inner_next_sibling: &DynamicDomSlot) { self.write(slot); - self.prev_next_sibling = Some(inner_next_sibling.clone_to_follower()); + self.prev_next_sibling = Some(inner_next_sibling.clone_to_handle()); } } impl Drop for SlotBulletin<'_> { diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs new file mode 100644 index 00000000000..27d4d0c1f41 --- /dev/null +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -0,0 +1,168 @@ +use slab::Slab; + +use super::{DomSlot, DomSlotVariant, Node}; + +#[derive(Default)] +pub struct LinkForest { + nodes: Slab, +} + +impl LinkForest { + pub fn insert(&mut self, link: DomSlot) -> LinkId { + let link = Link::new(link); + let parent = link.parent.link(); + let link_id = self.nodes.insert(link); + if let Some(parent) = parent { + self.node_mut(parent).add_ref(); + } + link_id.try_into().unwrap() + } + + fn node(&self, link: LinkId) -> &Link { + &self.nodes[link as usize] + } + + fn node_mut(&mut self, link: LinkId) -> &mut Link { + &mut self.nodes[link as usize] + } + + fn remove_node(&mut self, link: LinkId) -> Link { + self.nodes.remove(link as usize) + } + + pub fn remove(&mut self, link: LinkId) { + self.remove_link(link, true); + } + + pub fn leak(&mut self, link: LinkId) { + self.node_mut(link).leak(); + } + + fn remove_link(&mut self, link: LinkId, owner: bool) { + if !self.node_mut(link).dec_ref(owner) { + return; + } + let mut link = link; + loop { + let removed = self.remove_node(link); + let Some(parent) = removed.parent.link() else { + break; + }; + if !self.node_mut(parent).dec_ref(false) { + break; + } + link = parent; + } + // from time to time, clean up memory in the slab + // TODO: this needs more analysis under amortized runtime costs and a clever potential + // definition. shrink_to_fit will first check if there are any vacant slots at "the end". If + // there are, it will then do a full pass over empty and filled slots. The problem is that + // "the end" is not easily available from the public API. We know it's somewhere between + // len() and capacity(), and also past the link(s) we just removed. But we can't check the + // internal entries.len(). + const ALLOWED_SLACK: usize = 1024 * 1024 * 1024 / size_of::(); + let slots = &mut self.nodes; + if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { + slots.shrink_to_fit(); + } + } + + pub fn reassign(&mut self, link: LinkId, new_parent: DomSlot) { + let new_parent = LinkParent::from(new_parent); + debug_assert!( + self.node(link).has_owner(), + "owner must be alive to reassign" + ); + let old_parent = self.node(link).parent.link(); + match (old_parent, new_parent.link()) { + (None, None) => {} + (None, Some(new_parent)) => { + self.node_mut(new_parent).add_ref(); + } + (Some(old_parent), None) => { + self.remove_link(old_parent, false); + } + (Some(old_parent), Some(new_parent)) if old_parent == new_parent => return, + (Some(old_parent), Some(new_parent)) => { + self.node_mut(new_parent).add_ref(); + self.remove_link(old_parent, false); + } + } + self.node_mut(link).parent = new_parent; + } + + pub fn find_root(&self, mut link: LinkId) -> &Option { + loop { + match &self.node(link).parent { + // NOTE: We clone to drop the borrow and let f re-enter this method + LinkParent::Root(node) => break node, + &LinkParent::Some(p_link) => { + link = p_link; + } + } + } + } +} + +pub type LinkId = usize; + +enum LinkParent { + Root(Option), + Some(LinkId), +} + +impl From for LinkParent { + fn from(value: DomSlot) -> Self { + match value.variant { + DomSlotVariant::Node(node) => Self::Root(node), + DomSlotVariant::Chained(handle) => Self::Some(handle.link), + } + } +} + +impl LinkParent { + fn link(&self) -> Option { + let &Self::Some(parent) = self else { + return None; + }; + Some(parent) + } +} + +struct Link { + parent: LinkParent, + /// counts the owner + the number of links in LINK_FOREST that refer to this link. + /// to save a bit, the owner is counted in the lowest bit, handles are counted in the upper + /// bits + ref_count: usize, +} + +impl Link { + pub fn new(parent: DomSlot) -> Self { + Self { + parent: parent.into(), + ref_count: 1, + } + } + + fn leak(&mut self) { + self.add_ref(); + self.dec_ref(true); + } + + fn has_owner(&self) -> bool { + (self.ref_count & 0b1) != 0 + } + + fn dec_ref(&mut self, owner: bool) -> bool { + let weight = if owner { 1 } else { 2 }; + debug_assert!(self.ref_count >= weight, "must have refs"); + self.ref_count -= weight; + self.ref_count == 0 + } + + fn add_ref(&mut self) { + debug_assert!(self.ref_count > 0, "no revives"); + self.ref_count += 2; + } +} From 68f9ce7f391b7337b33152ade8192665b127e2a2 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 23 Jul 2026 18:06:31 +0200 Subject: [PATCH 12/17] implement cut link tree --- packages/yew/src/dom_bundle/position.rs | 2 +- .../yew/src/dom_bundle/position/forest.rs | 291 +++++++++++++++--- 2 files changed, 241 insertions(+), 52 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index 9f4b5d89c93..ad180b6e0fb 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -228,7 +228,7 @@ impl DynamicDomSlotHandle { fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { // We use an iterative approach to traverse a possible long chain of references. // See issue #3043 for why a recursive call is impossible for large lists in vdom. - let node = LINK_FOREST.with_borrow(|forest| forest.find_root(self.link).clone()); + let node = LINK_FOREST.with_borrow_mut(|forest| forest.find_root(self.link).clone()); f(node.as_ref()) } } diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index 27d4d0c1f41..08bd12e25e9 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -7,15 +7,41 @@ pub struct LinkForest { nodes: Slab, } +#[allow(unused)] +macro_rules! trace { + ($msg:literal $(,)?) => { + ::gloo::console::log!( + ::std::format!("%c[{}:{}] ", ::std::file!(), ::std::line!()), + "font-weight: bold", + ::std::format!($msg) + ) + }; + ($msg:literal , $( $args:tt ),*) => { + ::gloo::console::log!( + ::std::format!("%c[{}:{}] ", ::std::file!(), ::std::line!()), + "font-weight: bold", + ::std::format!($msg, $( $args ),* ), + ) + } +} + impl LinkForest { + #[allow(unused)] + fn print_all(&self) { + for (n, node) in &self.nodes { + gloo::console::console_dbg!(node.debug(n)); + } + } + pub fn insert(&mut self, link: DomSlot) -> LinkId { - let link = Link::new(link); - let parent = link.parent.link(); - let link_id = self.nodes.insert(link); + let entry = self.nodes.vacant_entry(); + let link_id = entry.key(); + let (link, parent) = Link::new(link, link_id); + entry.insert(link); if let Some(parent) = parent { self.node_mut(parent).add_ref(); } - link_id.try_into().unwrap() + link_id } fn node(&self, link: LinkId) -> &Link { @@ -42,16 +68,30 @@ impl LinkForest { if !self.node_mut(link).dec_ref(owner) { return; } - let mut link = link; + let mut n = link; loop { - let removed = self.remove_node(link); - let Some(parent) = removed.parent.link() else { + let node = self.remove_node(n); + debug_assert!( + node.right(n).is_none(), + "can't have children in the represented tree" + ); + let l = node.left(n); + let rep_p = node.rep_parent(n); + let p = node.parent; + if let &LinkParent::AuxParent(p) = &p { + debug_assert!(self.node(p).right(p) == Some(n)); + self.node_mut(p).set_right(p, l); + } + if let Some(l) = l { + self.node_mut(l).parent = p; + } + let Some(rep_p) = rep_p else { break; }; - if !self.node_mut(parent).dec_ref(false) { + if !self.node_mut(rep_p).dec_ref(false) { break; } - link = parent; + n = rep_p; } // from time to time, clean up memory in the slab // TODO: this needs more analysis under amortized runtime costs and a clever potential @@ -68,69 +108,160 @@ impl LinkForest { } pub fn reassign(&mut self, link: LinkId, new_parent: DomSlot) { - let new_parent = LinkParent::from(new_parent); debug_assert!( self.node(link).has_owner(), "owner must be alive to reassign" ); - let old_parent = self.node(link).parent.link(); - match (old_parent, new_parent.link()) { - (None, None) => {} - (None, Some(new_parent)) => { - self.node_mut(new_parent).add_ref(); - } - (Some(old_parent), None) => { - self.remove_link(old_parent, false); - } + let old_parent_id = self.node(link).rep_parent(link); + let (new_parent, new_parent_id) = match new_parent.variant { + DomSlotVariant::Chained(link) => (LinkParent::PathParent(link.link), Some(link.link)), + DomSlotVariant::Node(data) => (LinkParent::Root(data), None), + }; + match (old_parent_id, new_parent_id) { + // nothing to do (Some(old_parent), Some(new_parent)) if old_parent == new_parent => return, - (Some(old_parent), Some(new_parent)) => { - self.node_mut(new_parent).add_ref(); - self.remove_link(old_parent, false); + _ => {} + } + if let Some(new_parent_id) = new_parent_id { + self.node_mut(new_parent_id).add_ref(); + } + self.splay(link); + let l = self.node(link).left(link); + let parent = std::mem::replace(&mut self.node_mut(link).parent, new_parent); + self.node_mut(link).set_left(link, None); + self.node_mut(link).set_rep_parent(link, new_parent_id); + if let Some(l) = l { + self.node_mut(l).parent = parent; + } + if let Some(old_parent_id) = old_parent_id { + self.remove_link(old_parent_id, false); + } + } + + pub fn find_root(&mut self, link: LinkId) -> &Option { + self.access(link); + match &self.node(link).parent { + LinkParent::Root(node) => node, + _ => unreachable!("access method buggy"), + } + } + + // Splay operations on the auxiliary tree + // In fact, none of the splay operations change the refcount, since they do not modify the + // represented tree. + fn splay_parent(&self, link: LinkId) -> Result { + // Due to borrow issues (fixed with polonius?) we can't borrow data here, and do that + // in the caller with a double match :/ + match &self.node(link).parent { + &LinkParent::AuxParent(parent) => Ok(parent), + &LinkParent::PathParent(link) => Err(SplayResult::Link(link)), + LinkParent::Root(_) => Err(SplayResult::Root()), + } + } + + fn rotate(&mut self, x: LinkId, p: LinkId) { + let is_left = self.node(p).left(p) == Some(x); + let m; + if is_left { + m = self.node(x).right(x); + self.node_mut(x).set_right(x, Some(p)); + self.node_mut(p).set_left(p, m); + } else { + m = self.node(x).left(x); + self.node_mut(x).set_left(x, Some(p)); + self.node_mut(p).set_right(p, m); + }; + if let Some(m) = m { + debug_assert_eq!(self.node_mut(m).parent, LinkParent::AuxParent(x)); + self.node_mut(m).parent = LinkParent::AuxParent(p); + } + let g = std::mem::replace(&mut self.node_mut(p).parent, LinkParent::AuxParent(x)); + if let LinkParent::AuxParent(g) = g { + let p_was_left = self.node(g).left(g) == Some(p); + if p_was_left { + self.node_mut(g).set_left(g, Some(x)); + } else { + debug_assert!(self.node(g).right(g) == Some(p)); + self.node_mut(g).set_right(g, Some(x)); } } - self.node_mut(link).parent = new_parent; + self.node_mut(x).parent = g; } - pub fn find_root(&self, mut link: LinkId) -> &Option { + fn splay(&mut self, link: LinkId) -> SplayResult { + let x = link; loop { - match &self.node(link).parent { - // NOTE: We clone to drop the borrow and let f re-enter this method - LinkParent::Root(node) => break node, - &LinkParent::Some(p_link) => { - link = p_link; + let mut p = match self.splay_parent(x) { + Ok(p) => p, + Err(done) => return done, + }; + if let Ok(g) = self.splay_parent(p) { + // check for zig-zig or zig-zag + // zig-zig can be implemented by first rotating p and g, followed by x and p + // zig-zag can be implemented by first rotating x and p, followed by x and g + let x_is_left = self.node(p).left(p) == Some(x); + let p_is_left = self.node(g).left(g) == Some(p); + if x_is_left == p_is_left { + self.rotate(p, g); + } else { + self.rotate(x, p); + p = g; } } + self.rotate(x, p); + } + } + + // Link/cut operations + fn access(&mut self, link: LinkId) { + // Also does not change any refcounts + let (mut curr, mut prev) = (link, None); + loop { + let link = self.splay(curr); + // found a path-parent pointer. now we cut this one + let d = self.node_mut(curr).right(curr); + self.node_mut(curr).set_right(curr, prev); + if let Some(prev) = prev { + debug_assert_eq!(self.node(prev).parent, LinkParent::PathParent(curr)); + self.node_mut(prev).parent = LinkParent::AuxParent(curr); + } + if let Some(d) = d { + debug_assert_eq!(self.node(d).parent, LinkParent::AuxParent(curr)); + self.node_mut(d).parent = LinkParent::PathParent(curr); + } + let SplayResult::Link(link) = link else { break }; + (prev, curr) = (Some(curr), link); } + // now link is on the preferred path, so splay it one last time to put it on top. + let res = self.splay(link); + debug_assert!(matches!(res, SplayResult::Root())); } } pub type LinkId = usize; -enum LinkParent { - Root(Option), - Some(LinkId), -} - -impl From for LinkParent { - fn from(value: DomSlot) -> Self { - match value.variant { - DomSlotVariant::Node(node) => Self::Root(node), - DomSlotVariant::Chained(handle) => Self::Some(handle.link), - } - } +enum SplayResult { + Link(LinkId), + Root( + // &'a Option + ), } -impl LinkParent { - fn link(&self) -> Option { - let &Self::Some(parent) = self else { - return None; - }; - Some(parent) - } +#[derive(PartialEq, Debug)] +enum LinkParent { + Root(Option), + // parent is on the same preferred path + AuxParent(LinkId), + // "path-parent pointer" to some other preferred path + PathParent(LinkId), } struct Link { parent: LinkParent, + // We use a link's own id to signal that it has no right/left child or represented parent + left_aux: LinkId, + right_aux: LinkId, + rep_parent: LinkId, /// counts the owner + the number of links in LINK_FOREST that refer to this link. /// to save a bit, the owner is counted in the lowest bit, handles are counted in the upper /// bits @@ -138,10 +269,44 @@ struct Link { } impl Link { - pub fn new(parent: DomSlot) -> Self { - Self { - parent: parent.into(), + pub fn new(parent: DomSlot, this: LinkId) -> (Self, Option) { + let (parent, link) = match parent.variant { + DomSlotVariant::Node(node) => (LinkParent::Root(node), None), + DomSlotVariant::Chained(handle) => { + (LinkParent::PathParent(handle.link), Some(handle.link)) + } + }; + let this = Self { + parent, + left_aux: this, + right_aux: this, + rep_parent: link.unwrap_or(this), ref_count: 1, + }; + (this, link) + } + + fn debug(&self, this: LinkId) -> impl '_ + std::fmt::Debug { + #[expect(unused)] + #[derive(Debug)] + struct Link<'a> { + id: LinkId, + parent: &'a LinkParent, + left_aux: Option, + right_aux: Option, + rep_parent: Option, + ref_count: usize, + has_owner: bool, + } + let has_owner = self.has_owner(); + Link { + id: this, + parent: &self.parent, + left_aux: self.left(this), + right_aux: self.right(this), + rep_parent: self.rep_parent(this), + ref_count: self.ref_count / 2 + has_owner as usize, + has_owner, } } @@ -165,4 +330,28 @@ impl Link { debug_assert!(self.ref_count > 0, "no revives"); self.ref_count += 2; } + + fn left(&self, this: LinkId) -> Option { + (self.left_aux != this).then_some(self.left_aux) + } + + fn right(&self, this: LinkId) -> Option { + (self.right_aux != this).then_some(self.right_aux) + } + + fn set_left(&mut self, this: LinkId, left: Option) { + self.left_aux = left.unwrap_or(this); + } + + fn set_right(&mut self, this: LinkId, right: Option) { + self.right_aux = right.unwrap_or(this); + } + + fn rep_parent(&self, this: LinkId) -> Option { + (self.rep_parent != this).then_some(self.rep_parent) + } + + fn set_rep_parent(&mut self, this: LinkId, rep_parent: Option) { + self.rep_parent = rep_parent.unwrap_or(this); + } } From 05c16435f8df67232bb267c608d187588042c22e Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 23 Jul 2026 18:09:47 +0200 Subject: [PATCH 13/17] clippy fix --- packages/yew/src/dom_bundle/position/forest.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index 08bd12e25e9..3dbfa60ee60 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -45,15 +45,15 @@ impl LinkForest { } fn node(&self, link: LinkId) -> &Link { - &self.nodes[link as usize] + &self.nodes[link] } fn node_mut(&mut self, link: LinkId) -> &mut Link { - &mut self.nodes[link as usize] + &mut self.nodes[link] } fn remove_node(&mut self, link: LinkId) -> Link { - self.nodes.remove(link as usize) + self.nodes.remove(link) } pub fn remove(&mut self, link: LinkId) { From db88af9dc3bb4e83c49ba785ab17266c9637ca2a Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Thu, 23 Jul 2026 21:08:37 +0200 Subject: [PATCH 14/17] slightly work on readability --- .../yew/src/dom_bundle/position/forest.rs | 242 +++++++++++------- 1 file changed, 151 insertions(+), 91 deletions(-) diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index 3dbfa60ee60..91300ac384b 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -1,3 +1,5 @@ +use std::ops::{Deref, DerefMut}; + use slab::Slab; use super::{DomSlot, DomSlotVariant, Node}; @@ -29,7 +31,8 @@ impl LinkForest { #[allow(unused)] fn print_all(&self) { for (n, node) in &self.nodes { - gloo::console::console_dbg!(node.debug(n)); + let node = LinkRef::new(n, node); + gloo::console::console_dbg!(node.debug()); } } @@ -44,16 +47,16 @@ impl LinkForest { link_id } - fn node(&self, link: LinkId) -> &Link { - &self.nodes[link] + fn node(&self, link: LinkId) -> LinkRef<&Link> { + LinkRef::new(link, &self.nodes[link]) } - fn node_mut(&mut self, link: LinkId) -> &mut Link { - &mut self.nodes[link] + fn node_mut(&mut self, link: LinkId) -> LinkRef<&mut Link> { + LinkRef::new(link, &mut self.nodes[link]) } - fn remove_node(&mut self, link: LinkId) -> Link { - self.nodes.remove(link) + fn remove_node(&mut self, link: LinkId) -> LinkRef { + LinkRef::new(link, self.nodes.remove(link)) } pub fn remove(&mut self, link: LinkId) { @@ -72,15 +75,15 @@ impl LinkForest { loop { let node = self.remove_node(n); debug_assert!( - node.right(n).is_none(), + node.right().is_none(), "can't have children in the represented tree" ); - let l = node.left(n); - let rep_p = node.rep_parent(n); - let p = node.parent; + let l = node.left(); + let rep_p = node.rep_parent(); + let p = node.into_inner().parent; if let &LinkParent::AuxParent(p) = &p { - debug_assert!(self.node(p).right(p) == Some(n)); - self.node_mut(p).set_right(p, l); + debug_assert!(self.node(p).right() == Some(n)); + self.node_mut(p).set_right(l); } if let Some(l) = l { self.node_mut(l).parent = p; @@ -100,7 +103,7 @@ impl LinkForest { // "the end" is not easily available from the public API. We know it's somewhere between // len() and capacity(), and also past the link(s) we just removed. But we can't check the // internal entries.len(). - const ALLOWED_SLACK: usize = 1024 * 1024 * 1024 / size_of::(); + const ALLOWED_SLACK: usize = 64 * 1024 * 1024 / size_of::(); let slots = &mut self.nodes; if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { slots.shrink_to_fit(); @@ -108,28 +111,29 @@ impl LinkForest { } pub fn reassign(&mut self, link: LinkId, new_parent: DomSlot) { + // removes `link` from its represented tree and moves it to `new_parent`. + // we also have to keep track of ref counts. debug_assert!( self.node(link).has_owner(), "owner must be alive to reassign" ); - let old_parent_id = self.node(link).rep_parent(link); + let old_parent_id = self.node(link).rep_parent(); let (new_parent, new_parent_id) = match new_parent.variant { DomSlotVariant::Chained(link) => (LinkParent::PathParent(link.link), Some(link.link)), DomSlotVariant::Node(data) => (LinkParent::Root(data), None), }; - match (old_parent_id, new_parent_id) { - // nothing to do - (Some(old_parent), Some(new_parent)) if old_parent == new_parent => return, - _ => {} + if old_parent_id == new_parent_id && old_parent_id.is_some() { + // reassigned to its existing parent, no need to modify. + return; } if let Some(new_parent_id) = new_parent_id { self.node_mut(new_parent_id).add_ref(); } self.splay(link); - let l = self.node(link).left(link); - let parent = std::mem::replace(&mut self.node_mut(link).parent, new_parent); - self.node_mut(link).set_left(link, None); - self.node_mut(link).set_rep_parent(link, new_parent_id); + let l = self.node(link).left(); + let parent = self.node_mut(link).parent.replace(new_parent); + self.node_mut(link).set_left(None); + self.node_mut(link).set_rep_parent(new_parent_id); if let Some(l) = l { self.node_mut(l).parent = parent; } @@ -140,7 +144,7 @@ impl LinkForest { pub fn find_root(&mut self, link: LinkId) -> &Option { self.access(link); - match &self.node(link).parent { + match &self.node(link).into_inner().parent { LinkParent::Root(node) => node, _ => unreachable!("access method buggy"), } @@ -160,29 +164,31 @@ impl LinkForest { } fn rotate(&mut self, x: LinkId, p: LinkId) { - let is_left = self.node(p).left(p) == Some(x); + // shift the middle node `m` from `x` to `p`. let m; - if is_left { - m = self.node(x).right(x); - self.node_mut(x).set_right(x, Some(p)); - self.node_mut(p).set_left(p, m); + debug_assert!(self.node(p).right() == Some(x) || self.node(p).left() == Some(x)); + if self.node(p).left() == Some(x) { + m = self.node(x).right(); + self.node_mut(x).set_right(Some(p)); + self.node_mut(p).set_left(m); } else { - m = self.node(x).left(x); - self.node_mut(x).set_left(x, Some(p)); - self.node_mut(p).set_right(p, m); + m = self.node(x).left(); + self.node_mut(x).set_left(Some(p)); + self.node_mut(p).set_right(m); }; if let Some(m) = m { debug_assert_eq!(self.node_mut(m).parent, LinkParent::AuxParent(x)); self.node_mut(m).parent = LinkParent::AuxParent(p); } - let g = std::mem::replace(&mut self.node_mut(p).parent, LinkParent::AuxParent(x)); + // attach `x` to the parent of `p` + let g = self.node_mut(p).parent.replace(LinkParent::AuxParent(x)); if let LinkParent::AuxParent(g) = g { - let p_was_left = self.node(g).left(g) == Some(p); - if p_was_left { - self.node_mut(g).set_left(g, Some(x)); + let mut g = self.node_mut(g); + debug_assert!(g.right() == Some(p) || g.left() == Some(p)); + if g.left() == Some(p) { + g.set_left(Some(x)); } else { - debug_assert!(self.node(g).right(g) == Some(p)); - self.node_mut(g).set_right(g, Some(x)); + g.set_right(Some(x)); } } self.node_mut(x).parent = g; @@ -199,8 +205,8 @@ impl LinkForest { // check for zig-zig or zig-zag // zig-zig can be implemented by first rotating p and g, followed by x and p // zig-zag can be implemented by first rotating x and p, followed by x and g - let x_is_left = self.node(p).left(p) == Some(x); - let p_is_left = self.node(g).left(g) == Some(p); + let x_is_left = self.node(p).left() == Some(x); + let p_is_left = self.node(g).left() == Some(p); if x_is_left == p_is_left { self.rotate(p, g); } else { @@ -219,15 +225,18 @@ impl LinkForest { loop { let link = self.splay(curr); // found a path-parent pointer. now we cut this one - let d = self.node_mut(curr).right(curr); - self.node_mut(curr).set_right(curr, prev); + let d = self.node_mut(curr).right(); if let Some(prev) = prev { debug_assert_eq!(self.node(prev).parent, LinkParent::PathParent(curr)); self.node_mut(prev).parent = LinkParent::AuxParent(curr); - } - if let Some(d) = d { - debug_assert_eq!(self.node(d).parent, LinkParent::AuxParent(curr)); - self.node_mut(d).parent = LinkParent::PathParent(curr); + // small deviation from the original paper: we do not remove the tail + // of the preferred path the first node is already on. + // this would originally run unconditionally of prev.is_some() + self.node_mut(curr).set_right(Some(prev)); + if let Some(d) = d { + debug_assert_eq!(self.node(d).parent, LinkParent::AuxParent(curr)); + self.node_mut(d).parent = LinkParent::PathParent(curr); + } } let SplayResult::Link(link) = link else { break }; (prev, curr) = (Some(curr), link); @@ -256,6 +265,12 @@ enum LinkParent { PathParent(LinkId), } +impl LinkParent { + fn replace(&mut self, next: LinkParent) -> LinkParent { + std::mem::replace(self, next) + } +} + struct Link { parent: LinkParent, // We use a link's own id to signal that it has no right/left child or represented parent @@ -268,25 +283,49 @@ struct Link { ref_count: usize, } -impl Link { - pub fn new(parent: DomSlot, this: LinkId) -> (Self, Option) { - let (parent, link) = match parent.variant { - DomSlotVariant::Node(node) => (LinkParent::Root(node), None), - DomSlotVariant::Chained(handle) => { - (LinkParent::PathParent(handle.link), Some(handle.link)) - } - }; - let this = Self { - parent, - left_aux: this, - right_aux: this, - rep_parent: link.unwrap_or(this), - ref_count: 1, - }; - (this, link) +impl AsRef for Link { + fn as_ref(&self) -> &Link { + self + } +} + +impl AsMut for Link { + fn as_mut(&mut self) -> &mut Link { + self } +} - fn debug(&self, this: LinkId) -> impl '_ + std::fmt::Debug { +struct LinkRef { + id: LinkId, + link: L, +} + +impl> Deref for LinkRef { + type Target = Link; + + fn deref(&self) -> &Self::Target { + self.link.as_ref() + } +} + +impl + AsMut> DerefMut for LinkRef { + fn deref_mut(&mut self) -> &mut Self::Target { + self.link.as_mut() + } +} + +impl LinkRef { + fn new(id: LinkId, link: L) -> Self { + Self { id, link } + } + + fn into_inner(self) -> L { + self.link + } +} + +impl> LinkRef { + fn debug(&self) -> impl '_ + std::fmt::Debug { #[expect(unused)] #[derive(Debug)] struct Link<'a> { @@ -300,16 +339,61 @@ impl Link { } let has_owner = self.has_owner(); Link { - id: this, + id: self.id, parent: &self.parent, - left_aux: self.left(this), - right_aux: self.right(this), - rep_parent: self.rep_parent(this), + left_aux: self.left(), + right_aux: self.right(), + rep_parent: self.rep_parent(), ref_count: self.ref_count / 2 + has_owner as usize, has_owner, } } + fn left(&self) -> Option { + (self.left_aux != self.id).then_some(self.left_aux) + } + + fn right(&self) -> Option { + (self.right_aux != self.id).then_some(self.right_aux) + } + + fn rep_parent(&self) -> Option { + (self.rep_parent != self.id).then_some(self.rep_parent) + } +} + +impl> LinkRef { + fn set_left(&mut self, left: Option) { + self.link.as_mut().left_aux = left.unwrap_or(self.id); + } + + fn set_right(&mut self, right: Option) { + self.link.as_mut().right_aux = right.unwrap_or(self.id); + } + + fn set_rep_parent(&mut self, rep_parent: Option) { + self.link.as_mut().rep_parent = rep_parent.unwrap_or(self.id); + } +} + +impl Link { + pub fn new(parent: DomSlot, this: LinkId) -> (Self, Option) { + let (parent, link) = match parent.variant { + DomSlotVariant::Node(node) => (LinkParent::Root(node), None), + DomSlotVariant::Chained(handle) => { + (LinkParent::PathParent(handle.link), Some(handle.link)) + } + }; + let this = Self { + parent, + left_aux: this, + right_aux: this, + rep_parent: link.unwrap_or(this), + ref_count: 1, + }; + (this, link) + } + fn leak(&mut self) { self.add_ref(); self.dec_ref(true); @@ -330,28 +414,4 @@ impl Link { debug_assert!(self.ref_count > 0, "no revives"); self.ref_count += 2; } - - fn left(&self, this: LinkId) -> Option { - (self.left_aux != this).then_some(self.left_aux) - } - - fn right(&self, this: LinkId) -> Option { - (self.right_aux != this).then_some(self.right_aux) - } - - fn set_left(&mut self, this: LinkId, left: Option) { - self.left_aux = left.unwrap_or(this); - } - - fn set_right(&mut self, this: LinkId, right: Option) { - self.right_aux = right.unwrap_or(this); - } - - fn rep_parent(&self, this: LinkId) -> Option { - (self.rep_parent != this).then_some(self.rep_parent) - } - - fn set_rep_parent(&mut self, this: LinkId, rep_parent: Option) { - self.rep_parent = rep_parent.unwrap_or(this); - } } From 48039bb6edcd012d3fd911a7b7021eb3fdf5c3a4 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Mon, 17 Aug 2026 14:55:30 +0200 Subject: [PATCH 15/17] push a few more datastructures into forest --- packages/yew/src/dom_bundle/position.rs | 117 +++++++++--------- .../yew/src/dom_bundle/position/forest.rs | 106 ++++++++++++---- 2 files changed, 141 insertions(+), 82 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index ad180b6e0fb..bff29dcbae1 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -1,12 +1,7 @@ //! Structs for keeping track where in the DOM a node belongs -use std::cell::RefCell; -use std::marker::PhantomData; - use web_sys::{Element, Node}; -type PhantomNotSendNorSync = PhantomData<*const u8>; - /// A position in the list of children of an implicit parent [`Element`]. /// /// This can either be in front of a `DomSlot::at(next_sibling)`, at the end of the list with @@ -16,6 +11,12 @@ pub(crate) struct DomSlot { variant: DomSlotVariant, } +#[derive(Clone)] +enum DomSlotVariant { + Node(Option), + Chained(DynamicDomSlotHandle), +} + impl std::fmt::Debug for DomSlot { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { self.with_next_sibling(|n| { @@ -29,67 +30,49 @@ impl std::fmt::Debug for DomSlot { } } -#[derive(Clone)] -enum DomSlotVariant { - Node(Option), - Chained(DynamicDomSlotHandle), -} - mod forest; -use forest::{LinkForest, LinkId}; - -// This handle is only valid when trap nodes are active -const RESERVED_TRAP_HANDLE: DynamicDomSlotHandle = DynamicDomSlotHandle { - link: 0, - _phantom: PhantomData, -}; -thread_local! { - static LINK_FOREST: RefCell = { - let mut slots = LinkForest::default(); - trap_impl::with_trap_ref(|trap| { - if let Some(trap) = trap { - let trap_link = slots.insert(DomSlot::at(trap.clone())); - assert_eq!(trap_link, RESERVED_TRAP_HANDLE.link); - slots.leak(trap_link); - } - }); - RefCell::new(slots) - }; -} +use forest::{LinkHandle, LinkOwner, with_forest}; /// A dynamic dom slot can be reassigned. This change is also seen by the [`DomSlot`] from /// [`Self::to_position`] before the reassignment took place. pub(crate) struct DynamicDomSlot { - link: LinkId, - // The link is tied to this specific thread and can't be accessed elsewhere - _phantom: PhantomNotSendNorSync, + link: LinkOwner, } impl std::fmt::Debug for DynamicDomSlot { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "#{} -> {:?}", self.link, self.to_position()) + write!(f, "#{:?} -> {:?}", self.link, self.to_position()) } } #[derive(Clone)] struct DynamicDomSlotHandle { - link: LinkId, - // The link is tied to this specific thread and can't be accessed elsewhere - _phantom: PhantomNotSendNorSync, + link: LinkHandle, } mod trap_impl { - use super::Node; + use std::cell::OnceCell; + + use super::{LinkHandle, Node}; + + pub struct TrapContext { + // A special marker element that should not be referenced + pub trap: Node, + #[allow(unused)] + pub handle: OnceCell, + } #[cfg(all(debug_assertions, feature = "hydration"))] thread_local! { - // A special marker element that should not be referenced - static TRAP: Node = gloo::utils::document().create_element("div").unwrap().into(); + static CTX: TrapContext = TrapContext { + trap: gloo::utils::document().create_element("div").unwrap().into(), + handle: OnceCell::new(), + }; } #[inline] - pub fn with_trap_ref(f: impl FnOnce(Option<&Node>) -> R) -> R { + pub fn with_trap_ctx(f: impl FnOnce(Option<&TrapContext>) -> R) -> R { #[cfg(all(debug_assertions, feature = "hydration"))] { - TRAP.with(|trap| f(Some(trap))) + CTX.with(|ctx| f(Some(ctx))) } #[cfg(not(all(debug_assertions, feature = "hydration")))] { @@ -97,6 +80,10 @@ mod trap_impl { } } #[inline] + pub fn with_trap_ref(f: impl FnOnce(Option<&Node>) -> R) -> R { + with_trap_ctx(|ctx| f(ctx.map(|ctx| &ctx.trap))) + } + #[inline] pub fn is_trap(node: &Node) -> bool { with_trap_ref(|trap| trap == Some(node)) } @@ -172,11 +159,8 @@ impl DynamicDomSlot { /// Create a dynamic dom slot that initially represents ("targets") the same slot as the /// argument. pub fn new(initial_position: DomSlot) -> Self { - let link = LINK_FOREST.with_borrow_mut(|slots| slots.insert(initial_position)); - Self { - link, - _phantom: PhantomData, - } + let link = with_forest(|slots| slots.insert(initial_position.variant)); + Self { link } } /// Change the [`DomSlot`] that is targeted. Subsequently, this will behave as if `self` was @@ -198,15 +182,14 @@ impl DynamicDomSlot { /// point without prior synchronization, as long as they ensure that the owner is still alive. fn clone_to_handle(&self) -> DynamicDomSlotHandle { DynamicDomSlotHandle { - link: self.link, - _phantom: self._phantom, + link: self.link.handle(), } } } impl Drop for DynamicDomSlot { fn drop(&mut self) { - LINK_FOREST.with_borrow_mut(|links| links.remove(self.link)); + with_forest(|links| links.remove(&mut self.link)); } } @@ -220,15 +203,13 @@ impl DynamicDomSlotHandle { /// Reassign through a handle. This is only valid if the owning [DynamicDomSlot] is still alive. fn reassign_unchecked(&self, next_position: DomSlot) { // TODO: is not defensive against accidental reference loops - LINK_FOREST.with_borrow_mut(|forest| { - forest.reassign(self.link, next_position); + with_forest(|forest| { + forest.reassign(&self.link, next_position.variant); }); } fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { - // We use an iterative approach to traverse a possible long chain of references. - // See issue #3043 for why a recursive call is impossible for large lists in vdom. - let node = LINK_FOREST.with_borrow_mut(|forest| forest.find_root(self.link).clone()); + let node = with_forest(|forest| forest.find_root(&self.link).clone()); f(node.as_ref()) } } @@ -239,11 +220,29 @@ mod feat_hydration { use web_sys::Node; - use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle, RESERVED_TRAP_HANDLE}; + use super::{DomSlot, DynamicDomSlot, DynamicDomSlotHandle, with_forest}; + + #[inline] + fn with_trap_handle(f: impl FnOnce(Option) -> R) -> R { + super::trap_impl::with_trap_ctx(|ctx| { + let handle = ctx.map(|ctx| { + let trap_link = ctx.handle.get_or_init(|| { + with_forest(|forest| { + let trap_link = forest.insert(DomSlot::at(ctx.trap.clone()).variant); + forest.leak(trap_link) + }) + }); + DynamicDomSlotHandle { + link: trap_link.clone(), + } + }); + f(handle) + }) + } fn trapped_position() -> DomSlot { - super::trap_impl::with_trap_ref(|trap| match trap { - Some(_) => RESERVED_TRAP_HANDLE.into_position(), + with_trap_handle(|handle| match handle { + Some(handle) => handle.into_position(), None => DomSlot::at_end(), }) } diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index 91300ac384b..52bd6b2246a 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -1,14 +1,57 @@ +use std::cell::RefCell; +use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; use slab::Slab; -use super::{DomSlot, DomSlotVariant, Node}; +use super::{DomSlotVariant, Node}; + +type PhantomNotSendNorSync = PhantomData<*const u8>; #[derive(Default)] pub struct LinkForest { nodes: Slab, } +thread_local! { + static LINK_FOREST: RefCell = { + RefCell::new(LinkForest::default()) + }; +} + +pub fn with_forest(f: impl FnOnce(&mut LinkForest) -> R) -> R { + LINK_FOREST.with_borrow_mut(f) +} + +#[derive(Debug, PartialEq, Eq)] +pub struct LinkOwner { + id: LinkId, + // The link is tied to this specific thread and can't be accessed elsewhere + _phantom: PhantomNotSendNorSync, +} + +impl LinkOwner { + pub fn handle(&self) -> LinkHandle { + LinkHandle::from_raw(self.id) + } +} + +#[derive(Debug, PartialEq, Eq, Clone)] +pub struct LinkHandle { + id: LinkId, + // The link is tied to this specific thread and can't be accessed elsewhere + _phantom: PhantomNotSendNorSync, +} + +impl LinkHandle { + pub const fn from_raw(id: LinkId) -> Self { + Self { + id, + _phantom: PhantomData, + } + } +} + #[allow(unused)] macro_rules! trace { ($msg:literal $(,)?) => { @@ -36,7 +79,7 @@ impl LinkForest { } } - pub fn insert(&mut self, link: DomSlot) -> LinkId { + pub fn insert(&mut self, link: DomSlotVariant) -> LinkOwner { let entry = self.nodes.vacant_entry(); let link_id = entry.key(); let (link, parent) = Link::new(link, link_id); @@ -44,7 +87,10 @@ impl LinkForest { if let Some(parent) = parent { self.node_mut(parent).add_ref(); } - link_id + LinkOwner { + id: link_id, + _phantom: PhantomData, + } } fn node(&self, link: LinkId) -> LinkRef<&Link> { @@ -59,12 +105,8 @@ impl LinkForest { LinkRef::new(link, self.nodes.remove(link)) } - pub fn remove(&mut self, link: LinkId) { - self.remove_link(link, true); - } - - pub fn leak(&mut self, link: LinkId) { - self.node_mut(link).leak(); + pub fn remove(&mut self, link: &mut LinkOwner) { + self.remove_link(link.id, true); } fn remove_link(&mut self, link: LinkId, owner: bool) { @@ -110,7 +152,8 @@ impl LinkForest { } } - pub fn reassign(&mut self, link: LinkId, new_parent: DomSlot) { + pub fn reassign(&mut self, link: &LinkHandle, new_parent: DomSlotVariant) { + let link = link.id; // removes `link` from its represented tree and moves it to `new_parent`. // we also have to keep track of ref counts. debug_assert!( @@ -118,8 +161,10 @@ impl LinkForest { "owner must be alive to reassign" ); let old_parent_id = self.node(link).rep_parent(); - let (new_parent, new_parent_id) = match new_parent.variant { - DomSlotVariant::Chained(link) => (LinkParent::PathParent(link.link), Some(link.link)), + let (new_parent, new_parent_id) = match new_parent { + DomSlotVariant::Chained(link) => { + (LinkParent::PathParent(link.link.id), Some(link.link.id)) + } DomSlotVariant::Node(data) => (LinkParent::Root(data), None), }; if old_parent_id == new_parent_id && old_parent_id.is_some() { @@ -142,9 +187,9 @@ impl LinkForest { } } - pub fn find_root(&mut self, link: LinkId) -> &Option { - self.access(link); - match &self.node(link).into_inner().parent { + pub fn find_root(&mut self, link: &LinkHandle) -> &Option { + self.access(link.id); + match &self.node(link.id).into_inner().parent { LinkParent::Root(node) => node, _ => unreachable!("access method buggy"), } @@ -220,6 +265,8 @@ impl LinkForest { // Link/cut operations fn access(&mut self, link: LinkId) { + // We use an iterative approach to traverse a possible long chain of references. + // See issue #3043 for why a recursive call is impossible for large lists in vdom. // Also does not change any refcounts let (mut curr, mut prev) = (link, None); loop { @@ -377,11 +424,11 @@ impl> LinkRef { } impl Link { - pub fn new(parent: DomSlot, this: LinkId) -> (Self, Option) { - let (parent, link) = match parent.variant { + pub fn new(parent: DomSlotVariant, this: LinkId) -> (Self, Option) { + let (parent, link) = match parent { DomSlotVariant::Node(node) => (LinkParent::Root(node), None), DomSlotVariant::Chained(handle) => { - (LinkParent::PathParent(handle.link), Some(handle.link)) + (LinkParent::PathParent(handle.link.id), Some(handle.link.id)) } }; let this = Self { @@ -394,11 +441,6 @@ impl Link { (this, link) } - fn leak(&mut self) { - self.add_ref(); - self.dec_ref(true); - } - fn has_owner(&self) -> bool { (self.ref_count & 0b1) != 0 } @@ -415,3 +457,21 @@ impl Link { self.ref_count += 2; } } + +#[cfg(feature = "hydration")] +mod feat_hydration { + use super::*; + + impl LinkForest { + pub fn leak(&mut self, link: LinkOwner) -> LinkHandle { + self.node_mut(link.id).leak(); + LinkHandle::from_raw(link.id) + } + } + impl Link { + fn leak(&mut self) { + self.add_ref(); + self.dec_ref(true); + } + } +} From 6fd1ba65757540561fdca39dfc3cc66019c4fc49 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Tue, 18 Aug 2026 15:14:09 +0200 Subject: [PATCH 16/17] intrusively linked structure instead of a slab, data is contained in an Rcs for each DynamicDomSlot --- packages/yew/src/dom_bundle/position.rs | 2 +- .../yew/src/dom_bundle/position/forest.rs | 392 +++++++++--------- 2 files changed, 205 insertions(+), 189 deletions(-) diff --git a/packages/yew/src/dom_bundle/position.rs b/packages/yew/src/dom_bundle/position.rs index bff29dcbae1..2d61d3e72f4 100644 --- a/packages/yew/src/dom_bundle/position.rs +++ b/packages/yew/src/dom_bundle/position.rs @@ -209,7 +209,7 @@ impl DynamicDomSlotHandle { } fn with_next_sibling(&self, f: impl FnOnce(Option<&Node>) -> R) -> R { - let node = with_forest(|forest| forest.find_root(&self.link).clone()); + let node = with_forest(|forest| forest.find_root(&self.link)); f(node.as_ref()) } } diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index 52bd6b2246a..bdca95ef518 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -1,17 +1,16 @@ -use std::cell::RefCell; +use std::cell::{self, RefCell}; +use std::fmt::Debug; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; - -use slab::Slab; +use std::rc::Rc; use super::{DomSlotVariant, Node}; type PhantomNotSendNorSync = PhantomData<*const u8>; +/// A dummy struct that serves as a marker for the borrow of [`LINK_FOREST`]. #[derive(Default)] -pub struct LinkForest { - nodes: Slab, -} +pub struct LinkForest; thread_local! { static LINK_FOREST: RefCell = { @@ -23,33 +22,84 @@ pub fn with_forest(f: impl FnOnce(&mut LinkForest) -> R) -> R { LINK_FOREST.with_borrow_mut(f) } -#[derive(Debug, PartialEq, Eq)] +#[derive(Clone)] +struct RawLink( + /// SAFETY: Comes from `Rc::into_raw` + *const RefCell, +); + +impl RawLink { + fn new(link: Link) -> Self { + let the_rc = Rc::new(RefCell::new(link)); + let ptr = Rc::into_raw(the_rc); + Self(ptr) + } + + fn id(&self) -> usize { + self.0.addr() + } + + fn inc_strong(&self) { + unsafe { Rc::increment_strong_count(self.0) }; + } + + fn into_rc(self) -> Rc> { + unsafe { Rc::from_raw(self.0) } + } +} + +impl PartialEq for RawLink { + fn eq(&self, other: &Self) -> bool { + std::ptr::addr_eq(self.0, other.0) + } +} + +#[derive(PartialEq)] pub struct LinkOwner { - id: LinkId, + id: RawLink, // The link is tied to this specific thread and can't be accessed elsewhere _phantom: PhantomNotSendNorSync, } +impl Debug for LinkOwner { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "#{:x}", self.id.id()) + } +} + impl LinkOwner { pub fn handle(&self) -> LinkHandle { - LinkHandle::from_raw(self.id) + LinkHandle::from_raw(self.id.clone()) } } -#[derive(Debug, PartialEq, Eq, Clone)] +#[derive(Clone, PartialEq)] pub struct LinkHandle { - id: LinkId, + id: RawLink, // The link is tied to this specific thread and can't be accessed elsewhere _phantom: PhantomNotSendNorSync, } +impl Debug for LinkHandle { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "#<{:x}>", self.id.id()) + } +} impl LinkHandle { - pub const fn from_raw(id: LinkId) -> Self { + const fn from_raw(id: RawLink) -> Self { Self { id, _phantom: PhantomData, } } + + fn to_owner(&self) -> LinkOwner { + self.id.inc_strong(); + LinkOwner { + id: self.id.clone(), + _phantom: PhantomData, + } + } } #[allow(unused)] @@ -70,100 +120,91 @@ macro_rules! trace { } } -impl LinkForest { - #[allow(unused)] - fn print_all(&self) { - for (n, node) in &self.nodes { - let node = LinkRef::new(n, node); - gloo::console::console_dbg!(node.debug()); - } +struct AsRefRef<'a>(cell::Ref<'a, Link>); +impl AsRef for AsRefRef<'_> { + fn as_ref(&self) -> &Link { + &self.0 } +} +struct AsRefMut<'a>(cell::RefMut<'a, Link>); +impl AsRef for AsRefMut<'_> { + fn as_ref(&self) -> &Link { + &self.0 + } +} +impl AsMut for AsRefMut<'_> { + fn as_mut(&mut self) -> &mut Link { + &mut self.0 + } +} +impl LinkForest { pub fn insert(&mut self, link: DomSlotVariant) -> LinkOwner { - let entry = self.nodes.vacant_entry(); - let link_id = entry.key(); - let (link, parent) = Link::new(link, link_id); - entry.insert(link); - if let Some(parent) = parent { - self.node_mut(parent).add_ref(); - } + let raw = RawLink::new(Link::new(link)); LinkOwner { - id: link_id, + id: raw, _phantom: PhantomData, } } - fn node(&self, link: LinkId) -> LinkRef<&Link> { - LinkRef::new(link, &self.nodes[link]) + fn node(&self, link: &LinkHandle) -> LinkRef> { + let refcell = unsafe { &*link.id.0 }; + LinkRef::new(link.id.id(), AsRefRef(refcell.borrow())) } - fn node_mut(&mut self, link: LinkId) -> LinkRef<&mut Link> { - LinkRef::new(link, &mut self.nodes[link]) + fn node_mut(&mut self, link: &LinkHandle) -> LinkRef + AsMut> { + let refcell = unsafe { &*link.id.0 }; + LinkRef::new(link.id.id(), AsRefMut(refcell.borrow_mut())) } - fn remove_node(&mut self, link: LinkId) -> LinkRef { - LinkRef::new(link, self.nodes.remove(link)) + fn remove_node(&mut self, link: &LinkOwner) -> Option> { + // TODO: this should take the owner by value, but that's incompatible with calling it + // inside of a Drop method without adding a new "invalid" state. + let id = link.id.id(); + let owned = link.id.clone().into_rc(); + let inner = Rc::try_unwrap(owned).ok()?.into_inner(); + Some(LinkRef::new(id, inner)) } pub fn remove(&mut self, link: &mut LinkOwner) { - self.remove_link(link.id, true); + self.remove_link(link); } - fn remove_link(&mut self, link: LinkId, owner: bool) { - if !self.node_mut(link).dec_ref(owner) { - return; - } + fn remove_link(&mut self, link: &LinkOwner) { + let mut slot; let mut n = link; loop { - let node = self.remove_node(n); + let Some(mut node) = self.remove_node(n) else { + break; + }; debug_assert!( node.right().is_none(), "can't have children in the represented tree" ); - let l = node.left(); - let rep_p = node.rep_parent(); + let l = node.left().cloned(); + let rep_p = node.replace_rep_parent(None); let p = node.into_inner().parent; - if let &LinkParent::AuxParent(p) = &p { - debug_assert!(self.node(p).right() == Some(n)); - self.node_mut(p).set_right(l); + if let LinkParent::AuxParent(p) = &p { + debug_assert!(self.node(p).right() == Some(&n.handle())); + self.node_mut(p).set_right(l.clone()); } if let Some(l) = l { - self.node_mut(l).parent = p; + self.node_mut(&l).parent = p; } let Some(rep_p) = rep_p else { break; }; - if !self.node_mut(rep_p).dec_ref(false) { - break; - } - n = rep_p; - } - // from time to time, clean up memory in the slab - // TODO: this needs more analysis under amortized runtime costs and a clever potential - // definition. shrink_to_fit will first check if there are any vacant slots at "the end". If - // there are, it will then do a full pass over empty and filled slots. The problem is that - // "the end" is not easily available from the public API. We know it's somewhere between - // len() and capacity(), and also past the link(s) we just removed. But we can't check the - // internal entries.len(). - const ALLOWED_SLACK: usize = 64 * 1024 * 1024 / size_of::(); - let slots = &mut self.nodes; - if slots.capacity() / 4 > slots.len() && slots.capacity() - slots.len() > ALLOWED_SLACK { - slots.shrink_to_fit(); + slot = rep_p; + n = &slot; } } pub fn reassign(&mut self, link: &LinkHandle, new_parent: DomSlotVariant) { - let link = link.id; // removes `link` from its represented tree and moves it to `new_parent`. - // we also have to keep track of ref counts. - debug_assert!( - self.node(link).has_owner(), - "owner must be alive to reassign" - ); - let old_parent_id = self.node(link).rep_parent(); + let old_parent_id = self.node_mut(link).rep_parent(); let (new_parent, new_parent_id) = match new_parent { DomSlotVariant::Chained(link) => { - (LinkParent::PathParent(link.link.id), Some(link.link.id)) + (LinkParent::PathParent(link.link.clone()), Some(link.link)) } DomSlotVariant::Node(data) => (LinkParent::Root(data), None), }; @@ -171,26 +212,24 @@ impl LinkForest { // reassigned to its existing parent, no need to modify. return; } - if let Some(new_parent_id) = new_parent_id { - self.node_mut(new_parent_id).add_ref(); - } self.splay(link); - let l = self.node(link).left(); + let left = self.node(link).left().cloned(); let parent = self.node_mut(link).parent.replace(new_parent); self.node_mut(link).set_left(None); - self.node_mut(link).set_rep_parent(new_parent_id); - if let Some(l) = l { - self.node_mut(l).parent = parent; + let new_parent_id = new_parent_id.map(|handle| handle.to_owner()); + let old_parent_id = self.node_mut(link).replace_rep_parent(new_parent_id); + if let Some(l) = left { + self.node_mut(&l).parent = parent; } if let Some(old_parent_id) = old_parent_id { - self.remove_link(old_parent_id, false); + self.remove_link(&old_parent_id); } } - pub fn find_root(&mut self, link: &LinkHandle) -> &Option { - self.access(link.id); - match &self.node(link.id).into_inner().parent { - LinkParent::Root(node) => node, + pub fn find_root(&mut self, link: &LinkHandle) -> Option { + self.access(link); + match &self.node(link).parent { + LinkParent::Root(node) => node.clone(), _ => unreachable!("access method buggy"), } } @@ -198,91 +237,94 @@ impl LinkForest { // Splay operations on the auxiliary tree // In fact, none of the splay operations change the refcount, since they do not modify the // represented tree. - fn splay_parent(&self, link: LinkId) -> Result { + fn splay_parent(&self, link: &LinkHandle) -> Result { // Due to borrow issues (fixed with polonius?) we can't borrow data here, and do that // in the caller with a double match :/ - match &self.node(link).parent { - &LinkParent::AuxParent(parent) => Ok(parent), - &LinkParent::PathParent(link) => Err(SplayResult::Link(link)), + match &self.node(&link).parent { + LinkParent::AuxParent(parent) => Ok(parent.clone()), + LinkParent::PathParent(link) => Err(SplayResult::Link(link.clone())), LinkParent::Root(_) => Err(SplayResult::Root()), } } - fn rotate(&mut self, x: LinkId, p: LinkId) { + fn rotate(&mut self, x: &LinkHandle, p: &LinkHandle) { // shift the middle node `m` from `x` to `p`. let m; debug_assert!(self.node(p).right() == Some(x) || self.node(p).left() == Some(x)); if self.node(p).left() == Some(x) { - m = self.node(x).right(); - self.node_mut(x).set_right(Some(p)); - self.node_mut(p).set_left(m); + m = self.node(x).right().cloned(); + self.node_mut(x).set_right(Some(p.clone())); + self.node_mut(p).set_left(m.clone()); } else { - m = self.node(x).left(); - self.node_mut(x).set_left(Some(p)); - self.node_mut(p).set_right(m); + m = self.node(x).left().cloned(); + self.node_mut(x).set_left(Some(p.clone())); + self.node_mut(p).set_right(m.clone()); }; if let Some(m) = m { - debug_assert_eq!(self.node_mut(m).parent, LinkParent::AuxParent(x)); - self.node_mut(m).parent = LinkParent::AuxParent(p); + // debug_assert_eq!(self.node_mut(m).parent, LinkParent::AuxParent(x)); + self.node_mut(&m).parent = LinkParent::AuxParent(p.clone()); } // attach `x` to the parent of `p` - let g = self.node_mut(p).parent.replace(LinkParent::AuxParent(x)); - if let LinkParent::AuxParent(g) = g { + let g = self + .node_mut(p) + .parent + .replace(LinkParent::AuxParent(x.clone())); + if let LinkParent::AuxParent(g) = &g { let mut g = self.node_mut(g); debug_assert!(g.right() == Some(p) || g.left() == Some(p)); if g.left() == Some(p) { - g.set_left(Some(x)); + g.set_left(Some(x.clone())); } else { - g.set_right(Some(x)); + g.set_right(Some(x.clone())); } } self.node_mut(x).parent = g; } - fn splay(&mut self, link: LinkId) -> SplayResult { + fn splay(&mut self, link: &LinkHandle) -> SplayResult { let x = link; loop { let mut p = match self.splay_parent(x) { Ok(p) => p, Err(done) => return done, }; - if let Ok(g) = self.splay_parent(p) { + if let Ok(g) = self.splay_parent(&p) { // check for zig-zig or zig-zag // zig-zig can be implemented by first rotating p and g, followed by x and p // zig-zag can be implemented by first rotating x and p, followed by x and g - let x_is_left = self.node(p).left() == Some(x); - let p_is_left = self.node(g).left() == Some(p); + let x_is_left = self.node(&p).left() == Some(x); + let p_is_left = self.node(&g).left() == Some(&p); if x_is_left == p_is_left { - self.rotate(p, g); + self.rotate(&p, &g); } else { - self.rotate(x, p); + self.rotate(x, &p); p = g; } } - self.rotate(x, p); + self.rotate(x, &p); } } // Link/cut operations - fn access(&mut self, link: LinkId) { + fn access(&mut self, link: &LinkHandle) { // We use an iterative approach to traverse a possible long chain of references. // See issue #3043 for why a recursive call is impossible for large lists in vdom. // Also does not change any refcounts - let (mut curr, mut prev) = (link, None); + let (mut curr, mut prev) = (link.clone(), None); loop { - let link = self.splay(curr); + let link = self.splay(&curr); // found a path-parent pointer. now we cut this one - let d = self.node_mut(curr).right(); - if let Some(prev) = prev { - debug_assert_eq!(self.node(prev).parent, LinkParent::PathParent(curr)); - self.node_mut(prev).parent = LinkParent::AuxParent(curr); + let d = self.node_mut(&curr).right().cloned(); + if let Some(ref prev) = prev { + // debug_assert_eq!(self.node(prev).parent, LinkParent::PathParent(curr)); + self.node_mut(prev).parent = LinkParent::AuxParent(curr.clone()); // small deviation from the original paper: we do not remove the tail // of the preferred path the first node is already on. // this would originally run unconditionally of prev.is_some() - self.node_mut(curr).set_right(Some(prev)); + self.node_mut(&curr).set_right(Some(prev.clone())); if let Some(d) = d { - debug_assert_eq!(self.node(d).parent, LinkParent::AuxParent(curr)); - self.node_mut(d).parent = LinkParent::PathParent(curr); + // debug_assert_eq!(self.node(d).parent, LinkParent::AuxParent(curr)); + self.node_mut(&d).parent = LinkParent::PathParent(curr.clone()); } } let SplayResult::Link(link) = link else { break }; @@ -294,22 +336,20 @@ impl LinkForest { } } -pub type LinkId = usize; - enum SplayResult { - Link(LinkId), + Link(LinkHandle), Root( // &'a Option ), } -#[derive(PartialEq, Debug)] +#[derive(Debug)] enum LinkParent { Root(Option), // parent is on the same preferred path - AuxParent(LinkId), + AuxParent(LinkHandle), // "path-parent pointer" to some other preferred path - PathParent(LinkId), + PathParent(LinkHandle), } impl LinkParent { @@ -320,14 +360,9 @@ impl LinkParent { struct Link { parent: LinkParent, - // We use a link's own id to signal that it has no right/left child or represented parent - left_aux: LinkId, - right_aux: LinkId, - rep_parent: LinkId, - /// counts the owner + the number of links in LINK_FOREST that refer to this link. - /// to save a bit, the owner is counted in the lowest bit, handles are counted in the upper - /// bits - ref_count: usize, + left_aux: Option, + right_aux: Option, + rep_parent: Option, } impl AsRef for Link { @@ -343,7 +378,7 @@ impl AsMut for Link { } struct LinkRef { - id: LinkId, + id: usize, link: L, } @@ -362,7 +397,7 @@ impl + AsMut> DerefMut for LinkRef { } impl LinkRef { - fn new(id: LinkId, link: L) -> Self { + fn new(id: usize, link: L) -> Self { Self { id, link } } @@ -372,89 +407,71 @@ impl LinkRef { } impl> LinkRef { + #[allow(unused)] fn debug(&self) -> impl '_ + std::fmt::Debug { - #[expect(unused)] #[derive(Debug)] struct Link<'a> { - id: LinkId, + id: usize, parent: &'a LinkParent, - left_aux: Option, - right_aux: Option, - rep_parent: Option, - ref_count: usize, - has_owner: bool, + left_aux: Option, + right_aux: Option, + rep_parent: Option, } - let has_owner = self.has_owner(); Link { id: self.id, parent: &self.parent, - left_aux: self.left(), - right_aux: self.right(), - rep_parent: self.rep_parent(), - ref_count: self.ref_count / 2 + has_owner as usize, - has_owner, + left_aux: self.left_aux.as_ref().map(|l| l.id.id()), + right_aux: self.right_aux.as_ref().map(|l| l.id.id()), + rep_parent: self.rep_parent.as_ref().map(|l| l.id.id()), } } - fn left(&self) -> Option { - (self.left_aux != self.id).then_some(self.left_aux) - } - - fn right(&self) -> Option { - (self.right_aux != self.id).then_some(self.right_aux) + fn left(&self) -> Option<&LinkHandle> { + self.left_aux.as_ref() } - fn rep_parent(&self) -> Option { - (self.rep_parent != self.id).then_some(self.rep_parent) + fn right(&self) -> Option<&LinkHandle> { + self.right_aux.as_ref() } } impl> LinkRef { - fn set_left(&mut self, left: Option) { - self.link.as_mut().left_aux = left.unwrap_or(self.id); + fn set_left(&mut self, left: Option) { + self.link.as_mut().left_aux = left; + } + + fn set_right(&mut self, right: Option) { + self.link.as_mut().right_aux = right; } - fn set_right(&mut self, right: Option) { - self.link.as_mut().right_aux = right.unwrap_or(self.id); + fn rep_parent(&mut self) -> Option { + self.link + .as_mut() + .rep_parent + .as_ref() + .map(|parent| parent.handle()) } - fn set_rep_parent(&mut self, rep_parent: Option) { - self.link.as_mut().rep_parent = rep_parent.unwrap_or(self.id); + fn replace_rep_parent(&mut self, rep_parent: Option) -> Option { + std::mem::replace(&mut self.link.as_mut().rep_parent, rep_parent) } } impl Link { - pub fn new(parent: DomSlotVariant, this: LinkId) -> (Self, Option) { + pub fn new(parent: DomSlotVariant) -> Self { let (parent, link) = match parent { DomSlotVariant::Node(node) => (LinkParent::Root(node), None), - DomSlotVariant::Chained(handle) => { - (LinkParent::PathParent(handle.link.id), Some(handle.link.id)) - } + DomSlotVariant::Chained(handle) => ( + LinkParent::PathParent(handle.link.clone()), + Some(handle.link.to_owner()), + ), }; - let this = Self { + Self { parent, - left_aux: this, - right_aux: this, - rep_parent: link.unwrap_or(this), - ref_count: 1, - }; - (this, link) - } - - fn has_owner(&self) -> bool { - (self.ref_count & 0b1) != 0 - } - - fn dec_ref(&mut self, owner: bool) -> bool { - let weight = if owner { 1 } else { 2 }; - debug_assert!(self.ref_count >= weight, "must have refs"); - self.ref_count -= weight; - self.ref_count == 0 - } - - fn add_ref(&mut self) { - debug_assert!(self.ref_count > 0, "no revives"); - self.ref_count += 2; + left_aux: None, + right_aux: None, + rep_parent: link, + } } } @@ -464,14 +481,13 @@ mod feat_hydration { impl LinkForest { pub fn leak(&mut self, link: LinkOwner) -> LinkHandle { - self.node_mut(link.id).leak(); + self.node_mut(&link.handle()).leak(); LinkHandle::from_raw(link.id) } } impl Link { fn leak(&mut self) { - self.add_ref(); - self.dec_ref(true); + // self.dec_owner(); } } } From a11e043e2e8477b5d9bec637c145b624e36e1812 Mon Sep 17 00:00:00 2001 From: Martin Molzer Date: Tue, 18 Aug 2026 16:02:09 +0200 Subject: [PATCH 17/17] be more precise with ownership semantics --- .../yew/src/dom_bundle/position/forest.rs | 90 ++++++++++++------- 1 file changed, 58 insertions(+), 32 deletions(-) diff --git a/packages/yew/src/dom_bundle/position/forest.rs b/packages/yew/src/dom_bundle/position/forest.rs index bdca95ef518..43474807aa8 100644 --- a/packages/yew/src/dom_bundle/position/forest.rs +++ b/packages/yew/src/dom_bundle/position/forest.rs @@ -2,6 +2,7 @@ use std::cell::{self, RefCell}; use std::fmt::Debug; use std::marker::PhantomData; use std::ops::{Deref, DerefMut}; +use std::ptr::NonNull; use std::rc::Rc; use super::{DomSlotVariant, Node}; @@ -10,7 +11,9 @@ type PhantomNotSendNorSync = PhantomData<*const u8>; /// A dummy struct that serves as a marker for the borrow of [`LINK_FOREST`]. #[derive(Default)] -pub struct LinkForest; +pub struct LinkForest { + _priv: (), +} thread_local! { static LINK_FOREST: RefCell = { @@ -25,51 +28,74 @@ pub fn with_forest(f: impl FnOnce(&mut LinkForest) -> R) -> R { #[derive(Clone)] struct RawLink( /// SAFETY: Comes from `Rc::into_raw` - *const RefCell, + NonNull>, ); impl RawLink { fn new(link: Link) -> Self { let the_rc = Rc::new(RefCell::new(link)); - let ptr = Rc::into_raw(the_rc); - Self(ptr) + let ptr = Rc::into_raw(the_rc).cast_mut(); + // Pointers from Rc::into_raw are always non-null! + Self(NonNull::new(ptr).unwrap()) } fn id(&self) -> usize { - self.0.addr() + self.0.as_ptr().addr() } fn inc_strong(&self) { - unsafe { Rc::increment_strong_count(self.0) }; + unsafe { Rc::increment_strong_count(self.0.as_ptr()) }; } fn into_rc(self) -> Rc> { - unsafe { Rc::from_raw(self.0) } + unsafe { Rc::from_raw(self.0.as_ptr()) } + } + + fn as_ref(&self) -> &RefCell { + unsafe { &*self.0.as_ptr() } } } impl PartialEq for RawLink { fn eq(&self, other: &Self) -> bool { - std::ptr::addr_eq(self.0, other.0) + self.0 == other.0 } } #[derive(PartialEq)] pub struct LinkOwner { - id: RawLink, + id: Option, // The link is tied to this specific thread and can't be accessed elsewhere _phantom: PhantomNotSendNorSync, } impl Debug for LinkOwner { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "#{:x}", self.id.id()) + write!(f, "#{:x}", self.id()) } } impl LinkOwner { pub fn handle(&self) -> LinkHandle { - LinkHandle::from_raw(self.id.clone()) + LinkHandle::from_raw(self.id.as_ref().unwrap().clone()) + } + + fn into_inner(self) -> RawLink { + self.id.unwrap() + } + + fn take(&mut self) -> LinkOwner { + Self { + id: self.id.take(), + _phantom: PhantomData, + } + } + + fn id(&self) -> usize { + match &self.id { + Some(id) => id.id(), + None => 0, + } } } @@ -96,7 +122,7 @@ impl LinkHandle { fn to_owner(&self) -> LinkOwner { self.id.inc_strong(); LinkOwner { - id: self.id.clone(), + id: Some(self.id.clone()), _phantom: PhantomData, } } @@ -142,36 +168,37 @@ impl LinkForest { pub fn insert(&mut self, link: DomSlotVariant) -> LinkOwner { let raw = RawLink::new(Link::new(link)); LinkOwner { - id: raw, + id: Some(raw), _phantom: PhantomData, } } - fn node(&self, link: &LinkHandle) -> LinkRef> { - let refcell = unsafe { &*link.id.0 }; - LinkRef::new(link.id.id(), AsRefRef(refcell.borrow())) + fn node<'h>(&self, link: &'h LinkHandle) -> LinkRef> { + LinkRef::new(link.id.id(), AsRefRef(link.id.as_ref().borrow())) } - fn node_mut(&mut self, link: &LinkHandle) -> LinkRef + AsMut> { - let refcell = unsafe { &*link.id.0 }; - LinkRef::new(link.id.id(), AsRefMut(refcell.borrow_mut())) + fn node_mut<'h>( + &mut self, + link: &'h LinkHandle, + ) -> LinkRef + AsMut> { + LinkRef::new(link.id.id(), AsRefMut(link.id.as_ref().borrow_mut())) } - fn remove_node(&mut self, link: &LinkOwner) -> Option> { + fn remove_node(&mut self, link: LinkOwner) -> Option> { // TODO: this should take the owner by value, but that's incompatible with calling it // inside of a Drop method without adding a new "invalid" state. - let id = link.id.id(); - let owned = link.id.clone().into_rc(); + let inner = link.into_inner(); + let id = inner.id(); + let owned = inner.into_rc(); let inner = Rc::try_unwrap(owned).ok()?.into_inner(); Some(LinkRef::new(id, inner)) } pub fn remove(&mut self, link: &mut LinkOwner) { - self.remove_link(link); + self.remove_link(link.take()); } - fn remove_link(&mut self, link: &LinkOwner) { - let mut slot; + fn remove_link(&mut self, link: LinkOwner) { let mut n = link; loop { let Some(mut node) = self.remove_node(n) else { @@ -185,7 +212,7 @@ impl LinkForest { let rep_p = node.replace_rep_parent(None); let p = node.into_inner().parent; if let LinkParent::AuxParent(p) = &p { - debug_assert!(self.node(p).right() == Some(&n.handle())); + // debug_assert!(self.node(p).right() == Some(&n.handle())); self.node_mut(p).set_right(l.clone()); } if let Some(l) = l { @@ -194,8 +221,7 @@ impl LinkForest { let Some(rep_p) = rep_p else { break; }; - slot = rep_p; - n = &slot; + n = rep_p; } } @@ -222,7 +248,7 @@ impl LinkForest { self.node_mut(&l).parent = parent; } if let Some(old_parent_id) = old_parent_id { - self.remove_link(&old_parent_id); + self.remove_link(old_parent_id); } } @@ -240,7 +266,7 @@ impl LinkForest { fn splay_parent(&self, link: &LinkHandle) -> Result { // Due to borrow issues (fixed with polonius?) we can't borrow data here, and do that // in the caller with a double match :/ - match &self.node(&link).parent { + match &self.node(link).parent { LinkParent::AuxParent(parent) => Ok(parent.clone()), LinkParent::PathParent(link) => Err(SplayResult::Link(link.clone())), LinkParent::Root(_) => Err(SplayResult::Root()), @@ -422,7 +448,7 @@ impl> LinkRef { parent: &self.parent, left_aux: self.left_aux.as_ref().map(|l| l.id.id()), right_aux: self.right_aux.as_ref().map(|l| l.id.id()), - rep_parent: self.rep_parent.as_ref().map(|l| l.id.id()), + rep_parent: self.rep_parent.as_ref().map(|l| l.id()), } } @@ -482,7 +508,7 @@ mod feat_hydration { impl LinkForest { pub fn leak(&mut self, link: LinkOwner) -> LinkHandle { self.node_mut(&link.handle()).leak(); - LinkHandle::from_raw(link.id) + LinkHandle::from_raw(link.into_inner()) } } impl Link {