feat: use cppgc::Ptr (#818)

Uses denoland/rusty_v8#1523 to avoid
cppgc::Member overhead. Also move all Persistent logic into cppgc.rs to
clean up the macro code.

There is done using a new `Ptr` struct which can deref into the inner
member of `CppGcObject`. This is structured so as to play a little trick
and inline `T` into `CppGcObject<T>` which halves the allocations and
enables a future change for getting `&dyn Reference` from deno-managed
cppgc objects.

Author: devsnek

Cargo.lock

@@ -24,7 +24,7 @@ deno_ops = { version = "0.169.0", path = "./ops" }
 serde_v8 = { version = "0.202.0", path = "./serde_v8" }
 deno_core_testing = { path = "./testing" }
 
-v8 = { version = "0.97.0", default-features = false }
+v8 = { version = "0.98.0", default-features = false }
 deno_ast = { version = "=0.35.3", features = ["transpiling"] }
 deno_unsync = "0.3.10"
 deno_core_icudata = "0.0.73"

Cargo.toml

@@ -9,12 +9,12 @@ const CPPGC_TAG: u16 = 1;
 #[repr(C)]
 struct CppGcObject<T: GarbageCollected> {
   tag: TypeId,
-  member: v8::cppgc::Member<T>,
+  member: T,
 }
 
 impl<T: GarbageCollected> v8::cppgc::GarbageCollected for CppGcObject<T> {
   fn trace(&self, visitor: &v8::cppgc::Visitor) {
-    visitor.trace(&self.member);
+    self.member.trace(visitor);
   }
 }
 
@@ -48,7 +48,7 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>(
       heap,
       CppGcObject {
         tag: TypeId::of::<T>(),
-        member: v8::cppgc::make_garbage_collected(heap, t),
+        member: t,
       },
     )
   };
@@ -60,31 +60,54 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>(
   obj
 }
 
+#[doc(hidden)]
+pub struct Ptr<T: GarbageCollected> {
+  inner: v8::cppgc::Ptr<CppGcObject<T>>,
+  root: Option<v8::cppgc::Persistent<CppGcObject<T>>>,
+}
+
+#[doc(hidden)]
+impl<T: GarbageCollected> Ptr<T> {
+  /// If this pointer is used in an async function, it could leave the stack,
+  /// so this method can be called to root it in the GC and keep the reference
+  /// valid.
+  pub fn root(&mut self) {
+    if self.root.is_none() {
+      self.root = Some(v8::cppgc::Persistent::new(&self.inner));
+    }
+  }
+}
+
+impl<T: GarbageCollected> std::ops::Deref for Ptr<T> {
+  type Target = T;
+  fn deref(&self) -> &T {
+    &self.inner.member
+  }
+}
+
 #[doc(hidden)]
 #[allow(clippy::needless_lifetimes)]
 pub fn try_unwrap_cppgc_object<'sc, T: GarbageCollected + 'static>(
   scope: &mut v8::HandleScope<'sc>,
   val: v8::Local<'sc, v8::Value>,
-) -> v8::cppgc::Member<T> {
+) -> Option<Ptr<T>> {
   let Ok(obj): Result<v8::Local<v8::Object>, _> = val.try_into() else {
-    return v8::cppgc::Member::empty();
+    return None;
   };
 
   if !obj.is_api_wrapper() {
-    return v8::cppgc::Member::empty();
+    return None;
   }
 
-  let ptr =
-    unsafe { v8::Object::unwrap::<CPPGC_TAG, CppGcObject<T>>(scope, obj) };
-  let Some(obj) = ptr.borrow() else {
-    return v8::cppgc::Member::empty();
-  };
+  let obj =
+    unsafe { v8::Object::unwrap::<CPPGC_TAG, CppGcObject<T>>(scope, obj) }?;
 
   if obj.tag != TypeId::of::<T>() {
-    return v8::cppgc::Member::empty();
+    return None;
   }
 
-  let mut h = v8::cppgc::Member::empty();
-  h.set(&obj.member);
-  h
+  Some(Ptr {
+    inner: obj,
+    root: None,
+  })
 }

core/cppgc.rs

@@ -455,12 +455,12 @@ pub(crate) fn generate_dispatch_fast(
     generator_state.needs_fast_scope = true;
     generator_state.needs_fast_api_callback_options = true;
     gs_quote!(generator_state(self_ty, scope, fast_api_callback_options) => {
-      let self_handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, this.into());
-      let Some(self_) = self_handle_.borrow() else {
+      let Some(self_) = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, this.into()) else {
         #fast_api_callback_options.fallback = true;
         // SAFETY: All fast return types have zero as a valid value
         return unsafe { std::mem::zeroed() };
       };
+      let self_ = &*self_;
     })
   } else {
     quote!()
@@ -764,12 +764,12 @@ fn map_v8_fastcall_arg_to_arg(
       *needs_fast_scope = true;
       *needs_fast_api_callback_options = true;
       quote! {
-        let handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident);
-        let Some(#arg_ident) = handle_.borrow() else {
+        let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) else {
           #fast_api_callback_options.fallback = true;
           // SAFETY: All fast return types have zero as a valid value
           return unsafe { std::mem::zeroed() };
         };
+        let #arg_ident = &*#arg_ident;
       }
     }
     Arg::OptionCppGcResource(ty) => {
@@ -779,20 +779,16 @@ fn map_v8_fastcall_arg_to_arg(
       *needs_fast_scope = true;
       *needs_fast_api_callback_options = true;
       quote! {
-        let mut handle_ = deno_core::v8::cppgc::Member::empty();
         let #arg_ident = if #arg_ident.is_null_or_undefined() {
           None
+        } else if let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) {
+          Some(#arg_ident)
         } else {
-          handle_.set(&deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident));
-          match handle_.borrow() {
-            Some(r) => Some(r),
-            None => {
-              #fast_api_callback_options.fallback = true;
-              // SAFETY: All fast return types have zero as a valid value
-              return unsafe { std::mem::zeroed() };
-            }
-          }
+          #fast_api_callback_options.fallback = true;
+          // SAFETY: All fast return types have zero as a valid value
+          return unsafe { std::mem::zeroed() };
         };
+        let #arg_ident = #arg_ident.as_deref();
       }
     }
     _ => quote!(let #arg_ident = #arg_ident as _;),

ops/op2/dispatch_fast.rs

@@ -257,26 +257,23 @@ pub(crate) fn with_self(
   let tokens = if matches!(ret_val, RetVal::Future(_) | RetVal::FutureResult(_))
   {
     let tokens = gs_quote!(generator_state(self_ty, fn_args, scope) => {
-      let self_handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, #fn_args.this().into());
-      if self_handle_.borrow().is_none() {
+      let Some(mut self_) = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, #fn_args.this().into()) else {
         #throw_exception;
-      }
-      let mut self_persistent_ = deno_core::v8::cppgc::Persistent::empty();
-      self_persistent_.set(&self_handle_);
-      drop(self_handle_);
+      };
+      self_.root();
     });
 
     generator_state.moves.push(quote! {
-      let self_ = self_persistent_.borrow().unwrap();
+      let self_ = &*self_;
     });
 
     tokens
   } else {
     gs_quote!(generator_state(self_ty, fn_args, scope) => {
-      let self_handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, #fn_args.this().into());
-      let Some(self_) = self_handle_.borrow() else {
+      let Some(self_) = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(&mut #scope, #fn_args.this().into()) else {
         #throw_exception;
       };
+      let self_ = &*self_;
     })
   };
   Ok(tokens)
@@ -582,24 +579,21 @@ pub fn from_arg(
         syn::parse_str::<syn::Path>(ty).expect("Failed to reparse state type");
       if matches!(ret_val, RetVal::Future(_) | RetVal::FutureResult(_)) {
         let tokens = quote! {
-          let handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident);
-          if handle_.borrow().is_none() {
+          let Some(mut #arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) else {
             #throw_exception;
-          }
-          let mut #arg_ident = deno_core::v8::cppgc::Persistent::empty();
-          #arg_ident.set(&handle_);
-          drop(handle_);
+          };
+          #arg_ident.root();
         };
         generator_state.moves.push(quote! {
-          let #arg_ident = #arg_ident.borrow().unwrap();
+          let #arg_ident = &*#arg_ident;
         });
         tokens
       } else {
         quote! {
-          let handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident);
-          let Some(#arg_ident) = handle_.borrow() else {
+          let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) else {
             #throw_exception;
           };
+          let #arg_ident = &*#arg_ident;
         }
       }
     }
@@ -613,37 +607,30 @@ pub fn from_arg(
       if matches!(ret_val, RetVal::Future(_) | RetVal::FutureResult(_)) {
         let tokens = quote! {
           let #arg_ident = if #arg_ident.is_null_or_undefined() {
-            deno_core::v8::cppgc::Persistent::empty()
+            None
+          } else if let Some(mut #arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) {
+            #arg_ident.root();
+            Some(#arg_ident)
           } else {
-            let handle = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident);
-            if handle.borrow().is_none() {
-              #throw_exception;
-            }
-            let mut persistent = deno_core::v8::cppgc::Persistent::empty();
-            persistent.set(&handle);
-            persistent
+            #throw_exception;
           };
         };
 
         generator_state.moves.push(quote! {
-          let #arg_ident = #arg_ident.borrow();
+          let #arg_ident = #arg_ident.as_deref();
         });
 
         tokens
       } else {
         quote! {
-          let mut handle_ = deno_core::v8::cppgc::Member::empty();
           let #arg_ident = if #arg_ident.is_null_or_undefined() {
             None
+          } else if let Some(#arg_ident) = deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident) {
+            Some(#arg_ident)
           } else {
-            handle_.set(&deno_core::_ops::try_unwrap_cppgc_object::<#ty>(&mut #scope, #arg_ident));
-            match handle_.borrow() {
-              Some(r) => Some(r),
-              None => {
-                #throw_exception;
-              }
-            }
+            #throw_exception;
           };
+          let #arg_ident = #arg_ident.as_deref();
         }
       }
     }