Skip to content

Commit

Permalink
#[stack_trace] in fast calls (#959)
Browse files Browse the repository at this point in the history
  • Loading branch information
littledivy authored Nov 7, 2024
1 parent 1bcb645 commit a9ae661
Show file tree
Hide file tree
Showing 9 changed files with 241 additions and 23 deletions.
2 changes: 1 addition & 1 deletion ops/op2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ Only usable in `deno_core`.
```

</td><td>

</td><td>

</td><td>
Expand Down
26 changes: 24 additions & 2 deletions ops/op2/dispatch_fast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);)
Expand Down Expand Up @@ -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(..)
Expand All @@ -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<...>)
Expand Down
22 changes: 10 additions & 12 deletions ops/op2/dispatch_slow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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) => {
Expand Down
5 changes: 4 additions & 1 deletion ops/op2/test_cases/compiler_pass/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>) {}
96 changes: 94 additions & 2 deletions ops/op2/test_cases/sync/stack_trace.out

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ops/op2/test_cases/sync/stack_trace.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
107 changes: 105 additions & 2 deletions ops/op2/test_cases/sync/stack_trace_scope.out

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion ops/op2/test_cases/sync/stack_trace_scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>>,
Expand Down
2 changes: 1 addition & 1 deletion ops/op2/valid_args.md
Original file line number Diff line number Diff line change
Expand Up @@ -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. |

0 comments on commit a9ae661

Please sign in to comment.