From 24e398cf39ab694a55e65ee930bf12b0e73be7e2 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 3 Jul 2024 23:11:32 +0530 Subject: [PATCH 01/17] op2++ rework 1 --- core/cppgc.rs | 46 +++++++++++++++--------- core/extension_set.rs | 40 ++++++++++++++++----- core/extensions.rs | 26 +++++++++++++- core/lib.rs | 1 + core/ops.rs | 5 +++ core/runtime/bindings.rs | 39 +++++++++++++++++++-- core/runtime/jsrealm.rs | 4 +++ core/runtime/jsruntime.rs | 8 +++-- ops/Cargo.toml | 1 + ops/op2/config.rs | 12 ++++++- ops/op2/dispatch_slow.rs | 12 ++++++- ops/op2/generator_state.rs | 2 ++ ops/op2/mod.rs | 52 +++++++++++++++++++++++++++- testing/checkin/runner/extensions.rs | 3 ++ testing/checkin/runner/ops.rs | 43 +++++++++++++++++------ testing/unit/resource_test.ts | 8 +++++ 16 files changed, 257 insertions(+), 45 deletions(-) diff --git a/core/cppgc.rs b/core/cppgc.rs index 40dd147f6..b75505df6 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -41,35 +41,47 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( let func = templ.get_function(scope).unwrap(); let obj = func.new_instance(scope, &[]).unwrap(); - let heap = scope.get_cpp_heap().unwrap(); - - let member = unsafe { - v8::cppgc::make_garbage_collected( - heap, - CppGcObject { - tag: TypeId::of::(), - member: v8::cppgc::make_garbage_collected(heap, t), - }, - ) - }; - - unsafe { - v8::Object::wrap::>(scope, obj, &member); - } - - obj + wrap_object(scope, obj, t) } +pub fn wrap_object<'a, T: GarbageCollected + 'static>( + scope: &mut v8::HandleScope<'a>, + obj: v8::Local<'a, v8::Object>, + t: T +) -> v8::Local<'a, v8::Object> { + // Print string of T + println!("{:?}",obj.is_api_wrapper()); + let heap = scope.get_cpp_heap().unwrap(); + + let member = unsafe { + v8::cppgc::make_garbage_collected( + heap, + CppGcObject { + tag: TypeId::of::(), + member: v8::cppgc::make_garbage_collected(heap, t), + }, + ) + }; + + unsafe { + v8::Object::wrap::>(scope, obj, &member); + } + obj + } + #[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 { + + println!("{:?}", val.to_rust_string_lossy(scope)); let Ok(obj): Result, _> = val.try_into() else { return v8::cppgc::Member::empty(); }; + println!("{:?}", obj.is_api_wrapper()); if !obj.is_api_wrapper() { return v8::cppgc::Member::empty(); } diff --git a/core/extension_set.rs b/core/extension_set.rs index 938a771ea..fdfb2133f 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -11,6 +11,7 @@ use crate::extensions::GlobalTemplateMiddlewareFn; use crate::extensions::OpMiddlewareFn; use crate::modules::ModuleName; use crate::ops::OpCtx; +use crate::ops::OpMethodCtx; use crate::runtime::ExtensionTranspiler; use crate::runtime::JsRuntimeState; use crate::runtime::OpDriverImpl; @@ -20,6 +21,7 @@ use crate::FastString; use crate::GetErrorClassFn; use crate::ModuleCodeString; use crate::OpDecl; +use crate::_ops::OpMethodDecl; use crate::OpMetricsFactoryFn; use crate::OpState; use crate::SourceMapGetter; @@ -38,7 +40,7 @@ pub fn setup_op_state(op_state: &mut OpState, extensions: &mut [Extension]) { pub fn init_ops( deno_core_ops: &'static [OpDecl], extensions: &mut [Extension], -) -> Vec { +) -> (Vec, Vec) { // In debug build verify there that inter-Extension dependencies // are setup correctly. #[cfg(debug_assertions)] @@ -49,6 +51,7 @@ pub fn init_ops( .map(|e| e.op_count()) .fold(0, |ext_ops_count, count| count + ext_ops_count); let mut ops = Vec::with_capacity(no_of_ops + deno_core_ops.len()); + let mut op_methods = Vec::new(); // Collect all middlewares - deno_core extension must not have a middleware! let middlewares: Vec> = extensions @@ -77,13 +80,18 @@ pub fn init_ops( ..macroware(*ext_op) }); } + + let ext_method_ops = ext.init_method_ops(); + for ext_op in ext_method_ops { + op_methods.push(*ext_op); + } } // In debug build verify there are no duplicate ops. #[cfg(debug_assertions)] check_no_duplicate_op_names(&ops); - ops + (ops, op_methods) } /// This functions panics if any of the extensions is missing its dependencies. @@ -122,22 +130,24 @@ fn check_no_duplicate_op_names(ops: &[OpDecl]) { pub fn create_op_ctxs( op_decls: Vec, + op_method_decls: Vec, op_metrics_factory_fn: Option, op_driver: Rc, op_state: Rc>, runtime_state: Rc, get_error_class_fn: GetErrorClassFn, -) -> Box<[OpCtx]> { +) -> (Box<[OpCtx]>, Box<[OpMethodCtx]>) { let op_count = op_decls.len(); let mut op_ctxs = Vec::with_capacity(op_count); + let mut op_method_ctxs = vec![]; let runtime_state_ptr = runtime_state.as_ref() as *const _; - for (index, decl) in op_decls.into_iter().enumerate() { + let mut create_ctx = |index, decl| { let metrics_fn = op_metrics_factory_fn .as_ref() .and_then(|f| (f)(index as _, op_count, &decl)); - let op_ctx = OpCtx::new( + OpCtx::new( index as _, std::ptr::null_mut(), op_driver.clone(), @@ -146,12 +156,26 @@ pub fn create_op_ctxs( runtime_state_ptr, get_error_class_fn, metrics_fn, - ); + ) + }; + + for (index, decl) in op_decls.into_iter().enumerate() { + op_ctxs.push(create_ctx(index, decl)); + } - op_ctxs.push(op_ctx); + for (index, mut decl) in op_method_decls.into_iter().enumerate() { + decl.constructor.name = decl.name.0; + decl.constructor.name_fast = decl.name.1; + + op_method_ctxs.push(OpMethodCtx { + constructor: create_ctx(index, decl.constructor), + methods: decl.methods.into_iter().map(|method_decl| { + create_ctx(index, *method_decl) + }).collect(), + }); } - op_ctxs.into_boxed_slice() + (op_ctxs.into_boxed_slice(), op_method_ctxs.into_boxed_slice()) } pub fn get_middlewares_and_external_refs( diff --git a/core/extensions.rs b/core/extensions.rs index 40270b4ce..b199dad9f 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -168,10 +168,17 @@ pub type GlobalObjectMiddlewareFn = extern "C" fn noop() {} +#[derive(Clone, Copy)] +pub struct OpMethodDecl { + pub name: (&'static str, FastStaticString), + pub constructor: OpDecl, + pub methods: &'static [OpDecl], +} + #[derive(Clone, Copy)] pub struct OpDecl { pub name: &'static str, - pub(crate) name_fast: FastStaticString, + pub name_fast: FastStaticString, pub is_async: bool, pub is_reentrant: bool, pub arg_count: u8, @@ -391,6 +398,7 @@ macro_rules! extension { $(, bounds = [ $( $bound:path : $bound_type:ident ),+ ] )? $(, ops_fn = $ops_symbol:ident $( < $ops_param:ident > )? )? $(, ops = [ $( $(#[$m:meta])* $( $op:ident )::+ $( < $( $op_param:ident ),* > )? ),+ $(,)? ] )? + $(, objects = [ $( $object:ident )::+ ] )? $(, esm_entry_point = $esm_entry_point:expr )? $(, esm = [ $($esm:tt)* ] )? $(, lazy_loaded_esm = [ $($lazy_loaded_esm:tt)* ] )? @@ -454,6 +462,15 @@ macro_rules! extension { $( #[ $m ] )* $( $op )::+ $( :: < $($op_param),* > )? () }),+)?]), + // $object is a list of struct names have DECL methods + // Auto add ::DECL + // methods: ::std::borrow::Cow::Borrowed(&[ + // object_1::DECL, + // object_2::DECL, + // ]) + methods: ::std::borrow::Cow::Borrowed(&[ + $( $( $object )::+::DECL, )* + ]), external_references: ::std::borrow::Cow::Borrowed(&[ $( $external_reference ),* ]), global_template_middleware: ::std::option::Option::None, global_object_middleware: ::std::option::Option::None, @@ -587,6 +604,7 @@ pub struct Extension { pub lazy_loaded_esm_files: Cow<'static, [ExtensionFileSource]>, pub esm_entry_point: Option<&'static str>, pub ops: Cow<'static, [OpDecl]>, + pub methods: Cow<'static, [OpMethodDecl]>, pub external_references: Cow<'static, [v8::ExternalReference<'static>]>, pub global_template_middleware: Option, pub global_object_middleware: Option, @@ -610,6 +628,7 @@ impl Extension { lazy_loaded_esm_files: Cow::Borrowed(&[]), esm_entry_point: None, ops: self.ops.clone(), + methods: self.methods.clone(), external_references: self.external_references.clone(), global_template_middleware: self.global_template_middleware, global_object_middleware: self.global_object_middleware, @@ -628,6 +647,7 @@ impl Default for Extension { lazy_loaded_esm_files: Cow::Borrowed(&[]), esm_entry_point: None, ops: Cow::Borrowed(&[]), + methods: Cow::Borrowed(&[]), external_references: Cow::Borrowed(&[]), global_template_middleware: None, global_object_middleware: None, @@ -692,6 +712,10 @@ impl Extension { self.ops.as_ref() } + pub fn init_method_ops(&self) -> &[OpMethodDecl] { + self.methods.as_ref() + } + /// Allows setting up the initial op-state of an isolate at startup. pub fn take_state(&mut self, state: &mut OpState) { if let Some(op_fn) = self.op_state_fn.take() { diff --git a/core/lib.rs b/core/lib.rs index 31d6c896c..c4aa26e8c 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -181,6 +181,7 @@ pub mod _ops { pub use super::error_codes::get_error_code; pub use super::extensions::Op; pub use super::extensions::OpDecl; + pub use super::extensions::OpMethodDecl; #[cfg(debug_assertions)] pub use super::ops::reentrancy_check; pub use super::ops::OpCtx; diff --git a/core/ops.rs b/core/ops.rs index f071e012d..6f79de126 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -74,6 +74,11 @@ impl OpMetadata { } } +pub struct OpMethodCtx { + pub constructor: OpCtx, + pub methods: Vec, +} + /// Per-op context. /// // Note: We don't worry too much about the size of this struct because it's allocated once per realm, and is diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index 0b63fd63e..2f5418a84 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -22,6 +22,7 @@ use crate::modules::synthetic_module_evaluation_steps; use crate::modules::ImportAttributesKind; use crate::modules::ModuleMap; use crate::ops::OpCtx; +use crate::ops::OpMethodCtx; use crate::runtime::InitMode; use crate::runtime::JsRealm; use crate::FastStaticString; @@ -31,6 +32,7 @@ use crate::ModuleType; pub(crate) fn create_external_references( ops: &[OpCtx], + op_method_ctxs: &[OpMethodCtx], additional_references: &[v8::ExternalReference], sources: &[v8::OneByteConst], ops_in_snapshot: usize, @@ -81,6 +83,13 @@ pub(crate) fn create_external_references( }); } + for ctx in op_method_ctxs { + references.extend_from_slice(&ctx.constructor.external_references()); + for method in &ctx.methods { + references.extend_from_slice(&method.external_references()); + } + } + references.extend_from_slice(additional_references); for ctx in &ops[..ops_in_snapshot] { @@ -289,6 +298,7 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( scope: &mut v8::HandleScope<'s>, context: v8::Local<'s, v8::Context>, op_ctxs: &[OpCtx], + op_method_ctxs: &[OpMethodCtx], ) { let global = context.global(scope); @@ -323,15 +333,29 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( deno_core_ops_obj.set(scope, key.into(), op_fn.into()); } + + for op_method_ctx in op_method_ctxs { + let mut tmpl = op_ctx_template(scope, &op_method_ctx.constructor); + let prototype = tmpl.prototype_template(scope); + let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope); + + for method in op_method_ctx.methods.iter() { + let mut op_fn = op_ctx_template(scope, &method); + let method_key = method.decl.name_fast.v8_string(scope); + prototype.set(method_key.into(), op_fn.into()); + } + + let op_fn = tmpl.get_function(scope).unwrap(); + deno_core_ops_obj.set(scope, key.into(), op_fn.into()); + } } -fn op_ctx_function<'s>( +pub(crate) fn op_ctx_template<'s>( scope: &mut v8::HandleScope<'s>, op_ctx: &OpCtx, -) -> v8::Local<'s, v8::Function> { +) -> v8::Local<'s, v8::FunctionTemplate> { let op_ctx_ptr = op_ctx as *const OpCtx as *const c_void; let external = v8::External::new(scope, op_ctx_ptr as *mut c_void); - let v8name = op_ctx.decl.name_fast.v8_string(scope); let (slow_fn, fast_fn) = if op_ctx.metrics_enabled() { ( @@ -359,6 +383,15 @@ fn op_ctx_function<'s>( builder.build(scope) }; + template +} + +fn op_ctx_function<'s>( + scope: &mut v8::HandleScope<'s>, + op_ctx: &OpCtx, +) -> v8::Local<'s, v8::Function> { + let v8name = op_ctx.decl.name_fast.v8_string(scope); + let template = op_ctx_template(scope, op_ctx); let v8fn = template.get_function(scope).unwrap(); v8fn.set_name(v8name); v8fn diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs index e3bacc110..7d3cea322 100644 --- a/core/runtime/jsrealm.rs +++ b/core/runtime/jsrealm.rs @@ -13,6 +13,7 @@ use crate::modules::ModuleMap; use crate::modules::ModuleName; use crate::ops::ExternalOpsTracker; use crate::ops::OpCtx; +use crate::ops::OpMethodCtx; use crate::stats::RuntimeActivityTraces; use crate::tasks::V8TaskSpawnerFactory; use crate::web_timeout::WebTimers; @@ -68,6 +69,7 @@ pub struct ContextState { // We don't explicitly re-read this prop but need the slice to live alongside // the context pub(crate) op_ctxs: Box<[OpCtx]>, + pub(crate) op_method_ctxs: Box<[OpMethodCtx]>, pub(crate) isolate: Option<*mut v8::OwnedIsolate>, pub(crate) exception_state: Rc, pub(crate) has_next_tick_scheduled: Cell, @@ -81,6 +83,7 @@ impl ContextState { isolate_ptr: *mut v8::OwnedIsolate, get_error_class_fn: GetErrorClassFn, op_ctxs: Box<[OpCtx]>, + op_method_ctxs: Box<[OpMethodCtx]>, external_ops_tracker: ExternalOpsTracker, ) -> Self { Self { @@ -93,6 +96,7 @@ impl ContextState { wasm_instantiate_fn: Default::default(), activity_traces: Default::default(), op_ctxs, + op_method_ctxs, pending_ops: op_driver, task_spawner_factory: Default::default(), timers: Default::default(), diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index ce658acae..eb33225e3 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -712,15 +712,16 @@ impl JsRuntime { // ...now we're moving on to ops; set them up, create `OpCtx` for each op // and get ready to actually create V8 isolate... - let op_decls = + let (op_decls, op_method_decls) = extension_set::init_ops(crate::ops_builtin::BUILTIN_OPS, &mut extensions); let op_driver = Rc::new(OpDriverImpl::default()); let op_metrics_factory_fn = options.op_metrics_factory_fn.take(); let get_error_class_fn = options.get_error_class_fn.unwrap_or(&|_| "Error"); - let mut op_ctxs = extension_set::create_op_ctxs( + let (mut op_ctxs, op_method_ctxs) = extension_set::create_op_ctxs( op_decls, + op_method_decls, op_metrics_factory_fn, op_driver.clone(), op_state.clone(), @@ -768,6 +769,7 @@ impl JsRuntime { isolate_allocations.external_refs = Some(Box::new(bindings::create_external_references( &op_ctxs, + &op_method_ctxs, &additional_references, &isolate_allocations.externalized_sources, ops_in_snapshot, @@ -809,6 +811,7 @@ impl JsRuntime { isolate_ptr, options.get_error_class_fn.unwrap_or(&|_| "Error"), op_ctxs, + op_method_ctxs, op_state.borrow().external_ops_tracker.clone(), )); @@ -871,6 +874,7 @@ impl JsRuntime { scope, context, &context_state.op_ctxs, + &context_state.op_method_ctxs, ); } diff --git a/ops/Cargo.toml b/ops/Cargo.toml index 6d8ecae11..e45c609de 100644 --- a/ops/Cargo.toml +++ b/ops/Cargo.toml @@ -23,6 +23,7 @@ strum_macros.workspace = true syn.workspace = true thiserror.workspace = true +prettyplease.workspace = true [dev-dependencies] pretty_assertions.workspace = true prettyplease.workspace = true diff --git a/ops/op2/config.rs b/ops/op2/config.rs index 87b7b7dc9..38ed2c1fc 100644 --- a/ops/op2/config.rs +++ b/ops/op2/config.rs @@ -24,6 +24,8 @@ pub(crate) struct MacroConfig { pub reentrant: bool, /// Marks an op as a method on a wrapped object. pub method: Option, + /// Marks an op as a constructor + pub constructor: bool, } impl MacroConfig { @@ -61,7 +63,12 @@ impl MacroConfig { } for flag in flags { - if flag == "fast" { + if flag == "method" { + continue; + } + if flag == "constructor" { + config.constructor = true; + } else if flag == "fast" { config.fast = true; } else if flag.starts_with("fast(") { let tokens = @@ -140,6 +147,9 @@ impl MacroConfig { ( $($flags:tt $( ( $( $args:ty ),* ) )? ),+ ) => { Self::from_token_trees(flags, args) } + ( # [ $($flags:tt),+ ] ) => { + Self::from_flags(flags.into_iter().map(|flag| flag.to_string())) + } }) }) .map_err(|_| Op2Error::PatternMatchFailed("attribute", attr_string))??; diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs index 19cd5083e..bf7df8c01 100644 --- a/ops/op2/dispatch_slow.rs +++ b/ops/op2/dispatch_slow.rs @@ -875,6 +875,11 @@ pub fn return_value_infallible( ArgMarker::Number => { gs_quote!(generator_state(result) => (deno_core::_ops::RustToV8Marker::::from(#result))) } + ArgMarker::Cppgc if generator_state.use_this_cppgc => { + gs_quote!(generator_state(result, scope, retval) => ( + Some(deno_core::cppgc::wrap_object(&mut #scope, args.this(), #result)) + )) + } ArgMarker::Cppgc => { let marker = quote!(deno_core::_ops::RustToV8Marker::::from); if ret_type.is_option() { @@ -949,7 +954,12 @@ pub fn return_value_v8_value( quote!(deno_core::_ops::RustToV8Marker::::from(#result)) } ArgMarker::Cppgc => { - let marker = quote!(deno_core::_ops::RustToV8Marker::::from); + let marker = if !generator_state.use_this_cppgc { + quote!(deno_core::_ops::RustToV8Marker::::from) + } else { + quote!((|x| { deno_core::cppgc::wrap_object(#scope, args.this(), x) })) + }; + if ret_type.is_option() { quote!(#result.map(#marker)) } else { diff --git a/ops/op2/generator_state.rs b/ops/op2/generator_state.rs index 6769504a1..c15cb8345 100644 --- a/ops/op2/generator_state.rs +++ b/ops/op2/generator_state.rs @@ -51,6 +51,8 @@ pub struct GeneratorState { pub needs_fast_api_callback_options: bool, pub needs_fast_js_runtime_state: bool, pub needs_self: bool, + /// Wrap the `this` with cppgc object + pub use_this_cppgc: bool, } /// Quotes a set of generator_state fields, along with variables captured from diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 5638c2d15..7d6f64c5d 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -81,7 +81,56 @@ pub(crate) fn op2( attr: TokenStream, item: TokenStream, ) -> Result { - let func = parse2::(item)?; + let Ok(func) = parse2::(item.clone()) else { + let impl_block = parse2::(item)?; + // for each method in impl block, generate op2 + let mut tokens = TokenStream::new(); + let self_ty = impl_block.self_ty; + let mut constructor = None; + let mut methods = vec![]; + for item in impl_block.items { + if let syn::ImplItem::Fn(mut method) = item { + let attrs = method.attrs.swap_remove(0); + let func = ItemFn { + attrs: method.attrs.clone(), + vis: method.vis, + sig: method.sig.clone(), + block: Box::new(method.block), + }; + let mut config = MacroConfig::from_tokens(quote!{ + #attrs + })?; + use quote::ToTokens; + if config.constructor { + constructor = Some(method.sig.ident.clone()); + } else { + methods.push(method.sig.ident.clone()); + config.method = Some(self_ty.clone().to_token_stream().to_string()); + } + let op = generate_op2(config, func)?; + tokens.extend(op); + } + } + + let res = quote! { + impl #self_ty { + pub const DECL: deno_core::_ops::OpMethodDecl = deno_core::_ops::OpMethodDecl { + methods: &[ + #( + #self_ty::#methods(), + )* + ], + constructor: #self_ty::#constructor(), + name: ::deno_core::__op_name_fast!(#self_ty), + }; + + #tokens + } + }; + + return Ok(res); + }; + let config = MacroConfig::from_tokens(attr)?; generate_op2(config, func) } @@ -187,6 +236,7 @@ fn generate_op2( needs_fast_api_callback_options: false, needs_fast_js_runtime_state: false, needs_self: config.method.is_some(), + use_this_cppgc: config.constructor, }; let name = func.sig.ident; diff --git a/testing/checkin/runner/extensions.rs b/testing/checkin/runner/extensions.rs index bdabaa56d..a50b4d123 100644 --- a/testing/checkin/runner/extensions.rs +++ b/testing/checkin/runner/extensions.rs @@ -48,6 +48,9 @@ deno_core::extension!( ops_worker::op_worker_await_close, ops_worker::op_worker_terminate, ], + objects = [ + ops::Stateful + ], esm_entry_point = "ext:checkin_runtime/__init.js", esm = [ dir "checkin/runtime", diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs index 24eb3fe4c..46bb770d7 100644 --- a/testing/checkin/runner/ops.rs +++ b/testing/checkin/runner/ops.rs @@ -69,36 +69,57 @@ pub fn op_stats_delete( test_data.take::(name); } +#[derive(Debug, Clone)] pub struct Stateful { name: String, } impl GarbageCollected for Stateful {} +#[op2] impl Stateful { - #[op2(method(Stateful))] + #[constructor] + #[cppgc] + fn new(#[string] name: String) -> Stateful { + Stateful { name } + } + + #[method] + #[cppgc] + fn get(&self) -> Self { + self.clone() + } + + #[method] #[string] fn get_name(&self) -> String { self.name.clone() } - #[op2(fast, method(Stateful))] + #[fast] #[smi] fn len(&self) -> u32 { self.name.len() as u32 } +} - #[op2(async, method(Stateful))] - async fn delay(&self, #[smi] millis: u32) { - tokio::time::sleep(std::time::Duration::from_millis(millis as u64)).await; - println!("name: {}", self.name); - } +struct DOMPoint { + x: f64, + y: f64, + z: f64, + w: f64, } -// Make sure this compiles, we'll use it when we add registration. -#[allow(dead_code)] -const STATEFUL_DECL: [OpDecl; 3] = - [Stateful::get_name(), Stateful::len(), Stateful::delay()]; +impl GarbageCollected for DOMPoint {} + +#[op2] +impl DOMPoint { + #[constructor] + #[cppgc] + fn new(x: f64, y: f64, z: f64, w: f64) -> DOMPoint { + DOMPoint { x, y, z, w } + } +} #[op2(fast)] pub fn op_nop_generic(state: &mut OpState) { diff --git a/testing/unit/resource_test.ts b/testing/unit/resource_test.ts index 5b52a7d0f..780cd366a 100644 --- a/testing/unit/resource_test.ts +++ b/testing/unit/resource_test.ts @@ -6,6 +6,7 @@ const { op_file_open, op_async_make_cppgc_resource, op_async_get_cppgc_resource, + Stateful, } = Deno.core.ops; test(async function testPipe() { @@ -62,3 +63,10 @@ test(async function testCppgcAsync() { const resource = await op_async_make_cppgc_resource(); assertEquals(await op_async_get_cppgc_resource(resource), 42); }); + +test(function testOp2plusplus() { + const stateful = new Stateful("Hello"); + assertEquals(stateful.get_name(), "Hello"); + + let stateful2 = stateful.get(); +}); From f0426255ceb6eb0e328ad52da31a41129dc33c9f Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 09:54:15 +0530 Subject: [PATCH 02/17] Static members --- core/cppgc.rs | 63 ++++++++++-------- core/extension_set.rs | 23 +++++-- core/extensions.rs | 2 + core/gotham_state.rs | 10 +++ core/ops.rs | 2 + core/runtime/bindings.rs | 46 ++++++++++---- core/runtime/jsruntime.rs | 1 + ops/op2/config.rs | 8 ++- ops/op2/dispatch_slow.rs | 10 +-- ops/op2/mod.rs | 95 +++++++++++++++------------- testing/checkin/runner/extensions.rs | 2 +- testing/checkin/runner/ops.rs | 82 +++++++++++++----------- testing/unit/resource_test.ts | 22 +++++-- 13 files changed, 227 insertions(+), 139 deletions(-) diff --git a/core/cppgc.rs b/core/cppgc.rs index b75505df6..4a748dfa9 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -31,15 +31,29 @@ pub(crate) fn make_cppgc_template<'s>( v8::FunctionTemplate::new(scope, cppgc_template_constructor) } +pub(crate) struct FunctionTemplate { + pub template: v8::Global, +} + pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( scope: &mut v8::HandleScope<'a>, t: T, ) -> v8::Local<'a, v8::Object> { let state = JsRuntime::state_from(scope); - let templ = - v8::Local::new(scope, state.cppgc_template.borrow().as_ref().unwrap()); - let func = templ.get_function(scope).unwrap(); - let obj = func.new_instance(scope, &[]).unwrap(); + let opstate = state.op_state.borrow(); + + let id = TypeId::of::(); + let obj = + if let Some(templ) = opstate.try_borrow_untyped::(id) { + let templ = v8::Local::new(scope, &templ.template); + let inst = templ.instance_template(scope); + inst.new_instance(scope).unwrap() + } else { + let templ = + v8::Local::new(scope, state.cppgc_template.borrow().as_ref().unwrap()); + let func = templ.get_function(scope).unwrap(); + func.new_instance(scope, &[]).unwrap() + }; wrap_object(scope, obj, t) } @@ -47,27 +61,25 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( pub fn wrap_object<'a, T: GarbageCollected + 'static>( scope: &mut v8::HandleScope<'a>, obj: v8::Local<'a, v8::Object>, - t: T + t: T, ) -> v8::Local<'a, v8::Object> { - // Print string of T - println!("{:?}",obj.is_api_wrapper()); - let heap = scope.get_cpp_heap().unwrap(); - - let member = unsafe { - v8::cppgc::make_garbage_collected( - heap, - CppGcObject { - tag: TypeId::of::(), - member: v8::cppgc::make_garbage_collected(heap, t), - }, - ) - }; - - unsafe { - v8::Object::wrap::>(scope, obj, &member); - } - obj - } + let heap = scope.get_cpp_heap().unwrap(); + + let member = unsafe { + v8::cppgc::make_garbage_collected( + heap, + CppGcObject { + tag: TypeId::of::(), + member: v8::cppgc::make_garbage_collected(heap, t), + }, + ) + }; + + unsafe { + v8::Object::wrap::>(scope, obj, &member); + } + obj +} #[doc(hidden)] #[allow(clippy::needless_lifetimes)] @@ -75,13 +87,10 @@ pub fn try_unwrap_cppgc_object<'sc, T: GarbageCollected + 'static>( scope: &mut v8::HandleScope<'sc>, val: v8::Local<'sc, v8::Value>, ) -> v8::cppgc::Member { - - println!("{:?}", val.to_rust_string_lossy(scope)); let Ok(obj): Result, _> = val.try_into() else { return v8::cppgc::Member::empty(); }; - println!("{:?}", obj.is_api_wrapper()); if !obj.is_api_wrapper() { return v8::cppgc::Member::empty(); } diff --git a/core/extension_set.rs b/core/extension_set.rs index fdfb2133f..7f9122879 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -21,10 +21,10 @@ use crate::FastString; use crate::GetErrorClassFn; use crate::ModuleCodeString; use crate::OpDecl; -use crate::_ops::OpMethodDecl; use crate::OpMetricsFactoryFn; use crate::OpState; use crate::SourceMapGetter; +use crate::_ops::OpMethodDecl; /// Contribute to the `OpState` from each extension. pub fn setup_op_state(op_state: &mut OpState, extensions: &mut [Extension]) { @@ -83,7 +83,7 @@ pub fn init_ops( let ext_method_ops = ext.init_method_ops(); for ext_op in ext_method_ops { - op_methods.push(*ext_op); + op_methods.push(*ext_op); } } @@ -168,14 +168,25 @@ pub fn create_op_ctxs( decl.constructor.name_fast = decl.name.1; op_method_ctxs.push(OpMethodCtx { + id: (decl.id)(), constructor: create_ctx(index, decl.constructor), - methods: decl.methods.into_iter().map(|method_decl| { - create_ctx(index, *method_decl) - }).collect(), + methods: decl + .methods + .into_iter() + .map(|method_decl| create_ctx(index, *method_decl)) + .collect(), + static_methods: decl + .static_methods + .into_iter() + .map(|method_decl| create_ctx(index, *method_decl)) + .collect(), }); } - (op_ctxs.into_boxed_slice(), op_method_ctxs.into_boxed_slice()) + ( + op_ctxs.into_boxed_slice(), + op_method_ctxs.into_boxed_slice(), + ) } pub fn get_middlewares_and_external_refs( diff --git a/core/extensions.rs b/core/extensions.rs index b199dad9f..59eee5193 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -170,9 +170,11 @@ extern "C" fn noop() {} #[derive(Clone, Copy)] pub struct OpMethodDecl { + pub id: fn() -> std::any::TypeId, pub name: (&'static str, FastStaticString), pub constructor: OpDecl, pub methods: &'static [OpDecl], + pub static_methods: &'static [OpDecl], } #[derive(Clone, Copy)] diff --git a/core/gotham_state.rs b/core/gotham_state.rs index fad5c3f99..b56a6320a 100644 --- a/core/gotham_state.rs +++ b/core/gotham_state.rs @@ -22,6 +22,11 @@ impl GothamState { self.data.insert(type_id, Box::new(t)); } + // For internal use. + pub(crate) fn put_untyped(&mut self, t: TypeId, v: T) { + self.data.insert(t, Box::new(v)); + } + /// Determines if the current value exists in `GothamState` storage. pub fn has(&self) -> bool { let type_id = TypeId::of::(); @@ -34,6 +39,11 @@ impl GothamState { self.data.get(&type_id).and_then(|b| b.downcast_ref()) } + // For internal use. + pub fn try_borrow_untyped(&self, t: TypeId) -> Option<&T> { + self.data.get(&t).and_then(|b| b.downcast_ref()) + } + /// Borrows a value from the `GothamState` storage. pub fn borrow(&self) -> &T { self.try_borrow().unwrap_or_else(|| missing::()) diff --git a/core/ops.rs b/core/ops.rs index 6f79de126..bc29c72d5 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -75,8 +75,10 @@ impl OpMetadata { } pub struct OpMethodCtx { + pub id: std::any::TypeId, pub constructor: OpCtx, pub methods: Vec, + pub static_methods: Vec, } /// Per-op context. diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index 2f5418a84..ac5352091 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -88,6 +88,9 @@ pub(crate) fn create_external_references( for method in &ctx.methods { references.extend_from_slice(&method.external_references()); } + for method in &ctx.static_methods { + references.extend_from_slice(&method.external_references()); + } } references.extend_from_slice(additional_references); @@ -292,10 +295,13 @@ pub(crate) fn initialize_primordials_and_infra( Ok(()) } - +use deno_core::OpState; +use std::cell::RefCell; +use std::rc::Rc; /// Set up JavaScript bindings for ops. pub(crate) fn initialize_deno_core_ops_bindings<'s>( scope: &mut v8::HandleScope<'s>, + op_state: Rc>, context: v8::Local<'s, v8::Context>, op_ctxs: &[OpCtx], op_method_ctxs: &[OpMethodCtx], @@ -335,18 +341,32 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( } for op_method_ctx in op_method_ctxs { - let mut tmpl = op_ctx_template(scope, &op_method_ctx.constructor); - let prototype = tmpl.prototype_template(scope); - let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope); - - for method in op_method_ctx.methods.iter() { - let mut op_fn = op_ctx_template(scope, &method); - let method_key = method.decl.name_fast.v8_string(scope); - prototype.set(method_key.into(), op_fn.into()); - } + let mut tmpl = op_ctx_template(scope, &op_method_ctx.constructor); + let prototype = tmpl.prototype_template(scope); + let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope); + + for method in op_method_ctx.methods.iter() { + let mut op_fn = op_ctx_template(scope, &method); + let method_key = method.decl.name_fast.v8_string(scope); + prototype.set(method_key.into(), op_fn.into()); + } - let op_fn = tmpl.get_function(scope).unwrap(); - deno_core_ops_obj.set(scope, key.into(), op_fn.into()); + for method in op_method_ctx.static_methods.iter() { + let mut op_fn = op_ctx_template(scope, &method); + let method_key = method.decl.name_fast.v8_string(scope); + tmpl.set(method_key.into(), op_fn.into()); + } + + let op_fn = tmpl.get_function(scope).unwrap(); + deno_core_ops_obj.set(scope, key.into(), op_fn.into()); + + let id = op_method_ctx.id; + op_state.borrow_mut().put_untyped( + id, + deno_core::cppgc::FunctionTemplate { + template: v8::Global::new(scope, tmpl), + }, + ); } } @@ -391,7 +411,7 @@ fn op_ctx_function<'s>( op_ctx: &OpCtx, ) -> v8::Local<'s, v8::Function> { let v8name = op_ctx.decl.name_fast.v8_string(scope); - let template = op_ctx_template(scope, op_ctx); + let template = op_ctx_template(scope, op_ctx); let v8fn = template.get_function(scope).unwrap(); v8fn.set_name(v8name); v8fn diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index eb33225e3..04bbc0997 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -872,6 +872,7 @@ impl JsRuntime { if init_mode.needs_ops_bindings() { bindings::initialize_deno_core_ops_bindings( scope, + op_state, context, &context_state.op_ctxs, &context_state.op_method_ctxs, diff --git a/ops/op2/config.rs b/ops/op2/config.rs index 38ed2c1fc..cd633ccb1 100644 --- a/ops/op2/config.rs +++ b/ops/op2/config.rs @@ -26,6 +26,8 @@ pub(crate) struct MacroConfig { pub method: Option, /// Marks an op as a constructor pub constructor: bool, + /// Marks an op as a static member + pub static_member: bool, } impl MacroConfig { @@ -64,10 +66,12 @@ impl MacroConfig { for flag in flags { if flag == "method" { - continue; + continue; } if flag == "constructor" { - config.constructor = true; + config.constructor = true; + } else if flag == "static_method" { + config.static_member = true; } else if flag == "fast" { config.fast = true; } else if flag.starts_with("fast(") { diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs index bf7df8c01..7fe8b3497 100644 --- a/ops/op2/dispatch_slow.rs +++ b/ops/op2/dispatch_slow.rs @@ -876,9 +876,9 @@ pub fn return_value_infallible( gs_quote!(generator_state(result) => (deno_core::_ops::RustToV8Marker::::from(#result))) } ArgMarker::Cppgc if generator_state.use_this_cppgc => { - gs_quote!(generator_state(result, scope, retval) => ( - Some(deno_core::cppgc::wrap_object(&mut #scope, args.this(), #result)) - )) + gs_quote!(generator_state(result, scope, retval) => ( + Some(deno_core::cppgc::wrap_object(&mut #scope, args.this(), #result)) + )) } ArgMarker::Cppgc => { let marker = quote!(deno_core::_ops::RustToV8Marker::::from); @@ -955,9 +955,9 @@ pub fn return_value_v8_value( } ArgMarker::Cppgc => { let marker = if !generator_state.use_this_cppgc { - quote!(deno_core::_ops::RustToV8Marker::::from) + quote!(deno_core::_ops::RustToV8Marker::::from) } else { - quote!((|x| { deno_core::cppgc::wrap_object(#scope, args.this(), x) })) + quote!((|x| { deno_core::cppgc::wrap_object(#scope, args.this(), x) })) }; if ret_type.is_option() { diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 7d6f64c5d..8b0ee8e18 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -82,53 +82,62 @@ pub(crate) fn op2( item: TokenStream, ) -> Result { let Ok(func) = parse2::(item.clone()) else { - let impl_block = parse2::(item)?; - // for each method in impl block, generate op2 - let mut tokens = TokenStream::new(); - let self_ty = impl_block.self_ty; - let mut constructor = None; - let mut methods = vec![]; - for item in impl_block.items { - if let syn::ImplItem::Fn(mut method) = item { - let attrs = method.attrs.swap_remove(0); - let func = ItemFn { - attrs: method.attrs.clone(), - vis: method.vis, - sig: method.sig.clone(), - block: Box::new(method.block), - }; - let mut config = MacroConfig::from_tokens(quote!{ - #attrs - })?; - use quote::ToTokens; - if config.constructor { - constructor = Some(method.sig.ident.clone()); - } else { - methods.push(method.sig.ident.clone()); - config.method = Some(self_ty.clone().to_token_stream().to_string()); - } - let op = generate_op2(config, func)?; - tokens.extend(op); + let impl_block = parse2::(item)?; + // for each method in impl block, generate op2 + let mut tokens = TokenStream::new(); + let self_ty = impl_block.self_ty; + let mut constructor = None; + let mut methods = vec![]; + let mut static_methods = vec![]; + for item in impl_block.items { + if let syn::ImplItem::Fn(mut method) = item { + let attrs = method.attrs.swap_remove(0); + let func = ItemFn { + attrs: method.attrs.clone(), + vis: method.vis, + sig: method.sig.clone(), + block: Box::new(method.block), + }; + let mut config = MacroConfig::from_tokens(quote! { + #attrs + })?; + use quote::ToTokens; + if config.constructor { + constructor = Some(method.sig.ident.clone()); + } else if config.static_member { + static_methods.push(method.sig.ident.clone()); + } else { + methods.push(method.sig.ident.clone()); + config.method = Some(self_ty.clone().to_token_stream().to_string()); } + let op = generate_op2(config, func)?; + tokens.extend(op); } + } - let res = quote! { - impl #self_ty { - pub const DECL: deno_core::_ops::OpMethodDecl = deno_core::_ops::OpMethodDecl { - methods: &[ - #( - #self_ty::#methods(), - )* - ], - constructor: #self_ty::#constructor(), - name: ::deno_core::__op_name_fast!(#self_ty), - }; - - #tokens - } - }; + let res = quote! { + impl #self_ty { + pub const DECL: deno_core::_ops::OpMethodDecl = deno_core::_ops::OpMethodDecl { + methods: &[ + #( + #self_ty::#methods(), + )* + ], + static_methods: &[ + #( + #self_ty::#static_methods(), + )* + ], + constructor: #self_ty::#constructor(), + name: ::deno_core::__op_name_fast!(#self_ty), + id: || ::std::any::TypeId::of::<#self_ty>() + }; + + #tokens + } + }; - return Ok(res); + return Ok(res); }; let config = MacroConfig::from_tokens(attr)?; diff --git a/testing/checkin/runner/extensions.rs b/testing/checkin/runner/extensions.rs index a50b4d123..81bd8b576 100644 --- a/testing/checkin/runner/extensions.rs +++ b/testing/checkin/runner/extensions.rs @@ -49,7 +49,7 @@ deno_core::extension!( ops_worker::op_worker_terminate, ], objects = [ - ops::Stateful + ops::DOMPoint ], esm_entry_point = "ext:checkin_runtime/__init.js", esm = [ diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs index 46bb770d7..1e65f3d28 100644 --- a/testing/checkin/runner/ops.rs +++ b/testing/checkin/runner/ops.rs @@ -2,12 +2,14 @@ use std::cell::RefCell; use std::rc::Rc; +use deno_core::error::AnyError; use deno_core::op2; use deno_core::stats::RuntimeActivityDiff; use deno_core::stats::RuntimeActivitySnapshot; use deno_core::stats::RuntimeActivityStats; use deno_core::stats::RuntimeActivityStatsFactory; use deno_core::stats::RuntimeActivityStatsFilter; +use deno_core::v8; use deno_core::GarbageCollected; use deno_core::OpDecl; use deno_core::OpState; @@ -69,41 +71,7 @@ pub fn op_stats_delete( test_data.take::(name); } -#[derive(Debug, Clone)] -pub struct Stateful { - name: String, -} - -impl GarbageCollected for Stateful {} - -#[op2] -impl Stateful { - #[constructor] - #[cppgc] - fn new(#[string] name: String) -> Stateful { - Stateful { name } - } - - #[method] - #[cppgc] - fn get(&self) -> Self { - self.clone() - } - - #[method] - #[string] - fn get_name(&self) -> String { - self.name.clone() - } - - #[fast] - #[smi] - fn len(&self) -> u32 { - self.name.len() as u32 - } -} - -struct DOMPoint { +pub struct DOMPoint { x: f64, y: f64, z: f64, @@ -116,8 +84,48 @@ impl GarbageCollected for DOMPoint {} impl DOMPoint { #[constructor] #[cppgc] - fn new(x: f64, y: f64, z: f64, w: f64) -> DOMPoint { - DOMPoint { x, y, z, w } + fn new( + x: Option, + y: Option, + z: Option, + w: Option, + ) -> DOMPoint { + DOMPoint { + x: x.unwrap_or(0.0), + y: y.unwrap_or(0.0), + z: z.unwrap_or(0.0), + w: w.unwrap_or(0.0), + } + } + + #[static_method] + #[cppgc] + fn from_point( + scope: &mut v8::HandleScope, + other: v8::Local, + ) -> Result { + fn get( + scope: &mut v8::HandleScope, + other: v8::Local, + key: &str, + ) -> Option { + let key = v8::String::new(scope, key).unwrap(); + other + .get(scope, key.into()) + .map(|x| x.to_number(scope).unwrap().value()) + } + + Ok(DOMPoint { + x: get(scope, other, "x").unwrap_or(0.0), + y: get(scope, other, "y").unwrap_or(0.0), + z: get(scope, other, "z").unwrap_or(0.0), + w: get(scope, other, "w").unwrap_or(0.0), + }) + } + + #[fast] + fn x(&self) -> f64 { + self.x } } diff --git a/testing/unit/resource_test.ts b/testing/unit/resource_test.ts index 780cd366a..68741e2bf 100644 --- a/testing/unit/resource_test.ts +++ b/testing/unit/resource_test.ts @@ -6,7 +6,7 @@ const { op_file_open, op_async_make_cppgc_resource, op_async_get_cppgc_resource, - Stateful, + DOMPoint, } = Deno.core.ops; test(async function testPipe() { @@ -64,9 +64,21 @@ test(async function testCppgcAsync() { assertEquals(await op_async_get_cppgc_resource(resource), 42); }); -test(function testOp2plusplus() { - const stateful = new Stateful("Hello"); - assertEquals(stateful.get_name(), "Hello"); +test(function testDomPoint() { + const p1 = new DOMPoint(100, 100); + const p2 = new DOMPoint(); + const p3 = DOMPoint.from_point({ x: 200 }); + const p4 = DOMPoint.from_point({ x: 0, y: 100, z: 99.9, w: 100 }); + assertEquals(p1.x(), 100); + assertEquals(p2.x(), 0); + assertEquals(p3.x(), 200); + assertEquals(p4.x(), 0); - let stateful2 = stateful.get(); + let caught; + try { + new DOMPoint("bad"); + } catch (e) { + caught = e; + } + assert(caught); }); From 7e38d0874240cf3d01a4725fe6b49de1f337339c Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 10:02:51 +0530 Subject: [PATCH 03/17] some cleanup --- core/cppgc.rs | 27 ++++++++++++--------------- core/extension_set.rs | 2 +- core/extensions.rs | 17 ++++++----------- core/runtime/bindings.rs | 15 ++++++--------- ops/op2/dispatch_slow.rs | 2 +- testing/checkin/runner/ops.rs | 9 ++++----- 6 files changed, 30 insertions(+), 42 deletions(-) diff --git a/core/cppgc.rs b/core/cppgc.rs index 4a748dfa9..4478ab4e2 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -31,10 +31,6 @@ pub(crate) fn make_cppgc_template<'s>( v8::FunctionTemplate::new(scope, cppgc_template_constructor) } -pub(crate) struct FunctionTemplate { - pub template: v8::Global, -} - pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( scope: &mut v8::HandleScope<'a>, t: T, @@ -43,17 +39,18 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( let opstate = state.op_state.borrow(); let id = TypeId::of::(); - let obj = - if let Some(templ) = opstate.try_borrow_untyped::(id) { - let templ = v8::Local::new(scope, &templ.template); - let inst = templ.instance_template(scope); - inst.new_instance(scope).unwrap() - } else { - let templ = - v8::Local::new(scope, state.cppgc_template.borrow().as_ref().unwrap()); - let func = templ.get_function(scope).unwrap(); - func.new_instance(scope, &[]).unwrap() - }; + let obj = if let Some(templ) = + opstate.try_borrow_untyped::>(id) + { + let templ = v8::Local::new(scope, templ); + let inst = templ.instance_template(scope); + inst.new_instance(scope).unwrap() + } else { + let templ = + v8::Local::new(scope, state.cppgc_template.borrow().as_ref().unwrap()); + let func = templ.get_function(scope).unwrap(); + func.new_instance(scope, &[]).unwrap() + }; wrap_object(scope, obj, t) } diff --git a/core/extension_set.rs b/core/extension_set.rs index 7f9122879..26fec685c 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -142,7 +142,7 @@ pub fn create_op_ctxs( let mut op_method_ctxs = vec![]; let runtime_state_ptr = runtime_state.as_ref() as *const _; - let mut create_ctx = |index, decl| { + let create_ctx = |index, decl| { let metrics_fn = op_metrics_factory_fn .as_ref() .and_then(|f| (f)(index as _, op_count, &decl)); diff --git a/core/extensions.rs b/core/extensions.rs index 59eee5193..a74107a5d 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -464,13 +464,7 @@ macro_rules! extension { $( #[ $m ] )* $( $op )::+ $( :: < $($op_param),* > )? () }),+)?]), - // $object is a list of struct names have DECL methods - // Auto add ::DECL - // methods: ::std::borrow::Cow::Borrowed(&[ - // object_1::DECL, - // object_2::DECL, - // ]) - methods: ::std::borrow::Cow::Borrowed(&[ + objects: ::std::borrow::Cow::Borrowed(&[ $( $( $object )::+::DECL, )* ]), external_references: ::std::borrow::Cow::Borrowed(&[ $( $external_reference ),* ]), @@ -606,7 +600,7 @@ pub struct Extension { pub lazy_loaded_esm_files: Cow<'static, [ExtensionFileSource]>, pub esm_entry_point: Option<&'static str>, pub ops: Cow<'static, [OpDecl]>, - pub methods: Cow<'static, [OpMethodDecl]>, + pub objects: Cow<'static, [OpMethodDecl]>, pub external_references: Cow<'static, [v8::ExternalReference<'static>]>, pub global_template_middleware: Option, pub global_object_middleware: Option, @@ -630,7 +624,7 @@ impl Extension { lazy_loaded_esm_files: Cow::Borrowed(&[]), esm_entry_point: None, ops: self.ops.clone(), - methods: self.methods.clone(), + objects: self.objects.clone(), external_references: self.external_references.clone(), global_template_middleware: self.global_template_middleware, global_object_middleware: self.global_object_middleware, @@ -649,7 +643,7 @@ impl Default for Extension { lazy_loaded_esm_files: Cow::Borrowed(&[]), esm_entry_point: None, ops: Cow::Borrowed(&[]), - methods: Cow::Borrowed(&[]), + objects: Cow::Borrowed(&[]), external_references: Cow::Borrowed(&[]), global_template_middleware: None, global_object_middleware: None, @@ -714,8 +708,9 @@ impl Extension { self.ops.as_ref() } + /// Called at JsRuntime startup to initialize method ops in the isolate. pub fn init_method_ops(&self) -> &[OpMethodDecl] { - self.methods.as_ref() + self.objects.as_ref() } /// Allows setting up the initial op-state of an isolate at startup. diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index ac5352091..253897b70 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -341,18 +341,18 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( } for op_method_ctx in op_method_ctxs { - let mut tmpl = op_ctx_template(scope, &op_method_ctx.constructor); + let tmpl = op_ctx_template(scope, &op_method_ctx.constructor); let prototype = tmpl.prototype_template(scope); let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope); for method in op_method_ctx.methods.iter() { - let mut op_fn = op_ctx_template(scope, &method); + let op_fn = op_ctx_template(scope, &method); let method_key = method.decl.name_fast.v8_string(scope); prototype.set(method_key.into(), op_fn.into()); } for method in op_method_ctx.static_methods.iter() { - let mut op_fn = op_ctx_template(scope, &method); + let op_fn = op_ctx_template(scope, &method); let method_key = method.decl.name_fast.v8_string(scope); tmpl.set(method_key.into(), op_fn.into()); } @@ -361,12 +361,9 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( deno_core_ops_obj.set(scope, key.into(), op_fn.into()); let id = op_method_ctx.id; - op_state.borrow_mut().put_untyped( - id, - deno_core::cppgc::FunctionTemplate { - template: v8::Global::new(scope, tmpl), - }, - ); + op_state + .borrow_mut() + .put_untyped(id, v8::Global::new(scope, tmpl)); } } diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs index 7fe8b3497..79be2d563 100644 --- a/ops/op2/dispatch_slow.rs +++ b/ops/op2/dispatch_slow.rs @@ -876,7 +876,7 @@ pub fn return_value_infallible( gs_quote!(generator_state(result) => (deno_core::_ops::RustToV8Marker::::from(#result))) } ArgMarker::Cppgc if generator_state.use_this_cppgc => { - gs_quote!(generator_state(result, scope, retval) => ( + gs_quote!(generator_state(result, scope) => ( Some(deno_core::cppgc::wrap_object(&mut #scope, args.this(), #result)) )) } diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs index 1e65f3d28..7f5646e20 100644 --- a/testing/checkin/runner/ops.rs +++ b/testing/checkin/runner/ops.rs @@ -11,7 +11,6 @@ use deno_core::stats::RuntimeActivityStatsFactory; use deno_core::stats::RuntimeActivityStatsFilter; use deno_core::v8; use deno_core::GarbageCollected; -use deno_core::OpDecl; use deno_core::OpState; use super::extensions::SomeType; @@ -72,10 +71,10 @@ pub fn op_stats_delete( } pub struct DOMPoint { - x: f64, - y: f64, - z: f64, - w: f64, + pub x: f64, + pub y: f64, + pub z: f64, + pub w: f64, } impl GarbageCollected for DOMPoint {} From b5a43d20a6b5ebbbfec32c42aa01d0a56841c72b Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 12:44:16 +0530 Subject: [PATCH 04/17] Refactor --- ops/op2/mod.rs | 59 +----------------- ops/op2/object_wrap.rs | 134 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 137 insertions(+), 56 deletions(-) create mode 100644 ops/op2/object_wrap.rs diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 8b0ee8e18..6b56b02e2 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -32,6 +32,7 @@ pub mod dispatch_fast; pub mod dispatch_shared; pub mod dispatch_slow; pub mod generator_state; +pub mod object_wrap; pub mod signature; pub mod signature_retval; @@ -83,68 +84,14 @@ pub(crate) fn op2( ) -> Result { let Ok(func) = parse2::(item.clone()) else { let impl_block = parse2::(item)?; - // for each method in impl block, generate op2 - let mut tokens = TokenStream::new(); - let self_ty = impl_block.self_ty; - let mut constructor = None; - let mut methods = vec![]; - let mut static_methods = vec![]; - for item in impl_block.items { - if let syn::ImplItem::Fn(mut method) = item { - let attrs = method.attrs.swap_remove(0); - let func = ItemFn { - attrs: method.attrs.clone(), - vis: method.vis, - sig: method.sig.clone(), - block: Box::new(method.block), - }; - let mut config = MacroConfig::from_tokens(quote! { - #attrs - })?; - use quote::ToTokens; - if config.constructor { - constructor = Some(method.sig.ident.clone()); - } else if config.static_member { - static_methods.push(method.sig.ident.clone()); - } else { - methods.push(method.sig.ident.clone()); - config.method = Some(self_ty.clone().to_token_stream().to_string()); - } - let op = generate_op2(config, func)?; - tokens.extend(op); - } - } - - let res = quote! { - impl #self_ty { - pub const DECL: deno_core::_ops::OpMethodDecl = deno_core::_ops::OpMethodDecl { - methods: &[ - #( - #self_ty::#methods(), - )* - ], - static_methods: &[ - #( - #self_ty::#static_methods(), - )* - ], - constructor: #self_ty::#constructor(), - name: ::deno_core::__op_name_fast!(#self_ty), - id: || ::std::any::TypeId::of::<#self_ty>() - }; - - #tokens - } - }; - - return Ok(res); + return object_wrap::generate_impl_ops(impl_block); }; let config = MacroConfig::from_tokens(attr)?; generate_op2(config, func) } -fn generate_op2( +pub(crate) fn generate_op2( config: MacroConfig, func: ItemFn, ) -> Result { diff --git a/ops/op2/object_wrap.rs b/ops/op2/object_wrap.rs new file mode 100644 index 000000000..53451f89a --- /dev/null +++ b/ops/op2/object_wrap.rs @@ -0,0 +1,134 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +use proc_macro2::TokenStream; +use quote::quote; +use quote::ToTokens; +use syn::ImplItem; +use syn::ItemFn; +use syn::ItemImpl; + +use crate::op2::generate_op2; +use crate::op2::MacroConfig; +use crate::op2::Op2Error; + +// Object wrap for Cppgc-backed objects +// +// This module generates the glue code declarations +// for `impl` blocks to create JS objects in Rust +// using the op2 infra. +// +// ```rust +// #[op] +// impl MyObject { +// #[constructor] // <-- first attribute defines binding type +// #[cppgc] // <-- attributes for op2 +// fn new() -> MyObject { +// MyObject::new() +// } +// +// #[static_method] +// #[cppgc] +// fn static_method() -> MyObject { +// // ... +// } +// +// #[method] +// #[smi] +// fn x(&self) -> i32 { +// // ... +// } +// } +// +// The generated OpMethodDecl that can be passed to +// `deno_core::extension!` macro to register the object +// +// ```rust +// deno_core::extension!( +// ..., +// objects = [MyObject], +// ) +// ``` +// +// ```js +// import { MyObject } from "ext:core/ops"; +// ``` +// +// Currently supported bindings: +// - constructor +// - methods +// - static methods +// +// Planned support: +// - getters +// - setters +// +pub(crate) fn generate_impl_ops( + item: ItemImpl, +) -> Result { + let mut tokens = TokenStream::new(); + + let self_ty = &item.self_ty; + let self_ty_ident = self_ty.to_token_stream().to_string(); + + // State + let mut constructor = None; + let mut methods = Vec::new(); + let mut static_methods = Vec::new(); + + for item in item.items { + match item { + ImplItem::Fn(mut method) => { + /* First attribute idents, for all functions in block */ + let attrs = method.attrs.swap_remove(0); + + let ident = method.sig.ident.clone(); + let func = ItemFn { + attrs: method.attrs, + vis: method.vis, + sig: method.sig, + block: Box::new(method.block), + }; + + let mut config = MacroConfig::from_tokens(quote! { + #attrs + })?; + if config.constructor { + constructor = Some(ident); + } else if config.static_member { + static_methods.push(ident); + } else { + methods.push(ident); + config.method = Some(self_ty_ident.clone()); + } + + let op = generate_op2(config, func); + tokens.extend(op); + } + _ => {} + } + } + + let res = quote! { + impl #self_ty { + pub const DECL: deno_core::_ops::OpMethodDecl = deno_core::_ops::OpMethodDecl { + methods: &[ + #( + #self_ty::#methods(), + )* + ], + static_methods: &[ + #( + #self_ty::#static_methods(), + )* + ], + constructor: #self_ty::#constructor(), + name: ::deno_core::__op_name_fast!(#self_ty), + id: || ::std::any::TypeId::of::<#self_ty>() + }; + + #tokens + } + }; + + Ok(res) +} From a105ac99bc25dcae43fb9de652cf40473f5f145c Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 12:57:04 +0530 Subject: [PATCH 05/17] Add some comments --- core/cppgc.rs | 7 +++++++ core/extensions.rs | 3 +++ 2 files changed, 10 insertions(+) diff --git a/core/cppgc.rs b/core/cppgc.rs index 4478ab4e2..c67814497 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -38,6 +38,12 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( let state = JsRuntime::state_from(scope); let opstate = state.op_state.borrow(); + // To support initializing object wraps correctly, we store the function + // template in the opstate during binding with `T`'s TypeId as the key + // because it'll be pretty annoying to propogate `T` everywhere. + // + // Here we try to retrive a function template for `T`, falling back to + // the default cppgc template. let id = TypeId::of::(); let obj = if let Some(templ) = opstate.try_borrow_untyped::>(id) @@ -55,6 +61,7 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( wrap_object(scope, obj, t) } +// Wrap an API object (eg: `args.This()`) pub fn wrap_object<'a, T: GarbageCollected + 'static>( scope: &mut v8::HandleScope<'a>, obj: v8::Local<'a, v8::Object>, diff --git a/core/extensions.rs b/core/extensions.rs index a74107a5d..b2282bf5b 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -168,8 +168,11 @@ pub type GlobalObjectMiddlewareFn = extern "C" fn noop() {} +// Delcration for object wrappers. #[derive(Clone, Copy)] pub struct OpMethodDecl { + // TypeId::of::() is unstable-nightly in const context so + // we store the fn pointer instead. pub id: fn() -> std::any::TypeId, pub name: (&'static str, FastStaticString), pub constructor: OpDecl, From 4d0bd642382f0ed1fc71b340419d741dddf9ceda Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 12:58:50 +0530 Subject: [PATCH 06/17] x --- ops/op2/config.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ops/op2/config.rs b/ops/op2/config.rs index cd633ccb1..d154f8939 100644 --- a/ops/op2/config.rs +++ b/ops/op2/config.rs @@ -66,6 +66,7 @@ impl MacroConfig { for flag in flags { if flag == "method" { + // Doesn't need any special handling, its more of a marker. continue; } if flag == "constructor" { From 58d65fdf3767df2575b65658921352d6d75789ef Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 14:11:30 +0530 Subject: [PATCH 07/17] Tests --- core/extension_set.rs | 4 +-- core/runtime/bindings.rs | 33 ++++++++++++++++--- core/runtime/jsruntime.rs | 1 + ops/Cargo.toml | 1 - ops/op2/object_wrap.rs | 49 +++++++++++++--------------- testing/checkin.d.ts | 8 +++++ testing/checkin/runner/extensions.rs | 1 + testing/checkin/runtime/__init.js | 2 ++ testing/checkin/runtime/object.ts | 5 +++ testing/tsconfig.json | 3 +- testing/unit/resource_test.ts | 3 +- 11 files changed, 74 insertions(+), 36 deletions(-) create mode 100644 testing/checkin/runtime/object.ts diff --git a/core/extension_set.rs b/core/extension_set.rs index 26fec685c..f76510647 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -172,12 +172,12 @@ pub fn create_op_ctxs( constructor: create_ctx(index, decl.constructor), methods: decl .methods - .into_iter() + .iter() .map(|method_decl| create_ctx(index, *method_decl)) .collect(), static_methods: decl .static_methods - .into_iter() + .iter() .map(|method_decl| create_ctx(index, *method_decl)) .collect(), }); diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index 253897b70..f7260f636 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -1,8 +1,10 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. use anyhow::Context; +use std::cell::RefCell; use std::mem::MaybeUninit; use std::os::raw::c_void; use std::path::PathBuf; +use std::rc::Rc; use url::Url; use v8::MapFnTo; @@ -29,6 +31,7 @@ use crate::FastStaticString; use crate::FastString; use crate::JsRuntime; use crate::ModuleType; +use crate::OpState; pub(crate) fn create_external_references( ops: &[OpCtx], @@ -295,9 +298,7 @@ pub(crate) fn initialize_primordials_and_infra( Ok(()) } -use deno_core::OpState; -use std::cell::RefCell; -use std::rc::Rc; + /// Set up JavaScript bindings for ops. pub(crate) fn initialize_deno_core_ops_bindings<'s>( scope: &mut v8::HandleScope<'s>, @@ -346,18 +347,19 @@ pub(crate) fn initialize_deno_core_ops_bindings<'s>( let key = op_method_ctx.constructor.decl.name_fast.v8_string(scope); for method in op_method_ctx.methods.iter() { - let op_fn = op_ctx_template(scope, &method); + let op_fn = op_ctx_template(scope, method); let method_key = method.decl.name_fast.v8_string(scope); prototype.set(method_key.into(), op_fn.into()); } for method in op_method_ctx.static_methods.iter() { - let op_fn = op_ctx_template(scope, &method); + let op_fn = op_ctx_template(scope, method); let method_key = method.decl.name_fast.v8_string(scope); tmpl.set(method_key.into(), op_fn.into()); } let op_fn = tmpl.get_function(scope).unwrap(); + op_fn.set_name(key); deno_core_ops_obj.set(scope, key.into(), op_fn.into()); let id = op_method_ctx.id; @@ -822,6 +824,7 @@ where /// to a JavaScript function that executes and op. pub fn create_exports_for_ops_virtual_module<'s>( op_ctxs: &[OpCtx], + op_method_ctxs: &[OpMethodCtx], scope: &mut v8::HandleScope<'s>, global: v8::Local, ) -> Vec<(FastStaticString, v8::Local<'s, v8::Value>)> { @@ -854,5 +857,25 @@ pub fn create_exports_for_ops_virtual_module<'s>( exports.push((op_ctx.decl.name_fast, op_fn.into())); } + for ctx in op_method_ctxs { + let tmpl = op_ctx_template(scope, &ctx.constructor); + let prototype = tmpl.prototype_template(scope); + + for method in ctx.methods.iter() { + let op_fn = op_ctx_template(scope, method); + let method_key = method.decl.name_fast.v8_string(scope); + prototype.set(method_key.into(), op_fn.into()); + } + + for method in ctx.static_methods.iter() { + let op_fn = op_ctx_template(scope, method); + let method_key = method.decl.name_fast.v8_string(scope); + tmpl.set(method_key.into(), op_fn.into()); + } + + let op_fn = tmpl.get_function(scope).unwrap(); + exports.push((ctx.constructor.decl.name_fast, op_fn.into())); + } + exports } diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index 04bbc0997..f1740387b 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -1103,6 +1103,7 @@ impl JsRuntime { let global = context_local.global(scope); let synthetic_module_exports = create_exports_for_ops_virtual_module( &context_state.op_ctxs, + &context_state.op_method_ctxs, scope, global, ); diff --git a/ops/Cargo.toml b/ops/Cargo.toml index e45c609de..6d8ecae11 100644 --- a/ops/Cargo.toml +++ b/ops/Cargo.toml @@ -23,7 +23,6 @@ strum_macros.workspace = true syn.workspace = true thiserror.workspace = true -prettyplease.workspace = true [dev-dependencies] pretty_assertions.workspace = true prettyplease.workspace = true diff --git a/ops/op2/object_wrap.rs b/ops/op2/object_wrap.rs index 53451f89a..fe658a144 100644 --- a/ops/op2/object_wrap.rs +++ b/ops/op2/object_wrap.rs @@ -76,35 +76,32 @@ pub(crate) fn generate_impl_ops( let mut static_methods = Vec::new(); for item in item.items { - match item { - ImplItem::Fn(mut method) => { - /* First attribute idents, for all functions in block */ - let attrs = method.attrs.swap_remove(0); + if let ImplItem::Fn(mut method) = item { + /* First attribute idents, for all functions in block */ + let attrs = method.attrs.swap_remove(0); - let ident = method.sig.ident.clone(); - let func = ItemFn { - attrs: method.attrs, - vis: method.vis, - sig: method.sig, - block: Box::new(method.block), - }; - - let mut config = MacroConfig::from_tokens(quote! { - #attrs - })?; - if config.constructor { - constructor = Some(ident); - } else if config.static_member { - static_methods.push(ident); - } else { - methods.push(ident); - config.method = Some(self_ty_ident.clone()); - } + let ident = method.sig.ident.clone(); + let func = ItemFn { + attrs: method.attrs, + vis: method.vis, + sig: method.sig, + block: Box::new(method.block), + }; - let op = generate_op2(config, func); - tokens.extend(op); + let mut config = MacroConfig::from_tokens(quote! { + #attrs + })?; + if config.constructor { + constructor = Some(ident); + } else if config.static_member { + static_methods.push(ident); + } else { + methods.push(ident); + config.method = Some(self_ty_ident.clone()); } - _ => {} + + let op = generate_op2(config, func); + tokens.extend(op); } } diff --git a/testing/checkin.d.ts b/testing/checkin.d.ts index bf3c22d27..867f0f6bf 100644 --- a/testing/checkin.d.ts +++ b/testing/checkin.d.ts @@ -51,4 +51,12 @@ declare module "ext:core/ops" { function op_worker_send(...any): any; function op_worker_spawn(...any): any; function op_worker_terminate(...any): any; + + class DOMPoint { + constructor(x?: number, y?: number, z?: number, w?: number); + static from_point( + other: { x?: number; y?: number; z?: number; w?: number }, + ): DOMPoint; + x(): number; + } } diff --git a/testing/checkin/runner/extensions.rs b/testing/checkin/runner/extensions.rs index 81bd8b576..aa77b9f8f 100644 --- a/testing/checkin/runner/extensions.rs +++ b/testing/checkin/runner/extensions.rs @@ -57,6 +57,7 @@ deno_core::extension!( "__init.js", "checkin:async" = "async.ts", "checkin:console" = "console.ts", + "checkin:object" = "object.ts", "checkin:error" = "error.ts", "checkin:timers" = "timers.ts", "checkin:worker" = "worker.ts", diff --git a/testing/checkin/runtime/__init.js b/testing/checkin/runtime/__init.js index 9b4ac1e00..73ad97692 100644 --- a/testing/checkin/runtime/__init.js +++ b/testing/checkin/runtime/__init.js @@ -5,9 +5,11 @@ import * as error from "checkin:error"; import * as timers from "checkin:timers"; import * as worker from "checkin:worker"; import * as throw_ from "checkin:throw"; +import * as object from "checkin:object"; async; error; throw_; +object; globalThis.console = console.console; globalThis.setTimeout = timers.setTimeout; diff --git a/testing/checkin/runtime/object.ts b/testing/checkin/runtime/object.ts new file mode 100644 index 000000000..8ffc560c3 --- /dev/null +++ b/testing/checkin/runtime/object.ts @@ -0,0 +1,5 @@ +// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. + +import { DOMPoint } from "ext:core/ops"; + +export { DOMPoint }; diff --git a/testing/tsconfig.json b/testing/tsconfig.json index 7023f08bf..7e03b485a 100644 --- a/testing/tsconfig.json +++ b/testing/tsconfig.json @@ -9,7 +9,8 @@ "checkin:testing": ["checkin/runtime/testing.ts"], "checkin:throw": ["checkin/runtime/throw.ts"], "checkin:timers": ["checkin/runtime/timers.ts"], - "checkin:worker": ["checkin/runtime/worker.ts"] + "checkin:worker": ["checkin/runtime/worker.ts"], + "checkin:object": ["checkin/runtime/object.ts"] }, "lib": ["ESNext", "DOM", "ES2023.Array"], "module": "ESNext", diff --git a/testing/unit/resource_test.ts b/testing/unit/resource_test.ts index 68741e2bf..700a9275c 100644 --- a/testing/unit/resource_test.ts +++ b/testing/unit/resource_test.ts @@ -1,12 +1,12 @@ // Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. import { assert, assertArrayEquals, assertEquals, test } from "checkin:testing"; +import { DOMPoint } from "checkin:object"; const { op_pipe_create, op_file_open, op_async_make_cppgc_resource, op_async_get_cppgc_resource, - DOMPoint, } = Deno.core.ops; test(async function testPipe() { @@ -76,6 +76,7 @@ test(function testDomPoint() { let caught; try { + // @ts-expect-error bad arg test new DOMPoint("bad"); } catch (e) { caught = e; From f41415859dd28ee44f9ca786231555e5765b7915 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 14:29:36 +0530 Subject: [PATCH 08/17] Fixl int --- testing/checkin/runner/ops.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/testing/checkin/runner/ops.rs b/testing/checkin/runner/ops.rs index 7f5646e20..03d0866f2 100644 --- a/testing/checkin/runner/ops.rs +++ b/testing/checkin/runner/ops.rs @@ -126,6 +126,18 @@ impl DOMPoint { fn x(&self) -> f64 { self.x } + #[fast] + fn y(&self) -> f64 { + self.y + } + #[fast] + fn w(&self) -> f64 { + self.w + } + #[fast] + fn z(&self) -> f64 { + self.z + } } #[op2(fast)] From 19e91c6a0688e9e18e4de5a76f6dd7f8ded75e89 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Thu, 4 Jul 2024 14:33:20 +0530 Subject: [PATCH 09/17] Update core/cppgc.rs --- core/cppgc.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/core/cppgc.rs b/core/cppgc.rs index c67814497..600e6b1e4 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -38,9 +38,9 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( let state = JsRuntime::state_from(scope); let opstate = state.op_state.borrow(); - // To support initializing object wraps correctly, we store the function - // template in the opstate during binding with `T`'s TypeId as the key - // because it'll be pretty annoying to propogate `T` everywhere. + // To initialize object wraps correctly, we store the function + // template in OpState with `T`'s TypeId as the key when binding + // because it'll be pretty annoying to propogate `T` generic everywhere. // // Here we try to retrive a function template for `T`, falling back to // the default cppgc template. From 774f379e419f684fb5c4aaead20574d2eb77ad7a Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 9 Jul 2024 11:26:26 +0530 Subject: [PATCH 10/17] perf: use isolate instead of scope --- Cargo.lock | 4 ++-- Cargo.toml | 4 ++-- core/cppgc.rs | 15 ++++++--------- core/runtime/jsruntime.rs | 8 +++++++- ops/op2/dispatch_fast.rs | 39 ++++++++++++++++++++++++--------------- ops/op2/dispatch_slow.rs | 3 ++- 6 files changed, 43 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 41513d46e..6c3af73e3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2735,9 +2735,9 @@ dependencies = [ [[package]] name = "v8" -version = "0.95.0" +version = "0.97.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c4395d3c43b81368d91335ffc78d71cb6e3288d311ab6a55094634cb2afdc5c5" +checksum = "5ecc402c55b363c29901bdd0613c68213b01c5b2a3ee362d5e985cb74901b472" dependencies = [ "bindgen", "bitflags", diff --git a/Cargo.toml b/Cargo.toml index 410ef9804..a55360da0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,7 @@ deno_ops = { version = "0.168.0", path = "./ops" } serde_v8 = { version = "0.201.0", path = "./serde_v8" } deno_core_testing = { path = "./testing" } -v8 = { version = "0.95.0", default-features = false } +v8 = { version = "0.97.0", default-features = false } deno_ast = { version = "=0.35.3", features = ["transpiling"] } deno_unsync = "0.3.2" deno_core_icudata = "0.0.73" @@ -77,7 +77,7 @@ opt-level = 1 codegen-units = 1 incremental = true lto = true -opt-level = 'z' # Optimize for size +opt-level = 3 # Optimize for size # Build release with debug symbols: cargo build --profile=release-with-debug [profile.release-with-debug] diff --git a/core/cppgc.rs b/core/cppgc.rs index 600e6b1e4..532f57880 100644 --- a/core/cppgc.rs +++ b/core/cppgc.rs @@ -63,11 +63,11 @@ pub fn make_cppgc_object<'a, T: GarbageCollected + 'static>( // Wrap an API object (eg: `args.This()`) pub fn wrap_object<'a, T: GarbageCollected + 'static>( - scope: &mut v8::HandleScope<'a>, + isolate: &mut v8::Isolate, obj: v8::Local<'a, v8::Object>, t: T, ) -> v8::Local<'a, v8::Object> { - let heap = scope.get_cpp_heap().unwrap(); + let heap = isolate.get_cpp_heap().unwrap(); let member = unsafe { v8::cppgc::make_garbage_collected( @@ -80,7 +80,7 @@ pub fn wrap_object<'a, T: GarbageCollected + 'static>( }; unsafe { - v8::Object::wrap::>(scope, obj, &member); + v8::Object::wrap::>(isolate, obj, &member); } obj } @@ -88,27 +88,24 @@ pub fn wrap_object<'a, T: GarbageCollected + 'static>( #[doc(hidden)] #[allow(clippy::needless_lifetimes)] pub fn try_unwrap_cppgc_object<'sc, T: GarbageCollected + 'static>( - scope: &mut v8::HandleScope<'sc>, + isolate: &mut v8::Isolate, val: v8::Local<'sc, v8::Value>, ) -> v8::cppgc::Member { let Ok(obj): Result, _> = val.try_into() else { return v8::cppgc::Member::empty(); }; - if !obj.is_api_wrapper() { return v8::cppgc::Member::empty(); } - let ptr = - unsafe { v8::Object::unwrap::>(scope, obj) }; + unsafe { v8::Object::unwrap::>(isolate, obj) }; + let Some(obj) = ptr.borrow() else { return v8::cppgc::Member::empty(); }; - if obj.tag != TypeId::of::() { return v8::cppgc::Member::empty(); } - let mut h = v8::cppgc::Member::empty(); h.set(&obj.member); h diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index f1740387b..914156060 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -719,7 +719,7 @@ impl JsRuntime { let op_metrics_factory_fn = options.op_metrics_factory_fn.take(); let get_error_class_fn = options.get_error_class_fn.unwrap_or(&|_| "Error"); - let (mut op_ctxs, op_method_ctxs) = extension_set::create_op_ctxs( + let (mut op_ctxs, mut op_method_ctxs) = extension_set::create_op_ctxs( op_decls, op_method_decls, op_metrics_factory_fn, @@ -794,6 +794,12 @@ impl JsRuntime { for op_ctx in op_ctxs.iter_mut() { op_ctx.isolate = isolate.as_mut() as *mut Isolate; } + for op_method_ctx in op_method_ctxs.iter_mut() { + op_method_ctx.constructor.isolate = isolate.as_mut() as *mut Isolate; + for op in &mut op_method_ctx.methods { + op.isolate = isolate.as_mut() as *mut Isolate; + } + } // TODO(Bartlomieju): this can be simplified let isolate_ptr = setup::create_isolate_ptr(); diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs index e7f77b886..153fa22a7 100644 --- a/ops/op2/dispatch_fast.rs +++ b/ops/op2/dispatch_fast.rs @@ -438,24 +438,11 @@ pub(crate) fn generate_dispatch_fast( } else { quote!() }; - - let with_opctx = if generator_state.needs_fast_opctx { - generator_state.needs_fast_api_callback_options = true; - gs_quote!(generator_state(opctx, fast_api_callback_options) => { - let #opctx: &'s _ = unsafe { - &*(deno_core::v8::Local::::cast(unsafe { #fast_api_callback_options.data.data }).value() - as *const deno_core::_ops::OpCtx) - }; - }) - } else { - quote!() - }; - let with_self = if generator_state.needs_self { - generator_state.needs_fast_scope = true; + generator_state.needs_isolate = 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 self_handle_ = deno_core::_ops::try_unwrap_cppgc_object::<#self_ty>(#scope, this.into()); let Some(self_) = self_handle_.borrow() else { #fast_api_callback_options.fallback = true; // SAFETY: All fast return types have zero as a valid value @@ -466,6 +453,27 @@ pub(crate) fn generate_dispatch_fast( quote!() }; + let with_isolate = + if generator_state.needs_isolate && !generator_state.needs_fast_opctx { + generator_state.needs_fast_opctx = true; + gs_quote!(generator_state(opctx, scope) => + (let mut #scope = unsafe { &mut *#opctx.isolate };) + ) + } else { + quote!() + }; + + let with_opctx = if generator_state.needs_fast_opctx { + generator_state.needs_fast_api_callback_options = true; + gs_quote!(generator_state(opctx, fast_api_callback_options) => { + let #opctx: &'s _ = unsafe { + &*(deno_core::v8::Local::::cast(unsafe { #fast_api_callback_options.data.data }).value() + as *const deno_core::_ops::OpCtx) + }; + }) + } else { + quote!() + }; let name = &generator_state.name; let call = if generator_state.needs_self { quote!(self_. #name) @@ -553,6 +561,7 @@ pub(crate) fn generate_dispatch_fast( #with_scope #with_opctx #with_js_runtime_state + #with_isolate #with_self let #result = { #(#call_args)* diff --git a/ops/op2/dispatch_slow.rs b/ops/op2/dispatch_slow.rs index 79be2d563..044ae608f 100644 --- a/ops/op2/dispatch_slow.rs +++ b/ops/op2/dispatch_slow.rs @@ -258,7 +258,7 @@ pub(crate) fn with_self( { 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() { + if self_handle_..borrow().is_none() { #throw_exception; } let mut self_persistent_ = deno_core::v8::cppgc::Persistent::empty(); @@ -876,6 +876,7 @@ pub fn return_value_infallible( gs_quote!(generator_state(result) => (deno_core::_ops::RustToV8Marker::::from(#result))) } ArgMarker::Cppgc if generator_state.use_this_cppgc => { + generator_state.needs_isolate = true; gs_quote!(generator_state(result, scope) => ( Some(deno_core::cppgc::wrap_object(&mut #scope, args.this(), #result)) )) From 54aa520cecaaf7fc43c31fec7d1eee886d7c9d3e Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 9 Jul 2024 11:34:03 +0530 Subject: [PATCH 11/17] fix --- ops/op2/dispatch_fast.rs | 21 +++++++++++---------- ops/op2/generator_state.rs | 1 + ops/op2/mod.rs | 1 + 3 files changed, 13 insertions(+), 10 deletions(-) diff --git a/ops/op2/dispatch_fast.rs b/ops/op2/dispatch_fast.rs index 153fa22a7..1af96ccc9 100644 --- a/ops/op2/dispatch_fast.rs +++ b/ops/op2/dispatch_fast.rs @@ -439,7 +439,7 @@ pub(crate) fn generate_dispatch_fast( quote!() }; let with_self = if generator_state.needs_self { - generator_state.needs_isolate = true; + generator_state.needs_fast_isolate = 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>(#scope, this.into()); @@ -453,15 +453,16 @@ pub(crate) fn generate_dispatch_fast( quote!() }; - let with_isolate = - if generator_state.needs_isolate && !generator_state.needs_fast_opctx { - generator_state.needs_fast_opctx = true; - gs_quote!(generator_state(opctx, scope) => - (let mut #scope = unsafe { &mut *#opctx.isolate };) - ) - } else { - quote!() - }; + let with_isolate = if generator_state.needs_fast_isolate + && !generator_state.needs_fast_scope + { + generator_state.needs_fast_opctx = true; + gs_quote!(generator_state(opctx, scope) => + (let mut #scope = unsafe { &mut *#opctx.isolate };) + ) + } else { + quote!() + }; let with_opctx = if generator_state.needs_fast_opctx { generator_state.needs_fast_api_callback_options = true; diff --git a/ops/op2/generator_state.rs b/ops/op2/generator_state.rs index c15cb8345..daa7f1491 100644 --- a/ops/op2/generator_state.rs +++ b/ops/op2/generator_state.rs @@ -43,6 +43,7 @@ pub struct GeneratorState { pub needs_retval: bool, pub needs_scope: bool, pub needs_fast_scope: bool, + pub needs_fast_isolate: bool, pub needs_isolate: bool, pub needs_opstate: bool, pub needs_opctx: bool, diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 6b56b02e2..942c63a27 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -183,6 +183,7 @@ pub(crate) fn generate_op2( moves: vec![], needs_retval: false, needs_scope: false, + needs_fast_isolate: false, needs_isolate: false, needs_opctx: false, needs_opstate: false, From 3ecd6b3e8b6343965d19fa2e4f56a804588546dd Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 9 Jul 2024 11:47:23 +0530 Subject: [PATCH 12/17] reviews --- core/extension_set.rs | 9 +++++++-- core/extensions.rs | 6 +++++- core/gotham_state.rs | 2 +- core/ops.rs | 5 +++++ core/runtime/bindings.rs | 5 +++-- ops/op2/mod.rs | 2 ++ ops/op2/object_wrap.rs | 4 ++++ 7 files changed, 27 insertions(+), 6 deletions(-) diff --git a/core/extension_set.rs b/core/extension_set.rs index f76510647..6b0a687b5 100644 --- a/core/extension_set.rs +++ b/core/extension_set.rs @@ -51,7 +51,12 @@ pub fn init_ops( .map(|e| e.op_count()) .fold(0, |ext_ops_count, count| count + ext_ops_count); let mut ops = Vec::with_capacity(no_of_ops + deno_core_ops.len()); - let mut op_methods = Vec::new(); + + let no_of_methods = extensions + .iter() + .map(|e| e.method_op_count()) + .fold(0, |ext_ops_count, count| count + ext_ops_count); + let mut op_methods = Vec::with_capacity(no_of_methods); // Collect all middlewares - deno_core extension must not have a middleware! let middlewares: Vec> = extensions @@ -139,7 +144,7 @@ pub fn create_op_ctxs( ) -> (Box<[OpCtx]>, Box<[OpMethodCtx]>) { let op_count = op_decls.len(); let mut op_ctxs = Vec::with_capacity(op_count); - let mut op_method_ctxs = vec![]; + let mut op_method_ctxs = Vec::with_capacity(op_method_decls.len()); let runtime_state_ptr = runtime_state.as_ref() as *const _; let create_ctx = |index, decl| { diff --git a/core/extensions.rs b/core/extensions.rs index b2282bf5b..c8a8bb61c 100644 --- a/core/extensions.rs +++ b/core/extensions.rs @@ -168,7 +168,7 @@ pub type GlobalObjectMiddlewareFn = extern "C" fn noop() {} -// Delcration for object wrappers. +// Declaration for object wrappers. #[derive(Clone, Copy)] pub struct OpMethodDecl { // TypeId::of::() is unstable-nightly in const context so @@ -701,6 +701,10 @@ impl Extension { self.ops.len() } + pub fn method_op_count(&self) -> usize { + self.objects.len() + } + /// Called at JsRuntime startup to initialize ops in the isolate. pub fn init_ops(&mut self) -> &[OpDecl] { if !self.enabled { diff --git a/core/gotham_state.rs b/core/gotham_state.rs index b56a6320a..b7fb834f3 100644 --- a/core/gotham_state.rs +++ b/core/gotham_state.rs @@ -40,7 +40,7 @@ impl GothamState { } // For internal use. - pub fn try_borrow_untyped(&self, t: TypeId) -> Option<&T> { + pub(crate) fn try_borrow_untyped(&self, t: TypeId) -> Option<&T> { self.data.get(&t).and_then(|b| b.downcast_ref()) } diff --git a/core/ops.rs b/core/ops.rs index bc29c72d5..d8f25f6e6 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -74,10 +74,15 @@ impl OpMetadata { } } +/// Per-object contexts for members. pub struct OpMethodCtx { + /// TypeId of the wrapped type pub id: std::any::TypeId, + /// Op context for the constructor pub constructor: OpCtx, + /// Per-op context for the methods pub methods: Vec, + /// Per-op context for the static methods pub static_methods: Vec, } diff --git a/core/runtime/bindings.rs b/core/runtime/bindings.rs index f7260f636..a1c9dcea0 100644 --- a/core/runtime/bindings.rs +++ b/core/runtime/bindings.rs @@ -47,7 +47,8 @@ pub(crate) fn create_external_references( + BUILTIN_SOURCES.len() + (ops.len() * 4) + additional_references.len() - + sources.len(), + + sources.len() + + op_method_ctxs.len(), ); references.push(v8::ExternalReference { @@ -828,7 +829,7 @@ pub fn create_exports_for_ops_virtual_module<'s>( scope: &mut v8::HandleScope<'s>, global: v8::Local, ) -> Vec<(FastStaticString, v8::Local<'s, v8::Value>)> { - let mut exports = Vec::with_capacity(op_ctxs.len()); + let mut exports = Vec::with_capacity(op_ctxs.len() + op_method_ctxs.len()); let deno_obj = get(scope, global, DENO, "Deno"); let deno_core_obj = get(scope, deno_obj, CORE, "Deno.core"); diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 942c63a27..8e8bc70ab 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -62,6 +62,8 @@ pub enum Op2Error { TooManyFastAlternatives, #[error("The flags for this attribute were not sorted alphabetically. They should be listed as '({0})'.")] ImproperlySortedAttribute(String), + #[error("Only one constructor is allowed per object")] + MultipleConstructors, } #[derive(Debug, Error)] diff --git a/ops/op2/object_wrap.rs b/ops/op2/object_wrap.rs index fe658a144..77266d06e 100644 --- a/ops/op2/object_wrap.rs +++ b/ops/op2/object_wrap.rs @@ -92,6 +92,10 @@ pub(crate) fn generate_impl_ops( #attrs })?; if config.constructor { + if constructor.is_some() { + return Err(Op2Error::MultipleConstructors); + } + constructor = Some(ident); } else if config.static_member { static_methods.push(ident); From a00360615140abbd29299d20bbb6e0935c687dea Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 9 Jul 2024 11:48:27 +0530 Subject: [PATCH 13/17] x --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index a55360da0..53f98ddb1 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,7 +77,7 @@ opt-level = 1 codegen-units = 1 incremental = true lto = true -opt-level = 3 # Optimize for size +opt-level = 'z' # Optimize for size # Build release with debug symbols: cargo build --profile=release-with-debug [profile.release-with-debug] From 48ea286c69322d160fb30a44b4a6be13e11c735d Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 6 Aug 2024 11:02:35 +0200 Subject: [PATCH 14/17] x --- ops/op2/valid_args.md | 46 +++++++++++++++++++++---------------------- testing/ops.d.ts | 2 +- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/ops/op2/valid_args.md b/ops/op2/valid_args.md index 058c0d5a6..569613972 100644 --- a/ops/op2/valid_args.md +++ b/ops/op2/valid_args.md @@ -22,38 +22,38 @@ | X | &v8::**V8** | X | **V8** | | | X | v8::Local | X | any | | | X | v8::Local | X | **V8** | | -| X | #[global] v8::Global | | any | ⚠️ Slower than `v8::Local`. | -| X | #[global] v8::Global | | **V8** | ⚠️ Slower than `v8::Local`. | -| X | #[serde] SerdeType | | any | ⚠️ May be slow. | -| X | #[serde] (Tuple, Tuple) | | any | ⚠️ May be slow. | -| | #[anybuffer] &mut [u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[anybuffer] &[u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[anybuffer] *mut u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| | #[anybuffer] *const u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| X | #[arraybuffer] &mut [u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[arraybuffer] &[u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[arraybuffer] *mut u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| X | #[arraybuffer] *const u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[global] v8::Global | | any | ⚠️ Slower than `v8::Local`. | +| X | #[global] v8::Global | | **V8** | ⚠️ Slower than `v8::Local`. | +| X | #[serde] SerdeType | | any | ⚠️ May be slow. | +| X | #[serde] (Tuple, Tuple) | | any | ⚠️ May be slow. | +| | #[anybuffer] &mut [u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[anybuffer] &[u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[anybuffer] *mut u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[anybuffer] *const u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[arraybuffer] &mut [u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[arraybuffer] &[u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[arraybuffer] *mut u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[arraybuffer] *const u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | | X | #[arraybuffer(copy)] Vec | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | | X | #[arraybuffer(copy)] Box<[u8]> | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | | X | #[arraybuffer(copy)] bytes::Bytes | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | -| | #[buffer] &mut [u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[buffer] &[u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[buffer] *mut u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| | #[buffer] *const u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[buffer] &mut [u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[buffer] &[u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[buffer] *mut u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[buffer] *const u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | | X | #[buffer(copy)] Vec | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] Box<[u8]> | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] bytes::Bytes | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | -| X | #[buffer] &mut [u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[buffer] &[u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[buffer] &mut [u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[buffer] &[u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | | X | #[buffer(copy)] Vec | X | UInt32Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] Box<[u32]> | X | UInt32Array (resizable=true,false) | Safe, but forces a copy. | -| | #[buffer] V8Slice | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] V8Slice | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | | | #[buffer(detach)] V8Slice | X | ArrayBufferView (resizable=true,false) | Safe. | -| | #[buffer] V8ResizableSlice | X | ArrayBufferView (resizable=true) | ⚠️ JS may modify the contents of slices obtained from buffer. | -| | #[buffer] JsBuffer | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] V8ResizableSlice | X | ArrayBufferView (resizable=true) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] JsBuffer | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | | X | #[buffer(detach)] JsBuffer | | ArrayBufferView (resizable=true,false) | Safe. | -| | #[buffer(unsafe)] bytes::Bytes | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of the buffer. | +| | #[buffer(unsafe)] bytes::Bytes | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of the buffer. | | | #[buffer(detach)] bytes::Bytes | X | ArrayBufferView (resizable=true,false) | Safe. | | X | *const std::ffi::c_void | X | External | | | X | *mut std::ffi::c_void | X | External | | @@ -67,4 +67,4 @@ | X | #[state] &StateObject | X | | Extracts an object from `OpState`. | | 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 | *mut v8::Isolate | X | | ⚠️ Extremely dangerous, may crash if you don't use `nofast` depending on what you do. | diff --git a/testing/ops.d.ts b/testing/ops.d.ts index fe0fd1a71..cc45158c8 100644 --- a/testing/ops.d.ts +++ b/testing/ops.d.ts @@ -20,7 +20,7 @@ export function op_worker_send(...any: any[]): any; export function op_worker_spawn(...any: any[]): any; export function op_worker_terminate(...any: any[]): any; -class DOMPoint { +export class DOMPoint { constructor(x?: number, y?: number, z?: number, w?: number); static from_point( other: { x?: number; y?: number; z?: number; w?: number }, From 76b9ab2637b01a4ed508cab624313af13c72692b Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 6 Aug 2024 11:12:40 +0200 Subject: [PATCH 15/17] x --- ops/op2/valid_args.md | 46 +++++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/ops/op2/valid_args.md b/ops/op2/valid_args.md index 569613972..058c0d5a6 100644 --- a/ops/op2/valid_args.md +++ b/ops/op2/valid_args.md @@ -22,38 +22,38 @@ | X | &v8::**V8** | X | **V8** | | | X | v8::Local | X | any | | | X | v8::Local | X | **V8** | | -| X | #[global] v8::Global | | any | ⚠️ Slower than `v8::Local`. | -| X | #[global] v8::Global | | **V8** | ⚠️ Slower than `v8::Local`. | -| X | #[serde] SerdeType | | any | ⚠️ May be slow. | -| X | #[serde] (Tuple, Tuple) | | any | ⚠️ May be slow. | -| | #[anybuffer] &mut [u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[anybuffer] &[u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[anybuffer] *mut u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| | #[anybuffer] *const u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| X | #[arraybuffer] &mut [u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[arraybuffer] &[u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[arraybuffer] *mut u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| X | #[arraybuffer] *const u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[global] v8::Global | | any | ⚠️ Slower than `v8::Local`. | +| X | #[global] v8::Global | | **V8** | ⚠️ Slower than `v8::Local`. | +| X | #[serde] SerdeType | | any | ⚠️ May be slow. | +| X | #[serde] (Tuple, Tuple) | | any | ⚠️ May be slow. | +| | #[anybuffer] &mut [u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[anybuffer] &[u8] | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[anybuffer] *mut u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[anybuffer] *const u8 | X | ArrayBuffer, ArrayBufferView (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[arraybuffer] &mut [u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[arraybuffer] &[u8] | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[arraybuffer] *mut u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| X | #[arraybuffer] *const u8 | X | ArrayBuffer (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | | X | #[arraybuffer(copy)] Vec | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | | X | #[arraybuffer(copy)] Box<[u8]> | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | | X | #[arraybuffer(copy)] bytes::Bytes | X | ArrayBuffer (resizable=true,false) | Safe, but forces a copy. | -| | #[buffer] &mut [u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[buffer] &[u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| | #[buffer] *mut u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | -| | #[buffer] *const u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[buffer] &mut [u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[buffer] &[u8] | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| | #[buffer] *mut u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | +| | #[buffer] *const u8 | X | UInt8Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. Because of how V8 treats empty arrays in fastcalls, they will always be passed as null. | | X | #[buffer(copy)] Vec | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] Box<[u8]> | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] bytes::Bytes | X | UInt8Array (resizable=true,false) | Safe, but forces a copy. | -| X | #[buffer] &mut [u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | -| X | #[buffer] &[u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[buffer] &mut [u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | +| X | #[buffer] &[u32] | X | UInt32Array (resizable=true,false) | ⚠️ JS may modify the contents of the slice if V8 is called re-entrantly. | | X | #[buffer(copy)] Vec | X | UInt32Array (resizable=true,false) | Safe, but forces a copy. | | X | #[buffer(copy)] Box<[u32]> | X | UInt32Array (resizable=true,false) | Safe, but forces a copy. | -| | #[buffer] V8Slice | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] V8Slice | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | | | #[buffer(detach)] V8Slice | X | ArrayBufferView (resizable=true,false) | Safe. | -| | #[buffer] V8ResizableSlice | X | ArrayBufferView (resizable=true) | ⚠️ JS may modify the contents of slices obtained from buffer. | -| | #[buffer] JsBuffer | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] V8ResizableSlice | X | ArrayBufferView (resizable=true) | ⚠️ JS may modify the contents of slices obtained from buffer. | +| | #[buffer] JsBuffer | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of slices obtained from buffer. | | X | #[buffer(detach)] JsBuffer | | ArrayBufferView (resizable=true,false) | Safe. | -| | #[buffer(unsafe)] bytes::Bytes | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of the buffer. | +| | #[buffer(unsafe)] bytes::Bytes | X | ArrayBufferView (resizable=false) | ⚠️ JS may modify the contents of the buffer. | | | #[buffer(detach)] bytes::Bytes | X | ArrayBufferView (resizable=true,false) | Safe. | | X | *const std::ffi::c_void | X | External | | | X | *mut std::ffi::c_void | X | External | | @@ -67,4 +67,4 @@ | X | #[state] &StateObject | X | | Extracts an object from `OpState`. | | 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 | *mut v8::Isolate | X | | ⚠️ Extremely dangerous, may crash if you don't use `nofast` depending on what you do. | From f0725dd4ebcbd647b85d78a6b45bb2189b754d95 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 9 Oct 2024 10:18:02 +0100 Subject: [PATCH 16/17] Fix --- ops/op2/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/ops/op2/mod.rs b/ops/op2/mod.rs index 2609d1eff..ad1f6aff6 100644 --- a/ops/op2/mod.rs +++ b/ops/op2/mod.rs @@ -186,6 +186,7 @@ pub(crate) fn generate_op2( needs_retval: false, needs_scope: false, needs_fast_isolate: false, + needs_fast_scope: false, needs_isolate: false, needs_opctx: false, needs_opstate: false, From b2cd2eae8f28ea8801e50fb7a59f46f9dac84a06 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 9 Oct 2024 10:33:39 +0100 Subject: [PATCH 17/17] x --- Cargo.lock | 1 - dcore/Cargo.toml | 1 - 2 files changed, 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68149dba4..b48bed5a9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -500,7 +500,6 @@ dependencies = [ "deno_core", "deno_core_testing", "fastwebsockets", - "futures", "http", "http-body-util", "hyper", diff --git a/dcore/Cargo.toml b/dcore/Cargo.toml index f5ff2418c..432c9cf3c 100644 --- a/dcore/Cargo.toml +++ b/dcore/Cargo.toml @@ -23,7 +23,6 @@ deno_core_testing.workspace = true clap = "4.5.1" deno_core.workspace = true fastwebsockets = { version = "0.6", features = ["upgrade", "unstable-split"] } -futures.workspace = true http = { version = "1.0" } http-body-util = { version = "0.1" } hyper = { version = "=1.1.0", features = ["full"] }