Skip to content

Commit

Permalink
feat: Support #[stack_trace] op2 arg (#920)
Browse files Browse the repository at this point in the history
  • Loading branch information
bartlomieju authored Oct 14, 2024
1 parent c907543 commit e6fb17b
Show file tree
Hide file tree
Showing 15 changed files with 427 additions and 83 deletions.
2 changes: 2 additions & 0 deletions core/extension_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ pub fn create_op_ctxs(
op_state: Rc<RefCell<OpState>>,
runtime_state: Rc<JsRuntimeState>,
get_error_class_fn: GetErrorClassFn,
enable_stack_trace_arg_in_ops: bool,
) -> Box<[OpCtx]> {
let op_count = op_decls.len();
let mut op_ctxs = Vec::with_capacity(op_count);
Expand All @@ -145,6 +146,7 @@ pub fn create_op_ctxs(
runtime_state_ptr,
get_error_class_fn,
metrics_fn,
enable_stack_trace_arg_in_ops,
);

op_ctxs.push(op_ctx);
Expand Down
4 changes: 4 additions & 0 deletions core/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,8 @@ pub struct OpCtx {
pub state: Rc<RefCell<OpState>>,
#[doc(hidden)]
pub get_error_class_fn: GetErrorClassFn,
#[doc(hidden)]
pub enable_stack_trace_arg: bool,

pub(crate) decl: OpDecl,
pub(crate) fast_fn_info: Option<CFunction>,
Expand All @@ -106,6 +108,7 @@ impl OpCtx {
runtime_state: *const JsRuntimeState,
get_error_class_fn: GetErrorClassFn,
metrics_fn: Option<OpMetricsFn>,
enable_stack_trace_arg: bool,
) -> Self {
// If we want metrics for this function, create the fastcall `CFunctionInfo` from the metrics
// `CFunction`. For some extremely fast ops, the parameter list may change for the metrics
Expand All @@ -127,6 +130,7 @@ impl OpCtx {
fast_fn_info,
isolate,
metrics_fn,
enable_stack_trace_arg,
}
}

Expand Down
5 changes: 5 additions & 0 deletions core/runtime/jsruntime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -553,6 +553,10 @@ pub struct RuntimeOptions {
Option<(EvalContextGetCodeCacheCb, EvalContextCodeCacheReadyCb)>,

pub import_assertions_support: ImportAssertionsSupport,

/// Whether `#[stack_trace]` argument in ops should return `Some(frames)`. Use wisely,
/// as it's very expensive to collect stack traces on each op invocation.
pub enable_stack_trace_arg_in_ops: bool,
}

pub struct ImportAssertionsSupportCustomCallbackArgs {
Expand Down Expand Up @@ -873,6 +877,7 @@ impl JsRuntime {
op_state.clone(),
state_rc.clone(),
get_error_class_fn,
options.enable_stack_trace_arg_in_ops,
);

// ...ops are now almost fully set up; let's create a V8 isolate...
Expand Down
14 changes: 14 additions & 0 deletions ops/op2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -921,6 +921,20 @@ Only usable in `deno_core`.
</td><td>
⚠️ Extremely dangerous, may crash if you don't use `nofast` depending on what you do.
</td></tr>
<tr>
<td>

```text
#[stack_trace] Option<Vec<JsStackFrame>>
```

</td><td>

</td><td>

</td><td>
⚠️ 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.
</td></tr>
</table>

<!-- END ARGS -->
Expand Down
24 changes: 17 additions & 7 deletions ops/op2/dispatch_async.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use super::dispatch_slow::with_opstate;
use super::dispatch_slow::with_retval;
use super::dispatch_slow::with_scope;
use super::dispatch_slow::with_self;
use super::dispatch_slow::with_stack_trace;
use super::generator_state::gs_quote;
use super::generator_state::GeneratorState;
use super::signature::ParsedSignature;
Expand Down Expand Up @@ -120,11 +121,12 @@ pub(crate) fn generate_dispatch_async(
quote!()
};

let with_opctx = if generator_state.needs_opctx {
with_opctx(generator_state)
} else {
quote!()
};
let with_opctx =
if generator_state.needs_opctx | generator_state.needs_stack_trace {
with_opctx(generator_state)
} else {
quote!()
};

let with_retval = if generator_state.needs_retval {
with_retval(generator_state)
Expand All @@ -138,8 +140,15 @@ pub(crate) fn generate_dispatch_async(
quote!()
};

let with_scope = if generator_state.needs_scope {
with_scope(generator_state)
let with_scope =
if generator_state.needs_scope | generator_state.needs_stack_trace {
with_scope(generator_state)
} else {
quote!()
};

let with_stack_trace = if generator_state.needs_stack_trace {
with_stack_trace(generator_state)
} else {
quote!()
};
Expand All @@ -157,6 +166,7 @@ pub(crate) fn generate_dispatch_async(
#with_opctx
#with_opstate
#with_self
#with_stack_trace

#output
}
Expand Down
3 changes: 2 additions & 1 deletion ops/op2/dispatch_fast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -845,7 +845,8 @@ fn map_arg_to_v8_fastcall_type(
| Arg::OptionBuffer(..)
| Arg::SerdeV8(_)
| Arg::FromV8(_)
| Arg::Ref(..) => return Ok(None),
| Arg::Ref(..)
| Arg::Special(Special::StackTrace) => 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
44 changes: 37 additions & 7 deletions ops/op2/dispatch_slow.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,11 +92,12 @@ pub(crate) fn generate_dispatch_slow(
quote!()
};

let with_opctx = if generator_state.needs_opctx {
with_opctx(generator_state)
} else {
quote!()
};
let with_opctx =
if generator_state.needs_opctx | generator_state.needs_stack_trace {
with_opctx(generator_state)
} else {
quote!()
};

let with_retval = if generator_state.needs_retval {
with_retval(generator_state)
Expand All @@ -117,8 +118,15 @@ pub(crate) fn generate_dispatch_slow(
quote!()
};

let with_scope = if generator_state.needs_scope {
with_scope(generator_state)
let with_scope =
if generator_state.needs_scope | generator_state.needs_stack_trace {
with_scope(generator_state)
} else {
quote!()
};

let with_stack_trace = if generator_state.needs_stack_trace {
with_stack_trace(generator_state)
} else {
quote!()
};
Expand All @@ -136,6 +144,7 @@ pub(crate) fn generate_dispatch_slow(
#with_opctx
#with_isolate
#with_opstate
#with_stack_trace
#with_js_runtime_state
#with_self

Expand Down Expand Up @@ -183,6 +192,21 @@ pub(crate) fn with_scope(generator_state: &mut GeneratorState) -> TokenStream {
)
}

pub(crate) fn with_stack_trace(
generator_state: &mut GeneratorState,
) -> TokenStream {
generator_state.needs_opctx = true;
gs_quote!(generator_state(stack_trace, opctx, scope) =>
(let #stack_trace = if #opctx.enable_stack_trace_arg {
let hs = &mut deno_core::v8::HandleScope::new(&mut #scope);
let stack_trace_msg = deno_core::v8::String::empty(hs);
let stack_trace_error = deno_core::v8::Exception::error(hs, stack_trace_msg.into());
let js_error = deno_core::error::JsError::from_v8_exception(hs, stack_trace_error);
Some(js_error.frames)
} else { None };)
)
}

pub(crate) fn with_retval(generator_state: &mut GeneratorState) -> TokenStream {
gs_quote!(generator_state(retval, info) =>
(let mut #retval = deno_core::v8::ReturnValue::from_function_callback_info(#info);)
Expand Down Expand Up @@ -284,11 +308,13 @@ pub fn from_arg(
scope,
opstate,
opctx,
stack_trace,
js_runtime_state,
needs_scope,
needs_isolate,
needs_opstate,
needs_opctx,
needs_stack_trace,
needs_js_runtime_state,
..
} = &mut generator_state;
Expand Down Expand Up @@ -439,6 +465,10 @@ pub fn from_arg(
*needs_opctx = true;
quote!(let #arg_ident = #opctx.isolate;)
}
Arg::Special(Special::StackTrace) => {
*needs_stack_trace = true;
quote!(let #arg_ident = #stack_trace;)
}
Arg::Ref(_, Special::HandleScope) => {
*needs_scope = true;
quote!(let #arg_ident = &mut #scope;)
Expand Down
6 changes: 5 additions & 1 deletion ops/op2/generator_state.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
use proc_macro2::{Ident, TokenStream};
use proc_macro2::Ident;
use proc_macro2::TokenStream;

#[derive(Clone)]
pub struct GeneratorState {
Expand All @@ -19,6 +20,8 @@ pub struct GeneratorState {
pub opctx: Ident,
/// The `OpState` used for storing op state.
pub opstate: Ident,
// The stack trace used for storing a stack trace.
pub stack_trace: Ident,
/// The `JsRuntimeState` used for storing the `Rc<JsRuntimeState>``.
pub js_runtime_state: Ident,
/// The `FastApiCallbackOptions` used in fast calls for fallback returns.
Expand Down Expand Up @@ -46,6 +49,7 @@ pub struct GeneratorState {
pub needs_isolate: bool,
pub needs_opstate: bool,
pub needs_opctx: bool,
pub needs_stack_trace: bool,
pub needs_js_runtime_state: bool,
pub needs_fast_api_callback_options: bool,
pub needs_self: bool,
Expand Down
3 changes: 3 additions & 0 deletions ops/op2/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,7 @@ fn generate_op2(
let info = Ident::new("info", Span::call_site());
let opctx = Ident::new("opctx", Span::call_site());
let opstate = Ident::new("opstate", Span::call_site());
let stack_trace = Ident::new("stack_trace", Span::call_site());
let js_runtime_state = Ident::new("js_runtime_state", Span::call_site());
let promise_id = Ident::new("promise_id", Span::call_site());
let slow_function = Ident::new("v8_fn_ptr", Span::call_site());
Expand All @@ -165,6 +166,7 @@ fn generate_op2(
opctx,
opstate,
js_runtime_state,
stack_trace,
fast_api_callback_options,
result,
retval,
Expand All @@ -181,6 +183,7 @@ fn generate_op2(
needs_isolate: false,
needs_opctx: false,
needs_opstate: false,
needs_stack_trace: false,
needs_js_runtime_state: false,
needs_fast_api_callback_options: false,
needs_self: config.method.is_some(),
Expand Down
18 changes: 17 additions & 1 deletion ops/op2/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ pub enum Special {
JsRuntimeState,
FastApiCallbackOptions,
Isolate,
StackTrace,
}

#[derive(Copy, Clone, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -335,7 +336,8 @@ impl Arg {
| Special::OpState
| Special::JsRuntimeState
| Special::HandleScope
| Special::Isolate,
| Special::Isolate
| Special::StackTrace,
) => true,
Self::Ref(
_,
Expand Down Expand Up @@ -755,6 +757,8 @@ pub enum AttributeModifier {
Number,
/// #[cppgc], for a resource backed managed by cppgc.
CppGcResource,
/// #[stack_trace], for a stack trace of the op call site
StackTrace,
}

impl AttributeModifier {
Expand All @@ -771,6 +775,7 @@ impl AttributeModifier {
AttributeModifier::State => "state",
AttributeModifier::Global => "global",
AttributeModifier::CppGcResource => "cppgc",
AttributeModifier::StackTrace => "stack_trace",
}
}
}
Expand Down Expand Up @@ -1239,6 +1244,7 @@ fn parse_attribute(
(#[cppgc]) => Some(AttributeModifier::CppGcResource),
(#[to_v8]) => Some(AttributeModifier::ToV8),
(#[from_v8]) => Some(AttributeModifier::FromV8),
(#[stack_trace]) => Some(AttributeModifier::StackTrace),
(#[allow ($_rule:path)]) => None,
(#[doc = $_attr:literal]) => None,
(#[cfg $_cfg:tt]) => None,
Expand Down Expand Up @@ -1513,6 +1519,16 @@ pub(crate) fn parse_type(
"argument",
))
}
AttributeModifier::StackTrace => {
if position == Position::RetVal {
return Err(ArgError::InvalidAttributePosition(
primary.name(),
"argument",
));
}

return Ok(Arg::Special(Special::StackTrace));
}
AttributeModifier::ToV8 if position == Position::Arg => {
return Err(ArgError::InvalidAttributePosition(
primary.name(),
Expand Down
Loading

0 comments on commit e6fb17b

Please sign in to comment.