diff --git a/cli/tests/testdata/workers/test.ts b/cli/tests/testdata/workers/test.ts index d75ca499bd..8877c95645 100644 --- a/cli/tests/testdata/workers/test.ts +++ b/cli/tests/testdata/workers/test.ts @@ -655,7 +655,7 @@ Deno.test("Worker with invalid permission arg", function () { deno: { permissions: { env: "foo" } }, }), TypeError, - 'Error parsing args at position 1: (deno.permissions.env) invalid value: string "foo", expected "inherit" or boolean or string[]', + 'Error parsing args at position 0: (deno.permissions.env) invalid value: string "foo", expected "inherit" or boolean or string[]', ); }); diff --git a/core/01_core.js b/core/01_core.js index b37f0a0d5f..2683f4adf9 100644 --- a/core/01_core.js +++ b/core/01_core.js @@ -15,7 +15,6 @@ ArrayPrototypeMap, ErrorCaptureStackTrace, Promise, - ObjectEntries, ObjectFromEntries, MapPrototypeGet, MapPrototypeDelete, @@ -27,10 +26,6 @@ SymbolFor, } = window.__bootstrap.primordials; const ops = window.Deno.core.ops; - const opIds = Object.keys(ops).reduce((a, v, i) => { - a[v] = i; - return a; - }, {}); // Available on start due to bindings. const { refOp_, unrefOp_ } = window.Deno.core; @@ -154,7 +149,7 @@ function opAsync(opName, ...args) { const promiseId = nextPromiseId++; - const maybeError = ops[opName](opIds[opName], promiseId, ...args); + const maybeError = ops[opName](promiseId, ...args); // Handle sync error (e.g: error parsing args) if (maybeError) return unwrapOpResult(maybeError); let p = PromisePrototypeThen(setPromise(promiseId), unwrapOpResult); @@ -174,7 +169,7 @@ } function opSync(opName, ...args) { - return unwrapOpResult(ops[opName](opIds[opName], ...args)); + return unwrapOpResult(ops[opName](...args)); } function refOp(promiseId) { @@ -222,8 +217,8 @@ function metrics() { const [aggregate, perOps] = opSync("op_metrics"); aggregate.ops = ObjectFromEntries(ArrayPrototypeMap( - ObjectEntries(opIds), - ([opName, opId]) => [opName, perOps[opId]], + core.op_names, + (opName, opId) => [opName, perOps[opId]], )); return aggregate; } diff --git a/core/bindings.rs b/core/bindings.rs index c4555459cb..5e37232a1f 100644 --- a/core/bindings.rs +++ b/core/bindings.rs @@ -1,16 +1,15 @@ // Copyright 2018-2022 the Deno authors. All rights reserved. MIT license. use crate::error::is_instance_of_error; -use crate::extensions::OpDecl; use crate::modules::get_module_type_from_assertions; use crate::modules::parse_import_assertions; use crate::modules::validate_import_assertions; use crate::modules::ImportAssertionsKind; use crate::modules::ModuleMap; +use crate::ops::OpCtx; use crate::ops_builtin::WasmStreamingResource; use crate::resolve_url_or_path; use crate::JsRuntime; -use crate::OpState; use crate::PromiseId; use crate::ResourceId; use crate::ZeroCopyBuf; @@ -23,7 +22,6 @@ use serde_v8::to_v8; use std::cell::RefCell; use std::option::Option; use std::os::raw::c_void; -use std::rc::Rc; use url::Url; use v8::HandleScope; use v8::Local; @@ -147,9 +145,8 @@ pub fn module_origin<'a>( pub fn initialize_context<'s>( scope: &mut v8::HandleScope<'s, ()>, - ops: &[OpDecl], + op_ctxs: &[OpCtx], snapshot_loaded: bool, - op_state: Rc>, ) -> v8::Local<'s, v8::Context> { let scope = &mut v8::EscapableHandleScope::new(scope); @@ -165,14 +162,13 @@ pub fn initialize_context<'s>( // a really weird usecase. Remove this once all // tsc ops are static at snapshot time. if snapshot_loaded { - // Grab Deno.core.ops object + // Grab the Deno.core & Deno.core.ops objects + let core_obj = JsRuntime::grab_global::(scope, "Deno.core") + .expect("Deno.core to exist"); let ops_obj = JsRuntime::grab_global::(scope, "Deno.core.ops") .expect("Deno.core.ops to exist"); - - let raw_op_state = Rc::as_ptr(&op_state) as *const c_void; - for op in ops { - set_func_raw(scope, ops_obj, op.name, op.v8_fn_ptr, raw_op_state); - } + initialize_ops(scope, ops_obj, op_ctxs); + initialize_op_names(scope, core_obj, op_ctxs); return scope.escape(context); } @@ -236,14 +232,34 @@ pub fn initialize_context<'s>( set_func(scope, global, "queueMicrotask", queue_microtask); // Bind functions to Deno.core.ops.* - let ops_val = JsRuntime::ensure_objs(scope, global, "Deno.core.ops").unwrap(); - let raw_op_state = Rc::as_ptr(&op_state) as *const c_void; - for op in ops { - set_func_raw(scope, ops_val, op.name, op.v8_fn_ptr, raw_op_state); - } + let ops_obj = JsRuntime::ensure_objs(scope, global, "Deno.core.ops").unwrap(); + initialize_ops(scope, ops_obj, op_ctxs); + initialize_op_names(scope, core_val, op_ctxs); scope.escape(context) } +fn initialize_ops( + scope: &mut v8::HandleScope, + ops_obj: v8::Local, + op_ctxs: &[OpCtx], +) { + for ctx in op_ctxs { + let ctx_ptr = ctx as *const OpCtx as *const c_void; + set_func_raw(scope, ops_obj, ctx.decl.name, ctx.decl.v8_fn_ptr, ctx_ptr); + } +} + +fn initialize_op_names( + scope: &mut v8::HandleScope, + core_obj: v8::Local, + op_ctxs: &[OpCtx], +) { + let names: Vec<&str> = op_ctxs.iter().map(|o| o.decl.name).collect(); + let k = v8::String::new(scope, "op_names").unwrap().into(); + let v = serde_v8::to_v8(scope, names).unwrap(); + core_obj.set(scope, k, v); +} + pub fn set_func( scope: &mut v8::HandleScope<'_>, obj: v8::Local, diff --git a/core/examples/hello_world.rs b/core/examples/hello_world.rs index e551efa558..13a47221d9 100644 --- a/core/examples/hello_world.rs +++ b/core/examples/hello_world.rs @@ -5,7 +5,6 @@ use deno_core::op; use deno_core::Extension; use deno_core::JsRuntime; -use deno_core::OpState; use deno_core::RuntimeOptions; // This is a hack to make the `#[op]` macro work with diff --git a/core/lib.rs b/core/lib.rs index b9391aff6f..652ad2cd6a 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -108,6 +108,7 @@ pub mod _ops { pub use super::bindings::throw_type_error; pub use super::error_codes::get_error_code; pub use super::ops::to_op_result; + pub use super::ops::OpCtx; pub use super::runtime::queue_async_op; } diff --git a/core/ops.rs b/core/ops.rs index 42718b8ff7..b936fc7268 100644 --- a/core/ops.rs +++ b/core/ops.rs @@ -3,6 +3,7 @@ use crate::gotham_state::GothamState; use crate::resources::ResourceTable; use crate::runtime::GetErrorClassFn; +use crate::OpDecl; use crate::OpsTracker; use anyhow::Error; use futures::future::maybe_done; @@ -12,10 +13,12 @@ use futures::ready; use futures::task::noop_waker; use futures::Future; use serde::Serialize; +use std::cell::RefCell; use std::cell::UnsafeCell; use std::ops::Deref; use std::ops::DerefMut; use std::pin::Pin; +use std::rc::Rc; use std::task::Context; use std::task::Poll; @@ -134,6 +137,13 @@ pub fn to_op_result( } } +// TODO(@AaronO): optimize OpCtx(s) mem usage ? +pub struct OpCtx { + pub id: OpId, + pub state: Rc>, + pub decl: OpDecl, +} + /// Maintains the resources and ops inside a JS runtime. pub struct OpState { pub resource_table: ResourceTable, diff --git a/core/runtime.rs b/core/runtime.rs index cb8b242e98..50f7399d4c 100644 --- a/core/runtime.rs +++ b/core/runtime.rs @@ -164,6 +164,9 @@ pub(crate) struct JsRuntimeState { pub(crate) unrefed_ops: HashSet, pub(crate) have_unpolled_ops: bool, pub(crate) op_state: Rc>, + #[allow(dead_code)] + // We don't explicitly re-read this prop but need the slice to live alongside the isolate + pub(crate) op_ctxs: Box<[OpCtx]>, pub(crate) shared_array_buffer_store: Option, pub(crate) compiled_wasm_module_store: Option, waker: AtomicWaker, @@ -298,6 +301,16 @@ impl JsRuntime { } let op_state = Rc::new(RefCell::new(op_state)); + let op_ctxs = ops + .into_iter() + .enumerate() + .map(|(id, decl)| OpCtx { + id, + state: op_state.clone(), + decl, + }) + .collect::>() + .into_boxed_slice(); let global_context; let (mut isolate, maybe_snapshot_creator) = if options.will_snapshot { @@ -309,8 +322,7 @@ impl JsRuntime { let mut isolate = JsRuntime::setup_isolate(isolate); { let scope = &mut v8::HandleScope::new(&mut isolate); - let context = - bindings::initialize_context(scope, &ops, false, op_state.clone()); + let context = bindings::initialize_context(scope, &op_ctxs, false); global_context = v8::Global::new(scope, context); creator.set_default_context(context); } @@ -336,12 +348,8 @@ impl JsRuntime { let mut isolate = JsRuntime::setup_isolate(isolate); { let scope = &mut v8::HandleScope::new(&mut isolate); - let context = bindings::initialize_context( - scope, - &ops, - snapshot_loaded, - op_state.clone(), - ); + let context = + bindings::initialize_context(scope, &op_ctxs, snapshot_loaded); global_context = v8::Global::new(scope, context); } @@ -374,6 +382,7 @@ impl JsRuntime { shared_array_buffer_store: options.shared_array_buffer_store, compiled_wasm_module_store: options.compiled_wasm_module_store, op_state: op_state.clone(), + op_ctxs, have_unpolled_ops: false, waker: AtomicWaker::new(), }))); diff --git a/ops/lib.rs b/ops/lib.rs index 54f4212bcb..3dc7665e22 100644 --- a/ops/lib.rs +++ b/ops/lib.rs @@ -138,17 +138,19 @@ fn codegen_v8_async(core: &TokenStream2, f: &syn::ItemFn) -> TokenStream2 { quote! {} }; let rust_i0 = if uses_opstate { 1 } else { 0 }; - let (arg_decls, args_tail) = codegen_args(core, f, rust_i0, 2); + let (arg_decls, args_tail) = codegen_args(core, f, rust_i0, 1); let type_params = &f.sig.generics.params; quote! { use #core::futures::FutureExt; - // SAFETY: Called from Deno.core.opAsync. Which retrieves the index using opId table. - let op_id = unsafe { - #core::v8::Local::<#core::v8::Integer>::cast(args.get(0)) - }.value() as usize; + // SAFETY: #core guarantees args.data() is a v8 External pointing to an OpCtx for the isolates lifetime + let ctx = unsafe { + &*(#core::v8::Local::<#core::v8::External>::cast(args.data().unwrap_unchecked()).value() + as *const #core::_ops::OpCtx) + }; + let op_id = ctx.id; - let promise_id = args.get(1); + let promise_id = args.get(0); let promise_id = #core::v8::Local::<#core::v8::Integer>::try_from(promise_id) .map(|l| l.value() as #core::PromiseId) .map_err(#core::anyhow::Error::from); @@ -163,18 +165,8 @@ fn codegen_v8_async(core: &TokenStream2, f: &syn::ItemFn) -> TokenStream2 { #arg_decls - // SAFETY: Unchecked cast to external since #core guarantees args.data() is a v8 External. - let state_refcell_raw = unsafe { - #core::v8::Local::<#core::v8::External>::cast(args.data().unwrap_unchecked()) - }.value(); + let state = ctx.state.clone(); - // SAFETY: The Rc> is functionally pinned and is tied to the isolate's lifetime - let state = unsafe { - let ptr = state_refcell_raw as *const std::cell::RefCell<#core::OpState>; - // Increment so it will later be decremented/dropped by the underlaying func it is moved to - std::rc::Rc::increment_strong_count(ptr); - std::rc::Rc::from_raw(ptr) - }; // Track async call & get copy of get_error_class_fn let get_class = { let state = state.borrow(); @@ -199,30 +191,23 @@ fn codegen_v8_sync(core: &TokenStream2, f: &syn::ItemFn) -> TokenStream2 { quote! {} }; let rust_i0 = if uses_opstate { 1 } else { 0 }; - let (arg_decls, args_tail) = codegen_args(core, f, rust_i0, 1); + let (arg_decls, args_tail) = codegen_args(core, f, rust_i0, 0); let ret = codegen_sync_ret(core, &f.sig.output); let type_params = &f.sig.generics.params; quote! { - // SAFETY: Called from Deno.core.opSync. Which retrieves the index using opId table. - let op_id = unsafe { - #core::v8::Local::<#core::v8::Integer>::cast(args.get(0)).value() - } as usize; + // SAFETY: #core guarantees args.data() is a v8 External pointing to an OpCtx for the isolates lifetime + let ctx = unsafe { + &*(#core::v8::Local::<#core::v8::External>::cast(args.data().unwrap_unchecked()).value() + as *const #core::_ops::OpCtx) + }; #arg_decls - // SAFETY: Unchecked cast to external since #core guarantees args.data() is a v8 External. - let state_refcell_raw = unsafe { - #core::v8::Local::<#core::v8::External>::cast(args.data().unwrap_unchecked()) - }.value(); - - // SAFETY: The Rc> is functionally pinned and is tied to the isolate's lifetime - let state = unsafe { &*(state_refcell_raw as *const std::cell::RefCell<#core::OpState>) }; - - let op_state = &mut state.borrow_mut(); + let op_state = &mut ctx.state.borrow_mut(); let result = Self::call::<#type_params>(#args_head #args_tail); - op_state.tracker.track_sync(op_id); + op_state.tracker.track_sync(ctx.id); #ret }