More conservative node garbage collection

packages/core/src/diff.rs

@@ -135,6 +135,7 @@ impl<'b> DiffState<'b> {
             // these are *actual* elements, not wrappers around lists
             (Text(old), Text(new)) => {
                 if std::ptr::eq(old, new) {
+                    log::debug!("ptrs are equal, skipping diff");
                     return;
                 }
 
@@ -153,6 +154,7 @@ impl<'b> DiffState<'b> {
 
             (Placeholder(old), Placeholder(new)) => {
                 if std::ptr::eq(old, new) {
+                    log::debug!("ptrs are equal, skipping diff");
                     return;
                 }
 
@@ -167,6 +169,7 @@ impl<'b> DiffState<'b> {
 
             (Element(old), Element(new)) => {
                 if std::ptr::eq(old, new) {
+                    log::debug!("ptrs are equal, skipping diff");
                     return;
                 }
                 self.diff_element_nodes(old, new, old_node, new_node);
@@ -175,13 +178,15 @@ impl<'b> DiffState<'b> {
             // These two sets are pointers to nodes but are not actually nodes themselves
             (Component(old), Component(new)) => {
                 if std::ptr::eq(old, new) {
+                    log::debug!("ptrs are equal, skipping diff");
                     return;
                 }
                 self.diff_component_nodes(old_node, new_node, *old, *new);
             }
 
             (Fragment(old), Fragment(new)) => {
                 if std::ptr::eq(old, new) {
+                    log::debug!("ptrs are equal, skipping diff");
                     return;
                 }
                 self.diff_fragment_nodes(old, new);
@@ -274,6 +279,7 @@ impl<'b> DiffState<'b> {
         // this makes figure out when to drop the component more complicated
         let new_idx = if let Some(idx) = vcomponent.scope.get() {
             assert!(self.scopes.get_scope(idx).is_some());
+            self.scopes.set_active(idx, true);
             idx
         } else {
             // Insert a new scope into our component list
@@ -285,6 +291,8 @@ impl<'b> DiffState<'b> {
                 Some(parent_idx),
                 self.element_stack.last().copied().unwrap(),
                 0,
+                vcomponent.fn_name,
+                vcomponent.originator,
             )
         };
 
@@ -427,6 +435,7 @@ impl<'b> DiffState<'b> {
             .expect("existing component nodes should have a scope");
 
         if std::ptr::eq(old, new) {
+            log::debug!("ptrs are equal, skipping diff");
             return;
         }
 
@@ -443,6 +452,8 @@ impl<'b> DiffState<'b> {
                     .get_scope(scope_addr)
                     .unwrap_or_else(|| panic!("could not find {:?}", scope_addr));
 
+                scope.active.set(true);
+
                 // take the new props out regardless
                 // when memoizing, push to the existing scope if memoization happens
                 let new_props = new
@@ -473,17 +484,29 @@ impl<'b> DiffState<'b> {
                     self.scopes.run_scope(scope_addr);
                     self.mutations.mark_dirty_scope(scope_addr);
 
-                    self.diff_node(
-                        self.scopes.wip_head(scope_addr),
-                        self.scopes.fin_head(scope_addr),
+                    let old_head = self.scopes.wip_head(scope_addr);
+                    let new_head = self.scopes.fin_head(scope_addr);
+
+                    log::debug!(
+                        "running component diff {:?}, {:?}, {:?}",
+                        new.fn_name,
+                        old_head as *const _,
+                        new_head as *const _
                     );
+
+                    self.diff_node(old_head, new_head);
                 } else {
                     // memoization has taken place
                     drop(new_props);
                 };
             }
             self.leave_scope();
         } else {
+            log::warn!(
+                "component nodes with different components {:?}, {:?}",
+                old_node,
+                new_node
+            );
             self.replace_node(old_node, new_node);
         }
     }
@@ -521,6 +544,11 @@ impl<'b> DiffState<'b> {
     // to an element, and appending makes sense.
     fn diff_children(&mut self, old: &'b [VNode<'b>], new: &'b [VNode<'b>]) {
         if std::ptr::eq(old, new) {
+            log::debug!(
+                "ptrs are equal, skipping diff {:?}, {:?}",
+                old as *const _,
+                new as *const _
+            );
             return;
         }
 
@@ -890,6 +918,7 @@ impl<'b> DiffState<'b> {
     }
 
     fn replace_node(&mut self, old: &'b VNode<'b>, new: &'b VNode<'b>) {
+        log::debug!("replace_node: {:?} {:?}", old, new);
         let nodes_created = self.create_node(new);
         self.replace_inner(old, nodes_created);
     }
@@ -903,7 +932,7 @@ impl<'b> DiffState<'b> {
 
                 self.mutations.replace_with(id, nodes_created as u32);
                 self.remove_nodes(el.children, false);
-                self.scopes.collect_garbage(id);
+                self.collect_garbage(id);
             }
 
             VNode::Text(_) | VNode::Placeholder(_) => {
@@ -912,7 +941,7 @@ impl<'b> DiffState<'b> {
                     .unwrap_or_else(|| panic!("broke on {:?}", old));
 
                 self.mutations.replace_with(id, nodes_created as u32);
-                self.scopes.collect_garbage(id);
+                self.collect_garbage(id);
             }
 
             VNode::Fragment(f) => {
@@ -929,13 +958,20 @@ impl<'b> DiffState<'b> {
                 {
                     self.replace_inner(node, nodes_created);
 
-                    log::trace!("Replacing component x2 {:?}", old);
-
                     // we can only remove components if they are actively being diffed
+                    log::debug!(
+                        "attempting to remove component {:?}, {:?}, {:?}",
+                        old,
+                        self.scope_stack,
+                        self.mutations.dirty_scopes
+                    );
+
                     if self.scope_stack.contains(&c.originator) {
                         log::trace!("Removing component {:?}", old);
 
                         self.scopes.try_remove(scope_id).unwrap();
+                    } else {
+                        self.scopes.set_active(scope_id, false);
                     }
                 }
                 self.leave_scope();
@@ -949,7 +985,7 @@ impl<'b> DiffState<'b> {
                 VNode::Text(t) => {
                     // this check exists because our null node will be removed but does not have an ID
                     if let Some(id) = t.id.get() {
-                        self.scopes.collect_garbage(id);
+                        self.collect_garbage(id);
 
                         if gen_muts {
                             self.mutations.remove(id.as_u64());
@@ -958,7 +994,7 @@ impl<'b> DiffState<'b> {
                 }
                 VNode::Placeholder(a) => {
                     let id = a.id.get().unwrap();
-                    self.scopes.collect_garbage(id);
+                    self.collect_garbage(id);
 
                     if gen_muts {
                         self.mutations.remove(id.as_u64());
@@ -971,7 +1007,7 @@ impl<'b> DiffState<'b> {
                         self.mutations.remove(id.as_u64());
                     }
 
-                    self.scopes.collect_garbage(id);
+                    self.collect_garbage(id);
 
                     self.remove_nodes(e.children, false);
                 }
@@ -990,6 +1026,8 @@ impl<'b> DiffState<'b> {
                         // we can only remove this node if the originator is actively in our stackß
                         if self.scope_stack.contains(&c.originator) {
                             self.scopes.try_remove(scope_id).unwrap();
+                        } else {
+                            self.scopes.set_active(scope_id, false);
                         }
                     }
                     self.leave_scope();
@@ -1035,6 +1073,10 @@ impl<'b> DiffState<'b> {
         self.scope_stack.pop();
     }
 
+    fn collect_garbage(&mut self, element: ElementId) {
+        // self.scopes.collect_garbage(element);
+    }
+
     fn find_last_element(&self, vnode: &'b VNode<'b>) -> Option<ElementId> {
         let mut search_node = Some(vnode);
         loop {

packages/core/src/scopes.rs

@@ -89,6 +89,8 @@ impl ScopeArena {
         parent_scope: Option<ScopeId>,
         container: ElementId,
         subtree: u32,
+        fnname: &'static str,
+        originator: ScopeId,
     ) -> ScopeId {
         // Increment the ScopeId system. ScopeIDs are never reused
         let new_scope_id = ScopeId(self.scope_gen.get());
@@ -118,6 +120,8 @@ impl ScopeArena {
             scope.parent_scope = parent_scope;
             scope.height = height;
             scope.fnptr = fc_ptr;
+            scope.fnname = fnname;
+            scope.active.set(true);
             scope.props.get_mut().replace(vcomp);
             scope.subtree.set(subtree);
             scope.frames[0].reset();
@@ -147,6 +151,10 @@ impl ScopeArena {
                     parent_scope,
                     height,
                     fnptr: fc_ptr,
+                    fnname,
+                    originator,
+                    active: Cell::new(true),
+
                     props: RefCell::new(Some(vcomp)),
                     frames: [BumpFrame::new(node_capacity), BumpFrame::new(node_capacity)],
 
@@ -244,15 +252,17 @@ impl ScopeArena {
     }
 
     pub(crate) fn run_scope(&self, id: ScopeId) {
+        // 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") };
+
+        assert!(scope.active.get());
+
         // 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") };
-
-        log::trace!("running scope {:?} symbol: {:?}", id, scope.fnptr);
+        log::trace!("running scope {:?} name: {:?}", id, scope.fnname);
 
         // Safety:
         // - We dropped the listeners, so no more &mut T can be used while these are held
@@ -375,6 +385,11 @@ impl ScopeArena {
             None => None,
         }
     }
+
+    pub(crate) fn set_active(&self, scope_id: ScopeId, arg: bool) {
+        let scope = self.get_scope(scope_id).unwrap();
+        scope.active.set(arg);
+    }
 }
 
 /// Components in Dioxus use the "Context" object to interact with their lifecycle.
@@ -449,6 +464,9 @@ pub struct ScopeState {
     pub(crate) our_arena_idx: ScopeId,
     pub(crate) height: u32,
     pub(crate) fnptr: ComponentPtr,
+    pub(crate) fnname: &'static str,
+    pub(crate) originator: ScopeId,
+    pub(crate) active: Cell<bool>,
 
     // todo: subtrees
     pub(crate) is_subtree_root: Cell<bool>,
@@ -562,6 +580,22 @@ impl ScopeState {
         self.parent_scope.map(|p| unsafe { &*p }.our_arena_idx)
     }
 
+    pub fn container(&self) -> ElementId {
+        self.container
+    }
+
+    pub fn fnptr(&self) -> ComponentPtr {
+        self.fnptr
+    }
+
+    pub fn name(&self) -> &'static str {
+        self.fnname
+    }
+
+    pub fn originator(&self) -> ScopeId {
+        self.originator
+    }
+
     /// Get the ID of this Scope within this Dioxus VirtualDOM.
     ///
     /// This ID is not unique across Dioxus VirtualDOMs or across time. IDs will be reused when components are unmounted.

packages/core/src/virtual_dom.rs

@@ -222,6 +222,8 @@ impl VirtualDom {
             None,
             ElementId(0),
             0,
+            "app",
+            ScopeId(0),
         );
 
         Self {
@@ -472,6 +474,13 @@ impl VirtualDom {
             log::debug!("dirty_scopes: {:?}", self.dirty_scopes);
 
             if let Some(scopeid) = self.dirty_scopes.pop() {
+                let scope = scopes.get_scope(scopeid).unwrap();
+                let is_active = scope.active.get();
+                if !is_active {
+                    log::debug!("dirty scope {:?} is not active", scopeid);
+                    continue;
+                }
+
                 if !ran_scopes.contains(&scopeid) {
                     ran_scopes.insert(scopeid);
 

packages/router/src/components/route.rs

@@ -42,17 +42,19 @@ pub fn Route<'a>(cx: Scope<'a, RouteProps<'a>>) -> Element {
         router_root.register_total_route(
             route_context.total_route.clone(),
             cx.scope_id(),
+            cx.name(),
             cx.props.fallback,
         );
 
         Some(RouteInner {})
     });
 
-    // log::trace!("Checking route {}", cx.props.to);
-
     if router_root.should_render(cx.scope_id()) {
+        log::trace!("Route {} should render", cx.props.to);
+
         cx.render(rsx!(&cx.props.children))
     } else {
+        log::trace!("Route {} should not render", cx.props.to);
         None
     }
 }

packages/router/src/hooks/use_route.rs

@@ -61,7 +61,7 @@ impl UseRoute {
         self.router.current_location()
     }
 
-    fn path_segments(&self) -> Vec<String> {
+    pub fn path_segments(&self) -> Vec<String> {
         let location = self.router.current_location();
         let path = location.path();
         if path == "/" {