WIP: Use freelist to avoid frequent heap allocations

build.rs

@@ -61,6 +61,10 @@ fn build_v8() {
     vec!["is_debug=false".to_string()]
   };
 
+  if !cargo_gn::is_debug() {
+    gn_args.push("v8_enable_handle_zapping=false".to_string());
+  }
+
   if let Some(clang_base_path) = find_compatible_system_clang() {
     println!("clang_base_path {}", clang_base_path.display());
     gn_args.push(format!("clang_base_path={:?}", clang_base_path));

src/isolate.rs

@@ -142,6 +142,7 @@ impl Isolate {
     let (raw_create_params, create_param_allocations) = params.finalize();
     let cxx_isolate = unsafe { v8__Isolate__New(&raw_create_params) };
     let mut owned_isolate = OwnedIsolate::new(cxx_isolate);
+    ScopeData::create_scope_stack(&mut owned_isolate);
     owned_isolate.create_annex(create_param_allocations);
     owned_isolate
   }
@@ -201,27 +202,20 @@ impl Isolate {
   }
 
   /// Get a raw reference to the most recently entered scope.
-  pub(crate) fn get_current_scope(&self) -> Option<NonNull<ScopeData>> {
+  pub(crate) fn get_active_scope(&self) -> Option<NonNull<ScopeData>> {
     unsafe { NonNull::new(v8__Isolate__GetData(self, 1) as *mut ScopeData) }
   }
 
   /// Updates the most recently entered scope.
-  pub(crate) fn set_current_scope(
+  pub(crate) fn set_active_scope(
     &mut self,
-    current_scope: Option<NonNull<ScopeData>>,
+    active_scope: Option<NonNull<ScopeData>>,
   ) {
     let data =
-      current_scope.map(NonNull::as_ptr).unwrap_or_else(null_mut) as *mut _;
+      active_scope.map(NonNull::as_ptr).unwrap_or_else(null_mut) as *mut _;
     unsafe { v8__Isolate__SetData(self, 1, data) }
   }
 
-  /// Resets the scope stack, popping any remaining scopes that are no longer
-  /// in use but which have not yet been cleaned up. Note that calling this
-  /// function while a scope is active causes a panic.
-  pub(crate) fn reset_scopes(&mut self) {
-    ScopeData::reset(self)
-  }
-
   /// Get mutable reference to embedder data.
   pub fn get_slot_mut<T: 'static>(&self) -> Option<RefMut<T>> {
     let cell = self.get_annex().slots.get(&TypeId::of::<T>())?;
@@ -348,11 +342,13 @@ impl Isolate {
   /// Disposes the isolate.  The isolate must not be entered by any
   /// thread to be disposable.
   unsafe fn dispose(&mut self) {
-    let annex = self.get_annex_mut();
+    // Drop the scope stack.
+    ScopeData::drop_scope_stack(self);
 
     // Set the `isolate` pointer inside the annex struct to null, so any
     // IsolateHandle that outlives the isolate will know that it can't call
     // methods on the isolate.
+    let annex = self.get_annex_mut();
     {
       let _lock = annex.isolate_mutex.lock().unwrap();
       annex.isolate = null_mut();
@@ -554,7 +550,6 @@ impl OwnedIsolate {
 
 impl Drop for OwnedIsolate {
   fn drop(&mut self) {
-    self.reset_scopes();
     unsafe { self.cxx_isolate.as_mut().dispose() }
   }
 }

src/scope.rs

@@ -3,6 +3,7 @@ use std::cell::Cell;
 use std::marker::PhantomData;
 use std::mem::replace;
 use std::mem::size_of;
+use std::mem::take;
 use std::mem::MaybeUninit;
 use std::num::NonZeroUsize;
 use std::ops::Deref;
@@ -562,20 +563,79 @@ pub(crate) mod data {
 
   #[derive(Debug)]
   pub struct ScopeData {
+    // The first three fields do not change after initialization, and are even
+    // valid when a `ScopeData` allocation is shelved on the freelist.
+    prev_active: Option<NonNull<ScopeData>>,
+    next_free: Option<Box<ScopeData>>,
     pub(super) isolate: NonNull<Isolate>,
+    // The following fields are only valid for entered scopes.
     pub(super) context: Cell<Option<NonNull<Context>>>,
     pub(super) escape_slot: Option<NonNull<raw::EscapeSlot>>,
-    parent: Option<NonNull<ScopeData>>,
     deferred_drop: bool,
     type_specific_data: ScopeTypeSpecificData,
   }
 
+  impl Default for ScopeData {
+    fn default() -> Self {
+      let mut buf = MaybeUninit::<Self>::zeroed();
+      let p = buf.as_mut_ptr();
+      unsafe {
+        ptr::write(&mut (*p).isolate, NonNull::dangling());
+        ptr::write(&mut (*p).type_specific_data, Default::default());
+        buf.assume_init()
+      }
+    }
+  }
+
   impl ScopeData {
+    /// Initializes the scope stack by creating a 'dummy' frame at the very
+    /// bottom. This makes it possible to store the freelist of recyclable
+    // ScopeData object even when no scope is entered.
+    pub(crate) fn create_scope_stack(isolate: &mut Isolate) {
+      let isolate_nn = unsafe { NonNull::new_unchecked(isolate) };
+
+      let root_data = Self {
+        isolate: isolate_nn,
+        ..Self::default()
+      };
+      let root_data = Box::new(root_data);
+      let root_data = Box::leak(root_data);
+      let root_data = NonNull::from(root_data);
+
+      debug_assert!(isolate.get_active_scope().is_none());
+      isolate.set_active_scope(Some(root_data));
+    }
+
+    /// Clears the scope stack, popping any remaining scopes that are no longer
+    /// in use but which have not yet been cleaned up. Note that calling this
+    /// function while a scope is active causes a panic.
+    pub(crate) fn clear_scope_stack(isolate: &mut Isolate) {
+      loop {
+        match isolate.get_active_scope() {
+          Some(s) if unsafe { s.as_ref().prev_active.is_none() } => break,
+          Some(mut s) if unsafe { s.as_ref() }.deferred_drop => unsafe {
+            s.as_mut().drop_scope_data()
+          },
+          Some(_) => panic!("can't exit scope while entered"),
+          None => unreachable!(),
+        }
+      }
+    }
+
+    /// Drops the scope stack; this function should be called only when an
+    /// isolate is being dropped.
+    pub(crate) fn drop_scope_stack(isolate: &mut Isolate) {
+      Self::clear_scope_stack(isolate);
+      let root_data = isolate.get_active_scope().unwrap().as_ptr();
+      unsafe { Box::from_raw(root_data) };
+      isolate.set_active_scope(None);
+    }
+
     pub(super) fn new_context_scope<'s>(
       isolate: &'s mut Isolate,
       context: Local<'s, Context>,
     ) -> &'s mut Self {
-      Self::new_with(isolate, move |data| {
+      Self::new_scope_data_with(isolate, move |data| {
         data.context.set(Some(context.as_non_null()));
         data.type_specific_data =
           ScopeTypeSpecificData::ContextScope(raw::ContextScope::zeroed());
@@ -587,7 +647,7 @@ pub(crate) mod data {
     }
 
     pub(super) fn new_handle_scope(isolate: &mut Isolate) -> &mut Self {
-      Self::new_with(isolate, |data| {
+      Self::new_scope_data_with(isolate, |data| {
         data.type_specific_data =
           ScopeTypeSpecificData::HandleScope(raw::HandleScope::zeroed());
         match &mut data.type_specific_data {
@@ -602,7 +662,7 @@ pub(crate) mod data {
     pub(super) fn new_escapable_handle_scope(
       isolate: &mut Isolate,
     ) -> &mut Self {
-      Self::new_with(isolate, |data| {
+      Self::new_scope_data_with(isolate, |data| {
         data.type_specific_data = ScopeTypeSpecificData::EscapableHandleScope(
           raw::EscapableHandleScope::zeroed(),
         );
@@ -619,64 +679,95 @@ pub(crate) mod data {
       isolate: &'s mut Isolate,
       maybe_current_context: Option<Local<'s, Context>>,
     ) -> &'s mut Self {
-      Self::new_with(isolate, |data| {
+      Self::new_scope_data_with(isolate, |data| {
         data
           .context
           .set(maybe_current_context.map(|cx| cx.as_non_null()));
       })
     }
 
-    // TODO(piscisaureus): use something more efficient than a separate heap
-    // allocation for every scope.
-    fn new_with<F>(isolate: &mut Isolate, init_fn: F) -> &mut Self
+    fn new_scope_data_with<F>(isolate: &mut Isolate, init_fn: F) -> &mut Self
     where
       F: FnOnce(&mut Self),
     {
       let isolate_nn = unsafe { NonNull::new_unchecked(isolate) };
-      let mut parent = isolate.get_current_scope();
-      let context = parent
-        .as_mut()
-        .map(|p| unsafe { p.as_mut() })
-        .and_then(|p| p.context.get())
-        .map(NonNull::from);
-      let escape_slot = parent
-        .as_mut()
-        .map(|p| unsafe { p.as_mut() })
-        .and_then(|p| p.escape_slot)
-        .map(NonNull::from);
-      let data = Self {
-        isolate: isolate_nn,
-        parent,
-        context: Cell::new(context),
-        escape_slot,
+      let prev_active_nn = isolate.get_active_scope();
+      let scope_data_mut = match prev_active_nn
+        .map(NonNull::as_ptr)
+        .map(|p| unsafe { &mut (*p).next_free })
+        .unwrap()
+      {
+        Some(next_free) => {
+          // Recycle an unused `Box<ScopeData>` allocation.
+          debug_assert_eq!(next_free.isolate, isolate_nn);
+          debug_assert!(next_free.type_specific_data.is_none());
+          next_free.as_mut()
+        }
+        next_free @ None => {
+          // Allocate a new `Box<ScopeData>`.
+          next_free.replace(Box::new(Self {
+            isolate: isolate_nn,
+            prev_active: prev_active_nn,
+            ..Self::default()
+          }));
+          next_free.as_mut().unwrap()
+        }
+      };
+      *scope_data_mut = ScopeData {
+        context: prev_active_nn
+          .map(NonNull::as_ptr)
+          .map(|p| unsafe { &mut *p })
+          .and_then(|p| p.context.get())
+          .into(),
+        escape_slot: prev_active_nn
+          .map(NonNull::as_ptr)
+          .map(|p| unsafe { &mut *p })
+          .and_then(|p| p.escape_slot)
+          .map(NonNull::from),
         deferred_drop: false,
         type_specific_data: ScopeTypeSpecificData::default(),
+        ..take(scope_data_mut)
       };
-      let mut data_box = Box::new(data);
-      (init_fn)(&mut *data_box);
-      let data_ptr = Box::into_raw(data_box);
-      isolate.set_current_scope(NonNull::new(data_ptr));
-      unsafe { &mut *data_ptr }
+      (init_fn)(scope_data_mut);
+      isolate.set_active_scope(NonNull::new(scope_data_mut));
+      scope_data_mut
     }
 
-    fn drop(&mut self) {
-      // Make our parent scope 'current' again.
-      let parent = self.parent;
-      let isolate = self.get_isolate_mut();
-      isolate.set_current_scope(parent);
+    fn touch_scope_data(self_nn: NonNull<ScopeData>) {
+      loop {
+        let active_scope =
+          unsafe { self_nn.as_ref() }.get_isolate().get_active_scope();
+        match active_scope {
+          Some(active_scope_nn) if active_scope_nn == self_nn => break,
+          Some(mut active_scope_nn)
+            if unsafe { active_scope_nn.as_ref().deferred_drop } =>
+          unsafe { active_scope_nn.as_mut().drop_scope_data() }
+          _ => panic!("can't use scope that has been shadowed by another"),
+        }
+      }
+    }
+
+    fn drop_scope_data(&mut self) {
+      // Clear out the scope type specific data field. The other fields don't
+      // have destructors and there's no need to do any cleanup on them.
+      take(&mut self.type_specific_data);
 
-      // Turn the &mut self pointer back into a box and drop it.
-      let _ = unsafe { Box::from_raw(self) };
+      // Reactivate the previously active scope.
+      let prev_active = self.prev_active;
+      debug_assert!(prev_active.is_some());
+      let isolate = self.get_isolate_mut();
+      isolate.set_active_scope(prev_active);
     }
 
     pub(super) fn notify_scope_dropped(&mut self) {
-      // This function is called when the `api::Scope` object is dropped.
-      // With regard to (escapable) handle scopes: the Rust borrow checker
-      // allows these to be dropped before all the local handles that were
-      // created inside the HandleScope have gone out of scope. In order to
-      // avoid turning these locals into invalid references the HandleScope is
-      // kept alive for now -- it'll be actually dropped when the user touches
-      // the HandleScope's parent scope.
+      // This function is called when any of the publicly exported scope.
+      // objects (e.g `HandleScope`, `ContextScope`) is dropped.
+      // With respect to (Escapable)HandleScope: the Rust borrow checker allows
+      // them to be dropped a little bit earlier than the local handles that
+      // were created in these handle-keeping scopes. In order to avoid turning
+      // these locals into invalid references, a (Escapable)HandleScope will be
+      // kept alive for now; It'll be actually dropped when the user touches
+      // the parent scope.
       match &self.type_specific_data {
         ScopeTypeSpecificData::HandleScope(_)
         | ScopeTypeSpecificData::EscapableHandleScope(_) => {
@@ -686,7 +777,7 @@ pub(crate) mod data {
         }
         _ => {
           // Regular, immediate drop.
-          self.drop();
+          self.drop_scope_data();
         }
       }
     }
@@ -699,13 +790,13 @@ pub(crate) mod data {
 
     pub(super) fn get<S: api::Scope>(scope: &S) -> &Self {
       let self_nn = unsafe { *(scope as *const _ as *const NonNull<Self>) };
-      Self::touch(self_nn);
+      Self::touch_scope_data(self_nn);
       unsafe { &*self_nn.as_ptr() }
     }
 
     pub(super) fn get_mut<S: api::Scope>(scope: &mut S) -> &mut Self {
       let self_nn = unsafe { *(scope as *mut _ as *mut NonNull<Self>) };
-      Self::touch(self_nn);
+      Self::touch_scope_data(self_nn);
       unsafe { &mut *self_nn.as_ptr() }
     }
 
@@ -720,34 +811,6 @@ pub(crate) mod data {
     pub(super) fn get_isolate_ptr(&self) -> *mut Isolate {
       self.isolate.as_ptr()
     }
-
-    fn touch(self_nn: NonNull<ScopeData>) {
-      loop {
-        let current_scope = unsafe { self_nn.as_ref() }
-          .get_isolate()
-          .get_current_scope();
-        match current_scope {
-          Some(current_scope_nn) if current_scope_nn == self_nn => break,
-          Some(mut current_scope_nn)
-            if unsafe { current_scope_nn.as_ref().deferred_drop } =>
-          unsafe { current_scope_nn.as_mut().drop() }
-          _ => panic!("can't use scope that has been shadowed by another"),
-        }
-      }
-    }
-
-    pub(crate) fn reset(isolate: &mut Isolate) {
-      loop {
-        let current_scope = isolate.get_current_scope();
-        match current_scope {
-          None => break,
-          Some(mut current_scope_nn)
-            if unsafe { current_scope_nn.as_ref().deferred_drop } =>
-          unsafe { current_scope_nn.as_mut().drop() }
-          _ => panic!("can't reset scopes while they're in use"),
-        }
-      }
-    }
   }
 
   #[derive(Debug)]
@@ -763,6 +826,15 @@ pub(crate) mod data {
       Self::None
     }
   }
+
+  impl ScopeTypeSpecificData {
+    pub fn is_none(&self) -> bool {
+      match self {
+        Self::None => true,
+        _ => false,
+      }
+    }
+  }
 }
 
 mod raw {