From c4b8ebc1cf6196cc98defcfd87d45de9689ebfa7 Mon Sep 17 00:00:00 2001 From: Evan Almloff Date: Mon, 4 Mar 2024 13:04:22 -0600 Subject: [PATCH] separate task and scope tasks --- packages/core/src/diff/component.rs | 4 +- packages/core/src/dirty_scope.rs | 124 ++++++++++++++++++++++++---- packages/core/src/virtual_dom.rs | 60 +++++--------- packages/core/tests/task.rs | 10 +-- 4 files changed, 135 insertions(+), 63 deletions(-) diff --git a/packages/core/src/diff/component.rs b/packages/core/src/diff/component.rs index 8ef070c84..495f70549 100644 --- a/packages/core/src/diff/component.rs +++ b/packages/core/src/diff/component.rs @@ -2,7 +2,7 @@ use std::ops::{Deref, DerefMut}; use crate::{ any_props::AnyProps, - innerlude::{DirtyScope, ElementRef, MountId, VComponent, WriteMutations}, + innerlude::{DirtyScopes,ScopeOrder, ElementRef, MountId, VComponent, WriteMutations}, nodes::RenderReturn, nodes::VNode, scopes::ScopeId, @@ -91,7 +91,7 @@ impl VNode { dom.diff_scope(to, scope_id, new); let height = dom.runtime.get_state(scope_id).unwrap().height; - dom.dirty_scopes.remove(&DirtyScope::new(height, scope_id)); + dom.dirty_scopes.remove(&ScopeOrder::new(height, scope_id)); } fn replace_vcomponent( diff --git a/packages/core/src/dirty_scope.rs b/packages/core/src/dirty_scope.rs index 3bbe32a0a..a9b0cb245 100644 --- a/packages/core/src/dirty_scope.rs +++ b/packages/core/src/dirty_scope.rs @@ -4,8 +4,9 @@ use std::borrow::Borrow; use std::cell::Cell; use std::cell::RefCell; use std::hash::Hash; +use std::collections::BTreeSet; -#[derive(Debug, Clone, Eq)] +#[derive(Debug, Clone, Copy, Eq)] pub struct ScopeOrder { pub(crate) height: u32, pub(crate) id: ScopeId, @@ -41,62 +42,149 @@ impl Hash for ScopeOrder { } } +#[derive(Debug, Default)] +pub struct DirtyScopes { + pub(crate) scopes: BTreeSet, + pub(crate) tasks: BTreeSet, +} + +impl DirtyScopes { + pub fn queue_task(&mut self, task: Task, order: ScopeOrder) { + match self.tasks.get(&order) { + Some(scope) => scope.queue_task(task), + None => { + let mut scope = DirtyTasks::from(order); + scope.queue_task(task); + self.tasks.insert(scope); + } + } + } + + pub fn queue_scope(&mut self, order: ScopeOrder) { + self.scopes.insert(order); + } + + pub fn has_dirty_scopes(&self) -> bool { + !self.scopes.is_empty() + } + + pub fn pop_task(&mut self) -> Option { + self.tasks.pop_first() + } + + pub fn pop_scope(&mut self) -> Option { + self.scopes.pop_first() + } + + pub fn pop_work(&mut self) -> Option { + let dirty_scope = self.scopes.first(); + let dirty_task = self.tasks.first(); + match (dirty_scope, dirty_task) { + (Some(scope), Some(task)) => { + let tasks_order = task.borrow(); + if scope > tasks_order{ + let scope = self.scopes.pop_first().unwrap(); + Some(Work{ + scope: scope, + rerun_scope: true, + tasks: Vec::new(), + }) + } else if tasks_order> scope { + let task = self.tasks.pop_first().unwrap(); + Some(Work{ + scope: task.order, + rerun_scope: false, + tasks: task.tasks_queued.into_inner(), + }) + } + else { + let scope = self.scopes.pop_first().unwrap(); + let task = self.tasks.pop_first().unwrap(); + Some(Work{ + scope: scope, + rerun_scope: true, + tasks: task.tasks_queued.into_inner(), + }) + } + } + (Some(scope), None) => { + let scope = self.scopes.pop_first().unwrap(); + Some(Work{ + scope: scope, + rerun_scope: true, + tasks: Vec::new(), + }) + } + (None, Some(task)) => { + let task = self.tasks.pop_first().unwrap(); + Some(Work{ + scope: task.order, + rerun_scope: false, + tasks: task.tasks_queued.into_inner(), + }) + } + (None, None) => None + } + } + + pub fn remove(&mut self, scope: &ScopeOrder) { + self.scopes.remove(scope); + } +} + +pub struct Work { + pub scope: ScopeOrder, + pub rerun_scope: bool, + pub tasks: Vec, +} + #[derive(Debug, Clone, Eq)] -pub struct DirtyScope { +pub(crate) struct DirtyTasks { pub order: ScopeOrder, - pub rerun_queued: Cell, pub tasks_queued: RefCell>, } -impl From for DirtyScope { +impl From for DirtyTasks { fn from(order: ScopeOrder) -> Self { Self { order, - rerun_queued: false.into(), tasks_queued: Vec::new().into(), } } } -impl DirtyScope { - pub fn new(height: u32, id: ScopeId) -> Self { - ScopeOrder { height, id }.into() - } +impl DirtyTasks { pub fn queue_task(&self, task: Task) { self.tasks_queued.borrow_mut().push(task); } - - pub fn queue_rerun(&self) { - self.rerun_queued.set(true); - } } -impl Borrow for DirtyScope { +impl Borrow for DirtyTasks { fn borrow(&self) -> &ScopeOrder { &self.order } } -impl PartialOrd for DirtyScope { +impl PartialOrd for DirtyTasks { fn partial_cmp(&self, other: &Self) -> Option { Some(self.order.cmp(&other.order)) } } -impl Ord for DirtyScope { +impl Ord for DirtyTasks { fn cmp(&self, other: &Self) -> std::cmp::Ordering { self.order.cmp(&other.order) } } -impl PartialEq for DirtyScope { +impl PartialEq for DirtyTasks { fn eq(&self, other: &Self) -> bool { self.order == other.order } } -impl Hash for DirtyScope { +impl Hash for DirtyTasks { fn hash(&self, state: &mut H) { self.order.hash(state); } diff --git a/packages/core/src/virtual_dom.rs b/packages/core/src/virtual_dom.rs index b8e7ee9e6..e1657c071 100644 --- a/packages/core/src/virtual_dom.rs +++ b/packages/core/src/virtual_dom.rs @@ -8,7 +8,7 @@ use crate::{ any_props::AnyProps, arena::ElementId, innerlude::{ - DirtyScope, ElementRef, ErrorBoundary, NoOpMutations, SchedulerMsg, ScopeState, VNodeMount, + DirtyScopes, ElementRef, ErrorBoundary, NoOpMutations, SchedulerMsg, ScopeState, VNodeMount, VProps, WriteMutations, }, nodes::RenderReturn, @@ -184,7 +184,7 @@ use std::{any::Any, collections::BTreeSet, rc::Rc}; pub struct VirtualDom { pub(crate) scopes: Slab, - pub(crate) dirty_scopes: BTreeSet, + pub(crate) dirty_scopes: DirtyScopes, pub(crate) scopes_need_rerun: bool, // Maps a template path to a map of byte indexes to templates @@ -380,16 +380,7 @@ impl VirtualDom { tracing::trace!("Marking scope {:?} as dirty", id); self.scopes_need_rerun = true; let order = ScopeOrder::new(scope.height(), id); - match self.dirty_scopes.get(&order) { - Some(dirty) => { - dirty.queue_rerun(); - } - None => { - let dirty: DirtyScope = order.into(); - dirty.queue_rerun(); - self.dirty_scopes.insert(dirty); - } - } + self.dirty_scopes.queue_scope(order); } /// Mark a task as dirty @@ -401,16 +392,7 @@ impl VirtualDom { return; }; let order = ScopeOrder::new(scope.height(), scope.id); - match self.dirty_scopes.get(&order) { - Some(dirty) => { - dirty.queue_task(task); - } - None => { - let dirty: DirtyScope = order.into(); - dirty.queue_task(task); - self.dirty_scopes.insert(dirty); - } - } + self.dirty_scopes.queue_task(task, order); } /// Call a listener inside the VirtualDom with data from outside the VirtualDom. **The ElementId passed in must be the id of an element with a listener, not a static node or a text node.** @@ -509,15 +491,16 @@ impl VirtualDom { // Next, run any queued tasks // We choose not to poll the deadline since we complete pretty quickly anyways - while let Some(dirty) = self.dirty_scopes.pop_first() { + while let Some(task) = self.dirty_scopes.pop_task() { // If the scope doesn't exist for whatever reason, then we should skip it - if !self.scopes.contains(dirty.order.id.0) { + if !self.scopes.contains(task.order.id.0) { continue; } // Then poll any tasks that might be pending - for task in dirty.tasks_queued.borrow().iter() { - let _ = self.runtime.handle_task_wakeup(*task); + let tasks = std::mem::take(&mut *task.tasks_queued.borrow_mut()); + for task in tasks { + let _ = self.runtime.handle_task_wakeup(task); // Running that task, may mark a scope higher up as dirty. If it does, return from the function early self.queue_events(); if self.scopes_need_rerun { @@ -536,18 +519,19 @@ impl VirtualDom { pub fn replace_template(&mut self, template: Template) { self.register_template_first_byte_index(template); // iterating a slab is very inefficient, but this is a rare operation that will only happen during development so it's fine - for (_, scope) in self.scopes.iter() { + let mut dirty = Vec::new(); + for (id, scope) in self.scopes.iter() { if let Some(RenderReturn::Ready(sync)) = scope.try_root_node() { if sync.template.get().name.rsplit_once(':').unwrap().0 == template.name.rsplit_once(':').unwrap().0 { - let context = scope.state(); - let height = context.height; - self.dirty_scopes - .insert(DirtyScope::new(height, context.id)); + dirty.push(ScopeId(id)); } } } + for dirty in dirty { + self.mark_dirty(dirty); + } } /// Rebuild the virtualdom without handling any of the mutations @@ -608,9 +592,9 @@ impl VirtualDom { // Next, diff any dirty scopes // We choose not to poll the deadline since we complete pretty quickly anyways - while let Some(dirty) = self.dirty_scopes.pop_first() { + while let Some(work) = self.dirty_scopes.pop_work() { // If the scope doesn't exist for whatever reason, then we should skip it - if !self.scopes.contains(dirty.order.id.0) { + if !self.scopes.contains(work.scope.id.0) { continue; } @@ -618,14 +602,14 @@ impl VirtualDom { let _runtime = RuntimeGuard::new(self.runtime.clone()); // Then, poll any tasks that might be pending in the scope // This will run effects, so this **must** be done after the scope is diffed - for task in dirty.tasks_queued.borrow().iter() { - let _ = self.runtime.handle_task_wakeup(*task); + for task in work.tasks{ + let _ = self.runtime.handle_task_wakeup(task); } // If the scope is dirty, run the scope and get the mutations - if dirty.rerun_queued.get() { - let new_nodes = self.run_scope(dirty.order.id); + if work.rerun_scope { + let new_nodes = self.run_scope(work.scope.id); - self.diff_scope(to, dirty.order.id, new_nodes); + self.diff_scope(to, work.scope.id, new_nodes); } } } diff --git a/packages/core/tests/task.rs b/packages/core/tests/task.rs index 71e9bcb38..8828606f6 100644 --- a/packages/core/tests/task.rs +++ b/packages/core/tests/task.rs @@ -105,9 +105,9 @@ async fn flushing() { use_hook(|| { spawn(async move { for _ in 0..10 { + flush_sync().await; BROADCAST.with(|b| b.1.resubscribe()).recv().await.unwrap(); println!("Task 1 recved"); - flush_sync().await; println!("Task 1"); SEQUENCE.with(|s| s.borrow_mut().push(1)); } @@ -117,9 +117,9 @@ async fn flushing() { use_hook(|| { spawn(async move { for _ in 0..10 { + flush_sync().await; BROADCAST.with(|b| b.1.resubscribe()).recv().await.unwrap(); println!("Task 2 recved"); - flush_sync().await; println!("Task 2"); SEQUENCE.with(|s| s.borrow_mut().push(2)); } @@ -135,15 +135,15 @@ async fn flushing() { let fut = async { // Trigger the flush by waiting for work - for _ in 0..30 { + for i in 0..30 { BROADCAST.with(|b| b.0.send(()).unwrap()); - dom.mark_dirty(ScopeId(0)); tokio::select! { _ = dom.wait_for_work() => {} _ = tokio::time::sleep(Duration::from_millis(10)) => {} } + dom.mark_dirty(ScopeId(0)); dom.render_immediate(&mut dioxus_core::NoOpMutations); - println!("Flushed"); + println!("Flushed {}", i); } };