From 76d387fb93c741a5def8d120892be65e35164a01 Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Wed, 29 Jun 2022 13:43:33 +0530 Subject: [PATCH] perf(ext/ffi): optimize synchronous calls (#14945) --- ext/ffi/00_ffi.js | 66 +++---- ext/ffi/lib.rs | 428 ++++++++++++++++++++++++++++++++++++---------- 2 files changed, 368 insertions(+), 126 deletions(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index f308ecad9c..2730cae72e 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -253,8 +253,7 @@ symbols = {}; constructor(path, symbols) { - this.#rid = core.opSync("op_ffi_load", { path, symbols }); - + [this.#rid, this.symbols] = core.opSync("op_ffi_load", { path, symbols }); for (const symbol in symbols) { if ("type" in symbols[symbol]) { const type = symbols[symbol].type; @@ -285,48 +284,37 @@ } const isNonBlocking = symbols[symbol].nonblocking; - const resultType = symbols[symbol].result; - - let fn; if (isNonBlocking) { + const resultType = symbols[symbol].result; const needsUnpacking = isReturnedAsBigInt(resultType); - fn = (...parameters) => { - const promise = core.opAsync( - "op_ffi_call_nonblocking", - this.#rid, - symbol, - parameters, - ); + ObjectDefineProperty( + this.symbols, + symbol, + { + configurable: false, + enumerable: true, + value: (...parameters) => { + const promise = core.opAsync( + "op_ffi_call_nonblocking", + this.#rid, + symbol, + parameters, + ); - if (needsUnpacking) { - return PromisePrototypeThen( - promise, - (result) => unpackNonblockingReturnValue(resultType, result), - ); - } + if (needsUnpacking) { + return PromisePrototypeThen( + promise, + (result) => + unpackNonblockingReturnValue(resultType, result), + ); + } - return promise; - }; - } else { - fn = (...parameters) => - core.opSync( - "op_ffi_call", - this.#rid, - symbol, - parameters, - ); + return promise; + }, + writable: false, + }, + ); } - - ObjectDefineProperty( - this.symbols, - symbol, - { - configurable: false, - enumerable: true, - value: fn, - writable: false, - }, - ); } } diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index d90f20a29f..a5fc6b756a 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -105,7 +105,7 @@ unsafe impl Sync for PtrSymbol {} struct DynamicLibraryResource { lib: Library, - symbols: HashMap, + symbols: HashMap>, } impl Resource for DynamicLibraryResource { @@ -119,50 +119,6 @@ impl Resource for DynamicLibraryResource { } impl DynamicLibraryResource { - fn register( - &mut self, - name: String, - foreign_fn: ForeignFunction, - ) -> Result<(), AnyError> { - let symbol = match &foreign_fn.name { - Some(symbol) => symbol, - None => &name, - }; - // By default, Err returned by this function does not tell - // which symbol wasn't exported. So we'll modify the error - // message to include the name of symbol. - // - // SAFETY: The obtained T symbol is the size of a pointer. - let fn_ptr = match unsafe { self.lib.symbol::<*const c_void>(symbol) } { - Ok(value) => Ok(value), - Err(err) => Err(generic_error(format!( - "Failed to register symbol {}: {}", - symbol, err - ))), - }?; - let ptr = libffi::middle::CodePtr::from_ptr(fn_ptr as _); - let cif = libffi::middle::Cif::new( - foreign_fn - .parameters - .clone() - .into_iter() - .map(libffi::middle::Type::from), - foreign_fn.result.into(), - ); - - self.symbols.insert( - name, - Symbol { - cif, - ptr, - parameter_types: foreign_fn.parameters, - result_type: foreign_fn.result, - }, - ); - - Ok(()) - } - fn get_static(&self, symbol: String) -> Result<*const c_void, AnyError> { // By default, Err returned by this function does not tell // which symbol wasn't exported. So we'll modify the error @@ -196,7 +152,6 @@ pub fn init(unstable: bool) -> Extension { .ops(vec![ op_ffi_load::decl::

(), op_ffi_get_static::decl(), - op_ffi_call::decl(), op_ffi_call_nonblocking::decl(), op_ffi_call_ptr::decl::

(), op_ffi_call_ptr_nonblocking::decl::

(), @@ -378,6 +333,7 @@ impl NativeValue { } // SAFETY: native_type must correspond to the type of value represented by the union field + #[inline] unsafe fn to_v8<'scope>( &self, scope: &mut v8::HandleScope<'scope>, @@ -474,6 +430,8 @@ struct ForeignFunction { name: Option, parameters: Vec, result: NativeType, + #[serde(rename = "nonblocking")] + non_blocking: Option, } // ForeignStatic's name and type fields are read and used by @@ -575,11 +533,12 @@ pub(crate) fn format_error(e: dlopen::Error, path: String) -> String { } } -#[op] -fn op_ffi_load( +#[op(v8)] +fn op_ffi_load( + scope: &mut v8::HandleScope<'scope>, state: &mut deno_core::OpState, args: FfiLoadArgs, -) -> Result +) -> Result<(ResourceId, serde_v8::Value<'scope>), AnyError> where FP: FfiPermissions + 'static, { @@ -595,24 +554,118 @@ where format_error(e, path), )) })?; - let mut resource = DynamicLibraryResource { lib, symbols: HashMap::new(), }; + let obj = v8::Object::new(scope); - for (symbol, foreign_symbol) in args.symbols { + for (symbol_key, foreign_symbol) in args.symbols { match foreign_symbol { ForeignSymbol::ForeignStatic(_) => { // No-op: Statics will be handled separately and are not part of the Rust-side resource. } ForeignSymbol::ForeignFunction(foreign_fn) => { - resource.register(symbol, foreign_fn)?; + let symbol = match &foreign_fn.name { + Some(symbol) => symbol, + None => &symbol_key, + }; + // By default, Err returned by this function does not tell + // which symbol wasn't exported. So we'll modify the error + // message to include the name of symbol. + // + // SAFETY: The obtained T symbol is the size of a pointer. + let fn_ptr = + match unsafe { resource.lib.symbol::<*const c_void>(symbol) } { + Ok(value) => Ok(value), + Err(err) => Err(generic_error(format!( + "Failed to register symbol {}: {}", + symbol, err + ))), + }?; + let ptr = libffi::middle::CodePtr::from_ptr(fn_ptr as _); + let cif = libffi::middle::Cif::new( + foreign_fn + .parameters + .clone() + .into_iter() + .map(libffi::middle::Type::from), + foreign_fn.result.into(), + ); + + let func_key = v8::String::new(scope, &symbol_key).unwrap(); + let sym = Box::new(Symbol { + cif, + ptr, + parameter_types: foreign_fn.parameters, + result_type: foreign_fn.result, + }); + + resource.symbols.insert(symbol_key, sym.clone()); + match foreign_fn.non_blocking { + // Generate functions for synchronous calls. + Some(false) | None => { + let function = make_sync_fn(scope, Box::leak(sym)); + obj.set(scope, func_key.into(), function.into()); + } + // This optimization is not yet supported for non-blocking calls. + _ => {} + }; } } } - Ok(state.resource_table.add(resource)) + let rid = state.resource_table.add(resource); + Ok(( + rid, + serde_v8::Value { + v8_value: obj.into(), + }, + )) +} + +// Create a JavaScript function for synchronous FFI call to +// the given symbol. +fn make_sync_fn<'s>( + scope: &mut v8::HandleScope<'s>, + sym: *mut Symbol, +) -> v8::Local<'s, v8::Function> { + let func = v8::Function::builder( + |scope: &mut v8::HandleScope, + args: v8::FunctionCallbackArguments, + mut rv: v8::ReturnValue| { + let external: v8::Local = + args.data().unwrap().try_into().unwrap(); + // SAFETY: The pointer will not be deallocated until the function is + // garbage collected. + let symbol = unsafe { &*(external.value() as *const Symbol) }; + match ffi_call_sync(scope, args, symbol) { + Ok(result) => { + // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. + let result = unsafe { result.to_v8(scope, symbol.result_type) }; + rv.set(result.v8_value); + } + Err(err) => { + deno_core::_ops::throw_type_error(scope, err.to_string()); + } + }; + }, + ) + .data(v8::External::new(scope, sym as *mut _).into()) + .build(scope) + .unwrap(); + + let weak = v8::Weak::with_finalizer( + scope, + func, + Box::new(move |_| { + // SAFETY: This is never called twice. pointer obtained + // from Box::into_raw, hence, satisfies memory layout requirements. + unsafe { Box::from_raw(sym) }; + }), + ); + + weak.to_local(scope).unwrap() } fn ffi_parse_args<'scope>( @@ -787,6 +840,240 @@ where Ok(ffi_args) } +// A one-off synchronous FFI call. +fn ffi_call_sync<'scope>( + scope: &mut v8::HandleScope<'scope>, + args: v8::FunctionCallbackArguments, + symbol: &Symbol, +) -> Result +where + 'scope: 'scope, +{ + let Symbol { + parameter_types, + result_type, + cif, + ptr: fun_ptr, + } = symbol; + let mut ffi_args: Vec = + Vec::with_capacity(parameter_types.len()); + + for (index, native_type) in parameter_types.iter().enumerate() { + let value = args.get(index as i32); + match native_type { + NativeType::Void => { + unreachable!(); + } + NativeType::U8 => { + let value = value + .uint32_value(scope) + .ok_or_else(|| type_error("Invalid FFI u8 type, expected number"))? + as u8; + ffi_args.push(NativeValue { u8_value: value }); + } + NativeType::I8 => { + let value = value + .int32_value(scope) + .ok_or_else(|| type_error("Invalid FFI i8 type, expected number"))? + as i8; + + ffi_args.push(NativeValue { i8_value: value }); + } + NativeType::U16 => { + let value = value + .uint32_value(scope) + .ok_or_else(|| type_error("Invalid FFI u16 type, expected number"))? + as u16; + + ffi_args.push(NativeValue { u16_value: value }); + } + NativeType::I16 => { + let value = value + .int32_value(scope) + .ok_or_else(|| type_error("Invalid FFI i16 type, expected number"))? + as i16; + + ffi_args.push(NativeValue { i16_value: value }); + } + NativeType::U32 => { + let value = value + .uint32_value(scope) + .ok_or_else(|| type_error("Invalid FFI u32 type, expected number"))? + as u32; + + ffi_args.push(NativeValue { u32_value: value }); + } + NativeType::I32 => { + let value = value + .int32_value(scope) + .ok_or_else(|| type_error("Invalid FFI i32 type, expected number"))? + as i32; + + ffi_args.push(NativeValue { i32_value: value }); + } + NativeType::U64 => { + let value: u64 = + if let Ok(value) = v8::Local::::try_from(value) { + value.u64_value().0 + } else { + value.integer_value(scope).ok_or_else(|| { + type_error("Invalid FFI u64 type, expected number") + })? as u64 + }; + + ffi_args.push(NativeValue { u64_value: value }); + } + NativeType::I64 => { + let value: i64 = + if let Ok(value) = v8::Local::::try_from(value) { + value.i64_value().0 + } else { + value.integer_value(scope).ok_or_else(|| { + type_error("Invalid FFI i64 type, expected number") + })? as i64 + }; + + ffi_args.push(NativeValue { i64_value: value }); + } + NativeType::USize => { + let value: usize = + if let Ok(value) = v8::Local::::try_from(value) { + value.u64_value().0 as usize + } else { + value.integer_value(scope).ok_or_else(|| { + type_error("Invalid FFI usize type, expected number") + })? as usize + }; + + ffi_args.push(NativeValue { usize_value: value }); + } + NativeType::ISize => { + let value: isize = + if let Ok(value) = v8::Local::::try_from(value) { + value.i64_value().0 as isize + } else { + value.integer_value(scope).ok_or_else(|| { + type_error("Invalid FFI isize type, expected number") + })? as isize + }; + + ffi_args.push(NativeValue { isize_value: value }); + } + NativeType::F32 => { + let value = value + .number_value(scope) + .ok_or_else(|| type_error("Invalid FFI f32 type, expected number"))? + as f32; + + ffi_args.push(NativeValue { f32_value: value }); + } + NativeType::F64 => { + let value = value + .number_value(scope) + .ok_or_else(|| type_error("Invalid FFI f64 type, expected number"))? + as f64; + ffi_args.push(NativeValue { f64_value: value }); + } + NativeType::Pointer => { + if value.is_null() { + let value: *const u8 = ptr::null(); + + ffi_args.push(NativeValue { pointer: value }) + } else if let Ok(value) = v8::Local::::try_from(value) { + let value = value.u64_value().0 as *const u8; + + ffi_args.push(NativeValue { pointer: value }); + } else if let Ok(value) = + v8::Local::::try_from(value) + { + let byte_offset = value.byte_offset(); + let backing_store = value + .buffer(scope) + .ok_or_else(|| { + type_error( + "Invalid FFI ArrayBufferView, expected data in the buffer", + ) + })? + .get_backing_store(); + let pointer = &backing_store[byte_offset] as *const _ as *const u8; + + ffi_args.push(NativeValue { pointer }); + } else if let Ok(value) = v8::Local::::try_from(value) + { + let backing_store = value.get_backing_store(); + let pointer = &backing_store as *const _ as *const u8; + + ffi_args.push(NativeValue { pointer }); + } else { + return Err(type_error("Invalid FFI pointer type, expected null, BigInt, ArrayBuffer, or ArrayBufferView")); + } + } + NativeType::Function => { + if value.is_null() { + let value: *const u8 = ptr::null(); + ffi_args.push(NativeValue { pointer: value }) + } else if let Ok(value) = v8::Local::::try_from(value) { + let value = value.u64_value().0 as *const u8; + ffi_args.push(NativeValue { pointer: value }); + } else { + return Err(type_error( + "Invalid FFI function type, expected null, or BigInt", + )); + } + } + } + } + let call_args: Vec = ffi_args.iter().map(Arg::new).collect(); + // SAFETY: types in the `Cif` match the actual calling convention and + // types of symbol. + unsafe { + Ok(match result_type { + NativeType::Void => NativeValue { + void_value: cif.call::<()>(*fun_ptr, &call_args), + }, + NativeType::U8 => NativeValue { + u8_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::I8 => NativeValue { + i8_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::U16 => NativeValue { + u16_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::I16 => NativeValue { + i16_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::U32 => NativeValue { + u32_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::I32 => NativeValue { + i32_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::U64 => NativeValue { + u64_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::I64 => NativeValue { + i64_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::USize => NativeValue { + usize_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::ISize => NativeValue { + isize_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::F32 => NativeValue { + f32_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::F64 => NativeValue { + f64_value: cif.call::(*fun_ptr, &call_args), + }, + NativeType::Pointer | NativeType::Function => NativeValue { + pointer: cif.call::<*const u8>(*fun_ptr, &call_args), + }, + }) + } +} + fn ffi_call( call_args: Vec, cif: &libffi::middle::Cif, @@ -1404,39 +1691,6 @@ fn op_ffi_get_static<'scope>( }) } -#[op(v8)] -fn op_ffi_call<'scope>( - scope: &mut v8::HandleScope<'scope>, - state: Rc>, - rid: ResourceId, - symbol: String, - parameters: serde_v8::Value<'scope>, -) -> Result, AnyError> { - let symbol = { - let state = &mut state.borrow(); - let resource = state.resource_table.get::(rid)?; - - resource - .symbols - .get(&symbol) - .ok_or_else(|| type_error("Invalid FFI symbol name"))? - .clone() - }; - - let call_args = ffi_parse_args(scope, parameters, &symbol.parameter_types)?; - - let result = ffi_call( - call_args, - &symbol.cif, - symbol.ptr, - &symbol.parameter_types, - symbol.result_type, - )?; - // SAFETY: Same return type declared to libffi; trust user to have it right beyond that. - let result = unsafe { result.to_v8(scope, symbol.result_type) }; - Ok(result) -} - /// A non-blocking FFI call. #[op(v8)] fn op_ffi_call_nonblocking<'scope>( @@ -1450,7 +1704,7 @@ fn op_ffi_call_nonblocking<'scope>( let state = state.borrow(); let resource = state.resource_table.get::(rid)?; let symbols = &resource.symbols; - symbols + *symbols .get(&symbol) .ok_or_else(|| type_error("Invalid FFI symbol name"))? .clone()