diff --git a/packages/yew/Cargo.toml b/packages/yew/Cargo.toml index de6303fc6a4..ce95f74204a 100644 --- a/packages/yew/Cargo.toml +++ b/packages/yew/Cargo.toml @@ -79,7 +79,7 @@ trybuild = "1" [features] ssr = ["futures", "html-escape"] csr = [] -doc_test = ["csr"] +doc_test = ["csr", "ssr"] wasm_test = ["csr"] default = [] @@ -87,5 +87,5 @@ default = [] tokio = { version = "1.15.0", features = ["full"] } [package.metadata.docs.rs] -features = ["doc_test", "ssr"] +features = ["doc_test"] rustdoc-args = ["--cfg", "documenting"] diff --git a/packages/yew/src/html/component/lifecycle.rs b/packages/yew/src/html/component/lifecycle.rs index fef2d3086e2..92056f2a6f6 100644 --- a/packages/yew/src/html/component/lifecycle.rs +++ b/packages/yew/src/html/component/lifecycle.rs @@ -153,9 +153,7 @@ pub(crate) struct ComponentState { suspension: Option, - // Used for debug logging - #[cfg(debug_assertions)] - pub(crate) vcomp_id: usize, + pub(crate) comp_id: usize, } impl ComponentState { @@ -164,8 +162,7 @@ impl ComponentState { scope: Scope, props: Rc, ) -> Self { - #[cfg(debug_assertions)] - let vcomp_id = scope.vcomp_id; + let comp_id = scope.id; let context = Context { scope, props }; let inner = Box::new(CompStateInner { @@ -181,8 +178,7 @@ impl ComponentState { #[cfg(feature = "csr")] has_rendered: false, - #[cfg(debug_assertions)] - vcomp_id, + comp_id, } } @@ -208,7 +204,7 @@ impl Runnable for CreateRunner { let mut current_state = self.scope.state.borrow_mut(); if current_state.is_none() { #[cfg(debug_assertions)] - super::log_event(self.scope.vcomp_id, "create"); + super::log_event(self.scope.id, "create"); *current_state = Some(ComponentState::new( self.initial_render_state, @@ -240,6 +236,7 @@ impl Runnable for UpdateRunner { if let Some(state) = self.state.borrow_mut().as_mut() { let schedule_render = match self.event { UpdateEvent::Message => state.inner.flush_messages(), + #[cfg(feature = "csr")] UpdateEvent::Properties(props, next_node_ref, next_sibling) => { match state.render_state { @@ -297,16 +294,16 @@ impl Runnable for UpdateRunner { #[cfg(debug_assertions)] super::log_event( - state.vcomp_id, + state.comp_id, format!("update(schedule_render={})", schedule_render), ); if schedule_render { scheduler::push_component_render( - self.state.as_ptr() as usize, - RenderRunner { + state.comp_id, + Box::new(RenderRunner { state: self.state.clone(), - }, + }), ); // Only run from the scheduler, so no need to call `scheduler::start()` } @@ -325,7 +322,7 @@ impl Runnable for DestroyRunner { fn run(self: Box) { if let Some(mut state) = self.state.borrow_mut().take() { #[cfg(debug_assertions)] - super::log_event(state.vcomp_id, "destroy"); + super::log_event(state.comp_id, "destroy"); state.inner.destroy(); @@ -357,7 +354,7 @@ impl Runnable for RenderRunner { fn run(self: Box) { if let Some(state) = self.state.borrow_mut().as_mut() { #[cfg(debug_assertions)] - super::log_event(state.vcomp_id, "render"); + super::log_event(state.comp_id, "render"); match state.inner.view() { Ok(m) => self.render(state, m), @@ -373,14 +370,16 @@ impl RenderRunner { // suspension to parent element. let shared_state = self.state.clone(); + let comp_id = state.comp_id; + if suspension.resumed() { // schedule a render immediately if suspension is resumed. scheduler::push_component_render( - shared_state.as_ptr() as usize, - RenderRunner { + state.comp_id, + Box::new(RenderRunner { state: shared_state, - }, + }), ); } else { // We schedule a render after current suspension is resumed. @@ -393,10 +392,10 @@ impl RenderRunner { suspension.listen(Callback::from(move |_| { scheduler::push_component_render( - shared_state.as_ptr() as usize, - RenderRunner { + comp_id, + Box::new(RenderRunner { state: shared_state.clone(), - }, + }), ); scheduler::start(); })); @@ -442,11 +441,11 @@ impl RenderRunner { state.has_rendered = true; scheduler::push_component_rendered( - self.state.as_ptr() as usize, - RenderedRunner { + state.comp_id, + Box::new(RenderedRunner { state: self.state.clone(), first_render, - }, + }), first_render, ); } @@ -474,7 +473,7 @@ mod feat_csr { fn run(self: Box) { if let Some(state) = self.state.borrow_mut().as_mut() { #[cfg(debug_assertions)] - super::super::log_event(state.vcomp_id, "rendered"); + super::super::log_event(state.comp_id, "rendered"); if state.suspension.is_none() { state.inner.rendered(self.first_render); diff --git a/packages/yew/src/html/component/mod.rs b/packages/yew/src/html/component/mod.rs index add550a126f..613a664b48a 100644 --- a/packages/yew/src/html/component/mod.rs +++ b/packages/yew/src/html/component/mod.rs @@ -14,46 +14,36 @@ pub(crate) use scope::Scoped; pub use scope::{AnyScope, Scope, SendAsMessage}; use std::rc::Rc; +#[cfg(debug_assertions)] #[cfg(any(feature = "csr", feature = "ssr"))] mod feat_csr_ssr { - #[cfg(debug_assertions)] thread_local! { static EVENT_HISTORY: std::cell::RefCell>> = Default::default(); - static COMP_ID_COUNTER: AtomicUsize = AtomicUsize::new(0); } /// Push [Component] event to lifecycle debugging registry - #[cfg(debug_assertions)] - pub(crate) fn log_event(vcomp_id: usize, event: impl ToString) { + pub(crate) fn log_event(comp_id: usize, event: impl ToString) { EVENT_HISTORY.with(|h| { h.borrow_mut() - .entry(vcomp_id) + .entry(comp_id) .or_default() .push(event.to_string()) }); } /// Get [Component] event log from lifecycle debugging registry - #[cfg(debug_assertions)] #[allow(dead_code)] - pub(crate) fn get_event_log(vcomp_id: usize) -> Vec { + pub(crate) fn get_event_log(comp_id: usize) -> Vec { EVENT_HISTORY.with(|h| { h.borrow() - .get(&vcomp_id) + .get(&comp_id) .map(|l| (*l).clone()) .unwrap_or_default() }) } - - #[cfg(debug_assertions)] - pub(crate) fn next_id() -> usize { - COMP_ID_COUNTER.with(|m| m.fetch_add(1, Ordering::Relaxed)) - } - - #[cfg(debug_assertions)] - use std::sync::atomic::{AtomicUsize, Ordering}; } + #[cfg(debug_assertions)] #[cfg(any(feature = "csr", feature = "ssr"))] pub(crate) use feat_csr_ssr::*; diff --git a/packages/yew/src/html/component/scope.rs b/packages/yew/src/html/component/scope.rs index 0cbbd3e1209..04582114fb9 100644 --- a/packages/yew/src/html/component/scope.rs +++ b/packages/yew/src/html/component/scope.rs @@ -8,6 +8,7 @@ use std::cell::RefCell; #[cfg(any(feature = "csr", feature = "ssr"))] use super::lifecycle::{ComponentState, UpdateEvent, UpdateRunner}; use super::BaseComponent; + use crate::callback::Callback; use crate::context::{ContextHandle, ContextProvider}; use crate::html::IntoComponent; @@ -112,8 +113,7 @@ pub struct Scope { #[cfg(any(feature = "csr", feature = "ssr"))] pub(crate) state: Shared>, - #[cfg(debug_assertions)] - pub(crate) vcomp_id: usize, + pub(crate) id: usize, } impl fmt::Debug for Scope { @@ -134,8 +134,7 @@ impl Clone for Scope { #[cfg(any(feature = "csr", feature = "ssr"))] state: self.state.clone(), - #[cfg(debug_assertions)] - vcomp_id: self.vcomp_id, + id: self.id, } } } @@ -211,14 +210,15 @@ mod feat_ssr { let state = ComponentRenderState::Ssr { sender: Some(tx) }; scheduler::push_component_create( - CreateRunner { + self.id, + Box::new(CreateRunner { initial_render_state: state, props, scope: self.clone(), - }, - RenderRunner { + }), + Box::new(RenderRunner { state: self.state.clone(), - }, + }), ); scheduler::start(); @@ -227,12 +227,12 @@ mod feat_ssr { let self_any_scope = AnyScope::from(self.clone()); html.render_to_string(w, &self_any_scope).await; - scheduler::push_component_destroy(DestroyRunner { + scheduler::push_component_destroy(Box::new(DestroyRunner { state: self.state.clone(), #[cfg(feature = "csr")] parent_to_detach: false, - }); + })); scheduler::start(); } } @@ -269,6 +269,7 @@ mod feat_csr_ssr { use super::*; use crate::scheduler::{self, Shared}; use std::cell::Ref; + use std::sync::atomic::{AtomicUsize, Ordering}; #[derive(Debug)] pub(crate) struct MsgQueue(Shared>); @@ -308,6 +309,8 @@ mod feat_csr_ssr { } } + static COMP_ID_COUNTER: AtomicUsize = AtomicUsize::new(0); + impl Scope { /// Crate a scope with an optional parent scope pub(crate) fn new(parent: Option) -> Self { @@ -325,8 +328,7 @@ mod feat_csr_ssr { state, parent, - #[cfg(debug_assertions)] - vcomp_id: super::super::next_id(), + id: COMP_ID_COUNTER.fetch_add(1, Ordering::SeqCst), } } @@ -345,10 +347,10 @@ mod feat_csr_ssr { } pub(super) fn push_update(&self, event: UpdateEvent) { - scheduler::push_component_update(UpdateRunner { + scheduler::push_component_update(Box::new(UpdateRunner { state: self.state.clone(), event, - }); + })); // Not guaranteed to already have the scheduler started scheduler::start(); } @@ -415,14 +417,15 @@ mod feat_csr { }; scheduler::push_component_create( - CreateRunner { + self.id, + Box::new(CreateRunner { initial_render_state: state, props, scope: self.clone(), - }, - RenderRunner { + }), + Box::new(RenderRunner { state: self.state.clone(), - }, + }), ); // Not guaranteed to already have the scheduler started scheduler::start(); @@ -435,7 +438,7 @@ mod feat_csr { next_sibling: NodeRef, ) { #[cfg(debug_assertions)] - super::super::log_event(self.vcomp_id, "reuse"); + super::super::log_event(self.id, "reuse"); self.push_update(UpdateEvent::Properties(props, node_ref, next_sibling)); } @@ -470,10 +473,10 @@ mod feat_csr { /// Process an event to destroy a component fn destroy(self, parent_to_detach: bool) { - scheduler::push_component_destroy(DestroyRunner { + scheduler::push_component_destroy(Box::new(DestroyRunner { state: self.state, parent_to_detach, - }); + })); // Not guaranteed to already have the scheduler started scheduler::start(); } @@ -483,10 +486,10 @@ mod feat_csr { } fn shift_node(&self, parent: Element, next_sibling: NodeRef) { - scheduler::push_component_update(UpdateRunner { + scheduler::push_component_update(Box::new(UpdateRunner { state: self.state.clone(), event: UpdateEvent::Shift(parent, next_sibling), - }) + })) } } } diff --git a/packages/yew/src/scheduler.rs b/packages/yew/src/scheduler.rs index c9d1f7f31de..26b914d4909 100644 --- a/packages/yew/src/scheduler.rs +++ b/packages/yew/src/scheduler.rs @@ -1,7 +1,7 @@ //! This module contains a scheduler. use std::cell::RefCell; -use std::collections::VecDeque; +use std::collections::BTreeMap; use std::rc::Rc; /// Alias for Rc> @@ -24,15 +24,18 @@ struct Scheduler { destroy: Vec>, create: Vec>, update: Vec>, - render_first: VecDeque>, - #[cfg(any(feature = "ssr", feature = "csr"))] - render: RenderScheduler, - - /// Stacks to ensure child calls are always before parent calls - rendered_first: Vec>, - #[cfg(feature = "csr")] - rendered: RenderedScheduler, + /// The Binary Tree Map guarantees components with lower id (parent) is rendered first and + /// no more than 1 render can be scheduled before a component is rendered. + /// + /// Parent can destroy child components but not otherwise, we can save unnecessary render by + /// rendering parent first. + render_first: BTreeMap>, + render: BTreeMap>, + + /// Binary Tree Map to guarantee children rendered are always called before parent calls + rendered_first: BTreeMap>, + rendered: BTreeMap>, } /// Execute closure with a mutable reference to the scheduler @@ -60,92 +63,33 @@ pub fn push(runnable: Box) { #[cfg(any(feature = "ssr", feature = "csr"))] mod feat_csr_ssr { use super::*; - - use std::collections::{hash_map::Entry, HashMap}; - /// Push a component creation, first render and first rendered [Runnable]s to be executed pub(crate) fn push_component_create( - create: impl Runnable + 'static, - first_render: impl Runnable + 'static, + component_id: usize, + create: Box, + first_render: Box, ) { with(|s| { - s.create.push(Box::new(create)); - s.render_first.push_back(Box::new(first_render)); + s.create.push(create); + s.render_first.insert(component_id, first_render); }); } /// Push a component destruction [Runnable] to be executed - pub(crate) fn push_component_destroy(runnable: impl Runnable + 'static) { - with(|s| s.destroy.push(Box::new(runnable))); + pub(crate) fn push_component_destroy(runnable: Box) { + with(|s| s.destroy.push(runnable)); } /// Push a component render and rendered [Runnable]s to be executed - pub(crate) fn push_component_render(component_id: usize, render: impl Runnable + 'static) { + pub(crate) fn push_component_render(component_id: usize, render: Box) { with(|s| { - s.render.schedule(component_id, Box::new(render)); + s.render.insert(component_id, render); }); } /// Push a component update [Runnable] to be executed - pub(crate) fn push_component_update(runnable: impl Runnable + 'static) { - with(|s| s.update.push(Box::new(runnable))); - } - - /// Task to be executed for specific component - struct QueueTask { - /// Tasks in the queue to skip for this component - skip: usize, - - /// Runnable to execute - runnable: Box, - } - - /// Scheduler for non-first component renders with deduplication - #[derive(Default)] - pub(super) struct RenderScheduler { - /// Task registry by component ID - tasks: HashMap, - - /// Task queue by component ID - queue: VecDeque, - } - - impl RenderScheduler { - /// Schedule render task execution - pub fn schedule(&mut self, component_id: usize, runnable: Box) { - self.queue.push_back(component_id); - match self.tasks.entry(component_id) { - Entry::Vacant(e) => { - e.insert(QueueTask { skip: 0, runnable }); - } - Entry::Occupied(mut e) => { - let v = e.get_mut(); - v.skip += 1; - - // Technically the 2 runners should be functionally identical, but might as well - // overwrite it for good measure, accounting for future changes. We have it here - // anyway. - v.runnable = runnable; - } - } - } - - /// Try to pop a task from the queue, if any - pub fn pop(&mut self) -> Option> { - while let Some(id) = self.queue.pop_front() { - match self.tasks.entry(id) { - Entry::Occupied(mut e) => { - let v = e.get_mut(); - if v.skip == 0 { - return Some(e.remove().runnable); - } - v.skip -= 1; - } - Entry::Vacant(_) => (), - } - } - None - } + pub(crate) fn push_component_update(runnable: Box) { + with(|s| s.update.push(runnable)); } } @@ -156,51 +100,19 @@ pub(crate) use feat_csr_ssr::*; mod feat_csr { use super::*; - use std::collections::HashMap; - pub(crate) fn push_component_rendered( component_id: usize, - rendered: impl Runnable + 'static, + rendered: Box, first_render: bool, ) { with(|s| { - let rendered = Box::new(rendered); - if first_render { - s.rendered_first.push(rendered); + s.rendered_first.insert(component_id, rendered); } else { - s.rendered.schedule(component_id, rendered); + s.rendered.insert(component_id, rendered); } }); } - - /// Deduplicating scheduler for component rendered calls with deduplication - #[derive(Default)] - pub(super) struct RenderedScheduler { - /// Task registry by component ID - tasks: HashMap>, - - /// Task stack by component ID - stack: Vec, - } - - impl RenderedScheduler { - /// Schedule rendered task execution - pub fn schedule(&mut self, component_id: usize, runnable: Box) { - if self.tasks.insert(component_id, runnable).is_none() { - self.stack.push(component_id); - } - } - - /// Drain all tasks into `dst`, if any - pub fn drain_into(&mut self, dst: &mut Vec>) { - for id in self.stack.drain(..).rev() { - if let Some(t) = self.tasks.remove(&id) { - dst.push(t); - } - } - } - } } #[cfg(feature = "csr")] @@ -278,12 +190,26 @@ impl Scheduler { // Create events can be batched, as they are typically just for object creation to_run.append(&mut self.create); + // These typically do nothing and don't spawn any other events - can be batched. + // Should be run only after all first renders have finished. + if !to_run.is_empty() { + return; + } + // First render must never be skipped and takes priority over main, because it may need // to init `NodeRef`s // // Should be processed one at time, because they can spawn more create and rendered events // for their children. - if let Some(r) = self.render_first.pop_front() { + // + // To be replaced with BTreeMap::pop_first once it is stable. + if let Some(r) = self + .render_first + .keys() + .next() + .cloned() + .and_then(|m| self.render_first.remove(&m)) + { to_run.push(r); } @@ -292,7 +218,12 @@ impl Scheduler { if !to_run.is_empty() { return; } - to_run.extend(self.rendered_first.drain(..).rev()); + + if !self.rendered_first.is_empty() { + let rendered_first = std::mem::take(&mut self.rendered_first); + // Children rendered lifecycle happen before parents. + to_run.extend(rendered_first.into_values().rev()); + } // Updates are after the first render to ensure we always have the entire child tree // rendered, once an update is processed. @@ -303,29 +234,37 @@ impl Scheduler { // Likely to cause duplicate renders via component updates, so placed before them to_run.append(&mut self.main); - #[cfg(any(feature = "ssr", feature = "csr"))] + // Run after all possible updates to avoid duplicate renders. + // + // Should be processed one at time, because they can spawn more create and first render + // events for their children. + if !to_run.is_empty() { + return; + } + + // To be replaced with BTreeMap::pop_first once it is stable. + // Should be processed one at time, because they can spawn more create and rendered events + // for their children. + if let Some(r) = self + .render + .keys() + .next() + .cloned() + .and_then(|m| self.render.remove(&m)) { - // Run after all possible updates to avoid duplicate renders. - // - // Should be processed one at time, because they can spawn more create and first render - // events for their children. - if !to_run.is_empty() { - return; - } + to_run.push(r); + } - if let Some(r) = self.render.pop() { - to_run.push(r); - } + // These typically do nothing and don't spawn any other events - can be batched. + // Should be run only after all renders have finished. + if !to_run.is_empty() { + return; } - #[cfg(feature = "csr")] - { - // These typically do nothing and don't spawn any other events - can be batched. - // Should be run only after all renders have finished. - if !to_run.is_empty() { - return; - } - self.rendered.drain_into(to_run); + if !self.rendered.is_empty() { + let rendered = std::mem::take(&mut self.rendered); + // Children rendered lifecycle happen before parents. + to_run.extend(rendered.into_values().rev()); } } } diff --git a/packages/yew/tests/failed_tests/base_component_impl-fail.stderr b/packages/yew/tests/failed_tests/base_component_impl-fail.stderr index 563ac8e341e..a7e774e1dc7 100644 --- a/packages/yew/tests/failed_tests/base_component_impl-fail.stderr +++ b/packages/yew/tests/failed_tests/base_component_impl-fail.stderr @@ -1,12 +1,12 @@ error[E0277]: the trait bound `Comp: yew::Component` is not satisfied - --> tests/failed_tests/base_component_impl-fail.rs:6:6 - | -6 | impl BaseComponent for Comp { - | ^^^^^^^^^^^^^ the trait `yew::Component` is not implemented for `Comp` - | - = note: required because of the requirements on the impl of `html::component::sealed::SealedBaseComponent` for `Comp` + --> tests/failed_tests/base_component_impl-fail.rs:6:6 + | +6 | impl BaseComponent for Comp { + | ^^^^^^^^^^^^^ the trait `yew::Component` is not implemented for `Comp` + | + = note: required because of the requirements on the impl of `html::component::sealed::SealedBaseComponent` for `Comp` note: required by a bound in `BaseComponent` - --> src/html/component/mod.rs - | - | pub trait BaseComponent: sealed::SealedBaseComponent + Sized + 'static { - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BaseComponent` + --> src/html/component/mod.rs + | + | pub trait BaseComponent: sealed::SealedBaseComponent + Sized + 'static { + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `BaseComponent`