From e2e4d431e14e9e91b3301e994363c042400e687e Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Mon, 20 Dec 2021 22:33:13 -0500 Subject: [PATCH 1/2] wip: adjust memoization --- packages/core/examples/borrowed.rs | 2 +- packages/core/examples/hooks.rs | 4 - packages/core/examples/works.rs | 31 +- packages/core/src/diff.rs | 193 +++++----- packages/core/src/lib.rs | 18 +- packages/core/src/mutations.rs | 5 + packages/core/src/nodes.rs | 177 +++++----- packages/core/src/scopearena.rs | 355 ------------------- packages/core/src/{scope.rs => scopes.rs} | 408 +++++++++++++++++++--- packages/core/src/virtual_dom.rs | 224 ++++++++---- packages/core/tests/diffing.rs | 2 +- packages/core/tests/earlyabort.rs | 4 +- packages/core/tests/lifecycle.rs | 26 +- packages/core/tests/miri_stress.rs | 184 +++++++++- packages/ssr/src/lib.rs | 2 +- 15 files changed, 932 insertions(+), 703 deletions(-) delete mode 100644 packages/core/src/scopearena.rs rename packages/core/src/{scope.rs => scopes.rs} (55%) diff --git a/packages/core/examples/borrowed.rs b/packages/core/examples/borrowed.rs index 784f48740..f4dc7d955 100644 --- a/packages/core/examples/borrowed.rs +++ b/packages/core/examples/borrowed.rs @@ -20,7 +20,7 @@ struct Borrowed<'a> { fn App2<'a>(cx: Scope<'a, Borrowed<'a>>) -> Element { let g = eat2(&cx); - todo!() + rsx!(cx, "") } fn eat2(s: &ScopeState) {} diff --git a/packages/core/examples/hooks.rs b/packages/core/examples/hooks.rs index 969d9f3f2..131aae0e6 100644 --- a/packages/core/examples/hooks.rs +++ b/packages/core/examples/hooks.rs @@ -4,7 +4,3 @@ use dioxus_core_macro::*; use dioxus_html as dioxus_elements; fn main() {} - -fn App(cx: Scope<()>) -> Element { - todo!() -} diff --git a/packages/core/examples/works.rs b/packages/core/examples/works.rs index f96c40e88..d8e171042 100644 --- a/packages/core/examples/works.rs +++ b/packages/core/examples/works.rs @@ -6,16 +6,15 @@ use dioxus_core_macro::*; use dioxus_html as dioxus_elements; fn main() { - let _ = VirtualDom::new(Parent); + let _ = VirtualDom::new(parent); } -fn Parent(cx: Scope<()>) -> Element { +fn parent(cx: Scope<()>) -> Element { let value = cx.use_hook(|_| String::new(), |f| f); cx.render(rsx! { div { - Child { name: value } - Fragment { "asd" } + child( name: value ) } }) } @@ -25,11 +24,11 @@ struct ChildProps<'a> { name: &'a str, } -fn Child<'a>(cx: Scope<'a, ChildProps<'a>>) -> Element { +fn child<'a>(cx: Scope<'a, ChildProps<'a>>) -> Element { cx.render(rsx! { div { h1 { "it's nested" } - Child2 { name: cx.props.name } + grandchild( name: cx.props.name ) } }) } @@ -39,8 +38,26 @@ struct Grandchild<'a> { name: &'a str, } -fn Child2<'a>(cx: Scope<'a, Grandchild<'a>>) -> Element { +fn grandchild<'a>(cx: Scope<'a, Grandchild>) -> Element<'a> { cx.render(rsx! { div { "Hello {cx.props.name}!" } + great_grandchild( name: cx.props.name ) }) } + +fn great_grandchild<'a>(cx: Scope<'a, Grandchild>) -> Element<'a> { + cx.render(rsx! { + div { + h1 { "it's nested" } + } + }) +} + +/* +can we implement memoization as a wrapper or something? Like we just intercept the +render function? + + + + +*/ diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 7a5e00810..9603f1c0f 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -239,15 +239,6 @@ impl<'bump> DiffStack<'bump> { } impl<'bump> DiffState<'bump> { - pub fn diff_scope(&mut self, id: ScopeId) { - let (old, new) = (self.scopes.wip_head(id), self.scopes.fin_head(id)); - self.stack.push(DiffInstruction::Diff { old, new }); - self.stack.scope_stack.push(id); - let scope = self.scopes.get_scope(id).unwrap(); - self.stack.element_stack.push(scope.container); - self.work(|| false); - } - /// Progress the diffing for this "fiber" /// /// This method implements a depth-first iterative tree traversal. @@ -415,7 +406,7 @@ impl<'bump> DiffState<'bump> { } // todo: the settext optimization - + // // if children.len() == 1 { // if let VNode::Text(vtext) = children[0] { // self.mutations.set_text(vtext.text, real_id.as_u64()); @@ -436,40 +427,32 @@ impl<'bump> DiffState<'bump> { let parent_idx = self.stack.current_scope().unwrap(); // Insert a new scope into our component list - let parent_scope = self.scopes.get_scope(parent_idx).unwrap(); - let height = parent_scope.height + 1; - let subtree = parent_scope.subtree.get(); - - let parent_scope = self.scopes.get_scope_raw(parent_idx); - let caller = unsafe { std::mem::transmute(vcomponent.caller as *const _) }; - let fc_ptr = vcomponent.user_fc; - - let container = *self.stack.element_stack.last().unwrap(); - - let new_idx = - self.scopes - .new_with_key(fc_ptr, caller, parent_scope, container, height, subtree); + let caller = unsafe { std::mem::transmute(vcomponent.props as *const _) }; + let new_idx = self.scopes.new_with_key( + vcomponent.user_fc, + caller, + Some(parent_idx), + self.stack.element_stack.last().copied().unwrap(), + 0, + ); // Actually initialize the caller's slot with the right address - vcomponent.associated_scope.set(Some(new_idx)); + vcomponent.scope.set(Some(new_idx)); - if !vcomponent.can_memoize { - let cur_scope = self.scopes.get_scope(parent_idx).unwrap(); - let extended = unsafe { std::mem::transmute(vcomponent) }; - cur_scope.items.borrow_mut().borrowed_props.push(extended); - } else { - // the props are currently bump allocated but we need to move them to the heap + match vcomponent.can_memoize { + true => { + // promote the props onto the heap + // when we diff these nodes, we'll want to move it over + vcomponent.props.promote(); + } + false => { + // track this component internally so we know the right drop order + let cur_scope = self.scopes.get_scope(parent_idx).unwrap(); + let extended = unsafe { std::mem::transmute(vcomponent) }; + cur_scope.items.borrow_mut().borrowed_props.push(extended); + } } - // TODO: add noderefs to current noderef list Noderefs - let _new_component = self.scopes.get_scope(new_idx).unwrap(); - - log::debug!( - "initializing component {:?} with height {:?}", - new_idx, - height + 1 - ); - // Run the scope for one iteration to initialize it self.scopes.run_scope(new_idx); @@ -477,11 +460,6 @@ impl<'bump> DiffState<'bump> { let nextnode = self.scopes.fin_head(new_idx); self.stack.create_component(new_idx, nextnode); - // todo: subtrees - // if new_component.is_subtree_root.get() { - // self.stack.push_subtree(); - // } - // Finally, insert this scope as a seen node. self.mutations.dirty_scopes.insert(new_idx); } @@ -494,15 +472,23 @@ impl<'bump> DiffState<'bump> { use VNode::*; match (old_node, new_node) { // Check the most common cases first + // these are *actual* elements, not wrappers around lists (Text(old), Text(new)) => { self.diff_text_nodes(old, new, old_node, new_node); } + (Element(old), Element(new)) => self.diff_element_nodes(old, new, old_node, new_node), + (Placeholder(old), Placeholder(new)) => { + if let Some(root) = old.dom_id.get() { + self.scopes.update_node(new_node, root); + new.dom_id.set(Some(root)) + } + } + + // These two sets are pointers to nodes but are not actually nodes themselves (Component(old), Component(new)) => { self.diff_component_nodes(old_node, new_node, *old, *new) } (Fragment(old), Fragment(new)) => self.diff_fragment_nodes(old, new), - (Placeholder(old), Placeholder(new)) => self.diff_placeholder_nodes(old, new), - (Element(old), Element(new)) => self.diff_element_nodes(old, new, old_node, new_node), // The normal pathway still works, but generates slightly weird instructions // This pathway just ensures we get create and replace @@ -708,21 +694,14 @@ impl<'bump> DiffState<'bump> { old: &'bump VComponent<'bump>, new: &'bump VComponent<'bump>, ) { - let scope_addr = old.associated_scope.get().unwrap(); - - log::debug!( - "Diffing components. old_scope: {:?}, old_addr: {:?}, new_addr: {:?}", - scope_addr, - old.user_fc, - new.user_fc - ); + let scope_addr = old.scope.get().unwrap(); // Make sure we're dealing with the same component (by function pointer) if old.user_fc == new.user_fc { self.stack.scope_stack.push(scope_addr); // Make sure the new component vnode is referencing the right scope id - new.associated_scope.set(Some(scope_addr)); + new.scope.set(Some(scope_addr)); // make sure the component's caller function is up to date let scope = self @@ -730,12 +709,45 @@ impl<'bump> DiffState<'bump> { .get_scope(scope_addr) .unwrap_or_else(|| panic!("could not find {:?}", scope_addr)); - scope.caller.set(unsafe { std::mem::transmute(new.caller) }); + /* + old.props is the previous vcomponent + existing_props is the current ScopeState's + */ + let existing_props = unsafe { &*scope.render.get() }; + if old.can_memoize { + // let props_are_the_same = unsafe { existing_props.memoize(new.props) }; - // React doesn't automatically memoize, but we do. - let props_are_the_same = old.comparator.unwrap(); + if !props_are_the_same || self.force_diff { + // release the old props since they will be overwritten next frame + // existing_props.release(); - if self.force_diff || !props_are_the_same(new) { + // move the old props onto the heap + // todo: maybe we can reuse the old props box? + new.props.promote(); + + scope + .render + .set(unsafe { std::mem::transmute(new.props as *const dyn AnyProps) }); + + // this should auto drop the previous props + self.scopes.run_scope(scope_addr); + + self.diff_node( + self.scopes.wip_head(scope_addr), + self.scopes.fin_head(scope_addr), + ); + } else { + // release the old props since they will be overwritten next frame + if existing_props.as_ptr() != old.props.as_ptr() { + old.props.release(); + } + } + } else { + scope + .render + .set(unsafe { std::mem::transmute(new.props as *const dyn AnyProps) }); + + // this should auto drop the previous props self.scopes.run_scope(scope_addr); self.diff_node( self.scopes.wip_head(scope_addr), @@ -764,11 +776,6 @@ impl<'bump> DiffState<'bump> { self.diff_children(old.children, new.children); } - // probably will get inlined - fn diff_placeholder_nodes(&mut self, old: &'bump VPlaceholder, new: &'bump VPlaceholder) { - new.dom_id.set(old.dom_id.get()) - } - // ============================================= // Utilities for creating new diff instructions // ============================================= @@ -792,33 +799,8 @@ impl<'bump> DiffState<'bump> { // Remember, fragments can never be empty (they always have a single child) match (old, new) { ([], []) => {} - ([], _) => { - // we need to push the - self.stack.create_children(new, MountType::Append); - } - (_, []) => { - self.remove_nodes(old, true); - } - - // todo: placeholders explicitly replace fragments - // - // ([VNode::Placeholder(old_anchor)], [VNode::Placeholder(new_anchor)]) => { - // old_anchor.dom_id.set(new_anchor.dom_id.get()); - // } - // ([VNode::Placeholder(_)], _) => { - // self.stack - // .create_children(new, MountType::Replace { old: &old[0] }); - // } - // (_, [VNode::Placeholder(_)]) => { - // let new: &'bump VNode<'bump> = &new[0]; - // if let Some(first_old) = old.get(0) { - // self.remove_nodes(&old[1..], true); - // self.stack - // .create_node(new, MountType::Replace { old: first_old }); - // } else { - // self.stack.create_node(new, MountType::Append {}); - // } - // } + ([], _) => self.stack.create_children(new, MountType::Append), + (_, []) => self.remove_nodes(old, true), _ => { let new_is_keyed = new[0].key().is_some(); let old_is_keyed = old[0].key().is_some(); @@ -1216,7 +1198,7 @@ impl<'bump> DiffState<'bump> { search_node = frag.children.last(); } VNode::Component(el) => { - let scope_id = el.associated_scope.get().unwrap(); + let scope_id = el.scope.get().unwrap(); search_node = Some(self.scopes.root_node(scope_id)); } } @@ -1233,7 +1215,7 @@ impl<'bump> DiffState<'bump> { search_node = Some(&frag.children[0]); } VNode::Component(el) => { - let scope_id = el.associated_scope.get().unwrap(); + let scope_id = el.scope.get().unwrap(); search_node = Some(self.scopes.root_node(scope_id)); } VNode::Text(t) => break t.dom_id.get(), @@ -1250,8 +1232,6 @@ impl<'bump> DiffState<'bump> { .try_mounted_id() .unwrap_or_else(|| panic!("broke on {:?}", old)); - log::debug!("element parent is {:?}", el.parent_id.get()); - self.mutations.replace_with(id, nodes_created as u32); self.remove_nodes(el.children, false); } @@ -1270,11 +1250,11 @@ impl<'bump> DiffState<'bump> { } VNode::Component(c) => { - let node = self.scopes.fin_head(c.associated_scope.get().unwrap()); + let node = self.scopes.fin_head(c.scope.get().unwrap()); self.replace_node(node, nodes_created); - let scope_id = c.associated_scope.get().unwrap(); - log::debug!("Destroying scope {:?}", scope_id); + let scope_id = c.scope.get().unwrap(); + self.scopes.try_remove(scope_id).unwrap(); } } @@ -1282,7 +1262,7 @@ impl<'bump> DiffState<'bump> { /// schedules nodes for garbage collection and pushes "remove" to the mutation stack /// remove can happen whenever - fn remove_nodes( + pub(crate) fn remove_nodes( &mut self, nodes: impl IntoIterator>, gen_muts: bool, @@ -1323,20 +1303,17 @@ impl<'bump> DiffState<'bump> { } VNode::Component(c) => { - self.destroy_vomponent(c, gen_muts); + let scope_id = c.scope.get().unwrap(); + let root = self.scopes.root_node(scope_id); + self.remove_nodes(Some(root), gen_muts); + + log::debug!("Destroying scope {:?}", scope_id); + self.scopes.try_remove(scope_id).unwrap(); } } } } - fn destroy_vomponent(&mut self, vc: &VComponent, gen_muts: bool) { - let scope_id = vc.associated_scope.get().unwrap(); - let root = self.scopes.root_node(scope_id); - self.remove_nodes(Some(root), gen_muts); - log::debug!("Destroying scope {:?}", scope_id); - self.scopes.try_remove(scope_id).unwrap(); - } - /// Adds a listener closure to a scope during diff. fn attach_listener_to_scope(&mut self, listener: &'bump Listener<'bump>, scope: &ScopeState) { let long_listener = unsafe { std::mem::transmute(listener) }; diff --git a/packages/core/src/lib.rs b/packages/core/src/lib.rs index 9055d8729..aeb3b56f1 100644 --- a/packages/core/src/lib.rs +++ b/packages/core/src/lib.rs @@ -6,8 +6,7 @@ pub(crate) mod diff; pub(crate) mod lazynodes; pub(crate) mod mutations; pub(crate) mod nodes; -pub(crate) mod scope; -pub(crate) mod scopearena; +pub(crate) mod scopes; pub(crate) mod virtual_dom; pub(crate) mod innerlude { @@ -16,8 +15,8 @@ pub(crate) mod innerlude { pub use crate::lazynodes::*; pub use crate::mutations::*; pub use crate::nodes::*; - pub use crate::scope::*; - pub(crate) use crate::scopearena::*; + pub use crate::scopes::*; + pub(crate) use crate::scopes::*; pub use crate::virtual_dom::*; /// An [`Element`] is a possibly-none [`VNode`] created by calling `render` on [`Scope`] or [`ScopeState`]. @@ -61,7 +60,7 @@ pub(crate) mod innerlude { /// // ... /// }; /// ``` - pub type Component

= for<'a> fn(Scope<'a, P>) -> Element<'a>; + pub type Component

= fn(Scope

) -> Element; } pub use crate::innerlude::{ @@ -86,3 +85,12 @@ pub mod exports { pub use bumpalo; pub use futures_channel; } + +/// Functions that wrap unsafe functionality to prevent us from misusing it at the callsite +pub(crate) mod unsafe_utils { + use crate::VNode; + + pub unsafe fn extend_vnode<'a, 'b>(node: &'a VNode<'a>) -> &'b VNode<'b> { + std::mem::transmute(node) + } +} diff --git a/packages/core/src/mutations.rs b/packages/core/src/mutations.rs index 862a42df4..2e6333386 100644 --- a/packages/core/src/mutations.rs +++ b/packages/core/src/mutations.rs @@ -45,6 +45,11 @@ pub enum DomEdit<'bump> { many: u32, }, + // // save a possibly-fragment node as a template + // SaveAsTemplate { + // many: u32, + // }, + // "Root" refers to the item directly // it's a waste of an instruction to push the root directly ReplaceWith { diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index a6f288a52..3d14a332c 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -6,6 +6,7 @@ use crate::{ innerlude::{Element, Properties, Scope, ScopeId, ScopeState}, lazynodes::LazyNodes, + Component, }; use bumpalo::{boxed::Box as BumpBox, Bump}; use std::{ @@ -84,7 +85,7 @@ pub enum VNode<'src> { /// /// ```rust, ignore /// fn Example(cx: Scope<()>) -> Element { - /// todo!() + /// ... /// } /// /// let mut vdom = VirtualDom::new(); @@ -186,6 +187,7 @@ impl Debug for VNode<'_> { write!(s, "VNode::VFragment {{ children: {:?} }}", frag.children) } VNode::Component(comp) => write!(s, "VNode::VComponent {{ fc: {:?}}}", comp.user_fc), + // VNode::Component(comp) => write!(s, "VNode::VComponent {{ fc: {:?}}}", comp.user_fc), } } } @@ -354,26 +356,89 @@ impl Clone for EventHandler<'_> { /// Only supports the functional syntax pub struct VComponent<'src> { pub key: Option<&'src str>, - - pub associated_scope: Cell>, - - // Function pointer to the FC that was used to generate this component + pub scope: Cell>, + pub can_memoize: bool, pub user_fc: *const (), + pub props: Option>>, +} - pub(crate) can_memoize: bool, +pub(crate) struct VComponentProps

{ + // Props start local + // If they are static, then they can get promoted onto the heap + pub render_fn: Component

, + pub props: P, + pub memo: unsafe fn(&P, &P) -> bool, +} - pub(crate) _hard_allocation: Cell>, +pub trait AnyProps { + fn as_ptr(&self) -> *const (); + fn release(&self); + fn replace_heap(&self, new: Box); - // Raw pointer into the bump arena for the props of the component - pub(crate) bump_props: *const (), + // move the props from the bump arena onto the heap + fn promote(&self); + fn render<'a>(&'a self, bump: &'a ScopeState) -> Element<'a>; + unsafe fn memoize(&self, other: &dyn AnyProps) -> bool; +} - // during the "teardown" process we'll take the caller out so it can be dropped properly - pub(crate) caller: &'src dyn Fn(&'src ScopeState) -> Element, +impl

AnyProps for VComponentProps

{ + fn as_ptr(&self) -> *const () { + &self.props as *const _ as *const () + // // todo: maybe move this into an enum + // let heap_props = self.heap_props.borrow(); + // if let Some(b) = heap_props.as_ref() { + // b.as_ref() as *const _ as *const () + // } else { + // let local_props = self.local_props.borrow(); + // local_props.as_ref().unwrap() as *const _ as *const () + // } + } - pub(crate) comparator: Option<&'src dyn Fn(&VComponent) -> bool>, + // this will downcat the other ptr as our swallowed type! + // you *must* make this check *before* calling this method + // if your functions are not the same, then you will downcast a pointer into a different type (UB) + unsafe fn memoize(&self, other: &dyn AnyProps) -> bool { + let real_other: &P = &*(other.as_ptr() as *const _ as *const P); + let real_us: &P = &*(self.as_ptr() as *const _ as *const P); + (self.memo)(real_us, real_other) + } - // todo: re-add this - pub(crate) drop_props: RefCell>>, + fn release(&self) { + panic!("don't release props anymore"); + // if let Some(heap_props) = self.heap_props.borrow_mut().take() { + // dbg!("releasing heap"); + // drop(heap_props) + // } else if let Some(local_props) = self.local_props.borrow_mut().take() { + // dbg!("releasing local"); + // drop(local_props) + // } else { + // dbg!("trying to do a double release"); + // } + } + + fn promote(&self) { + panic!("don't promote props anymore"); + // dbg!("promoting props"); + // if let Some(props) = self.local_props.borrow_mut().take() { + // let mut heap_slot = self.heap_props.borrow_mut(); + // *heap_slot = Some(Box::new(props)); + // } + } + + fn render<'a>(&'a self, scope: &'a ScopeState) -> Element<'a> { + let props: &P = &self.props; + let inline_props = unsafe { std::mem::transmute::<&P, &P>(props) }; + + (self.render_fn)(Scope { + scope, + props: inline_props, + }) + } + + fn replace_heap(&self, new: Box) { + // let new = new.downcast::

().unwrap(); + // self.heap_props.borrow_mut().replace(new); + } } /// This struct provides an ergonomic API to quickly build VNodes. @@ -517,75 +582,23 @@ impl<'a> NodeFactory<'a> { where P: Properties + 'a, { - let bump = self.bump; - let props = bump.alloc(props); - let bump_props = props as *mut P as *mut (); - let user_fc = component as *const (); + VNode::Component(self.bump.alloc(VComponent { + key: key.map(|f| self.raw_text(f).0), + scope: Default::default(), + can_memoize: P::IS_STATIC, + user_fc: component as *const (), + props: Some(RefCell::new(Box::new(VComponentProps { + // local_props: RefCell::new(Some(props)), + // heap_props: RefCell::new(None), + props, + memo: P::memoize, // smuggle the memoization function across borders - let comparator: &mut dyn Fn(&VComponent) -> bool = bump.alloc_with(|| { - move |other: &VComponent| { - if user_fc == other.user_fc { - // Safety - // - We guarantee that Component

is the same by function pointer - // - Because Component

is the same, then P must be the same (even with generics) - // - Non-static P are autoderived to memoize as false - // - This comparator is only called on a corresponding set of bumpframes - // - // It's only okay to memoize if there are no children and the props can be memoized - // Implementing memoize is unsafe and done automatically with the props trait - unsafe { - let real_other: &P = &*(other.bump_props as *const _ as *const P); - props.memoize(real_other) - } - } else { - false - } - } - }); - - let drop_props = { - // create a closure to drop the props - let mut has_dropped = false; - - let drop_props: &mut dyn FnMut() = bump.alloc_with(|| { - move || unsafe { - if !has_dropped { - let real_other = bump_props as *mut _ as *mut P; - let b = BumpBox::from_raw(real_other); - std::mem::drop(b); - - has_dropped = true; - } else { - panic!("Drop props called twice - this is an internal failure of Dioxus"); - } - } - }); - - let drop_props = unsafe { BumpBox::from_raw(drop_props) }; - - RefCell::new(Some(drop_props)) - }; - - let key = key.map(|f| self.raw_text(f).0); - - let caller: &'a mut dyn Fn(&'a ScopeState) -> Element = - bump.alloc(move |scope: &ScopeState| -> Element { - let props: &'_ P = unsafe { &*(bump_props as *const P) }; - component(Scope { scope, props }) - }); - - let can_memoize = P::IS_STATIC; - - VNode::Component(bump.alloc(VComponent { - user_fc, - comparator: Some(comparator), - bump_props, - caller, - key, - can_memoize, - drop_props, - associated_scope: Cell::new(None), - _hard_allocation: Cell::new(None), + // i'm sorry but I just need to bludgeon the lifetimes into place here + // this is safe because we're managing all lifetimes to originate from previous calls + // the intricacies of Rust's lifetime system make it difficult to properly express + // the transformation from this specific lifetime to the for<'a> lifetime + render_fn: unsafe { std::mem::transmute(component) }, + }))), })) } diff --git a/packages/core/src/scopearena.rs b/packages/core/src/scopearena.rs deleted file mode 100644 index a08455895..000000000 --- a/packages/core/src/scopearena.rs +++ /dev/null @@ -1,355 +0,0 @@ -use bumpalo::Bump; -use futures_channel::mpsc::UnboundedSender; -use fxhash::{FxHashMap, FxHashSet}; -use slab::Slab; -use std::{ - borrow::Borrow, - cell::{Cell, RefCell}, -}; - -use crate::innerlude::*; - -pub(crate) type FcSlot = *const (); - -pub(crate) struct Heuristic { - hook_arena_size: usize, - node_arena_size: usize, -} - -// a slab-like arena with stable references even when new scopes are allocated -// uses a bump arena as a backing -// -// has an internal heuristics engine to pre-allocate arenas to the right size -pub(crate) struct ScopeArena { - pub pending_futures: RefCell>, - scope_counter: Cell, - pub(crate) sender: UnboundedSender, - bump: Bump, - - pub scopes: RefCell>, - pub heuristics: RefCell>, - free_scopes: RefCell>, - nodes: RefCell>>, -} - -impl ScopeArena { - pub(crate) fn new(sender: UnboundedSender) -> Self { - let bump = Bump::new(); - - // allocate a container for the root element - // this will *never* show up in the diffing process - let el = bump.alloc(VElement { - tag_name: "root", - namespace: None, - key: None, - dom_id: Cell::new(Some(ElementId(0))), - parent_id: Default::default(), - listeners: &[], - attributes: &[], - children: &[], - }); - - let node = bump.alloc(VNode::Element(el)); - let mut nodes = Slab::new(); - let root_id = nodes.insert(unsafe { std::mem::transmute(node as *const _) }); - - debug_assert_eq!(root_id, 0); - - Self { - scope_counter: Cell::new(0), - bump, - pending_futures: RefCell::new(FxHashSet::default()), - scopes: RefCell::new(FxHashMap::default()), - heuristics: RefCell::new(FxHashMap::default()), - free_scopes: RefCell::new(Vec::new()), - nodes: RefCell::new(nodes), - sender, - } - } - - /// Safety: - /// - Obtaining a mutable refernece to any Scope is unsafe - /// - Scopes use interior mutability when sharing data into components - pub(crate) fn get_scope(&self, id: ScopeId) -> Option<&ScopeState> { - unsafe { self.scopes.borrow().get(&id).map(|f| &**f) } - } - - pub(crate) fn get_scope_raw(&self, id: ScopeId) -> Option<*mut ScopeState> { - self.scopes.borrow().get(&id).copied() - } - - pub(crate) fn new_with_key( - &self, - fc_ptr: *const (), - caller: *const dyn Fn(&ScopeState) -> Element, - parent_scope: Option<*mut ScopeState>, - container: ElementId, - height: u32, - subtree: u32, - ) -> ScopeId { - let new_scope_id = ScopeId(self.scope_counter.get()); - self.scope_counter.set(self.scope_counter.get() + 1); - - /* - This scopearena aggressively reuse old scopes when possible. - We try to minimize the new allocations for props/arenas. - - However, this will probably lead to some sort of fragmentation. - I'm not exactly sure how to improve this today. - */ - match self.free_scopes.borrow_mut().pop() { - // No free scope, make a new scope - None => { - let (node_capacity, hook_capacity) = self - .heuristics - .borrow() - .get(&fc_ptr) - .map(|h| (h.node_arena_size, h.hook_arena_size)) - .unwrap_or_default(); - - let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; - - let scope = self.bump.alloc(ScopeState { - sender: self.sender.clone(), - container, - our_arena_idx: new_scope_id, - parent_scope, - height, - frames, - subtree: Cell::new(subtree), - is_subtree_root: Cell::new(false), - - caller: Cell::new(caller), - generation: 0.into(), - - shared_contexts: Default::default(), - - items: RefCell::new(SelfReferentialItems { - listeners: Default::default(), - borrowed_props: Default::default(), - tasks: Default::default(), - }), - - hook_arena: Bump::new(), - hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), - hook_idx: Default::default(), - }); - - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); - } - - // Reuse a free scope - Some(old_scope) => { - let scope = unsafe { &mut *old_scope }; - scope.caller.set(caller); - scope.parent_scope = parent_scope; - scope.height = height; - scope.subtree = Cell::new(subtree); - scope.our_arena_idx = new_scope_id; - scope.container = container; - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); - } - } - - new_scope_id - } - - // Removes a scope and its descendents from the arena - pub fn try_remove(&self, id: ScopeId) -> Option<()> { - self.ensure_drop_safety(id); - - // Safety: - // - ensure_drop_safety ensures that no references to this scope are in use - // - this raw pointer is removed from the map - let scope = unsafe { &mut *self.scopes.borrow_mut().remove(&id).unwrap() }; - scope.reset(); - - self.free_scopes.borrow_mut().push(scope); - - Some(()) - } - - pub fn reserve_node(&self, node: &VNode) -> ElementId { - let mut els = self.nodes.borrow_mut(); - let entry = els.vacant_entry(); - let key = entry.key(); - let id = ElementId(key); - let node: *const VNode = node as *const _; - let node = unsafe { std::mem::transmute::<*const VNode, *const VNode>(node) }; - entry.insert(node); - id - } - - pub fn update_node(&self, node: &VNode, id: ElementId) { - let node = unsafe { std::mem::transmute::<*const VNode, *const VNode>(node) }; - *self.nodes.borrow_mut().get_mut(id.0).unwrap() = node; - } - - pub fn collect_garbage(&self, id: ElementId) { - self.nodes.borrow_mut().remove(id.0); - } - - /// This method cleans up any references to data held within our hook list. This prevents mutable aliasing from - /// causing UB in our tree. - /// - /// This works by cleaning up our references from the bottom of the tree to the top. The directed graph of components - /// essentially forms a dependency tree that we can traverse from the bottom to the top. As we traverse, we remove - /// any possible references to the data in the hook list. - /// - /// References to hook data can only be stored in listeners and component props. During diffing, we make sure to log - /// all listeners and borrowed props so we can clear them here. - /// - /// This also makes sure that drop order is consistent and predictable. All resources that rely on being dropped will - /// be dropped. - pub(crate) fn ensure_drop_safety(&self, scope_id: ScopeId) { - if let Some(scope) = self.get_scope(scope_id) { - let mut items = scope.items.borrow_mut(); - - // make sure we drop all borrowed props manually to guarantee that their drop implementation is called before we - // run the hooks (which hold an &mut Reference) - // recursively call ensure_drop_safety on all children - items.borrowed_props.drain(..).for_each(|comp| { - let scope_id = comp - .associated_scope - .get() - .expect("VComponents should be associated with a valid Scope"); - - self.ensure_drop_safety(scope_id); - - let mut drop_props = comp.drop_props.borrow_mut().take().unwrap(); - drop_props(); - }); - - // Now that all the references are gone, we can safely drop our own references in our listeners. - items - .listeners - .drain(..) - .for_each(|listener| drop(listener.callback.callback.borrow_mut().take())); - } - } - - // pub(crate) fn run_scope(&self, id: ScopeId) -> bool { - - pub(crate) fn run_scope(&self, id: ScopeId) { - // Cycle to the next frame and then reset it - // This breaks any latent references, invalidating every pointer referencing into it. - // Remove all the outdated listeners - self.ensure_drop_safety(id); - - // todo: we *know* that this is aliased by the contents of the scope itself - let scope = unsafe { &mut *self.get_scope_raw(id).expect("could not find scope") }; - - // Safety: - // - We dropped the listeners, so no more &mut T can be used while these are held - // - All children nodes that rely on &mut T are replaced with a new reference - scope.hook_idx.set(0); - - // book keeping to ensure safety around the borrowed data - { - // Safety: - // - We've dropped all references to the wip bump frame with "ensure_drop_safety" - unsafe { scope.reset_wip_frame() }; - - let mut items = scope.items.borrow_mut(); - - // just forget about our suspended nodes while we're at it - items.tasks.clear(); - - // guarantee that we haven't screwed up - there should be no latent references anywhere - debug_assert!(items.listeners.is_empty()); - debug_assert!(items.borrowed_props.is_empty()); - debug_assert!(items.tasks.is_empty()); - } - - let render: &dyn Fn(&ScopeState) -> Element = unsafe { &*scope.caller.get() }; - - /* - If the component returns None, then we fill in a placeholder node. This will wipe what was there. - - An alternate approach is to leave the Real Dom the same, but that can lead to safety issues and a lot more checks. - */ - if let Some(node) = render(scope) { - if !scope.items.borrow().tasks.is_empty() { - self.pending_futures.borrow_mut().insert(id); - } - - let frame = scope.wip_frame(); - let node = frame.bump.alloc(node); - frame.node.set(unsafe { std::mem::transmute(node) }); - } else { - let frame = scope.wip_frame(); - let node = frame - .bump - .alloc(VNode::Placeholder(frame.bump.alloc(VPlaceholder { - dom_id: Default::default(), - }))); - frame.node.set(unsafe { std::mem::transmute(node) }); - } - - // make the "wip frame" contents the "finished frame" - // any future dipping into completed nodes after "render" will go through "fin head" - scope.cycle_frame(); - } - - pub fn call_listener_with_bubbling(&self, event: UserEvent, element: ElementId) { - let nodes = self.nodes.borrow(); - let mut cur_el = Some(element); - - while let Some(id) = cur_el.take() { - if let Some(el) = nodes.get(id.0) { - let real_el = unsafe { &**el }; - if let VNode::Element(real_el) = real_el { - // - for listener in real_el.listeners.borrow().iter() { - if listener.event == event.name { - let mut cb = listener.callback.callback.borrow_mut(); - if let Some(cb) = cb.as_mut() { - (cb)(event.data.clone()); - } - } - } - - cur_el = real_el.parent_id.get(); - } - } - } - } - - // The head of the bumpframe is the first linked NodeLink - pub fn wip_head(&self, id: ScopeId) -> &VNode { - let scope = self.get_scope(id).unwrap(); - let frame = scope.wip_frame(); - let node = unsafe { &*frame.node.get() }; - unsafe { std::mem::transmute::<&VNode, &VNode>(node) } - } - - // The head of the bumpframe is the first linked NodeLink - pub fn fin_head(&self, id: ScopeId) -> &VNode { - let scope = self.get_scope(id).unwrap(); - let frame = scope.fin_frame(); - let node = unsafe { &*frame.node.get() }; - unsafe { std::mem::transmute::<&VNode, &VNode>(node) } - } - - pub fn root_node(&self, id: ScopeId) -> &VNode { - self.fin_head(id) - } -} - -// when dropping the virtualdom, we need to make sure and drop everything important -impl Drop for ScopeArena { - fn drop(&mut self) { - for (_, scopeptr) in self.scopes.get_mut().drain() { - let scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; - drop(scope); - } - - // these are probably complete invalid unfortunately ? - for scopeptr in self.free_scopes.get_mut().drain(..) { - let scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; - drop(scope); - } - } -} diff --git a/packages/core/src/scope.rs b/packages/core/src/scopes.rs similarity index 55% rename from packages/core/src/scope.rs rename to packages/core/src/scopes.rs index 40bcd8aca..96c29c684 100644 --- a/packages/core/src/scope.rs +++ b/packages/core/src/scopes.rs @@ -1,8 +1,9 @@ -use crate::innerlude::*; - use futures_channel::mpsc::UnboundedSender; +use fxhash::{FxHashMap, FxHashSet}; +use slab::Slab; use std::{ any::{Any, TypeId}, + borrow::Borrow, cell::{Cell, RefCell}, collections::HashMap, future::Future, @@ -10,8 +11,351 @@ use std::{ rc::Rc, }; +use crate::{innerlude::*, unsafe_utils::extend_vnode}; use bumpalo::{boxed::Box as BumpBox, Bump}; +pub(crate) type FcSlot = *const (); + +pub(crate) struct Heuristic { + hook_arena_size: usize, + node_arena_size: usize, +} + +// a slab-like arena with stable references even when new scopes are allocated +// uses a bump arena as a backing +// +// has an internal heuristics engine to pre-allocate arenas to the right size +pub(crate) struct ScopeArena { + pub pending_futures: RefCell>, + pub scope_counter: Cell, + pub sender: UnboundedSender, + pub bump: Bump, + pub scopes: RefCell>, + pub heuristics: RefCell>, + pub free_scopes: RefCell>, + pub nodes: RefCell>>, +} + +impl ScopeArena { + pub(crate) fn new(sender: UnboundedSender) -> Self { + let bump = Bump::new(); + + // allocate a container for the root element + // this will *never* show up in the diffing process + // todo: figure out why this is necessary. i forgot. whoops. + let el = bump.alloc(VElement { + tag_name: "root", + namespace: None, + key: None, + dom_id: Cell::new(Some(ElementId(0))), + parent_id: Default::default(), + listeners: &[], + attributes: &[], + children: &[], + }); + + let node = bump.alloc(VNode::Element(el)); + let mut nodes = Slab::new(); + let root_id = nodes.insert(unsafe { std::mem::transmute(node as *const _) }); + + debug_assert_eq!(root_id, 0); + + Self { + scope_counter: Cell::new(0), + bump, + pending_futures: RefCell::new(FxHashSet::default()), + scopes: RefCell::new(FxHashMap::default()), + heuristics: RefCell::new(FxHashMap::default()), + free_scopes: RefCell::new(Vec::new()), + nodes: RefCell::new(nodes), + sender, + } + } + + /// Safety: + /// - Obtaining a mutable refernece to any Scope is unsafe + /// - Scopes use interior mutability when sharing data into components + pub(crate) fn get_scope(&self, id: ScopeId) -> Option<&ScopeState> { + unsafe { self.scopes.borrow().get(&id).map(|f| &**f) } + } + + pub(crate) fn get_scope_raw(&self, id: ScopeId) -> Option<*mut ScopeState> { + self.scopes.borrow().get(&id).copied() + } + + pub(crate) fn new_with_key( + &self, + fc_ptr: *const (), + vcomp: Box, + parent_scope: Option, + container: ElementId, + subtree: u32, + ) -> ScopeId { + let new_scope_id = ScopeId(self.scope_counter.get()); + self.scope_counter.set(self.scope_counter.get() + 1); + let parent_scope = parent_scope.map(|f| self.get_scope_raw(f)).flatten(); + let height = parent_scope + .as_ref() + .map(|f| unsafe { &**f }) + .map(|f| f.height + 1) + .unwrap_or_default(); + + /* + This scopearena aggressively reuse old scopes when possible. + We try to minimize the new allocations for props/arenas. + + However, this will probably lead to some sort of fragmentation. + I'm not exactly sure how to improve this today. + */ + match self.free_scopes.borrow_mut().pop() { + // No free scope, make a new scope + None => { + let (node_capacity, hook_capacity) = self + .heuristics + .borrow() + .get(&fc_ptr) + .map(|h| (h.node_arena_size, h.hook_arena_size)) + .unwrap_or_default(); + + let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; + + let scope = self.bump.alloc(ScopeState { + sender: self.sender.clone(), + container, + our_arena_idx: new_scope_id, + parent_scope, + height, + frames, + subtree: Cell::new(subtree), + is_subtree_root: Cell::new(false), + + props: vcomp, + generation: 0.into(), + + shared_contexts: Default::default(), + + items: RefCell::new(SelfReferentialItems { + listeners: Default::default(), + borrowed_props: Default::default(), + tasks: Default::default(), + }), + + hook_arena: Bump::new(), + hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), + hook_idx: Default::default(), + }); + + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); + debug_assert!(any_item.is_none()); + } + + // Reuse a free scope + Some(old_scope) => { + let scope = unsafe { &mut *old_scope }; + scope.props.set(vcomp); + scope.parent_scope = parent_scope; + scope.height = height; + scope.subtree = Cell::new(subtree); + scope.our_arena_idx = new_scope_id; + scope.container = container; + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); + debug_assert!(any_item.is_none()); + } + } + + new_scope_id + } + + // Removes a scope and its descendents from the arena + pub fn try_remove(&self, id: ScopeId) -> Option<()> { + self.ensure_drop_safety(id); + + // Safety: + // - ensure_drop_safety ensures that no references to this scope are in use + // - this raw pointer is removed from the map + let scope = unsafe { &mut *self.scopes.borrow_mut().remove(&id).unwrap() }; + scope.reset(); + + todo!("drop"); + // unsafe { &*scope.render.get() }.release(); + + self.free_scopes.borrow_mut().push(scope); + + Some(()) + } + + pub fn reserve_node<'a>(&self, node: &'a VNode<'a>) -> ElementId { + let mut els = self.nodes.borrow_mut(); + let entry = els.vacant_entry(); + let key = entry.key(); + let id = ElementId(key); + let node = unsafe { extend_vnode(node) }; + entry.insert(node as *const _); + id + } + + pub fn update_node<'a>(&self, node: &'a VNode<'a>, id: ElementId) { + let node = unsafe { extend_vnode(node) }; + *self.nodes.borrow_mut().get_mut(id.0).unwrap() = node; + } + + pub fn collect_garbage(&self, id: ElementId) { + self.nodes.borrow_mut().remove(id.0); + } + + /// This method cleans up any references to data held within our hook list. This prevents mutable aliasing from + /// causing UB in our tree. + /// + /// This works by cleaning up our references from the bottom of the tree to the top. The directed graph of components + /// essentially forms a dependency tree that we can traverse from the bottom to the top. As we traverse, we remove + /// any possible references to the data in the hook list. + /// + /// References to hook data can only be stored in listeners and component props. During diffing, we make sure to log + /// all listeners and borrowed props so we can clear them here. + /// + /// This also makes sure that drop order is consistent and predictable. All resources that rely on being dropped will + /// be dropped. + pub(crate) fn ensure_drop_safety(&self, scope_id: ScopeId) { + if let Some(scope) = self.get_scope(scope_id) { + let mut items = scope.items.borrow_mut(); + + // make sure we drop all borrowed props manually to guarantee that their drop implementation is called before we + // run the hooks (which hold an &mut Reference) + // recursively call ensure_drop_safety on all children + items.borrowed_props.drain(..).for_each(|comp| { + let scope_id = comp + .scope + .get() + .expect("VComponents should be associated with a valid Scope"); + + self.ensure_drop_safety(scope_id); + + let g = comp.props.take(); + // comp.props.release(); + }); + + // Now that all the references are gone, we can safely drop our own references in our listeners. + items + .listeners + .drain(..) + .for_each(|listener| drop(listener.callback.callback.borrow_mut().take())); + } + } + + pub(crate) fn run_scope(&self, id: ScopeId) { + // Cycle to the next frame and then reset it + // This breaks any latent references, invalidating every pointer referencing into it. + // Remove all the outdated listeners + self.ensure_drop_safety(id); + + // todo: we *know* that this is aliased by the contents of the scope itself + let scope = unsafe { &mut *self.get_scope_raw(id).expect("could not find scope") }; + + // Safety: + // - We dropped the listeners, so no more &mut T can be used while these are held + // - All children nodes that rely on &mut T are replaced with a new reference + scope.hook_idx.set(0); + + // book keeping to ensure safety around the borrowed data + { + // Safety: + // - We've dropped all references to the wip bump frame with "ensure_drop_safety" + unsafe { scope.reset_wip_frame() }; + + let mut items = scope.items.borrow_mut(); + + // just forget about our suspended nodes while we're at it + items.tasks.clear(); + + // guarantee that we haven't screwed up - there should be no latent references anywhere + debug_assert!(items.listeners.is_empty()); + debug_assert!(items.borrowed_props.is_empty()); + debug_assert!(items.tasks.is_empty()); + } + + // safety: this is definitely not dropped + + /* + If the component returns None, then we fill in a placeholder node. This will wipe what was there. + An alternate approach is to leave the Real Dom the same, but that can lead to safety issues and a lot more checks. + + Instead, we just treat the `None` as a shortcut to placeholder. + If the developer wants to prevent a scope from updating, they should control its memoization instead. + + Also, the way we implement hooks allows us to cut rendering short before the next hook is recalled. + I'm not sure if React lets you abort the component early, but we let you do that. + */ + if let Some(node) = scope.props.render(scope) { + if !scope.items.borrow().tasks.is_empty() { + self.pending_futures.borrow_mut().insert(id); + } + + let frame = scope.wip_frame(); + let node = frame.bump.alloc(node); + frame.node.set(unsafe { extend_vnode(node) }); + } else { + let frame = scope.wip_frame(); + let node = frame + .bump + .alloc(VNode::Placeholder(frame.bump.alloc(VPlaceholder { + dom_id: Default::default(), + }))); + frame.node.set(unsafe { extend_vnode(node) }); + } + + // make the "wip frame" contents the "finished frame" + // any future dipping into completed nodes after "render" will go through "fin head" + scope.cycle_frame(); + } + + pub fn call_listener_with_bubbling(&self, event: UserEvent, element: ElementId) { + let nodes = self.nodes.borrow(); + let mut cur_el = Some(element); + + while let Some(id) = cur_el.take() { + if let Some(el) = nodes.get(id.0) { + let real_el = unsafe { &**el }; + if let VNode::Element(real_el) = real_el { + for listener in real_el.listeners.borrow().iter() { + if listener.event == event.name { + let mut cb = listener.callback.callback.borrow_mut(); + if let Some(cb) = cb.as_mut() { + // todo: arcs are pretty heavy to clone + // we really want to convert arc to rc + // unfortunately, the SchedulerMsg must be send/sync to be sent across threads + // we could convert arc to rc internally or something + (cb)(event.data.clone()); + } + } + } + + cur_el = real_el.parent_id.get(); + } + } + } + } + + // The head of the bumpframe is the first linked NodeLink + pub fn wip_head(&self, id: ScopeId) -> &VNode { + let scope = self.get_scope(id).unwrap(); + let frame = scope.wip_frame(); + let node = unsafe { &*frame.node.get() }; + unsafe { extend_vnode(node) } + } + + // The head of the bumpframe is the first linked NodeLink + pub fn fin_head(&self, id: ScopeId) -> &VNode { + let scope = self.get_scope(id).unwrap(); + let frame = scope.fin_frame(); + let node = unsafe { &*frame.node.get() }; + unsafe { extend_vnode(node) } + } + + pub fn root_node(&self, id: ScopeId) -> &VNode { + self.fin_head(id) + } +} + /// Components in Dioxus use the "Context" object to interact with their lifecycle. /// /// This lets components access props, schedule updates, integrate hooks, and expose shared state. @@ -72,34 +416,29 @@ pub struct ScopeId(pub usize); /// use case they might have. pub struct ScopeState { pub(crate) parent_scope: Option<*mut ScopeState>, - pub(crate) container: ElementId, - pub(crate) our_arena_idx: ScopeId, - pub(crate) height: u32, + pub(crate) sender: UnboundedSender, + // todo: subtrees + pub(crate) is_subtree_root: Cell, pub(crate) subtree: Cell, - pub(crate) is_subtree_root: Cell, - - pub(crate) generation: Cell, + pub(crate) props: Option>>, + // nodes, items pub(crate) frames: [BumpFrame; 2], - - pub(crate) caller: Cell<*const dyn Fn(&ScopeState) -> Element>, - + pub(crate) generation: Cell, pub(crate) items: RefCell>, + // hooks pub(crate) hook_arena: Bump, - pub(crate) hook_vals: RefCell>, - pub(crate) hook_idx: Cell, + // shared state -> todo: move this out of scopestate pub(crate) shared_contexts: RefCell>>, - - pub(crate) sender: UnboundedSender, } pub struct SelfReferentialItems<'a> { @@ -143,7 +482,6 @@ impl ScopeState { /// /// ```rust, ignore /// fn App(cx: Scope<()>) -> Element { - /// todo!(); /// rsx!(cx, div { "Subtree {id}"}) /// }; /// ``` @@ -251,7 +589,7 @@ impl ScopeState { /// Get the Root Node of this scope pub fn root_node(&self) -> &VNode { - let node = unsafe { &*self.wip_frame().node.get() }; + let node = unsafe { &*self.fin_frame().node.get() }; unsafe { std::mem::transmute(node) } } @@ -465,45 +803,37 @@ impl ScopeState { &self.wip_frame().bump } - pub(crate) fn drop_hooks(&mut self) { - self.hook_vals.get_mut().drain(..).for_each(|state| { - let as_mut = unsafe { &mut *state }; - let boxed = unsafe { bumpalo::boxed::Box::from_raw(as_mut) }; - drop(boxed); - }); - } - + // todo: disable bookkeeping on drop (unncessary) pub(crate) fn reset(&mut self) { - // we're just reusing scopes so we need to clear it out - self.drop_hooks(); - + // first: book keaping self.hook_idx.set(0); - - self.hook_arena.reset(); - self.shared_contexts.get_mut().clear(); self.parent_scope = None; self.generation.set(0); self.is_subtree_root.set(false); self.subtree.set(0); - self.frames[0].reset(); - self.frames[1].reset(); + // next: shared context data + self.shared_contexts.get_mut().clear(); + // next: reset the node data let SelfReferentialItems { borrowed_props, listeners, tasks, } = self.items.get_mut(); - borrowed_props.clear(); listeners.clear(); tasks.clear(); - } -} + self.frames[0].reset(); + self.frames[1].reset(); -impl Drop for ScopeState { - fn drop(&mut self) { - self.drop_hooks(); + // Finally, free up the hook values + self.hook_arena.reset(); + self.hook_vals.get_mut().drain(..).for_each(|state| { + let as_mut = unsafe { &mut *state }; + let boxed = unsafe { bumpalo::boxed::Box::from_raw(as_mut) }; + drop(boxed); + }); } } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 13551d337..55902831c 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -8,7 +8,10 @@ use futures_util::{Future, StreamExt}; use fxhash::FxHashSet; use indexmap::IndexSet; use smallvec::SmallVec; -use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; +use std::{ + any::Any, cell::RefCell, collections::VecDeque, iter::FromIterator, pin::Pin, sync::Arc, + task::Poll, +}; /// A virtual node s ystem that progresses user events and diffs UI trees. /// @@ -103,10 +106,8 @@ use std::{any::Any, collections::VecDeque, pin::Pin, sync::Arc, task::Poll}; /// } /// ``` pub struct VirtualDom { - scopes: Box, + scopes: ScopeArena, - // should always be ScopeId(0) - base_scope: ScopeId, pending_messages: VecDeque, dirty_scopes: IndexSet, @@ -114,8 +115,6 @@ pub struct VirtualDom { UnboundedSender, UnboundedReceiver, ), - - caller: *mut dyn for<'r> Fn(&'r ScopeState) -> Element<'r>, } // Methods to create the VirtualDom @@ -174,7 +173,10 @@ impl VirtualDom { /// let mut dom = VirtualDom::new_with_props(Example, SomeProps { name: "jane" }); /// let mutations = dom.rebuild(); /// ``` - pub fn new_with_props(root: Component

, root_props: P) -> Self { + pub fn new_with_props

(root: Component

, root_props: P) -> Self + where + P: 'static, + { Self::new_with_props_and_scheduler( root, root_props, @@ -188,8 +190,8 @@ impl VirtualDom { /// VirtualDom to be created just to retrieve its channel receiver. /// /// ```rust - /// let (sender, receiver) = futures_channel::mpsc::unbounded(); - /// let dom = VirtualDom::new_with_scheduler(Example, (), sender, receiver); + /// let channel = futures_channel::mpsc::unbounded(); + /// let dom = VirtualDom::new_with_scheduler(Example, (), channel); /// ``` pub fn new_with_props_and_scheduler( root: Component

, @@ -199,36 +201,25 @@ impl VirtualDom { UnboundedReceiver, ), ) -> Self { - // move these two things onto the heap so we have stable ptrs even if the VirtualDom itself is moved - let scopes = Box::new(ScopeArena::new(channel.0.clone())); - let root_props = Box::new(root_props); + let scopes = ScopeArena::new(channel.0.clone()); - // effectively leak the caller so we can drop it when the VirtualDom is dropped - let caller: *mut dyn for<'r> Fn(&'r ScopeState) -> Element<'r> = - Box::into_raw(Box::new(move |scope: &ScopeState| -> Element { - // Safety: The props at this pointer can never be moved. - // Also, this closure will never be ran when the VirtualDom is destroyed. - // This is where the root lifetime of the VirtualDom originates. - let props = unsafe { std::mem::transmute::<&P, &P>(root_props.as_ref()) }; - let scp = Scope { scope, props }; - root(scp) - })); - - let base_scope = scopes.new_with_key(root as _, caller, None, ElementId(0), 0, 0); - - let mut dirty_scopes = IndexSet::new(); - dirty_scopes.insert(base_scope); - - // todo: add a pending message to the scheduler to start the scheduler? - let pending_messages = VecDeque::new(); + scopes.new_with_key( + root as *const _, + Box::new(VComponentProps { + props: root_props, + memo: |_a, _b| false, + render_fn: unsafe { std::mem::transmute(root) }, + }), + None, + ElementId(0), + 0, + ); Self { scopes, - base_scope, - caller, - pending_messages, - dirty_scopes, channel, + dirty_scopes: IndexSet::from_iter([ScopeId(0)]), + pending_messages: VecDeque::new(), } } @@ -237,9 +228,18 @@ impl VirtualDom { /// This is useful for traversing the tree from the root for heuristics or alternsative renderers that use Dioxus /// directly. /// + /// This method is equivalent to calling `get_scope(ScopeId(0))` + /// /// # Example + /// + /// ```rust + /// let mut dom = VirtualDom::new(example); + /// dom.rebuild(); + /// + /// + /// ``` pub fn base_scope(&self) -> &ScopeState { - self.get_scope(self.base_scope).unwrap() + self.get_scope(ScopeId(0)).unwrap() } /// Get the [`ScopeState`] for a component given its [`ScopeId`] @@ -248,7 +248,7 @@ impl VirtualDom { /// /// /// - pub fn get_scope<'a>(&'a self, id: ScopeId) -> Option<&'a ScopeState> { + pub fn get_scope(&self, id: ScopeId) -> Option<&ScopeState> { self.scopes.get_scope(id) } @@ -260,7 +260,7 @@ impl VirtualDom { /// let dom = VirtualDom::new(App); /// let sender = dom.get_scheduler_channel(); /// ``` - pub fn get_scheduler_channel(&self) -> futures_channel::mpsc::UnboundedSender { + pub fn get_scheduler_channel(&self) -> UnboundedSender { self.channel.0.clone() } @@ -273,10 +273,12 @@ impl VirtualDom { /// # Example /// ```rust, ignore /// let dom = VirtualDom::new(App); - /// dom.insert_scheduler_message(SchedulerMsg::Immediate(ScopeId(0))); + /// dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); /// ``` - pub fn insert_scheduler_message(&self, msg: SchedulerMsg) { - self.channel.0.unbounded_send(msg).unwrap() + pub fn handle_message(&mut self, msg: SchedulerMsg) { + if self.channel.0.unbounded_send(msg).is_ok() { + self.process_all_messages(); + } } /// Check if the [`VirtualDom`] has any pending updates or work to be done. @@ -326,26 +328,38 @@ impl VirtualDom { } } - while let Ok(Some(msg)) = self.channel.1.try_next() { - self.pending_messages.push_front(msg); - } + // Move all the messages into the queue + self.process_all_messages(); + } + } - if let Some(msg) = self.pending_messages.pop_back() { - match msg { - // just keep looping, the task is now saved but we should actually poll it - SchedulerMsg::NewTask(id) => { - self.scopes.pending_futures.borrow_mut().insert(id); - } - SchedulerMsg::UiEvent(event) => { - if let Some(element) = event.element { - self.scopes.call_listener_with_bubbling(event, element); - } - } - SchedulerMsg::Immediate(s) => { - self.dirty_scopes.insert(s); - } + /// Manually kick the VirtualDom to process any + pub fn process_all_messages(&mut self) { + // clear out the scheduler queue + while let Ok(Some(msg)) = self.channel.1.try_next() { + self.pending_messages.push_front(msg); + } + + // process all the messages pulled from the queue + while let Some(msg) = self.pending_messages.pop_back() { + self.process_message(msg); + } + } + + pub fn process_message(&mut self, msg: SchedulerMsg) { + match msg { + // just keep looping, the task is now saved but we should actually poll it + SchedulerMsg::NewTask(id) => { + self.scopes.pending_futures.borrow_mut().insert(id); + } + SchedulerMsg::UiEvent(event) => { + if let Some(element) = event.element { + self.scopes.call_listener_with_bubbling(event, element); } } + SchedulerMsg::Immediate(s) => { + self.dirty_scopes.insert(s); + } } } @@ -466,7 +480,7 @@ impl VirtualDom { /// apply_edits(edits); /// ``` pub fn rebuild(&mut self) -> Mutations { - let scope_id = self.base_scope; + let scope_id = ScopeId(0); let mut diff_state = DiffState::new(&self.scopes); self.scopes.run_scope(scope_id); @@ -510,12 +524,24 @@ impl VirtualDom { /// /// let edits = dom.diff(); /// ``` - pub fn hard_diff<'a>(&'a mut self, scope_id: ScopeId) -> Option> { + pub fn hard_diff(&mut self, scope_id: ScopeId) -> Mutations { let mut diff_machine = DiffState::new(&self.scopes); self.scopes.run_scope(scope_id); + + let (old, new) = ( + diff_machine.scopes.wip_head(scope_id), + diff_machine.scopes.fin_head(scope_id), + ); + diff_machine.force_diff = true; - diff_machine.diff_scope(scope_id); - Some(diff_machine.mutations) + diff_machine.stack.push(DiffInstruction::Diff { old, new }); + diff_machine.stack.scope_stack.push(scope_id); + + let scope = diff_machine.scopes.get_scope(scope_id).unwrap(); + diff_machine.stack.element_stack.push(scope.container); + diff_machine.work(|| false); + + diff_machine.mutations } /// Renders an `rsx` call into the Base Scope's allocator. @@ -531,7 +557,7 @@ impl VirtualDom { /// let nodes = dom.render_nodes(rsx!("div")); /// ``` pub fn render_vnodes<'a>(&'a self, lazy_nodes: Option>) -> &'a VNode<'a> { - let scope = self.scopes.get_scope(self.base_scope).unwrap(); + let scope = self.scopes.get_scope(ScopeId(0)).unwrap(); let frame = scope.wip_frame(); let factory = NodeFactory { bump: &frame.bump }; let node = lazy_nodes.unwrap().call(factory); @@ -554,7 +580,7 @@ impl VirtualDom { let mut machine = DiffState::new(&self.scopes); machine.stack.push(DiffInstruction::Diff { new, old }); machine.stack.element_stack.push(ElementId(0)); - machine.stack.scope_stack.push(self.base_scope); + machine.stack.scope_stack.push(ScopeId(0)); machine.work(|| false); machine.mutations } @@ -572,11 +598,12 @@ impl VirtualDom { /// let dom = VirtualDom::new(Base); /// let nodes = dom.render_nodes(rsx!("div")); /// ``` - pub fn create_vnodes<'a>(&'a self, left: Option>) -> Mutations<'a> { - let nodes = self.render_vnodes(left); + pub fn create_vnodes<'a>(&'a self, nodes: Option>) -> Mutations<'a> { let mut machine = DiffState::new(&self.scopes); machine.stack.element_stack.push(ElementId(0)); - machine.stack.create_node(nodes, MountType::Append); + machine + .stack + .create_node(self.render_vnodes(nodes), MountType::Append); machine.work(|| false); machine.mutations } @@ -602,29 +629,78 @@ impl VirtualDom { let (old, new) = (self.render_vnodes(left), self.render_vnodes(right)); let mut create = DiffState::new(&self.scopes); - create.stack.scope_stack.push(self.base_scope); + create.stack.scope_stack.push(ScopeId(0)); create.stack.element_stack.push(ElementId(0)); create.stack.create_node(old, MountType::Append); create.work(|| false); let mut edit = DiffState::new(&self.scopes); - edit.stack.scope_stack.push(self.base_scope); + edit.stack.scope_stack.push(ScopeId(0)); edit.stack.element_stack.push(ElementId(0)); edit.stack.push(DiffInstruction::Diff { old, new }); - edit.work(&mut || false); + edit.work(|| false); (create.mutations, edit.mutations) } } +/* +Scopes and ScopeArenas are never dropped internally. +An app will always occupy as much memory as its biggest form. +This means we need to handle all specifics of drop *here*. It's easier +to reason about centralizing all the drop logic in one spot rather than scattered in each module. + +Broadly speaking, we want to use the remove_nodes method to clean up *everything* +This will drop listeners, borrowed props, and hooks for all components. +We need to do this in the correct order - nodes at the very bottom must be dropped first to release +the borrow chain. + +Once the contents of the tree have been cleaned up, we can finally clean up the +memory used by ScopeState itself. + +questions: +should we build a vcomponent for the root? +- probably - yes? +- store the vcomponent in the root dom + +- 1: Use remove_nodes to use the ensure_drop_safety pathway to safely drop the tree +- 2: Drop the ScopeState itself + + + +*/ impl Drop for VirtualDom { fn drop(&mut self) { - // ensure the root component's caller is dropped - let caller = unsafe { Box::from_raw(self.caller) }; - drop(caller); + // the best way to drop the dom is to replace the root scope with a dud + // the diff infrastructure will then finish the rest + let scope = self.scopes.get_scope(ScopeId(0)).unwrap(); - // destroy the root component (which will destroy all of the other scopes) - // load the root node and descend + // todo: move the remove nodes method onto scopearena + // this will clear *all* scopes *except* the root scope + let mut machine = DiffState::new(&self.scopes); + machine.remove_nodes([scope.root_node()], false); + + // Now, clean up the root scope + // safety: there are no more references to the root scope + let scope = unsafe { &mut *self.scopes.get_scope_raw(ScopeId(0)).unwrap() }; + scope.reset(); + + // make sure there are no "live" components + for (_, scopeptr) in self.scopes.scopes.get_mut().drain() { + let scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; + drop(scope); + } + + for scopeptr in self.scopes.free_scopes.get_mut().drain(..) { + let mut scope = unsafe { bumpalo::boxed::Box::from_raw(scopeptr) }; + scope.reset(); + drop(scope); + } + + // Safety: all refereneces to this caller have been dropped + // the pointer is still valid since we haven't moved it since the virtualdom was created + // ensure the root component's caller is dropped + // drop(unsafe { Box::from_raw(self.caller) }); } } diff --git a/packages/core/tests/diffing.rs b/packages/core/tests/diffing.rs index b8c86e615..2125cb9fb 100644 --- a/packages/core/tests/diffing.rs +++ b/packages/core/tests/diffing.rs @@ -15,7 +15,7 @@ mod test_logging; fn new_dom() -> VirtualDom { const IS_LOGGING_ENABLED: bool = false; test_logging::set_up_logging(IS_LOGGING_ENABLED); - VirtualDom::new(|cx| todo!()) + VirtualDom::new(|cx| rsx!(cx, "hi")) } use DomEdit::*; diff --git a/packages/core/tests/earlyabort.rs b/packages/core/tests/earlyabort.rs index 713fe7ccb..ef77beac9 100644 --- a/packages/core/tests/earlyabort.rs +++ b/packages/core/tests/earlyabort.rs @@ -55,13 +55,13 @@ fn test_early_abort() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [CreatePlaceholder { root: 3 }, ReplaceWith { root: 1, m: 1 },], ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ diff --git a/packages/core/tests/lifecycle.rs b/packages/core/tests/lifecycle.rs index 2da5b5bd6..798870250 100644 --- a/packages/core/tests/lifecycle.rs +++ b/packages/core/tests/lifecycle.rs @@ -46,13 +46,13 @@ fn manual_diffing() { #[test] fn events_generate() { static App: Component<()> = |cx| { - let mut count = use_state(&cx, || 0); + let count = cx.use_hook(|_| 0, |f| f); let inner = match *count { 0 => { rsx! { div { - onclick: move |_| count += 1, + onclick: move |_| *count += 1, div { "nested" } @@ -105,8 +105,8 @@ fn events_generate() { #[test] fn components_generate() { static App: Component<()> = |cx| { - let mut render_phase = use_state(&cx, || 0); - render_phase += 1; + let render_phase = cx.use_hook(|_| 0, |f| f); + *render_phase += 1; cx.render(match *render_phase { 0 => rsx!("Text0"), @@ -140,7 +140,7 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -152,7 +152,7 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -164,7 +164,7 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -173,13 +173,13 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [CreatePlaceholder { root: 5 }, ReplaceWith { root: 4, m: 1 },] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -191,7 +191,7 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -207,7 +207,7 @@ fn components_generate() { ] ); - let edits = dom.hard_diff(ScopeId(0)).unwrap(); + let edits = dom.hard_diff(ScopeId(0)); assert_eq!( edits.edits, [ @@ -222,8 +222,8 @@ fn components_generate() { fn component_swap() { // simple_logger::init(); static App: Component<()> = |cx| { - let mut render_phase = use_state(&cx, || 0); - render_phase += 1; + let render_phase = cx.use_hook(|_| 0, |f| f); + *render_phase += 1; cx.render(match *render_phase { 0 => rsx!( diff --git a/packages/core/tests/miri_stress.rs b/packages/core/tests/miri_stress.rs index c51daa536..2091a8e3a 100644 --- a/packages/core/tests/miri_stress.rs +++ b/packages/core/tests/miri_stress.rs @@ -9,7 +9,7 @@ Specifically: - [ ] Async isn't busted */ -use dioxus::{prelude::*, DomEdit, ScopeId}; +use dioxus::{prelude::*, DomEdit, SchedulerMsg, ScopeId}; use dioxus_core as dioxus; use dioxus_core_macro::*; use dioxus_html as dioxus_elements; @@ -55,12 +55,6 @@ fn test_memory_leak() { )) } - fn child(cx: Scope<()>) -> Element { - rsx!(cx, div { - "goodbye world" - }) - } - #[derive(Props)] struct BorrowedProps<'a> { na: &'a str, @@ -74,6 +68,10 @@ fn test_memory_leak() { }) } + fn child(cx: Scope<()>) -> Element { + rsx!(cx, div { "goodbye world" }) + } + let mut dom = new_dom(app, ()); dom.rebuild(); @@ -100,7 +98,7 @@ fn memo_works_properly() { cx.render(rsx!( div { "Hello, world!" } - child(na: "asd".to_string()) + child(na: "asdfg".to_string()) )) } @@ -110,9 +108,7 @@ fn memo_works_properly() { } fn child(cx: Scope) -> Element { - rsx!(cx, div { - "goodbye world" - }) + rsx!(cx, div { "goodbye world" }) } let mut dom = new_dom(app, ()); @@ -123,4 +119,170 @@ fn memo_works_properly() { dom.hard_diff(ScopeId(0)); dom.hard_diff(ScopeId(0)); dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); + dom.hard_diff(ScopeId(0)); +} + +#[test] +fn free_works_on_root_props() { + fn app(cx: Scope) -> Element { + cx.render(rsx! { + child(a: "alpha") + child(a: "beta") + child(a: "gamma") + child(a: "delta") + }) + } + + #[derive(Props, PartialEq)] + struct ChildProps { + a: &'static str, + } + + fn child(cx: Scope) -> Element { + rsx!(cx, "child {cx.props.a}") + } + + struct Custom { + val: String, + } + + impl Drop for Custom { + fn drop(&mut self) { + dbg!("dropped!"); + } + } + + let mut dom = new_dom( + app, + Custom { + val: String::from("asd"), + }, + ); + dom.rebuild(); +} + +#[test] +fn free_works_on_borrowed() { + fn app(cx: Scope<()>) -> Element { + cx.render(rsx! { + child(a: "alpha", b: "asd".to_string()) + }) + } + #[derive(Props)] + struct ChildProps<'a> { + a: &'a str, + b: String, + } + + fn child<'a>(cx: Scope<'a, ChildProps<'a>>) -> Element { + dbg!("rendering child"); + rsx!(cx, "child {cx.props.a}, {cx.props.b}") + } + + impl Drop for ChildProps<'_> { + fn drop(&mut self) { + dbg!("dropped child!"); + } + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); +} + +#[test] +fn free_works_on_root_hooks() { + /* + On Drop, scopearena drops all the hook contents. + */ + + struct Droppable(T); + impl Drop for Droppable { + fn drop(&mut self) { + dbg!("dropping droppable"); + } + } + + fn app(cx: Scope<()>) -> Element { + let name = cx.use_hook(|_| Droppable(String::from("asd")), |f| f); + rsx!(cx, div { "{name.0}" }) + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); +} + +#[test] +fn old_props_arent_stale() { + fn app(cx: Scope<()>) -> Element { + dbg!("rendering parent"); + let cnt = cx.use_hook(|_| 0, |f| f); + *cnt += 1; + + if *cnt == 1 { + rsx!(cx, div { child(a: "abcdef".to_string()) }) + } else { + rsx!(cx, div { child(a: "abcdef".to_string()) }) + } + } + + #[derive(Props, PartialEq)] + struct ChildProps { + a: String, + } + fn child(cx: Scope) -> Element { + dbg!("rendering child", &cx.props.a); + rsx!(cx, div { "child {cx.props.a}" }) + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dbg!("forcing update to child"); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(1))); + dom.work_with_deadline(|| false); +} + +#[test] +fn basic() { + fn app(cx: Scope<()>) -> Element { + rsx!(cx, div { + child(a: "abcdef".to_string()) + }) + } + + #[derive(Props, PartialEq)] + struct ChildProps { + a: String, + } + + fn child(cx: Scope) -> Element { + dbg!("rendering child", &cx.props.a); + rsx!(cx, div { "child {cx.props.a}" }) + } + + let mut dom = new_dom(app, ()); + let _ = dom.rebuild(); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); + + dom.handle_message(SchedulerMsg::Immediate(ScopeId(0))); + dom.work_with_deadline(|| false); } diff --git a/packages/ssr/src/lib.rs b/packages/ssr/src/lib.rs index a29a7db1c..e0f5a91bb 100644 --- a/packages/ssr/src/lib.rs +++ b/packages/ssr/src/lib.rs @@ -244,7 +244,7 @@ impl<'a> TextRenderer<'a, '_> { } } VNode::Component(vcomp) => { - let idx = vcomp.associated_scope.get().unwrap(); + let idx = vcomp.scope.get().unwrap(); if let (Some(vdom), false) = (self.vdom, self.cfg.skip_components) { let new_node = vdom.get_scope(idx).unwrap().root_node(); From 78d9056e35deb6e87ea2929cc5369e35b8a20847 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Mon, 20 Dec 2021 23:31:33 -0500 Subject: [PATCH 2/2] feat: it works with a new bump each time!! --- packages/core/src/diff.rs | 71 +++++++-------- packages/core/src/nodes.rs | 4 +- packages/core/src/scopes.rs | 148 ++++++++++++++++++------------- packages/core/src/virtual_dom.rs | 9 +- 4 files changed, 121 insertions(+), 111 deletions(-) diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 9603f1c0f..886195a86 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -88,6 +88,8 @@ //! More info on how to improve this diffing algorithm: //! - https://hacks.mozilla.org/2019/03/fast-bump-allocated-virtual-doms-with-rust-and-wasm/ +use std::borrow::BorrowMut; + use crate::innerlude::*; use fxhash::{FxHashMap, FxHashSet}; use smallvec::{smallvec, SmallVec}; @@ -427,10 +429,11 @@ impl<'bump> DiffState<'bump> { let parent_idx = self.stack.current_scope().unwrap(); // Insert a new scope into our component list - let caller = unsafe { std::mem::transmute(vcomponent.props as *const _) }; + let props: Box = vcomponent.props.borrow_mut().take().unwrap(); + let props: Box = unsafe { std::mem::transmute(props) }; let new_idx = self.scopes.new_with_key( vcomponent.user_fc, - caller, + props, Some(parent_idx), self.stack.element_stack.last().copied().unwrap(), 0, @@ -441,9 +444,7 @@ impl<'bump> DiffState<'bump> { match vcomponent.can_memoize { true => { - // promote the props onto the heap - // when we diff these nodes, we'll want to move it over - vcomponent.props.promote(); + // todo: implement promotion logic. save us from boxing props that we don't need } false => { // track this component internally so we know the right drop order @@ -709,43 +710,30 @@ impl<'bump> DiffState<'bump> { .get_scope(scope_addr) .unwrap_or_else(|| panic!("could not find {:?}", scope_addr)); - /* - old.props is the previous vcomponent - existing_props is the current ScopeState's - */ - let existing_props = unsafe { &*scope.render.get() }; - if old.can_memoize { - // let props_are_the_same = unsafe { existing_props.memoize(new.props) }; + // take the new props out regardless + // when memoizing, push to the existing scope if memoization happens + let new_props = new.props.borrow_mut().take().unwrap(); - if !props_are_the_same || self.force_diff { - // release the old props since they will be overwritten next frame - // existing_props.release(); - - // move the old props onto the heap - // todo: maybe we can reuse the old props box? - new.props.promote(); - - scope - .render - .set(unsafe { std::mem::transmute(new.props as *const dyn AnyProps) }); - - // this should auto drop the previous props - self.scopes.run_scope(scope_addr); - - self.diff_node( - self.scopes.wip_head(scope_addr), - self.scopes.fin_head(scope_addr), - ); + let should_run = { + if old.can_memoize { + let props_are_the_same = unsafe { + scope + .props + .borrow() + .as_ref() + .unwrap() + .memoize(new_props.as_ref()) + }; + !props_are_the_same || self.force_diff } else { - // release the old props since they will be overwritten next frame - if existing_props.as_ptr() != old.props.as_ptr() { - old.props.release(); - } + true } - } else { - scope - .render - .set(unsafe { std::mem::transmute(new.props as *const dyn AnyProps) }); + }; + + if should_run { + let _old_props = scope + .props + .replace(unsafe { std::mem::transmute(Some(new_props)) }); // this should auto drop the previous props self.scopes.run_scope(scope_addr); @@ -753,7 +741,10 @@ impl<'bump> DiffState<'bump> { self.scopes.wip_head(scope_addr), self.scopes.fin_head(scope_addr), ); - } + } else { + // memoization has taken place + drop(new_props); + }; self.stack.scope_stack.pop(); } else { diff --git a/packages/core/src/nodes.rs b/packages/core/src/nodes.rs index 3d14a332c..419264ec8 100644 --- a/packages/core/src/nodes.rs +++ b/packages/core/src/nodes.rs @@ -359,7 +359,7 @@ pub struct VComponent<'src> { pub scope: Cell>, pub can_memoize: bool, pub user_fc: *const (), - pub props: Option>>, + pub props: RefCell>>, } pub(crate) struct VComponentProps

{ @@ -587,7 +587,7 @@ impl<'a> NodeFactory<'a> { scope: Default::default(), can_memoize: P::IS_STATIC, user_fc: component as *const (), - props: Some(RefCell::new(Box::new(VComponentProps { + props: RefCell::new(Some(Box::new(VComponentProps { // local_props: RefCell::new(Some(props)), // heap_props: RefCell::new(None), props, diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 96c29c684..7536202b0 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -91,15 +91,18 @@ impl ScopeArena { container: ElementId, subtree: u32, ) -> ScopeId { + // Increment the ScopeId system. ScopeIDs are never reused let new_scope_id = ScopeId(self.scope_counter.get()); self.scope_counter.set(self.scope_counter.get() + 1); - let parent_scope = parent_scope.map(|f| self.get_scope_raw(f)).flatten(); + + // Get the height of the scope let height = parent_scope - .as_ref() - .map(|f| unsafe { &**f }) - .map(|f| f.height + 1) + .map(|id| self.get_scope(id).map(|scope| scope.height)) + .flatten() .unwrap_or_default(); + let parent_scope = parent_scope.map(|f| self.get_scope_raw(f)).flatten(); + /* This scopearena aggressively reuse old scopes when possible. We try to minimize the new allocations for props/arenas. @@ -107,60 +110,39 @@ impl ScopeArena { However, this will probably lead to some sort of fragmentation. I'm not exactly sure how to improve this today. */ - match self.free_scopes.borrow_mut().pop() { - // No free scope, make a new scope - None => { - let (node_capacity, hook_capacity) = self - .heuristics - .borrow() - .get(&fc_ptr) - .map(|h| (h.node_arena_size, h.hook_arena_size)) - .unwrap_or_default(); + if let Some(old_scope) = self.free_scopes.borrow_mut().pop() { + // reuse the old scope + let scope = unsafe { &mut *old_scope }; + scope.props.get_mut().replace(vcomp); + scope.parent_scope = parent_scope; + scope.height = height; + scope.subtree.set(subtree); + scope.our_arena_idx = new_scope_id; + scope.container = container; + let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); + debug_assert!(any_item.is_none()); + } else { + // else create a new scope + let (node_capacity, hook_capacity) = self + .heuristics + .borrow() + .get(&fc_ptr) + .map(|h| (h.node_arena_size, h.hook_arena_size)) + .unwrap_or_default(); - let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; - - let scope = self.bump.alloc(ScopeState { - sender: self.sender.clone(), - container, - our_arena_idx: new_scope_id, - parent_scope, + self.scopes.borrow_mut().insert( + new_scope_id, + self.bump.alloc(ScopeState::new( height, - frames, - subtree: Cell::new(subtree), - is_subtree_root: Cell::new(false), - - props: vcomp, - generation: 0.into(), - - shared_contexts: Default::default(), - - items: RefCell::new(SelfReferentialItems { - listeners: Default::default(), - borrowed_props: Default::default(), - tasks: Default::default(), - }), - - hook_arena: Bump::new(), - hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), - hook_idx: Default::default(), - }); - - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); - } - - // Reuse a free scope - Some(old_scope) => { - let scope = unsafe { &mut *old_scope }; - scope.props.set(vcomp); - scope.parent_scope = parent_scope; - scope.height = height; - scope.subtree = Cell::new(subtree); - scope.our_arena_idx = new_scope_id; - scope.container = container; - let any_item = self.scopes.borrow_mut().insert(new_scope_id, scope); - debug_assert!(any_item.is_none()); - } + container, + new_scope_id, + self.sender.clone(), + parent_scope, + vcomp, + node_capacity, + hook_capacity, + )), + ); } new_scope_id @@ -176,9 +158,6 @@ impl ScopeArena { let scope = unsafe { &mut *self.scopes.borrow_mut().remove(&id).unwrap() }; scope.reset(); - todo!("drop"); - // unsafe { &*scope.render.get() }.release(); - self.free_scopes.borrow_mut().push(scope); Some(()) @@ -230,8 +209,7 @@ impl ScopeArena { self.ensure_drop_safety(scope_id); - let g = comp.props.take(); - // comp.props.release(); + drop(comp.props.take()); }); // Now that all the references are gone, we can safely drop our own references in our listeners. @@ -285,7 +263,10 @@ impl ScopeArena { Also, the way we implement hooks allows us to cut rendering short before the next hook is recalled. I'm not sure if React lets you abort the component early, but we let you do that. */ - if let Some(node) = scope.props.render(scope) { + + let props = scope.props.borrow(); + let render = props.as_ref().unwrap(); + if let Some(node) = render.render(scope) { if !scope.items.borrow().tasks.is_empty() { self.pending_futures.borrow_mut().insert(id); } @@ -425,7 +406,7 @@ pub struct ScopeState { pub(crate) is_subtree_root: Cell, pub(crate) subtree: Cell, - pub(crate) props: Option>>, + pub(crate) props: RefCell>>, // nodes, items pub(crate) frames: [BumpFrame; 2], @@ -449,6 +430,47 @@ pub struct SelfReferentialItems<'a> { // Public methods exposed to libraries and components impl ScopeState { + fn new( + height: u32, + container: ElementId, + our_arena_idx: ScopeId, + sender: UnboundedSender, + parent_scope: Option<*mut ScopeState>, + vcomp: Box, + node_capacity: usize, + hook_capacity: usize, + ) -> Self { + let frames = [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)]; + + ScopeState { + sender, + container, + our_arena_idx, + parent_scope, + props: RefCell::new(Some(vcomp)), + height, + frames, + + // todo: subtrees + subtree: Cell::new(0), + is_subtree_root: Cell::new(false), + + generation: 0.into(), + + shared_contexts: Default::default(), + + items: RefCell::new(SelfReferentialItems { + listeners: Default::default(), + borrowed_props: Default::default(), + tasks: Default::default(), + }), + + hook_arena: Bump::new(), + hook_vals: RefCell::new(Vec::with_capacity(hook_capacity)), + hook_idx: Default::default(), + } + } + /// Get the subtree ID that this scope belongs to. /// /// Each component has its own subtree ID - the root subtree has an ID of 0. This ID is used by the renderer to route diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index 55902831c..40205d9ca 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -8,10 +8,7 @@ use futures_util::{Future, StreamExt}; use fxhash::FxHashSet; use indexmap::IndexSet; use smallvec::SmallVec; -use std::{ - any::Any, cell::RefCell, collections::VecDeque, iter::FromIterator, pin::Pin, sync::Arc, - task::Poll, -}; +use std::{any::Any, collections::VecDeque, iter::FromIterator, pin::Pin, sync::Arc, task::Poll}; /// A virtual node s ystem that progresses user events and diffs UI trees. /// @@ -207,8 +204,8 @@ impl VirtualDom { root as *const _, Box::new(VComponentProps { props: root_props, - memo: |_a, _b| false, - render_fn: unsafe { std::mem::transmute(root) }, + memo: |_a, _b| unreachable!("memo on root will neve be run"), + render_fn: root, }), None, ElementId(0),