feat(ops): Automatic fast ops creation (denoland#15527)

DavidDeSimone · Sep 22, 2022 · 707e9e3 · 707e9e3
1 parent cc32a29
commit 707e9e3
Show file tree

Hide file tree

Showing 8 changed files with 49 additions and 41 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/cli/bench/deno_common.js b/cli/bench/deno_common.js
@@ -11,7 +11,7 @@ Deno.bench("date_now", { n: 5e5 }, () => {
   const { op_add } = Deno.core.ops;
   // deno-lint-ignore no-inner-declarations
   function add(a, b) {
-    return op_add.fast(a, b);
+    return op_add(a, b);
   }
   // deno-lint-ignore no-inner-declarations
   function addJS(a, b) {
@@ -24,7 +24,7 @@ Deno.bench("date_now", { n: 5e5 }, () => {
 // deno-lint-ignore camelcase
 const { op_void_sync } = Deno.core.ops;
 function sync() {
-  return op_void_sync.fast();
+  return op_void_sync();
 }
 sync(); // Warmup
 

diff --git a/core/01_core.js b/core/01_core.js
@@ -312,7 +312,7 @@
     deserialize: (buffer, options) => ops.op_deserialize(buffer, options),
     getPromiseDetails: (promise) => ops.op_get_promise_details(promise),
     getProxyDetails: (proxy) => ops.op_get_proxy_details(proxy),
-    isProxy: (value) => ops.op_is_proxy.fast(value),
+    isProxy: (value) => ops.op_is_proxy(value),
     memoryUsage: () => ops.op_memory_usage(),
     setWasmStreamingCallback: (fn) => ops.op_set_wasm_streaming_callback(fn),
     abortWasmStreaming: (

diff --git a/core/Cargo.toml b/core/Cargo.toml
@@ -33,7 +33,7 @@ serde_json = { version = "1.0.79", features = ["preserve_order"] }
 serde_v8 = { version = "0.62.0", path = "../serde_v8" }
 sourcemap = "6.1"
 url = { version = "2.3.1", features = ["serde", "expose_internals"] }
-v8 = { version = "0.50.0", default-features = false }
+v8 = { version = "0.51.0", default-features = false }
 
 [[example]]
 name = "http_bench_json_ops"

diff --git a/core/bindings.rs b/core/bindings.rs
@@ -140,33 +140,21 @@ fn initialize_ops(
   op_ctxs: &[OpCtx],
   snapshot_loaded: bool,
 ) {
-  let object_template = v8::ObjectTemplate::new(scope);
-  assert!(object_template.set_internal_field_count(
-    (crate::runtime::V8_WRAPPER_OBJECT_INDEX + 1) as usize
-  ));
-
   for ctx in op_ctxs {
     let ctx_ptr = ctx as *const OpCtx as *const c_void;
 
     // If this is a fast op, we don't want it to be in the snapshot.
     // Only initialize once snapshot is loaded.
     if ctx.decl.fast_fn.is_some() && snapshot_loaded {
-      let method_obj = object_template.new_instance(scope).unwrap();
-      method_obj.set_aligned_pointer_in_internal_field(
-        crate::runtime::V8_WRAPPER_OBJECT_INDEX,
-        ctx_ptr,
-      );
       set_func_raw(
         scope,
-        method_obj,
-        "fast",
+        ops_obj,
+        ctx.decl.name,
         ctx.decl.v8_fn_ptr,
         ctx_ptr,
         &ctx.decl.fast_fn,
         snapshot_loaded,
       );
-      let method_key = v8::String::new(scope, ctx.decl.name).unwrap();
-      ops_obj.set(scope, method_key.into(), method_obj.into());
     } else {
       set_func_raw(
         scope,

diff --git a/ext/web/02_timers.js b/ext/web/02_timers.js
@@ -31,7 +31,7 @@
   const hrU8 = new Uint8Array(8);
   const hr = new Uint32Array(hrU8.buffer);
   function opNow() {
-    ops.op_now.fast(hrU8);
+    ops.op_now(hrU8);
     return (hr[0] * 1000 + hr[1] / 1e6);
   }
 

diff --git a/ops/lib.rs b/ops/lib.rs
@@ -294,27 +294,27 @@ fn codegen_fast_impl(
   is_async: bool,
   must_be_fast: bool,
 ) -> (TokenStream2, TokenStream2) {
-  if !must_be_fast {
+  if is_async {
+    if must_be_fast {
+      panic!("async op cannot be a fast api. enforced by #[op(fast)]")
+    }
     return (quote! {}, quote! { None });
   }
   let fast_info = can_be_fast_api(core, f);
   if must_be_fast && fast_info.is_none() {
     panic!("op cannot be a fast api. enforced by #[op(fast)]")
   }
-  if must_be_fast && is_async {
-    panic!("async op cannot be a fast api. enforced by #[op(fast)]")
-  }
   if !is_async {
     if let Some(FastApiSyn {
       args,
       ret,
-      use_recv,
+      use_op_state,
       use_fast_cb_opts,
       v8_values,
       slices,
     }) = fast_info
     {
-      let offset = if use_recv { 1 } else { 0 };
+      let offset = if use_op_state { 1 } else { 0 };
       let mut inputs = f
         .sig
         .inputs
@@ -341,7 +341,7 @@ fn codegen_fast_impl(
           quote!(#arg)
         })
         .collect::<Vec<_>>();
-      if !slices.is_empty() && !use_fast_cb_opts {
+      if (!slices.is_empty() || use_op_state) && !use_fast_cb_opts {
         inputs.push(quote! { fast_api_callback_options: *mut #core::v8::fast_api::FastApiCallbackOptions });
       }
       let input_idents = f
@@ -390,8 +390,16 @@ fn codegen_fast_impl(
         (
           quote! {
             fn func(recv: #core::v8::Local<#core::v8::Object>, __promise_id: u32, #(#inputs),*) {
-              let op_ctx = recv.get_aligned_pointer_from_internal_field(#core::_ops::V8_WRAPPER_OBJECT_INDEX);
-              let op_id = op_ctx.op_id;
+              // SAFETY: V8 calling convention guarantees that the callback options pointer is non-null.
+              let opts: &#core::v8::fast_api::FastApiCallbackOptions = unsafe { &*fast_api_callback_options };
+              // SAFETY: data union is always created as the `v8::Local<v8::Value>` version
+              let data = unsafe { opts.data.data };
+              // SAFETY: #core guarantees data is a v8 External pointing to an OpCtx for the isolates lifetime
+              let ctx = unsafe {
+                &*(#core::v8::Local::<#core::v8::External>::cast(data).value()
+                as *const #core::_ops::OpCtx)
+              };
+              let op_id = ctx.op_id;
               #core::_ops::queue_async_op(scope, async move {
                 let result = Self::call(#args);
                 (__promise_id, __op_id, #core::_ops::OpResult::Ok(result))
@@ -404,19 +412,27 @@ fn codegen_fast_impl(
       } else {
         let output = &f.sig.output;
         let func_name = format_ident!("func_{}", name);
-        let recv_decl = if use_recv {
+        let recv_decl = if use_op_state {
+          let op_state_name = input_idents.first();
           quote! {
-            let ptr = unsafe { recv.get_aligned_pointer_from_internal_field(#core::_ops::V8_WRAPPER_OBJECT_INDEX) };
-            let op_ctx = unsafe { &*(ptr as *const #core::_ops::OpCtx) };
-            let state = &mut op_ctx.state.borrow_mut();
+            // SAFETY: V8 calling convention guarantees that the callback options pointer is non-null.
+            let opts: &#core::v8::fast_api::FastApiCallbackOptions = unsafe { &*fast_api_callback_options };
+            // SAFETY: data union is always created as the `v8::Local<v8::Value>` version.
+            let data = unsafe { opts.data.data };
+            // SAFETY: #core guarantees data is a v8 External pointing to an OpCtx for the isolates lifetime
+            let ctx = unsafe {
+              &*(#core::v8::Local::<#core::v8::External>::cast(data).value()
+              as *const #core::_ops::OpCtx)
+            };
+            let #op_state_name = &mut ctx.state.borrow_mut();
           }
         } else {
           quote!()
         };
 
         (
           quote! {
-            fn #func_name #generics (recv: #core::v8::Local<#core::v8::Object>, #(#inputs),*) #output #where_clause {
+            fn #func_name #generics (_recv: #core::v8::Local<#core::v8::Object>, #(#inputs),*) #output #where_clause {
               #recv_decl
               #name::call::<#type_params>(#(#input_idents),*)
             }
@@ -510,7 +526,7 @@ fn codegen_v8_sync(
 struct FastApiSyn {
   args: TokenStream2,
   ret: TokenStream2,
-  use_recv: bool,
+  use_op_state: bool,
   use_fast_cb_opts: bool,
   v8_values: Vec<usize>,
   slices: HashMap<usize, TokenStream2>,
@@ -526,20 +542,19 @@ fn can_be_fast_api(core: &TokenStream2, f: &syn::ItemFn) -> Option<FastApiSyn> {
     },
   };
 
-  let mut use_recv = false;
+  let mut use_op_state = false;
   let mut use_fast_cb_opts = false;
   let mut v8_values = Vec::new();
   let mut slices = HashMap::new();
   let mut args = vec![quote! { #core::v8::fast_api::Type::V8Value }];
   for (pos, input) in inputs.iter().enumerate() {
     if pos == inputs.len() - 1 && is_optional_fast_callback_option(input) {
       use_fast_cb_opts = true;
-      args.push(quote! { #core::v8::fast_api::Type::CallbackOptions });
       continue;
     }
 
     if pos == 0 && is_mut_ref_opstate(input) {
-      use_recv = true;
+      use_op_state = true;
       continue;
     }
 
@@ -573,6 +588,11 @@ fn can_be_fast_api(core: &TokenStream2, f: &syn::ItemFn) -> Option<FastApiSyn> {
     }
   }
 
+  if use_fast_cb_opts || use_op_state {
+    // Push CallbackOptions into args; it must be the last argument.
+    args.push(quote! { #core::v8::fast_api::Type::CallbackOptions });
+  }
+
   let args = args
     .iter()
     .map(|arg| format!("{}", arg))
@@ -581,7 +601,7 @@ fn can_be_fast_api(core: &TokenStream2, f: &syn::ItemFn) -> Option<FastApiSyn> {
   Some(FastApiSyn {
     args: args.parse().unwrap(),
     ret,
-    use_recv,
+    use_op_state,
     slices,
     v8_values,
     use_fast_cb_opts,

diff --git a/serde_v8/Cargo.toml b/serde_v8/Cargo.toml
@@ -18,7 +18,7 @@ derive_more = "0.99.17"
 serde = { version = "1.0.136", features = ["derive"] }
 serde_bytes = "0.11"
 smallvec = { version = "1.8", features = ["union"] }
-v8 = { version = "0.50.0", default-features = false }
+v8 = { version = "0.51.0", default-features = false }
 
 [dev-dependencies]
 bencher = "0.1"