Browse Source

feat: move to slotmap

Jonathan Kelley 4 years ago
parent
commit
7665f2c6cf

+ 2 - 4
packages/core/Cargo.toml

@@ -13,9 +13,6 @@ description = "Core functionality for Dioxus - a concurrent renderer-agnostic Vi
 # todo: use wast for faster load/compile
 dioxus-core-macro = { path="../core-macro", version="0.1.1" }
 
-# Backs scopes and graphs between parent and children
-generational-arena = { version="0.2.8" }
-
 # Bumpalo is used as a micro heap backing each component
 bumpalo = { version="3.6.0", features=["collections", "boxed"] }
 
@@ -47,4 +44,5 @@ futures = "0.3.15"
 
 [features]
 default = []
-serialize = ["generational-arena/serde"]
+serialize = []
+# serialize = ["generational-arena/serde"]

+ 8 - 8
packages/core/src/arena.rs

@@ -4,15 +4,15 @@ use std::{
     rc::Rc,
 };
 
-use generational_arena::Arena;
-
 use crate::innerlude::*;
+use slotmap::{DefaultKey, SlotMap};
 
 #[derive(Clone)]
 pub struct ScopeArena(pub Rc<RefCell<ScopeArenaInner>>);
+pub type ScopeMap = SlotMap<DefaultKey, Scope>;
 
 pub struct ScopeArenaInner {
-    pub(crate) arena: UnsafeCell<Arena<Scope>>,
+    pub(crate) arena: UnsafeCell<ScopeMap>,
     locks: HashMap<ScopeIdx, MutStatus>,
 }
 
@@ -91,7 +91,7 @@ enum MutStatus {
 //     }
 // }
 impl ScopeArena {
-    pub fn new(arena: Arena<Scope>) -> Self {
+    pub fn new(arena: ScopeMap) -> Self {
         ScopeArena(Rc::new(RefCell::new(ScopeArenaInner {
             arena: UnsafeCell::new(arena),
             locks: Default::default(),
@@ -114,17 +114,17 @@ impl ScopeArena {
         scope.ok_or_else(|| Error::FatalInternal("Scope not found"))
     }
 
-    fn inner(&self) -> &Arena<Scope> {
+    fn inner(&self) -> &ScopeMap {
         todo!()
     }
 
-    fn inner_mut(&mut self) -> &mut Arena<Scope> {
+    fn inner_mut(&mut self) -> &mut ScopeMap {
         todo!()
     }
 
     /// THIS METHOD IS CURRENTLY UNSAFE
     /// THERE ARE NO CHECKS TO VERIFY THAT WE ARE ALLOWED TO DO THIS
-    pub fn with<T>(&self, f: impl FnOnce(&mut Arena<Scope>) -> T) -> Result<T> {
+    pub fn with<T>(&self, f: impl FnOnce(&mut ScopeMap) -> T) -> Result<T> {
         let inner = unsafe { &mut *self.0.borrow().arena.get() };
         Ok(f(inner))
         // todo!()
@@ -155,7 +155,7 @@ impl ScopeArena {
             .ok_or_else(|| Error::FatalInternal("Scope not found"))
     }
 
-    unsafe fn inner_unchecked<'s>() -> &'s mut Arena<Scope> {
+    unsafe fn inner_unchecked<'s>() -> &'s mut ScopeMap {
         todo!()
     }
 }

+ 30 - 12
packages/core/src/diff.rs

@@ -175,9 +175,19 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
                         }
                         1 => {
                             // replace
+                            self.create(&new.children[0]);
+                            self.dom.replace_with();
                         }
                         _ => {
+                            todo!()
                             // remove and mount many
+                            // self.remove_self_and_next_siblings(old)
+                            //
+                            // let iter = ChildIterator::new(new_node, self.components).rev();
+                            // for child in iter {}
+                            // for child in new.children.iter().skip(1).rev() {
+                            //     self.dom.remove();
+                            // }
                         }
                     }
                 }
@@ -231,8 +241,9 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
                             // Make sure we're dealing with the same component (by function pointer)
 
                             // Make sure the new component vnode is referencing the right scope id
-                            let scope_id = old.ass_scope.borrow().clone();
-                            *new.ass_scope.borrow_mut() = scope_id;
+                            let scope_id = old.ass_scope.get();
+                            new.ass_scope.set(scope_id);
+                            new.mounted_root.set(old.mounted_root.get());
 
                             // make sure the component's caller function is up to date
                             self.components
@@ -243,6 +254,7 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
 
                             // React doesn't automatically memoize, but we do.
                             // The cost is low enough to make it worth checking
+                            // Rust produces fairly performant comparison methods, sometimes SIMD accelerated
                             let should_render = match old.comparator {
                                 Some(comparator) => comparator(new),
                                 None => true,
@@ -277,7 +289,7 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
                             // self.create_and_repalce(new_node, old.mounted_root.get());
 
                             // Now we need to remove the old scope and all of its descendents
-                            let old_scope = old.ass_scope.borrow().as_ref().unwrap().clone();
+                            let old_scope = old.ass_scope.get().unwrap();
                             self.destroy_scopes(old_scope);
                         }
                     }
@@ -408,7 +420,7 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
                 let idx = self
                     .components
                     .with(|components| {
-                        components.insert_with(|new_idx| {
+                        components.insert_with_key(|new_idx| {
                             let parent_scope = self.components.try_get(parent_idx).unwrap();
                             let height = parent_scope.height + 1;
                             Scope::new(
@@ -437,7 +449,7 @@ impl<'real, 'bump, Dom: RealDom<'bump>> DiffMachine<'real, 'bump, Dom> {
                 let new_component = inner.get_mut(idx).unwrap();
 
                 // Actually initialize the caller's slot with the right address
-                *component.ass_scope.borrow_mut() = Some(idx);
+                component.ass_scope.set(Some(idx));
 
                 // Run the scope for one iteration to initialize it
                 new_component.run_scope().unwrap();
@@ -1108,20 +1120,20 @@ impl<'a, 'bump, Dom: RealDom<'bump>> DiffMachine<'a, 'bump, Dom> {
             // self.dom.go_to_sibling(i);
             // [... parent this_child]
 
-            let did = old_child.get_mounted_id().unwrap();
+            let did = old_child.get_mounted_id(self.components).unwrap();
             if did.0 == 0 {
-                log::debug!("Root is bad {:#?}", old_child);
+                log::debug!("Root is bad: {:#?}", old_child);
             }
             self.dom.push_root(did);
             self.diff_node(old_child, new_child);
 
-            let old_id = old_child.get_mounted_id().unwrap();
-            let new_id = new_child.get_mounted_id().unwrap();
+            let old_id = old_child.get_mounted_id(self.components).unwrap();
+            let new_id = new_child.get_mounted_id(self.components).unwrap();
 
             log::debug!(
                 "pushed root. {:?}, {:?}",
-                old_child.get_mounted_id().unwrap(),
-                new_child.get_mounted_id().unwrap()
+                old_child.get_mounted_id(self.components).unwrap(),
+                new_child.get_mounted_id(self.components).unwrap()
             );
             if old_id != new_id {
                 log::debug!("Mismatch: {:?}", new_child);
@@ -1261,6 +1273,12 @@ impl<'a> ChildIterator<'a> {
     }
 }
 
+// impl<'a> DoubleEndedIterator for ChildIterator<'a> {
+//     fn next_back(&mut self) -> Option<Self::Item> {
+//         todo!()
+//     }
+// }
+
 impl<'a> Iterator for ChildIterator<'a> {
     type Item = &'a VNode<'a>;
 
@@ -1296,7 +1314,7 @@ impl<'a> Iterator for ChildIterator<'a> {
 
                     // For components, we load their root and push them onto the stack
                     VNode::Component(sc) => {
-                        let scope = self.scopes.try_get(sc.ass_scope.borrow().unwrap()).unwrap();
+                        let scope = self.scopes.try_get(sc.ass_scope.get().unwrap()).unwrap();
 
                         // Simply swap the current node on the stack with the root of the component
                         *node = scope.root();

+ 16 - 14
packages/core/src/nodes.rs

@@ -4,6 +4,7 @@
 //! These VNodes should be *very* cheap and *very* fast to construct - building a full tree should be insanely quick.
 
 use crate::{
+    arena::ScopeArena,
     events::VirtualEvent,
     innerlude::{Context, Properties, Scope, ScopeIdx, FC},
     nodebuilder::{text3, NodeCtx},
@@ -160,13 +161,13 @@ impl<'a> VNode<'a> {
         }
     }
 
-    pub fn get_mounted_id(&self) -> Option<RealDomNode> {
+    pub fn get_mounted_id(&self, components: &ScopeArena) -> Option<RealDomNode> {
         match self {
             VNode::Element(el) => Some(el.dom_id.get()),
             VNode::Text(te) => Some(te.dom_id.get()),
             VNode::Fragment(_) => todo!(),
             VNode::Suspended { .. } => todo!(),
-            VNode::Component(_) => todo!(),
+            VNode::Component(el) => Some(el.mounted_root.get()),
         }
     }
 }
@@ -312,9 +313,9 @@ pub struct VComponent<'src> {
 
     pub mounted_root: Cell<RealDomNode>,
 
-    pub ass_scope: RefCell<VCompAssociatedScope>,
+    pub ass_scope: Cell<VCompAssociatedScope>,
 
-    // pub comparator: Rc<dyn Fn(&VComponent) -> bool + 'src>,
+    // todo: swap the RC out with
     pub caller: Rc<dyn Fn(&Scope) -> VNode>,
 
     pub children: &'src [VNode<'src>],
@@ -347,8 +348,8 @@ impl<'a> VComponent<'a> {
         let props = bump.alloc(props);
         let raw_props = props as *const P as *const ();
 
-        let comparator: Option<&dyn Fn(&VComponent) -> bool> = {
-            Some(bump.alloc(move |other: &VComponent| {
+        let comparator: Option<&dyn Fn(&VComponent) -> bool> = Some(bump.alloc_with(|| {
+            move |other: &VComponent| {
                 // Safety:
                 // ------
                 //
@@ -358,6 +359,7 @@ impl<'a> VComponent<'a> {
                 // - Lifetime of P borrows from its parent
                 // - The parent scope still exists when method is called
                 // - Casting from T to *const () is portable
+                // - Casting raw props to P can only happen when P is static
                 //
                 // Explanation:
                 //   We are guaranteed that the props will be of the same type because
@@ -378,7 +380,12 @@ impl<'a> VComponent<'a> {
                 } else {
                     false
                 }
-            }))
+            }
+        }));
+
+        let key = match key {
+            Some(key) => NodeKey::new(key),
+            None => NodeKey(None),
         };
 
         Self {
@@ -386,11 +393,8 @@ impl<'a> VComponent<'a> {
             comparator,
             raw_props,
             children,
-            ass_scope: RefCell::new(None),
-            key: match key {
-                Some(key) => NodeKey::new(key),
-                None => NodeKey(None),
-            },
+            ass_scope: Cell::new(None),
+            key,
             caller: create_closure(component, raw_props),
             mounted_root: Cell::new(RealDomNode::empty()),
         }
@@ -403,11 +407,9 @@ fn create_closure<'a, P: 'a>(
     component: FC<P>,
     raw_props: *const (),
 ) -> Rc<dyn for<'r> Fn(&'r Scope) -> VNode<'r>> {
-    // ) -> impl for<'r> Fn(&'r Scope) -> VNode<'r> {
     let g: Captured = Rc::new(move |scp: &Scope| -> VNode {
         // cast back into the right lifetime
         let safe_props: &'_ P = unsafe { &*(raw_props as *const P) };
-        // let cx: Context<P2> = todo!();
         let cx: Context<P> = Context {
             props: safe_props,
             scope: scp,

+ 6 - 4
packages/core/src/virtual_dom.rs

@@ -22,7 +22,8 @@
 use crate::{arena::ScopeArena, innerlude::*};
 use bumpalo::Bump;
 use futures::FutureExt;
-use generational_arena::Arena;
+use slotmap::DefaultKey;
+use slotmap::SlotMap;
 use std::{
     any::{Any, TypeId},
     cell::{Cell, RefCell},
@@ -33,7 +34,8 @@ use std::{
     pin::Pin,
     rc::{Rc, Weak},
 };
-pub type ScopeIdx = generational_arena::Index;
+pub type ScopeIdx = DefaultKey;
+// pub type ScopeIdx = generational_arena::Index;
 
 /// An integrated virtual node system that progresses events and diffs UI trees.
 /// Differences are converted into patches which a renderer can use to draw the UI.
@@ -148,7 +150,7 @@ impl VirtualDom {
     /// let dom = VirtualDom::new(Example);
     /// ```
     pub fn new_with_props<P: Properties + 'static>(root: FC<P>, root_props: P) -> Self {
-        let components = ScopeArena::new(Arena::new());
+        let components = ScopeArena::new(SlotMap::new());
 
         // Normally, a component would be passed as a child in the RSX macro which automatically produces OpaqueComponents
         // Here, we need to make it manually, using an RC to force the Weak reference to stick around for the main scope.
@@ -173,7 +175,7 @@ impl VirtualDom {
 
         let base_scope = components
             .with(|arena| {
-                arena.insert_with(move |myidx| {
+                arena.insert_with_key(move |myidx| {
                     let event_channel = _event_queue.new_channel(0, myidx);
                     Scope::new(caller_ref, myidx, None, 0, event_channel, link, &[])
                 })

+ 1 - 0
packages/web/examples/framework_benchmark.rs

@@ -16,6 +16,7 @@ use dioxus_web::WebsysRenderer;
 fn main() {
     wasm_logger::init(wasm_logger::Config::new(log::Level::Debug));
     console_error_panic_hook::set_once();
+    log::debug!("starting!");
     wasm_bindgen_futures::spawn_local(WebsysRenderer::start(App));
 }
 

+ 78 - 80
packages/web/src/new.rs

@@ -84,7 +84,7 @@ impl WebsysDom {
     }
 }
 
-impl dioxus_core::diff::RealDom for WebsysDom {
+impl<'a> dioxus_core::diff::RealDom<'a> for WebsysDom {
     fn push_root(&mut self, root: dioxus_core::virtual_dom::RealDomNode) {
         log::debug!("Called [push_root] {:?}", root);
         let key: DefaultKey = KeyData::from_ffi(root.0).into();
@@ -161,7 +161,7 @@ impl dioxus_core::diff::RealDom for WebsysDom {
     }
 
     fn create_placeholder(&mut self) -> RealDomNode {
-        self.create_element("pre")
+        self.create_element("pre", None)
     }
     fn create_text_node(&mut self, text: &str) -> dioxus_core::virtual_dom::RealDomNode {
         // let nid = self.node_counter.next();
@@ -179,38 +179,30 @@ impl dioxus_core::diff::RealDom for WebsysDom {
         RealDomNode::new(nid)
     }
 
-    fn create_element(&mut self, tag: &str) -> dioxus_core::virtual_dom::RealDomNode {
-        let el = self
-            .document
-            .create_element(tag)
-            .unwrap()
-            .dyn_into::<Node>()
-            .unwrap();
-
-        self.stack.push(el.clone());
-        // let nid = self.node_counter.?next();
-        let nid = self.nodes.insert(el).data().as_ffi();
-        log::debug!("Called [`create_element`]: {}, {:?}", tag, nid);
-        RealDomNode::new(nid)
-    }
-
-    fn create_element_ns(
+    fn create_element(
         &mut self,
         tag: &str,
-        namespace: &str,
+        ns: Option<&'static str>,
     ) -> dioxus_core::virtual_dom::RealDomNode {
-        let el = self
-            .document
-            .create_element_ns(Some(namespace), tag)
-            .unwrap()
-            .dyn_into::<Node>()
-            .unwrap();
+        let el = match ns {
+            Some(ns) => self
+                .document
+                .create_element_ns(Some(ns), tag)
+                .unwrap()
+                .dyn_into::<Node>()
+                .unwrap(),
+            None => self
+                .document
+                .create_element(tag)
+                .unwrap()
+                .dyn_into::<Node>()
+                .unwrap(),
+        };
 
         self.stack.push(el.clone());
+        // let nid = self.node_counter.?next();
         let nid = self.nodes.insert(el).data().as_ffi();
-        // let nid = self.node_counter.next();
-        // self.nodes.insert(nid, el);
-        log::debug!("Called [`create_element_ns`]: {:}", nid);
+        log::debug!("Called [`create_element`]: {}, {:?}", tag, nid);
         RealDomNode::new(nid)
     }
 
@@ -252,60 +244,11 @@ impl dioxus_core::diff::RealDom for WebsysDom {
             entry.0 += 1;
         } else {
             let trigger = self.trigger.clone();
+
             let handler = Closure::wrap(Box::new(move |event: &web_sys::Event| {
                 // "Result" cannot be received from JS
                 // Instead, we just build and immediately execute a closure that returns result
-                let res = || -> anyhow::Result<EventTrigger> {
-                    log::debug!("Handling event!");
-
-                    let target = event
-                        .target()
-                        .expect("missing target")
-                        .dyn_into::<Element>()
-                        .expect("not a valid element");
-
-                    let typ = event.type_();
-                    use anyhow::Context;
-                    let val: String = target
-                        .get_attribute(&format!("dioxus-event-{}", typ))
-                        .context("")?;
-
-                    let mut fields = val.splitn(4, ".");
-
-                    let gi_id = fields
-                        .next()
-                        .and_then(|f| f.parse::<usize>().ok())
-                        .context("")?;
-                    let gi_gen = fields
-                        .next()
-                        .and_then(|f| f.parse::<u64>().ok())
-                        .context("")?;
-                    let el_id = fields
-                        .next()
-                        .and_then(|f| f.parse::<usize>().ok())
-                        .context("")?;
-                    let real_id = fields
-                        .next()
-                        .and_then(|f| f.parse::<u64>().ok().map(RealDomNode::new))
-                        .context("")?;
-
-                    // Call the trigger
-                    log::debug!(
-                        "decoded gi_id: {},  gi_gen: {},  li_idx: {}",
-                        gi_id,
-                        gi_gen,
-                        el_id
-                    );
-
-                    let triggered_scope = ScopeIdx::from_raw_parts(gi_id, gi_gen);
-                    Ok(EventTrigger::new(
-                        virtual_event_from_websys_event(event),
-                        triggered_scope,
-                        real_id,
-                    ))
-                };
-
-                match res() {
+                match decode_trigger(event) {
                     Ok(synthetic_event) => trigger.as_ref()(synthetic_event),
                     Err(_) => log::error!("Error decoding Dioxus event attribute."),
                 };
@@ -330,7 +273,7 @@ impl dioxus_core::diff::RealDom for WebsysDom {
         self.stack.top().set_text_content(Some(text))
     }
 
-    fn set_attribute(&mut self, name: &str, value: &str, is_namespaced: bool) {
+    fn set_attribute(&mut self, name: &str, value: &str, ns: Option<&str>) {
         log::debug!("Called [`set_attribute`]: {}, {}", name, value);
         if name == "class" {
             if let Some(el) = self.stack.top().dyn_ref::<Element>() {
@@ -601,3 +544,58 @@ fn virtual_event_from_websys_event(event: &web_sys::Event) -> VirtualEvent {
         _ => VirtualEvent::OtherEvent,
     }
 }
+
+/// This function decodes a websys event and produces an EventTrigger
+/// With the websys implementation, we attach a unique key to the nodes
+fn decode_trigger(event: &web_sys::Event) -> anyhow::Result<EventTrigger> {
+    log::debug!("Handling event!");
+
+    let target = event
+        .target()
+        .expect("missing target")
+        .dyn_into::<Element>()
+        .expect("not a valid element");
+
+    let typ = event.type_();
+
+    use anyhow::Context;
+
+    // The error handling here is not very descriptive and needs to be replaced with a zero-cost error system
+    let val: String = target
+        .get_attribute(&format!("dioxus-event-{}", typ))
+        .context("")?;
+
+    let mut fields = val.splitn(4, ".");
+
+    let gi_id = fields
+        .next()
+        .and_then(|f| f.parse::<usize>().ok())
+        .context("")?;
+    let gi_gen = fields
+        .next()
+        .and_then(|f| f.parse::<u64>().ok())
+        .context("")?;
+    let el_id = fields
+        .next()
+        .and_then(|f| f.parse::<usize>().ok())
+        .context("")?;
+    let real_id = fields
+        .next()
+        .and_then(|f| f.parse::<u64>().ok().map(RealDomNode::new))
+        .context("")?;
+
+    // Call the trigger
+    log::debug!(
+        "decoded gi_id: {},  gi_gen: {},  li_idx: {}",
+        gi_id,
+        gi_gen,
+        el_id
+    );
+
+    let triggered_scope = ScopeIdx::from_raw_parts(gi_id, gi_gen);
+    Ok(EventTrigger::new(
+        virtual_event_from_websys_event(event),
+        triggered_scope,
+        real_id,
+    ))
+}