Repository: denoland/deno_core
Pull request #959 (2024-11-07 12:43:03)

#[stack_trace] in fast calls

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/denoland/deno_core/pull/959?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=denoland) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 81.33%. Comparing base [(`0c7f83e`)](https://app.codecov.io/gh/denoland/deno_core/commit/0c7f83ea8daabe8f541fc93ae97529dc446f5f24?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=denoland) to head [(`bd6d89c`)](https://app.codecov.io/gh/denoland/deno_core/commit/bd6d89cf3b6962639828a050373def9331723fd4?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=denoland).
> Report is 161 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #959      +/-   ##
==========================================
- Coverage   81.43%   81.33%   -0.11%     
==========================================
  Files          97       97              
  Lines       23877    24988    +1111     
==========================================
+ Hits        19445    20324     +879     
- Misses       4432     4664     +232     
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/denoland/deno_core/pull/959?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=denoland).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=denoland).


Patch:
diff --git a/ops/op2/README.md b/ops/op2/README.md
index bfbb74d..1628a81 100644
--- a/ops/op2/README.md
+++ b/ops/op2/README.md
@@ -929,7 +929,7 @@ Only usable in `deno_core`.
 ```
 
 </td><td>
-
+✅
 </td><td>
 
 </td><td>
diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs
index c0b31e3..0eea673 100644
--- a/ops/op2/dispatch_fast.rs
+++ b/ops/op2/dispatch_fast.rs
@@ -434,6 +434,21 @@ pub(crate) fn generate_dispatch_fast(
     })
   };
 
+  let with_stack_trace = if generator_state.needs_stack_trace {
+    generator_state.needs_opctx = true;
+    generator_state.needs_scope = true;
+    gs_quote!(generator_state(stack_trace, opctx, scope) =>
+    (let #stack_trace = if #opctx.enable_stack_trace_arg {
+      let stack_trace_msg = deno_core::v8::String::empty(&mut #scope);
+      let stack_trace_error = deno_core::v8::Exception::error(&mut #scope, stack_trace_msg.into());
+      let js_error = deno_core::error::JsError::from_v8_exception(&mut #scope, stack_trace_error);
+      Some(js_error.frames)
+    } else { None };)
+    )
+  } else {
+    quote!()
+  };
+
   let with_js_runtime_state = if generator_state.needs_js_runtime_state {
     generator_state.needs_opctx = true;
     gs_quote!(generator_state(js_runtime_state, opctx) => {
@@ -552,6 +567,7 @@ pub(crate) fn generate_dispatch_fast(
       #with_fast_api_callback_options
       #with_scope
       #with_opctx
+      #with_stack_trace
       #with_js_runtime_state
       #with_self
       let #result = {
@@ -590,9 +606,11 @@ fn map_v8_fastcall_arg_to_arg(
     opctx,
     js_runtime_state,
     scope,
+    stack_trace,
     needs_scope,
     needs_opctx,
     needs_fast_api_callback_options,
+    needs_stack_trace,
     needs_js_runtime_state,
     ..
   } = generator_state;
@@ -663,6 +681,10 @@ fn map_v8_fastcall_arg_to_arg(
        let #arg_ident = #fast_api_callback_options.isolate;
       })
     }
+    Arg::Special(Special::StackTrace) => {
+      *needs_stack_trace = true;
+      quote!(let #arg_ident = #stack_trace;)
+    }
     Arg::Ref(RefType::Ref, Special::OpState) => {
       *needs_opctx = true;
       quote!(let #arg_ident = &::std::cell::RefCell::borrow(&#opctx.state);)
@@ -847,6 +869,7 @@ fn map_arg_to_v8_fastcall_type(
     | Arg::Ref(RefType::Ref, Special::JsRuntimeState)
     | Arg::State(..)
     | Arg::Special(Special::Isolate)
+    | Arg::Special(Special::StackTrace)
     | Arg::OptionState(..) => V8FastCallType::Virtual,
     // Other types + ref types are not handled
     Arg::OptionNumeric(..)
@@ -855,8 +878,7 @@ fn map_arg_to_v8_fastcall_type(
     | Arg::OptionBuffer(..)
     | Arg::SerdeV8(_)
     | Arg::FromV8(_)
-    | Arg::Ref(..)
-    | Arg::Special(Special::StackTrace) => return Ok(None),
+    | Arg::Ref(..) => return Ok(None),
     // We don't support v8 global arguments
     Arg::V8Global(_) | Arg::OptionV8Global(_) => return Ok(None),
     // We do support v8 type arguments (including Option<...>)
diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs
index 9126e6b..9ffff1c 100644
--- a/ops/op2/dispatch_slow.rs
+++ b/ops/op2/dispatch_slow.rs
@@ -98,12 +98,11 @@ pub(crate) fn generate_dispatch_slow(
     quote!()
   };
 
-  let with_opctx =
-    if generator_state.needs_opctx | generator_state.needs_stack_trace {
-      with_opctx(generator_state)
-    } else {
-      quote!()
-    };
+  let with_opctx = if generator_state.needs_opctx {
+    with_opctx(generator_state)
+  } else {
+    quote!()
+  };
 
   let with_retval = if generator_state.needs_retval {
     with_retval(generator_state)
@@ -124,12 +123,11 @@ pub(crate) fn generate_dispatch_slow(
     quote!()
   };
 
-  let with_scope =
-    if generator_state.needs_scope | generator_state.needs_stack_trace {
-      with_scope(generator_state)
-    } else {
-      quote!()
-    };
+  let with_scope = if generator_state.needs_scope {
+    with_scope(generator_state)
+  } else {
+    quote!()
+  };
 
   Ok(
     gs_quote!(generator_state(opctx, info, slow_function, slow_function_metrics) => {
diff --git a/ops/op2/valid_args.md b/ops/op2/valid_args.md
index 704a40e..5aa81bf 100644
--- a/ops/op2/valid_args.md
+++ b/ops/op2/valid_args.md
@@ -64,4 +64,4 @@
 | X         | #[state] &mut StateObject                | X        |                                                     | Extracts an object from `OpState`.                                                                                                                                                                             |
 | X         | &JsRuntimeState                          | X        |                                                     | Only usable in `deno_core`.                                                                                                                                                                                    |
 | X         | *mut v8::Isolate                         | X        |                                                     | ⚠️ Extremely dangerous, may crash if you don't use `nofast` depending on what you do.                                                                                                                          |
-| X         | #[stack_trace] Option<Vec<JsStackFrame>> |          |                                                     | ⚠️ This argument is very slow as it needs to create an error instance and collect a whole stack frame. It only returns `Some(frames)` if `RuntimeOptions::enable_stack_trace_arg_in_ops` is set to true.       |
+| X         | #[stack_trace] Option<Vec<JsStackFrame>> | X        |                                                     | ⚠️ This argument is very slow as it needs to create an error instance and collect a whole stack frame. It only returns `Some(frames)` if `RuntimeOptions::enable_stack_trace_arg_in_ops` is set to true.       |


Test patch:
diff --git a/ops/op2/test_cases/compiler_pass/sync.rs b/ops/op2/test_cases/compiler_pass/sync.rs
index 562aea5..5b86859 100644
--- a/ops/op2/test_cases/compiler_pass/sync.rs
+++ b/ops/op2/test_cases/compiler_pass/sync.rs
@@ -60,8 +60,11 @@ fn op_smi_signed_return(
   a as Int32 + b as Int32 + c as Int32 + d as Int32
 }
 
-#[op2]
+#[op2(fast)]
 fn op_stack_trace(
   #[string] _: String,
   #[stack_trace] _: Option<Vec<JsStackFrame>>,
 ) {}
+
+#[op2(fast)]
+fn op_stack_trace_fast(#[stack_trace] _: Option<Vec<JsStackFrame>>) {}
diff --git a/ops/op2/test_cases/sync/stack_trace.out b/ops/op2/test_cases/sync/stack_trace.out
index 3ae095c..0d41d1b 100644
--- a/ops/op2/test_cases/sync/stack_trace.out
+++ b/ops/op2/test_cases/sync/stack_trace.out
@@ -14,8 +14,30 @@ const fn op_stack_trace() -> ::deno_core::_ops::OpDecl {
             false,
             Self::v8_fn_ptr as _,
             Self::v8_fn_ptr_metrics as _,
-            None,
-            None,
+            Some({
+                use deno_core::v8::fast_api::Type as CType;
+                use deno_core::v8;
+                deno_core::v8::fast_api::CFunction::new(
+                    Self::v8_fn_ptr_fast as _,
+                    &deno_core::v8::fast_api::CFunctionInfo::new(
+                        CType::Void.scalar(),
+                        &[CType::V8Value.scalar(), CType::CallbackOptions.scalar()],
+                        deno_core::v8::fast_api::Int64Representation::BigInt,
+                    ),
+                )
+            }),
+            Some({
+                use deno_core::v8::fast_api::Type as CType;
+                use deno_core::v8;
+                deno_core::v8::fast_api::CFunction::new(
+                    Self::v8_fn_ptr_fast_metrics as _,
+                    &deno_core::v8::fast_api::CFunctionInfo::new(
+                        CType::Void.scalar(),
+                        &[CType::V8Value.scalar(), CType::CallbackOptions.scalar()],
+                        deno_core::v8::fast_api::Int64Representation::BigInt,
+                    ),
+                )
+            }),
             ::deno_core::OpMetadata {
                 ..::deno_core::OpMetadata::default()
             },
@@ -25,6 +47,76 @@ const fn op_stack_trace() -> ::deno_core::_ops::OpDecl {
         pub const fn name() -> &'static str {
             <Self as deno_core::_ops::Op>::NAME
         }
+        #[allow(clippy::too_many_arguments)]
+        extern "C" fn v8_fn_ptr_fast_metrics<'s>(
+            this: deno_core::v8::Local<deno_core::v8::Object>,
+            fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions<
+                's,
+            >,
+        ) -> () {
+            let fast_api_callback_options: &'s mut _ = unsafe {
+                &mut *fast_api_callback_options
+            };
+            let opctx: &'s _ = unsafe {
+                &*(deno_core::v8::Local::<
+                    deno_core::v8::External,
+                >::cast_unchecked(unsafe { fast_api_callback_options.data })
+                    .value() as *const deno_core::_ops::OpCtx)
+            };
+            deno_core::_ops::dispatch_metrics_fast(
+                opctx,
+                deno_core::_ops::OpMetricsEvent::Dispatched,
+            );
+            let res = Self::v8_fn_ptr_fast(this, fast_api_callback_options);
+            deno_core::_ops::dispatch_metrics_fast(
+                opctx,
+                deno_core::_ops::OpMetricsEvent::Completed,
+            );
+            res
+        }
+        #[allow(clippy::too_many_arguments)]
+        extern "C" fn v8_fn_ptr_fast<'s>(
+            this: deno_core::v8::Local<deno_core::v8::Object>,
+            fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions<
+                's,
+            >,
+        ) -> () {
+            #[cfg(debug_assertions)]
+            let _reentrancy_check_guard = deno_core::_ops::reentrancy_check(
+                &<Self as deno_core::_ops::Op>::DECL,
+            );
+            let fast_api_callback_options: &'s mut _ = unsafe {
+                &mut *fast_api_callback_options
+            };
+            let mut scope = unsafe {
+                deno_core::v8::CallbackScope::new(&*fast_api_callback_options)
+            };
+            let opctx: &'s _ = unsafe {
+                &*(deno_core::v8::Local::<
+                    deno_core::v8::External,
+                >::cast_unchecked(unsafe { fast_api_callback_options.data })
+                    .value() as *const deno_core::_ops::OpCtx)
+            };
+            let stack_trace = if opctx.enable_stack_trace_arg {
+                let stack_trace_msg = deno_core::v8::String::empty(&mut scope);
+                let stack_trace_error = deno_core::v8::Exception::error(
+                    &mut scope,
+                    stack_trace_msg.into(),
+                );
+                let js_error = deno_core::error::JsError::from_v8_exception(
+                    &mut scope,
+                    stack_trace_error,
+                );
+                Some(js_error.frames)
+            } else {
+                None
+            };
+            let result = {
+                let arg0 = stack_trace;
+                Self::call(arg0)
+            };
+            result as _
+        }
         #[inline(always)]
         fn slow_function_impl<'s>(
             info: &'s deno_core::v8::FunctionCallbackInfo,
diff --git a/ops/op2/test_cases/sync/stack_trace.rs b/ops/op2/test_cases/sync/stack_trace.rs
index 0e58548..f03da91 100644
--- a/ops/op2/test_cases/sync/stack_trace.rs
+++ b/ops/op2/test_cases/sync/stack_trace.rs
@@ -4,7 +4,7 @@ deno_ops_compile_test_runner::prelude!();
 
 use deno_core::error::JsStackFrame;
 
-#[op2]
+#[op2(fast)]
 fn op_stack_trace(#[stack_trace] s: Option<Vec<JsStackFrame>>) {
   drop(s);
 }
diff --git a/ops/op2/test_cases/sync/stack_trace_scope.out b/ops/op2/test_cases/sync/stack_trace_scope.out
index 2018632..07d3bc2 100644
--- a/ops/op2/test_cases/sync/stack_trace_scope.out
+++ b/ops/op2/test_cases/sync/stack_trace_scope.out
@@ -14,8 +14,38 @@ const fn op_stack_trace() -> ::deno_core::_ops::OpDecl {
             false,
             Self::v8_fn_ptr as _,
             Self::v8_fn_ptr_metrics as _,
-            None,
-            None,
+            Some({
+                use deno_core::v8::fast_api::Type as CType;
+                use deno_core::v8;
+                deno_core::v8::fast_api::CFunction::new(
+                    Self::v8_fn_ptr_fast as _,
+                    &deno_core::v8::fast_api::CFunctionInfo::new(
+                        CType::Void.scalar(),
+                        &[
+                            CType::V8Value.scalar(),
+                            CType::SeqOneByteString.scalar(),
+                            CType::CallbackOptions.scalar(),
+                        ],
+                        deno_core::v8::fast_api::Int64Representation::BigInt,
+                    ),
+                )
+            }),
+            Some({
+                use deno_core::v8::fast_api::Type as CType;
+                use deno_core::v8;
+                deno_core::v8::fast_api::CFunction::new(
+                    Self::v8_fn_ptr_fast_metrics as _,
+                    &deno_core::v8::fast_api::CFunctionInfo::new(
+                        CType::Void.scalar(),
+                        &[
+                            CType::V8Value.scalar(),
+                            CType::SeqOneByteString.scalar(),
+                            CType::CallbackOptions.scalar(),
+                        ],
+                        deno_core::v8::fast_api::Int64Representation::BigInt,
+                    ),
+                )
+            }),
             ::deno_core::OpMetadata {
                 ..::deno_core::OpMetadata::default()
             },
@@ -25,6 +55,79 @@ const fn op_stack_trace() -> ::deno_core::_ops::OpDecl {
         pub const fn name() -> &'static str {
             <Self as deno_core::_ops::Op>::NAME
         }
+        #[allow(clippy::too_many_arguments)]
+        extern "C" fn v8_fn_ptr_fast_metrics<'s>(
+            this: deno_core::v8::Local<deno_core::v8::Object>,
+            arg0: *mut deno_core::v8::fast_api::FastApiOneByteString,
+            fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions<
+                's,
+            >,
+        ) -> () {
+            let fast_api_callback_options: &'s mut _ = unsafe {
+                &mut *fast_api_callback_options
+            };
+            let opctx: &'s _ = unsafe {
+                &*(deno_core::v8::Local::<
+                    deno_core::v8::External,
+                >::cast_unchecked(unsafe { fast_api_callback_options.data })
+                    .value() as *const deno_core::_ops::OpCtx)
+            };
+            deno_core::_ops::dispatch_metrics_fast(
+                opctx,
+                deno_core::_ops::OpMetricsEvent::Dispatched,
+            );
+            let res = Self::v8_fn_ptr_fast(this, arg0, fast_api_callback_options);
+            deno_core::_ops::dispatch_metrics_fast(
+                opctx,
+                deno_core::_ops::OpMetricsEvent::Completed,
+            );
+            res
+        }
+        #[allow(clippy::too_many_arguments)]
+        extern "C" fn v8_fn_ptr_fast<'s>(
+            this: deno_core::v8::Local<deno_core::v8::Object>,
+            arg0: *mut deno_core::v8::fast_api::FastApiOneByteString,
+            fast_api_callback_options: *mut deno_core::v8::fast_api::FastApiCallbackOptions<
+                's,
+            >,
+        ) -> () {
+            #[cfg(debug_assertions)]
+            let _reentrancy_check_guard = deno_core::_ops::reentrancy_check(
+                &<Self as deno_core::_ops::Op>::DECL,
+            );
+            let fast_api_callback_options: &'s mut _ = unsafe {
+                &mut *fast_api_callback_options
+            };
+            let mut scope = unsafe {
+                deno_core::v8::CallbackScope::new(&*fast_api_callback_options)
+            };
+            let opctx: &'s _ = unsafe {
+                &*(deno_core::v8::Local::<
+                    deno_core::v8::External,
+                >::cast_unchecked(unsafe { fast_api_callback_options.data })
+                    .value() as *const deno_core::_ops::OpCtx)
+            };
+            let stack_trace = if opctx.enable_stack_trace_arg {
+                let stack_trace_msg = deno_core::v8::String::empty(&mut scope);
+                let stack_trace_error = deno_core::v8::Exception::error(
+                    &mut scope,
+                    stack_trace_msg.into(),
+                );
+                let js_error = deno_core::error::JsError::from_v8_exception(
+                    &mut scope,
+                    stack_trace_error,
+                );
+                Some(js_error.frames)
+            } else {
+                None
+            };
+            let result = {
+                let arg0 = deno_core::_ops::to_string_ptr(unsafe { &mut *arg0 });
+                let arg1 = stack_trace;
+                Self::call(arg0, arg1)
+            };
+            result as _
+        }
         #[inline(always)]
         fn slow_function_impl<'s>(
             info: &'s deno_core::v8::FunctionCallbackInfo,
diff --git a/ops/op2/test_cases/sync/stack_trace_scope.rs b/ops/op2/test_cases/sync/stack_trace_scope.rs
index 1b29342..b0ee648 100644
--- a/ops/op2/test_cases/sync/stack_trace_scope.rs
+++ b/ops/op2/test_cases/sync/stack_trace_scope.rs
@@ -4,7 +4,7 @@ deno_ops_compile_test_runner::prelude!();
 
 use deno_core::error::JsStackFrame;
 
-#[op2]
+#[op2(fast)]
 fn op_stack_trace(
   #[string] _: String,
   #[stack_trace] _: Option<Vec<JsStackFrame>>,