From c0aa68a1e2a34777c01fd6f718b7718b14e3a18d Mon Sep 17 00:00:00 2001 From: snek Date: Mon, 19 Aug 2024 07:51:16 -0700 Subject: [PATCH] feat: upgrade deno_core (#25042) - Update ffi turbocall to use revised fast call api - Remove `v8_version` function calls - `*mut OwnedIsolate` is no longer stored in OpCtx gotham store --- Cargo.lock | 16 ++--- Cargo.toml | 2 +- cli/args/flags.rs | 2 +- cli/build.rs | 2 +- ext/ffi/dlfcn.rs | 134 ++++++++++++++++++------------------ ext/ffi/turbocall.rs | 69 +++++++++++-------- ext/napi/lib.rs | 15 ++-- runtime/inspector_server.rs | 2 +- runtime/ops/bootstrap.rs | 2 +- 9 files changed, 129 insertions(+), 115 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3b410ea087..4d1b269100 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1389,9 +1389,9 @@ dependencies = [ [[package]] name = "deno_core" -version = "0.303.0" +version = "0.304.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a9cd2cc931f61dee2db67ce9d032d229dda981be29d68dfd530a3dc1187ddd6b" +checksum = "bcb02f25a743fe58a117ed5cfd47c4e640761926e294a1f4ba964a0e2c68b4dc" dependencies = [ "anyhow", "bincode", @@ -1888,9 +1888,9 @@ dependencies = [ [[package]] name = "deno_ops" -version = "0.179.0" +version = "0.180.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1dbb9802b8b976e73872ae6e03303532e6056236467053aa6df02f7deb33488c" +checksum = "8bbba184551dfe009836a42c0268fea6b4ad036b5154aec5a7625e7a7ce4e315" dependencies = [ "proc-macro-rules", "proc-macro2", @@ -6252,9 +6252,9 @@ dependencies = [ [[package]] name = "serde_v8" -version = "0.212.0" +version = "0.213.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bf9a8693e4e54bf21fe51b953b10f98a0e32040671f18c4065f6014f0317ae80" +checksum = "1831a65bad8086cfc993eaead4cbcb00579c085e43b4952a90024e1243f23b4e" dependencies = [ "num-bigint", "serde", @@ -7910,9 +7910,9 @@ dependencies = [ [[package]] name = "v8" -version = "0.102.0" +version = "0.103.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "88a710d5b95bff79a90708203cf9f74384e080d21fc6664aa4df463f2c66ac83" +checksum = "56d72310e5b559c0a8165a5d6bdf4c975ba77c61461039ccf615ce3bbe489ca5" dependencies = [ "bindgen", "bitflags 2.5.0", diff --git a/Cargo.toml b/Cargo.toml index c7f31c7365..1176656e20 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -45,7 +45,7 @@ repository = "https://github.com/denoland/deno" [workspace.dependencies] deno_ast = { version = "=0.41.2", features = ["transpiling"] } -deno_core = { version = "0.303.0" } +deno_core = { version = "0.304.0" } deno_bench_util = { version = "0.158.0", path = "./bench_util" } deno_lockfile = "0.21.1" diff --git a/cli/args/flags.rs b/cli/args/flags.rs index 5518b4b7db..d0a6900453 100644 --- a/cli/args/flags.rs +++ b/cli/args/flags.rs @@ -1427,7 +1427,7 @@ pub fn clap_root() -> Command { crate::version::DENO_VERSION_INFO.release_channel.name(), env!("PROFILE"), env!("TARGET"), - deno_core::v8_version(), + deno_core::v8::VERSION_STRING, crate::version::DENO_VERSION_INFO.typescript ); diff --git a/cli/build.rs b/cli/build.rs index 05636f7f9f..0b964ad433 100644 --- a/cli/build.rs +++ b/cli/build.rs @@ -332,7 +332,7 @@ fn create_cli_snapshot(snapshot_path: PathBuf) { let snapshot_options = SnapshotOptions { ts_version: ts::version(), - v8_version: deno_core::v8_version(), + v8_version: deno_core::v8::VERSION_STRING, target: std::env::var("TARGET").unwrap(), }; diff --git a/ext/ffi/dlfcn.rs b/ext/ffi/dlfcn.rs index 2bc9ab341d..261a62cd30 100644 --- a/ext/ffi/dlfcn.rs +++ b/ext/ffi/dlfcn.rs @@ -5,11 +5,13 @@ use crate::ir::out_buffer_as_ptr; use crate::symbol::NativeType; use crate::symbol::Symbol; use crate::turbocall; +use crate::turbocall::Turbocall; use crate::FfiPermissions; use deno_core::error::generic_error; use deno_core::error::AnyError; use deno_core::op2; use deno_core::v8; +use deno_core::GarbageCollected; use deno_core::OpState; use deno_core::Resource; use dlopen2::raw::Library; @@ -208,84 +210,84 @@ where Ok(out.into()) } +struct FunctionData { + // Held in a box to keep memory while function is alive. + #[allow(unused)] + symbol: Box, + // Held in a box to keep inner data alive while function is alive. + #[allow(unused)] + turbocall: Option, +} + +impl GarbageCollected for FunctionData {} + // Create a JavaScript function for synchronous FFI call to // the given symbol. fn make_sync_fn<'s>( scope: &mut v8::HandleScope<'s>, - sym: Box, + symbol: Box, ) -> v8::Local<'s, v8::Function> { - let sym = Box::leak(sym); - let builder = v8::FunctionTemplate::builder( - |scope: &mut v8::HandleScope, - args: v8::FunctionCallbackArguments, - mut rv: v8::ReturnValue| { - let external: v8::Local = args.data().try_into().unwrap(); - // SAFETY: The pointer will not be deallocated until the function is - // garbage collected. - let symbol = unsafe { &*(external.value() as *const Symbol) }; - let out_buffer = match symbol.result_type { - NativeType::Struct(_) => { - let argc = args.length(); - out_buffer_as_ptr( - scope, - Some( - v8::Local::::try_from(args.get(argc - 1)) - .unwrap(), - ), - ) - } - _ => None, - }; - match crate::call::ffi_call_sync(scope, args, symbol, out_buffer) { - Ok(result) => { - let result = - // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. - unsafe { result.to_v8(scope, symbol.result_type.clone()) }; - rv.set(result); - } - Err(err) => { - deno_core::_ops::throw_type_error(scope, err.to_string()); - } - }; - }, - ) - .data(v8::External::new(scope, sym as *mut Symbol as *mut _).into()); + let turbocall = if turbocall::is_compatible(&symbol) { + let trampoline = turbocall::compile_trampoline(&symbol); + let turbocall = turbocall::make_template(&symbol, trampoline); + Some(turbocall) + } else { + None + }; - let mut fast_call_alloc = None; + let c_function = turbocall.as_ref().map(|turbocall| { + v8::fast_api::CFunction::new( + turbocall.trampoline.ptr(), + &turbocall.c_function_info, + ) + }); - let func = if turbocall::is_compatible(sym) { - let trampoline = turbocall::compile_trampoline(sym); - let func = builder.build_fast( - scope, - &turbocall::make_template(sym, &trampoline), - None, - None, - None, - ); - fast_call_alloc = Some(Box::into_raw(Box::new(trampoline))); - func + let data = FunctionData { symbol, turbocall }; + let data = deno_core::cppgc::make_cppgc_object(scope, data); + + let builder = v8::FunctionTemplate::builder(sync_fn_impl).data(data.into()); + + let func = if let Some(c_function) = c_function { + builder.build_fast(scope, &[c_function]) } else { builder.build(scope) }; - let func = func.get_function(scope).unwrap(); + func.get_function(scope).unwrap() +} - let weak = v8::Weak::with_finalizer( +fn sync_fn_impl<'s>( + scope: &mut v8::HandleScope<'s>, + args: v8::FunctionCallbackArguments<'s>, + mut rv: v8::ReturnValue, +) { + let data = deno_core::cppgc::try_unwrap_cppgc_object::( scope, - func, - Box::new(move |_| { - // SAFETY: This is never called twice. pointer obtained - // from Box::into_raw, hence, satisfies memory layout requirements. - let _ = unsafe { Box::from_raw(sym) }; - if let Some(fast_call_ptr) = fast_call_alloc { - // fast-call compiled trampoline is unmapped when the MMAP handle is dropped - // SAFETY: This is never called twice. pointer obtained - // from Box::into_raw, hence, satisfies memory layout requirements. - let _ = unsafe { Box::from_raw(fast_call_ptr) }; - } - }), - ); - - weak.to_local(scope).unwrap() + args.data(), + ) + .unwrap(); + let out_buffer = match data.symbol.result_type { + NativeType::Struct(_) => { + let argc = args.length(); + out_buffer_as_ptr( + scope, + Some( + v8::Local::::try_from(args.get(argc - 1)).unwrap(), + ), + ) + } + _ => None, + }; + match crate::call::ffi_call_sync(scope, args, &data.symbol, out_buffer) { + Ok(result) => { + let result = + // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. + unsafe { result.to_v8(scope, data.symbol.result_type.clone()) }; + rv.set(result); + } + Err(err) => { + deno_core::_ops::throw_type_error(scope, err.to_string()); + } + }; } // `path` is only used on Windows. diff --git a/ext/ffi/turbocall.rs b/ext/ffi/turbocall.rs index b1cd5177b3..38b4062ab7 100644 --- a/ext/ffi/turbocall.rs +++ b/ext/ffi/turbocall.rs @@ -40,26 +40,39 @@ pub(crate) fn compile_trampoline(sym: &Symbol) -> Trampoline { } } -pub(crate) fn make_template( - sym: &Symbol, - trampoline: &Trampoline, -) -> fast_api::FastFunction { - let params = once(fast_api::Type::V8Value) // Receiver +pub(crate) struct Turbocall { + pub trampoline: Trampoline, + // Held in a box to keep the memory alive for CFunctionInfo + #[allow(unused)] + pub param_info: Box<[fast_api::CTypeInfo]>, + // Held in a box to keep the memory alive for V8 + #[allow(unused)] + pub c_function_info: Box, +} + +pub(crate) fn make_template(sym: &Symbol, trampoline: Trampoline) -> Turbocall { + let param_info = once(fast_api::Type::V8Value.scalar()) // Receiver .chain(sym.parameter_types.iter().map(|t| t.into())) - .collect::>(); + .collect::>(); let ret = if sym.result_type == NativeType::Buffer { // Buffer can be used as a return type and converts differently than in parameters. - fast_api::CType::Pointer + fast_api::Type::Pointer.scalar() } else { - fast_api::CType::from(&fast_api::Type::from(&sym.result_type)) + (&sym.result_type).into() }; - fast_api::FastFunction::new_with_bigint( - Box::leak(params.into_boxed_slice()), + let c_function_info = Box::new(fast_api::CFunctionInfo::new( ret, - trampoline.ptr(), - ) + ¶m_info, + fast_api::Int64Representation::BigInt, + )); + + Turbocall { + trampoline, + param_info, + c_function_info, + } } /// Trampoline for fast-call FFI functions @@ -68,33 +81,33 @@ pub(crate) fn make_template( pub(crate) struct Trampoline(ExecutableBuffer); impl Trampoline { - fn ptr(&self) -> *const c_void { + pub(crate) fn ptr(&self) -> *const c_void { &self.0[0] as *const u8 as *const c_void } } -impl From<&NativeType> for fast_api::Type { +impl From<&NativeType> for fast_api::CTypeInfo { fn from(native_type: &NativeType) -> Self { match native_type { - NativeType::Bool => fast_api::Type::Bool, + NativeType::Bool => fast_api::Type::Bool.scalar(), NativeType::U8 | NativeType::U16 | NativeType::U32 => { - fast_api::Type::Uint32 + fast_api::Type::Uint32.scalar() } NativeType::I8 | NativeType::I16 | NativeType::I32 => { - fast_api::Type::Int32 + fast_api::Type::Int32.scalar() } - NativeType::F32 => fast_api::Type::Float32, - NativeType::F64 => fast_api::Type::Float64, - NativeType::Void => fast_api::Type::Void, - NativeType::I64 => fast_api::Type::Int64, - NativeType::U64 => fast_api::Type::Uint64, - NativeType::ISize => fast_api::Type::Int64, - NativeType::USize => fast_api::Type::Uint64, - NativeType::Pointer | NativeType::Function => fast_api::Type::Pointer, - NativeType::Buffer => fast_api::Type::TypedArray(fast_api::CType::Uint8), - NativeType::Struct(_) => { - fast_api::Type::TypedArray(fast_api::CType::Uint8) + NativeType::F32 => fast_api::Type::Float32.scalar(), + NativeType::F64 => fast_api::Type::Float64.scalar(), + NativeType::Void => fast_api::Type::Void.scalar(), + NativeType::I64 => fast_api::Type::Int64.scalar(), + NativeType::U64 => fast_api::Type::Uint64.scalar(), + NativeType::ISize => fast_api::Type::Int64.scalar(), + NativeType::USize => fast_api::Type::Uint64.scalar(), + NativeType::Pointer | NativeType::Function => { + fast_api::Type::Pointer.scalar() } + NativeType::Buffer => fast_api::Type::Uint8.typed_array(), + NativeType::Struct(_) => fast_api::Type::Uint8.typed_array(), } } } diff --git a/ext/napi/lib.rs b/ext/napi/lib.rs index 8298398389..faf8a57777 100644 --- a/ext/napi/lib.rs +++ b/ext/napi/lib.rs @@ -345,7 +345,7 @@ impl EnvShared { #[repr(C)] pub struct Env { context: NonNull, - pub isolate_ptr: *mut v8::OwnedIsolate, + pub isolate_ptr: *mut v8::Isolate, pub open_handle_scopes: usize, pub shared: *mut EnvShared, pub async_work_sender: V8CrossThreadTaskSpawner, @@ -364,7 +364,7 @@ unsafe impl Sync for Env {} impl Env { #[allow(clippy::too_many_arguments)] pub fn new( - isolate_ptr: *mut v8::OwnedIsolate, + isolate_ptr: *mut v8::Isolate, context: v8::Global, global: v8::Global, buffer_constructor: v8::Global, @@ -409,8 +409,8 @@ impl Env { } #[inline] - pub fn isolate(&mut self) -> &mut v8::OwnedIsolate { - // SAFETY: Lifetime of `OwnedIsolate` is longer than `Env`. + pub fn isolate(&mut self) -> &mut v8::Isolate { + // SAFETY: Lifetime of `Isolate` is longer than `Env`. unsafe { &mut *self.isolate_ptr } } @@ -496,6 +496,7 @@ impl NapiPermissions for deno_permissions::PermissionsContainer { #[op2(reentrant)] fn op_napi_open( scope: &mut v8::HandleScope<'scope>, + isolate: *mut v8::Isolate, op_state: Rc>, #[string] path: String, global: v8::Local<'scope, v8::Object>, @@ -507,15 +508,13 @@ where { // We must limit the OpState borrow because this function can trigger a // re-borrow through the NAPI module. - let (async_work_sender, isolate_ptr, cleanup_hooks, external_ops_tracker) = { + let (async_work_sender, cleanup_hooks, external_ops_tracker) = { let mut op_state = op_state.borrow_mut(); let permissions = op_state.borrow_mut::(); permissions.check(Some(&PathBuf::from(&path)))?; let napi_state = op_state.borrow::(); - let isolate_ptr = op_state.borrow::<*mut v8::OwnedIsolate>(); ( op_state.borrow::().clone(), - *isolate_ptr, napi_state.env_cleanup_hooks.clone(), op_state.external_ops_tracker.clone(), ) @@ -536,7 +535,7 @@ where let ctx = scope.get_current_context(); let mut env = Env::new( - isolate_ptr, + isolate, v8::Global::new(scope, ctx), v8::Global::new(scope, global), v8::Global::new(scope, buffer_constructor), diff --git a/runtime/inspector_server.rs b/runtime/inspector_server.rs index 48d8e0a8fe..1f8cd5e714 100644 --- a/runtime/inspector_server.rs +++ b/runtime/inspector_server.rs @@ -270,7 +270,7 @@ async fn server( let json_version_response = json!({ "Browser": name, "Protocol-Version": "1.3", - "V8-Version": deno_core::v8_version(), + "V8-Version": deno_core::v8::VERSION_STRING, }); // Create the server manually so it can use the Local Executor diff --git a/runtime/ops/bootstrap.rs b/runtime/ops/bootstrap.rs index 88475f4154..0d8c1dab86 100644 --- a/runtime/ops/bootstrap.rs +++ b/runtime/ops/bootstrap.rs @@ -54,7 +54,7 @@ impl Default for SnapshotOptions { Self { ts_version: "n/a".to_owned(), - v8_version: deno_core::v8_version(), + v8_version: deno_core::v8::VERSION_STRING, target, } }