From ce17574aa0223b9cfb5d6eab6565a48fe302b21c Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Fri, 4 Feb 2022 18:05:55 -0500 Subject: [PATCH 1/7] wip: add tracking for tasks --- packages/core/src/scopes.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 00c3024bf..1e58dcfd2 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -7,7 +7,7 @@ use std::{ any::{Any, TypeId}, borrow::Borrow, cell::{Cell, RefCell}, - collections::HashMap, + collections::{HashMap, HashSet}, future::Future, pin::Pin, rc::Rc, @@ -179,6 +179,9 @@ impl ScopeArena { log::trace!("removing scope {:?}", id); self.ensure_drop_safety(id); + // + let tasks = self.tasks.tasks.borrow_mut(); + // Safety: // - ensure_drop_safety ensures that no references to this scope are in use // - this raw pointer is removed from the map @@ -933,14 +936,17 @@ impl BumpFrame { pub(crate) struct TaskQueue { pub(crate) tasks: RefCell>, + pub(crate) task_map: RefCell>>, gen: Cell, sender: UnboundedSender, } + pub(crate) type InnerTask = Pin>>; impl TaskQueue { fn new(sender: UnboundedSender) -> Rc { Rc::new(Self { tasks: RefCell::new(FxHashMap::default()), + task_map: RefCell::new(FxHashMap::default()), gen: Cell::new(0), sender, }) @@ -957,10 +963,6 @@ impl TaskQueue { fn remove_fut(&self, id: TaskId) { if let Ok(mut tasks) = self.tasks.try_borrow_mut() { let _ = tasks.remove(&id); - } else { - // todo: it should be okay to remote a fut while the queue is being polled - // However, it's not currently possible to do that. - log::trace!("Unable to remove task from task queue. This is probably a bug."); } } pub(crate) fn has_tasks(&self) -> bool { From 92059fe3b8dcaaaf987bd4b25c5ae3207a3d225e Mon Sep 17 00:00:00 2001 From: YuKun Liu Date: Sun, 27 Feb 2022 17:05:01 +0800 Subject: [PATCH 2/7] Update docs.yml --- .github/workflows/docs.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 0e19ecaec..197251a97 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -25,6 +25,7 @@ jobs: cd guide && mdbook build -d ../nightly/guide && cd .. && cd reference && mdbook build -d ../nightly/reference && cd .. && cd router && mdbook build -d ../nightly/router && cd .. + cd cli && mdbook build -d ../nightly/cli && cd .. - name: Deploy 🚀 uses: JamesIves/github-pages-deploy-action@v4.2.3 From caa61f8ca8d504ac689ee5fe7619e298fbe681e8 Mon Sep 17 00:00:00 2001 From: YuKun Liu Date: Sun, 27 Feb 2022 17:06:58 +0800 Subject: [PATCH 3/7] Update docs.yml --- .github/workflows/docs.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/docs.yml b/.github/workflows/docs.yml index 197251a97..81b1b64cb 100644 --- a/.github/workflows/docs.yml +++ b/.github/workflows/docs.yml @@ -24,7 +24,7 @@ jobs: run: cd docs && cd guide && mdbook build -d ../nightly/guide && cd .. && cd reference && mdbook build -d ../nightly/reference && cd .. && - cd router && mdbook build -d ../nightly/router && cd .. + cd router && mdbook build -d ../nightly/router && cd .. && cd cli && mdbook build -d ../nightly/cli && cd .. - name: Deploy 🚀 From 35a3eaabdf82d828e339fa3578e60a486ed0ec39 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 27 Feb 2022 14:49:47 -0500 Subject: [PATCH 4/7] fix: actually remove nodes in keyed_diff_middle --- packages/core/src/diff.rs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/core/src/diff.rs b/packages/core/src/diff.rs index 4e9a11c34..5682772be 100644 --- a/packages/core/src/diff.rs +++ b/packages/core/src/diff.rs @@ -813,6 +813,15 @@ impl<'b> DiffState<'b> { return; } + // remove any old children that are not shared + // todo: make this an iterator + for child in old { + let key = child.key().unwrap(); + if !shared_keys.contains(&key) { + self.remove_nodes([child], true); + } + } + // 4. Compute the LIS of this list let mut lis_sequence = Vec::default(); lis_sequence.reserve(new_index_to_old_index.len()); From c2c6d73916a8ab0e972179ac7917feefef8c8ca9 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 27 Feb 2022 14:59:36 -0500 Subject: [PATCH 5/7] fix: tests should reflect removes --- packages/core/tests/diffing.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/core/tests/diffing.rs b/packages/core/tests/diffing.rs index 6bc9b5596..9184d4e4a 100644 --- a/packages/core/tests/diffing.rs +++ b/packages/core/tests/diffing.rs @@ -623,16 +623,17 @@ fn controlled_keyed_diffing_out_of_order() { assert_eq!( changes.edits, [ + Remove { root: 4 }, // move 4 to after 6 PushRoot { root: 1 }, InsertAfter { n: 1, root: 3 }, // remove 7 // create 9 and insert before 6 - CreateElement { root: 5, tag: "div" }, + CreateElement { root: 4, tag: "div" }, InsertBefore { n: 1, root: 3 }, // create 0 and insert before 5 - CreateElement { root: 6, tag: "div" }, + CreateElement { root: 5, tag: "div" }, InsertBefore { n: 1, root: 2 }, ] ); @@ -659,7 +660,8 @@ fn controlled_keyed_diffing_out_of_order_max_test() { assert_eq!( changes.edits, [ - CreateElement { root: 6, tag: "div" }, + Remove { root: 5 }, + CreateElement { root: 5, tag: "div" }, InsertBefore { n: 1, root: 3 }, PushRoot { root: 4 }, InsertBefore { n: 1, root: 1 }, From 70177ed49dbf72b2d13cbb7dc213e10b9e7e0040 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 27 Feb 2022 17:03:43 -0500 Subject: [PATCH 6/7] feat: tasks are dropped when components are unmounted --- packages/core/src/scopes.rs | 59 ++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 17 deletions(-) diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 65725ad4f..7d50fd2a5 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -68,7 +68,12 @@ impl ScopeArena { heuristics: RefCell::new(FxHashMap::default()), free_scopes: RefCell::new(Vec::new()), nodes: RefCell::new(nodes), - tasks: TaskQueue::new(sender), + tasks: Rc::new(TaskQueue { + tasks: RefCell::new(FxHashMap::default()), + task_map: RefCell::new(FxHashMap::default()), + gen: Cell::new(0), + sender, + }), } } @@ -179,8 +184,14 @@ impl ScopeArena { log::trace!("removing scope {:?}", id); self.ensure_drop_safety(id); - // - let tasks = self.tasks.tasks.borrow_mut(); + // Dispose of any ongoing tasks + let mut tasks = self.tasks.tasks.borrow_mut(); + let mut task_map = self.tasks.task_map.borrow_mut(); + if let Some(cur_tasks) = task_map.remove(&id) { + for task in cur_tasks { + tasks.remove(&task); + } + } // Safety: // - ensure_drop_safety ensures that no references to this scope are in use @@ -438,7 +449,13 @@ pub struct ScopeId(pub usize); /// once a Task has been completed. #[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug)] -pub struct TaskId(pub usize); +pub struct TaskId { + /// The global ID of the task + pub id: usize, + + /// The original scope that this task was scheduled in + pub scope: ScopeId, +} /// Every component in Dioxus is represented by a `ScopeState`. /// @@ -748,7 +765,7 @@ impl ScopeState { .unbounded_send(SchedulerMsg::NewTask(self.our_arena_idx)) .unwrap(); - self.tasks.push_fut(fut) + self.tasks.spawn(self.our_arena_idx, fut) } /// Spawns the future but does not return the TaskId @@ -759,7 +776,7 @@ impl ScopeState { /// Informs the scheduler that this task is no longer needed and should be removed /// on next poll. pub fn remove_future(&self, id: TaskId) { - self.tasks.remove_fut(id); + self.tasks.remove(id); } /// Take a lazy VNode structure and actually build it with the context of the VDom's efficient VNode allocator. @@ -950,28 +967,36 @@ pub(crate) struct TaskQueue { pub(crate) type InnerTask = Pin>>; impl TaskQueue { - fn new(sender: UnboundedSender) -> Rc { - Rc::new(Self { - tasks: RefCell::new(FxHashMap::default()), - task_map: RefCell::new(FxHashMap::default()), - gen: Cell::new(0), - sender, - }) - } - fn push_fut(&self, task: impl Future + 'static) -> TaskId { + fn spawn(&self, scope: ScopeId, task: impl Future + 'static) -> TaskId { let pinned = Box::pin(task); let id = self.gen.get(); self.gen.set(id + 1); - let tid = TaskId(id); + let tid = TaskId { id, scope }; self.tasks.borrow_mut().insert(tid, pinned); + + // also add to the task map + // when the component is unmounted we know to remove it from the map + self.task_map + .borrow_mut() + .entry(scope) + .or_default() + .insert(tid); + tid } - fn remove_fut(&self, id: TaskId) { + + fn remove(&self, id: TaskId) { if let Ok(mut tasks) = self.tasks.try_borrow_mut() { let _ = tasks.remove(&id); } + + // the task map is still around, but it'll be removed when the scope is unmounted + if let Some(task_map) = self.task_map.borrow_mut().get_mut(&id.scope) { + task_map.remove(&id); + } } + pub(crate) fn has_tasks(&self) -> bool { !self.tasks.borrow().is_empty() } From e416e31bc733999f20c39dc303ecf74263e540c3 Mon Sep 17 00:00:00 2001 From: Jonathan Kelley Date: Sun, 27 Feb 2022 17:22:21 -0500 Subject: [PATCH 7/7] feat: new spawn_forever for tasks that never die --- packages/core/src/scopes.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/core/src/scopes.rs b/packages/core/src/scopes.rs index 7d50fd2a5..7e415801b 100644 --- a/packages/core/src/scopes.rs +++ b/packages/core/src/scopes.rs @@ -773,6 +773,20 @@ impl ScopeState { self.push_future(fut); } + /// Spawn a future that Dioxus will never clean up + /// + /// This is good for tasks that need to be run after the component has been dropped. + pub fn spawn_forever(&self, fut: impl Future + 'static) -> TaskId { + // wake up the scheduler if it is sleeping + self.tasks + .sender + .unbounded_send(SchedulerMsg::NewTask(self.our_arena_idx)) + .unwrap(); + + // The root scope will never be unmounted so we can just add the task at the top of the app + self.tasks.spawn(ScopeId(0), fut) + } + /// Informs the scheduler that this task is no longer needed and should be removed /// on next poll. pub fn remove_future(&self, id: TaskId) {