From 244c00d95b7ec8f30a5e81b743b4b618049b6c37 Mon Sep 17 00:00:00 2001 From: Aapo Alasuutari Date: Fri, 22 Jul 2022 14:07:35 +0300 Subject: [PATCH] perf(ext/ffi): Optimise common pointer related APIs (#15144) --- ext/ffi/00_ffi.js | 24 ++++++++++---------- ext/ffi/lib.rs | 49 +++++++++++++++++++++++++++++++++-------- test_ffi/tests/bench.js | 14 ++++++++++++ 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index 22e442e519..cdaf69e0d9 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -32,77 +32,77 @@ getUint8(offset = 0) { return core.opSync( "op_ffi_read_u8", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getInt8(offset = 0) { return core.opSync( "op_ffi_read_i8", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getUint16(offset = 0) { return core.opSync( "op_ffi_read_u16", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getInt16(offset = 0) { return core.opSync( "op_ffi_read_i16", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getUint32(offset = 0) { return core.opSync( "op_ffi_read_u32", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getInt32(offset = 0) { return core.opSync( "op_ffi_read_i32", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getBigUint64(offset = 0) { return core.opSync( "op_ffi_read_u64", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getBigInt64(offset = 0) { return core.opSync( "op_ffi_read_u64", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getFloat32(offset = 0) { return core.opSync( "op_ffi_read_f32", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getFloat64(offset = 0) { return core.opSync( "op_ffi_read_f64", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } getCString(offset = 0) { return core.opSync( "op_ffi_cstr_read", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, ); } @@ -115,7 +115,7 @@ copyInto(destination, offset = 0) { core.opSync( "op_ffi_buf_copy_into", - this.pointer + BigInt(offset), + offset ? this.pointer + BigInt(offset) : this.pointer, destination, destination.byteLength, ); diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 926d88ac7d..05dcba362c 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -1873,7 +1873,7 @@ fn op_ffi_call_nonblocking<'scope>( fn op_ffi_ptr_of( scope: &mut v8::HandleScope<'scope>, state: &mut deno_core::OpState, - buf: ZeroCopyBuf, + buf: serde_v8::Value<'scope>, ) -> Result, AnyError> where FP: FfiPermissions + 'static, @@ -1882,8 +1882,33 @@ where let permissions = state.borrow_mut::(); permissions.check(None)?; + let pointer = if let Ok(value) = + v8::Local::::try_from(buf.v8_value) + { + let backing_store = value + .buffer(scope) + .ok_or_else(|| { + type_error("Invalid FFI ArrayBufferView, expected data in the buffer") + })? + .get_backing_store(); + let byte_offset = value.byte_offset(); + if byte_offset > 0 { + &backing_store[byte_offset..] as *const _ as *const u8 + } else { + &backing_store[..] as *const _ as *const u8 + } + } else if let Ok(value) = v8::Local::::try_from(buf.v8_value) + { + let backing_store = value.get_backing_store(); + &backing_store[..] as *const _ as *const u8 + } else { + return Err(type_error( + "Invalid FFI buffer, expected ArrayBuffer, or ArrayBufferView", + )); + }; + let big_int: v8::Local = - v8::BigInt::new_from_u64(scope, buf.as_ptr() as u64).into(); + v8::BigInt::new_from_u64(scope, pointer as u64).into(); Ok(big_int.into()) } @@ -1915,11 +1940,12 @@ where } } -#[op] -fn op_ffi_cstr_read( +#[op(v8)] +fn op_ffi_cstr_read( + scope: &mut v8::HandleScope<'scope>, state: &mut deno_core::OpState, ptr: u64, -) -> Result +) -> Result, AnyError> where FP: FfiPermissions + 'static, { @@ -1928,10 +1954,15 @@ where let permissions = state.borrow_mut::(); permissions.check(None)?; - let ptr = ptr as *const c_char; - // SAFETY: ptr is user provided - // lifetime validity is not an issue because we allocate a new string. - Ok(unsafe { CStr::from_ptr(ptr) }.to_str()?.to_string()) + // SAFETY: Pointer is user provided. + let cstr = unsafe { CStr::from_ptr(ptr as *const c_char) }.to_bytes(); + let value: v8::Local = + v8::String::new_from_utf8(scope, cstr, v8::NewStringType::Normal) + .ok_or_else(|| { + type_error("Invalid CString pointer, string exceeds max length") + })? + .into(); + Ok(value.into()) } #[op] diff --git a/test_ffi/tests/bench.js b/test_ffi/tests/bench.js index 54a9e0acc4..165b395e28 100644 --- a/test_ffi/tests/bench.js +++ b/test_ffi/tests/bench.js @@ -571,3 +571,17 @@ Deno.bench("nop_many_parameters_nonblocking()", () => { buffer2, ); }); + +Deno.bench("Deno.UnsafePointer.of", () => { + Deno.UnsafePointer.of(buffer); +}); + +const cstringBuffer = new TextEncoder().encode("Best believe it!\0"); +// Make sure the buffer does not get collected +globalThis.cstringBuffer = cstringBuffer; +const cstringPointerView = new Deno.UnsafePointerView( + Deno.UnsafePointer.of(cstringBuffer), +); +Deno.bench("Deno.UnsafePointerView#getCString", () => { + cstringPointerView.getCString(); +});