1
0
Эх сурвалжийг харах

Fix: dont use bumpslab anymore, just box scopestates

Jonathan Kelley 1 жил өмнө
parent
commit
4240f8428c

+ 0 - 2
packages/core/Cargo.toml

@@ -33,8 +33,6 @@ log = { workspace = true }
 # Serialize the Edits for use in Webview/Liveview instances
 serde = { version = "1", features = ["derive"], optional = true }
 
-bumpslab = { version = "0.2.0" }
-
 [dev-dependencies]
 tokio = { workspace = true, features = ["full"] }
 dioxus = { workspace = true }

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

@@ -91,21 +91,21 @@ impl VirtualDom {
     // Note: This will not remove any ids from the arena
     pub(crate) fn drop_scope(&mut self, id: ScopeId, recursive: bool) {
         self.dirty_scopes.remove(&DirtyScope {
-            height: self.scopes[id].height,
+            height: self.scopes[id.0].height,
             id,
         });
 
         self.ensure_drop_safety(id);
 
         if recursive {
-            if let Some(root) = self.scopes[id].try_root_node() {
+            if let Some(root) = self.scopes[id.0].try_root_node() {
                 if let RenderReturn::Ready(node) = unsafe { root.extend_lifetime_ref() } {
                     self.drop_scope_inner(node)
                 }
             }
         }
 
-        let scope = &mut self.scopes[id];
+        let scope = &mut self.scopes[id.0];
 
         // Drop all the hooks once the children are dropped
         // this means we'll drop hooks bottom-up
@@ -116,7 +116,7 @@ impl VirtualDom {
             scope.tasks.remove(task_id);
         }
 
-        self.scopes.remove(id);
+        self.scopes.remove(id.0);
     }
 
     fn drop_scope_inner(&mut self, node: &VNode) {
@@ -137,7 +137,7 @@ impl VirtualDom {
 
     /// Descend through the tree, removing any borrowed props and listeners
     pub(crate) fn ensure_drop_safety(&self, scope_id: ScopeId) {
-        let scope = &self.scopes[scope_id];
+        let scope = &self.scopes[scope_id.0];
 
         // make sure we drop all borrowed props manually to guarantee that their drop implementation is called before we
         // run the hooks (which hold an &mut Reference)

+ 9 - 9
packages/core/src/diff.rs

@@ -15,7 +15,7 @@ use DynamicNode::*;
 
 impl<'b> VirtualDom {
     pub(super) fn diff_scope(&mut self, scope: ScopeId) {
-        let scope_state = &mut self.scopes[scope];
+        let scope_state = &mut self.scopes[scope.0];
 
         self.scope_stack.push(scope);
         unsafe {
@@ -191,7 +191,7 @@ impl<'b> VirtualDom {
         right.scope.set(Some(scope_id));
 
         // copy out the box for both
-        let old = self.scopes[scope_id].props.as_ref();
+        let old = self.scopes[scope_id.0].props.as_ref();
         let new: Box<dyn AnyProps> = right.props.take().unwrap();
         let new: Box<dyn AnyProps> = unsafe { std::mem::transmute(new) };
 
@@ -203,14 +203,14 @@ impl<'b> VirtualDom {
         }
 
         // First, move over the props from the old to the new, dropping old props in the process
-        self.scopes[scope_id].props = Some(new);
+        self.scopes[scope_id.0].props = Some(new);
 
         // Now run the component and diff it
         self.run_scope(scope_id);
         self.diff_scope(scope_id);
 
         self.dirty_scopes.remove(&DirtyScope {
-            height: self.scopes[scope_id].height,
+            height: self.scopes[scope_id.0].height,
             id: scope_id,
         });
     }
@@ -714,7 +714,7 @@ impl<'b> VirtualDom {
 
                     Component(comp) => {
                         let scope = comp.scope.get().unwrap();
-                        match unsafe { self.scopes[scope].root_node().extend_lifetime_ref() } {
+                        match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
                             RenderReturn::Ready(node) => self.push_all_real_nodes(node),
                             RenderReturn::Aborted(_node) => todo!(),
                         }
@@ -915,13 +915,13 @@ impl<'b> VirtualDom {
             .expect("VComponents to always have a scope");
 
         // Remove the component from the dom
-        match unsafe { self.scopes[scope].root_node().extend_lifetime_ref() } {
+        match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
             RenderReturn::Ready(t) => self.remove_node(t, gen_muts),
             RenderReturn::Aborted(placeholder) => self.remove_placeholder(placeholder, gen_muts),
         };
 
         // Restore the props back to the vcomponent in case it gets rendered again
-        let props = self.scopes[scope].props.take();
+        let props = self.scopes[scope.0].props.take();
         *comp.props.borrow_mut() = unsafe { std::mem::transmute(props) };
 
         // Now drop all the resouces
@@ -936,7 +936,7 @@ impl<'b> VirtualDom {
             Some(Placeholder(t)) => t.id.get().unwrap(),
             Some(Component(comp)) => {
                 let scope = comp.scope.get().unwrap();
-                match unsafe { self.scopes[scope].root_node().extend_lifetime_ref() } {
+                match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
                     RenderReturn::Ready(t) => self.find_first_element(t),
                     _ => todo!("cannot handle nonstandard nodes"),
                 }
@@ -952,7 +952,7 @@ impl<'b> VirtualDom {
             Some(Placeholder(t)) => t.id.get().unwrap(),
             Some(Component(comp)) => {
                 let scope = comp.scope.get().unwrap();
-                match unsafe { self.scopes[scope].root_node().extend_lifetime_ref() } {
+                match unsafe { self.scopes[scope.0].root_node().extend_lifetime_ref() } {
                     RenderReturn::Ready(t) => self.find_last_element(t),
                     _ => todo!("cannot handle nonstandard nodes"),
                 }

+ 1 - 1
packages/core/src/scheduler/wait.rs

@@ -20,7 +20,7 @@ impl VirtualDom {
         // If the task completes...
         if task.task.borrow_mut().as_mut().poll(&mut cx).is_ready() {
             // Remove it from the scope so we dont try to double drop it when the scope dropes
-            let scope = &self.scopes[task.scope];
+            let scope = &self.scopes[task.scope.0];
             scope.spawned_tasks.borrow_mut().remove(&id);
 
             // Remove it from the scheduler

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

@@ -16,9 +16,9 @@ impl VirtualDom {
         let parent = self.acquire_current_scope_raw();
         let entry = self.scopes.vacant_entry();
         let height = unsafe { parent.map(|f| (*f).height + 1).unwrap_or(0) };
-        let id = entry.key();
+        let id = ScopeId(entry.key());
 
-        entry.insert(ScopeState {
+        entry.insert(Box::new(ScopeState {
             parent,
             id,
             height,
@@ -35,13 +35,13 @@ impl VirtualDom {
             shared_contexts: Default::default(),
             borrowed_props: Default::default(),
             attributes_to_drop: Default::default(),
-        })
+        }))
     }
 
     fn acquire_current_scope_raw(&self) -> Option<*const ScopeState> {
         let id = self.scope_stack.last().copied()?;
-        let scope = self.scopes.get(id)?;
-        Some(scope)
+        let scope = self.scopes.get(id.0)?;
+        Some(scope.as_ref())
     }
 
     pub(crate) fn run_scope(&mut self, scope_id: ScopeId) -> &RenderReturn {
@@ -51,9 +51,9 @@ impl VirtualDom {
         self.ensure_drop_safety(scope_id);
 
         let new_nodes = unsafe {
-            self.scopes[scope_id].previous_frame().bump_mut().reset();
+            self.scopes[scope_id.0].previous_frame().bump_mut().reset();
 
-            let scope = &self.scopes[scope_id];
+            let scope = &self.scopes[scope_id.0];
             scope.suspended.set(false);
 
             scope.hook_idx.set(0);
@@ -65,7 +65,7 @@ impl VirtualDom {
             props.render(scope).extend_lifetime()
         };
 
-        let scope = &self.scopes[scope_id];
+        let scope = &self.scopes[scope_id.0];
 
         // We write on top of the previous frame and then make it the current by pushing the generation forward
         let frame = scope.previous_frame();

+ 0 - 92
packages/core/src/scopes.rs

@@ -9,15 +9,12 @@ use crate::{
     AnyValue, Attribute, AttributeValue, Element, Event, Properties, TaskId,
 };
 use bumpalo::{boxed::Box as BumpBox, Bump};
-use bumpslab::{BumpSlab, Slot};
 use rustc_hash::FxHashSet;
-use slab::{Slab, VacantEntry};
 use std::{
     any::{Any, TypeId},
     cell::{Cell, RefCell, UnsafeCell},
     fmt::{Arguments, Debug},
     future::Future,
-    ops::{Index, IndexMut},
     rc::Rc,
     sync::Arc,
 };
@@ -65,95 +62,6 @@ impl<'a, T> std::ops::Deref for Scoped<'a, T> {
 #[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, PartialOrd, Ord)]
 pub struct ScopeId(pub usize);
 
-/// A thin wrapper around a BumpSlab that uses ids to index into the slab.
-pub(crate) struct ScopeSlab {
-    slab: BumpSlab<ScopeState>,
-    // a slab of slots of stable pointers to the ScopeState in the bump slab
-    entries: Slab<Slot<'static, ScopeState>>,
-}
-
-impl Drop for ScopeSlab {
-    fn drop(&mut self) {
-        // Bump slab doesn't drop its contents, so we need to do it manually
-        for slot in self.entries.drain() {
-            self.slab.remove(slot);
-        }
-    }
-}
-
-impl Default for ScopeSlab {
-    fn default() -> Self {
-        Self {
-            slab: BumpSlab::new(),
-            entries: Slab::new(),
-        }
-    }
-}
-
-impl ScopeSlab {
-    pub(crate) fn get(&self, id: ScopeId) -> Option<&ScopeState> {
-        self.entries.get(id.0).map(|slot| unsafe { &*slot.ptr() })
-    }
-
-    pub(crate) fn get_mut(&mut self, id: ScopeId) -> Option<&mut ScopeState> {
-        self.entries
-            .get(id.0)
-            .map(|slot| unsafe { &mut *slot.ptr_mut() })
-    }
-
-    pub(crate) fn vacant_entry(&mut self) -> ScopeSlabEntry {
-        let entry = self.entries.vacant_entry();
-        ScopeSlabEntry {
-            slab: &mut self.slab,
-            entry,
-        }
-    }
-
-    pub(crate) fn remove(&mut self, id: ScopeId) {
-        self.slab.remove(self.entries.remove(id.0));
-    }
-
-    pub(crate) fn contains(&self, id: ScopeId) -> bool {
-        self.entries.contains(id.0)
-    }
-
-    pub(crate) fn iter(&self) -> impl Iterator<Item = &ScopeState> {
-        self.entries.iter().map(|(_, slot)| unsafe { &*slot.ptr() })
-    }
-}
-
-pub(crate) struct ScopeSlabEntry<'a> {
-    slab: &'a mut BumpSlab<ScopeState>,
-    entry: VacantEntry<'a, Slot<'static, ScopeState>>,
-}
-
-impl<'a> ScopeSlabEntry<'a> {
-    pub(crate) fn key(&self) -> ScopeId {
-        ScopeId(self.entry.key())
-    }
-
-    pub(crate) fn insert(self, scope: ScopeState) -> &'a ScopeState {
-        let slot = self.slab.push(scope);
-        // this is safe because the slot is only ever accessed with the lifetime of the borrow of the slab
-        let slot = unsafe { std::mem::transmute(slot) };
-        let entry = self.entry.insert(slot);
-        unsafe { &*entry.ptr() }
-    }
-}
-
-impl Index<ScopeId> for ScopeSlab {
-    type Output = ScopeState;
-    fn index(&self, id: ScopeId) -> &Self::Output {
-        self.get(id).unwrap()
-    }
-}
-
-impl IndexMut<ScopeId> for ScopeSlab {
-    fn index_mut(&mut self, id: ScopeId) -> &mut Self::Output {
-        self.get_mut(id).unwrap()
-    }
-}
-
 /// A component's state separate from its props.
 ///
 /// This struct exists to provide a common interface for all scopes without relying on generics.

+ 7 - 7
packages/core/src/virtual_dom.rs

@@ -5,7 +5,7 @@
 use crate::{
     any_props::VProps,
     arena::{ElementId, ElementRef},
-    innerlude::{DirtyScope, ErrorBoundary, Mutations, Scheduler, SchedulerMsg, ScopeSlab},
+    innerlude::{DirtyScope, ErrorBoundary, Mutations, Scheduler, SchedulerMsg},
     mutations::Mutation,
     nodes::RenderReturn,
     nodes::{Template, TemplateId},
@@ -176,7 +176,7 @@ use std::{any::Any, cell::Cell, collections::BTreeSet, future::Future, rc::Rc};
 pub struct VirtualDom {
     // Maps a template path to a map of byteindexes to templates
     pub(crate) templates: FxHashMap<TemplateId, FxHashMap<usize, Template<'static>>>,
-    pub(crate) scopes: ScopeSlab,
+    pub(crate) scopes: Slab<Box<ScopeState>>,
     pub(crate) dirty_scopes: BTreeSet<DirtyScope>,
     pub(crate) scheduler: Rc<Scheduler>,
 
@@ -281,14 +281,14 @@ impl VirtualDom {
     ///
     /// This is useful for inserting or removing contexts from a scope, or rendering out its root node
     pub fn get_scope(&self, id: ScopeId) -> Option<&ScopeState> {
-        self.scopes.get(id)
+        self.scopes.get(id.0).map(|f| f.as_ref())
     }
 
     /// Get the single scope at the top of the VirtualDom tree that will always be around
     ///
     /// This scope has a ScopeId of 0 and is the root of the tree
     pub fn base_scope(&self) -> &ScopeState {
-        self.scopes.get(ScopeId(0)).unwrap()
+        self.get_scope(ScopeId(0)).unwrap()
     }
 
     /// Build the virtualdom with a global context inserted into the base scope
@@ -303,7 +303,7 @@ impl VirtualDom {
     ///
     /// Whenever the VirtualDom "works", it will re-render this scope
     pub fn mark_dirty(&mut self, id: ScopeId) {
-        if let Some(scope) = self.scopes.get(id) {
+        if let Some(scope) = self.get_scope(id) {
             let height = scope.height;
             self.dirty_scopes.insert(DirtyScope { height, id });
         }
@@ -496,7 +496,7 @@ impl VirtualDom {
     pub fn replace_template(&mut self, template: Template<'static>) {
         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() {
+        for (_, 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
@@ -601,7 +601,7 @@ impl VirtualDom {
                 self.dirty_scopes.remove(&dirty);
 
                 // If the scope doesn't exist for whatever reason, then we should skip it
-                if !self.scopes.contains(dirty.id) {
+                if !self.scopes.contains(dirty.id.0) {
                     continue;
                 }
 

+ 4 - 0
packages/core/tests/fuzzing.rs

@@ -1,3 +1,5 @@
+#![cfg(not(miri))]
+
 use dioxus::prelude::Props;
 use dioxus_core::*;
 use std::{cell::Cell, collections::HashSet};
@@ -314,6 +316,7 @@ fn create_random_element(cx: Scope<DepthProps>) -> Element {
 }
 
 // test for panics when creating random nodes and templates
+#[cfg(not(miri))]
 #[test]
 fn create() {
     for _ in 0..1000 {
@@ -325,6 +328,7 @@ fn create() {
 
 // test for panics when diffing random nodes
 // This test will change the template every render which is not very realistic, but it helps stress the system
+#[cfg(not(miri))]
 #[test]
 fn diff() {
     for _ in 0..100000 {

+ 13 - 8
packages/core/tests/suspense.rs

@@ -1,17 +1,22 @@
 use dioxus::prelude::*;
 
-#[tokio::test]
-async fn it_works() {
+#[test]
+fn it_works() {
     // wait just a moment, not enough time for the boundary to resolve
 
-    let mut dom = VirtualDom::new(app);
-    _ = dom.rebuild();
-    dom.wait_for_suspense().await;
-    let out = dioxus_ssr::pre_render(&dom);
+    tokio::runtime::Builder::new_current_thread()
+        .build()
+        .unwrap()
+        .block_on(async {
+            let mut dom = VirtualDom::new(app);
+            _ = dom.rebuild();
+            dom.wait_for_suspense().await;
+            let out = dioxus_ssr::pre_render(&dom);
 
-    assert_eq!(out, "<div>Waiting for... child</div>");
+            assert_eq!(out, "<div>Waiting for... child</div>");
 
-    dbg!(out);
+            dbg!(out);
+        });
 }
 
 fn app(cx: Scope) -> Element {

+ 1 - 0
packages/core/tests/task.rs

@@ -5,6 +5,7 @@ use std::{sync::atomic::AtomicUsize, time::Duration};
 
 static POLL_COUNT: AtomicUsize = AtomicUsize::new(0);
 
+#[cfg(not(miri))]
 #[tokio::test]
 async fn it_works() {
     let mut dom = VirtualDom::new(app);