From e34260c5b05f2a93f695d807b7cb54a50605885e Mon Sep 17 00:00:00 2001 From: Divy Srivastava Date: Tue, 23 Aug 2022 09:16:43 +0530 Subject: [PATCH] BREAKING(ext/ffi): specialized `buffer` type (#15518) --- cli/dts/lib.deno.unstable.d.ts | 9 +++- ext/ffi/00_ffi.js | 2 +- ext/ffi/jit_trampoline.rs | 22 +++++--- ext/ffi/lib.rs | 93 +++++++++++++++++++++++----------- test_ffi/tests/bench.js | 57 +++------------------ test_ffi/tests/ffi_types.ts | 38 ++++++++++++-- test_ffi/tests/test.js | 17 ++++--- 7 files changed, 137 insertions(+), 101 deletions(-) diff --git a/cli/dts/lib.deno.unstable.d.ts b/cli/dts/lib.deno.unstable.d.ts index d88d86cd32..29e64379ec 100644 --- a/cli/dts/lib.deno.unstable.d.ts +++ b/cli/dts/lib.deno.unstable.d.ts @@ -395,6 +395,8 @@ declare namespace Deno { type NativePointerType = "pointer"; + type NativeBufferType = "buffer"; + type NativeFunctionType = "function"; type NativeVoidType = "void"; @@ -407,6 +409,7 @@ declare namespace Deno { | NativeNumberType | NativeBigIntType | NativePointerType + | NativeBufferType | NativeFunctionType; /** @category FFI */ @@ -416,8 +419,9 @@ declare namespace Deno { type ToNativeTypeMap = & Record & Record - & Record - & Record; + & Record + & Record + & Record; /** Type conversion for foreign symbol parameters and unsafe callback return * types. @@ -452,6 +456,7 @@ declare namespace Deno { & Record & Record & Record + & Record & Record; /** Type conversion for foreign symbol return types and unsafe callback diff --git a/ext/ffi/00_ffi.js b/ext/ffi/00_ffi.js index ca9cb34742..6957b94188 100644 --- a/ext/ffi/00_ffi.js +++ b/ext/ffi/00_ffi.js @@ -216,7 +216,7 @@ } function isPointerType(type) { - return type === "pointer" || + return type === "buffer" || type === "pointer" || typeof type === "object" && type !== null && "function" in type; } diff --git a/ext/ffi/jit_trampoline.rs b/ext/ffi/jit_trampoline.rs index 4785fd0920..75cdae51de 100644 --- a/ext/ffi/jit_trampoline.rs +++ b/ext/ffi/jit_trampoline.rs @@ -32,8 +32,8 @@ fn native_arg_to_c(ty: &NativeType) -> &'static str { NativeType::I64 => "int64_t", NativeType::ISize => "intptr_t", NativeType::USize => "uintptr_t", - NativeType::Pointer => "struct FastApiTypedArray*", - NativeType::Function => "void*", + NativeType::Buffer => "struct FastApiTypedArray*", + NativeType::Function | NativeType::Pointer => "void*", } } @@ -52,7 +52,7 @@ fn native_to_c(ty: &NativeType) -> &'static str { NativeType::I64 => "int64_t", NativeType::ISize => "intptr_t", NativeType::USize => "uintptr_t", - NativeType::Pointer | NativeType::Function => "void*", + NativeType::Pointer | NativeType::Buffer | NativeType::Function => "void*", } } @@ -97,7 +97,7 @@ pub(crate) fn codegen(sym: &crate::Symbol) -> String { if i > 0 { call_s += ", "; } - if matches!(ty, NativeType::Pointer) { + if matches!(ty, NativeType::Buffer) { let _ = write!(call_s, "p{i}->data"); } else { let _ = write!(call_s, "p{i}"); @@ -195,14 +195,14 @@ mod tests { }\n\n", ); assert_codegen( - codegen(vec![NativeType::Pointer, NativeType::U32], NativeType::U32), + codegen(vec![NativeType::Buffer, NativeType::U32], NativeType::U32), "extern uint32_t func(void* p0, uint32_t p1);\n\n\ uint32_t func_trampoline(void* recv, struct FastApiTypedArray* p0, uint32_t p1) {\ \n return func(p0->data, p1);\n\ }\n\n", ); assert_codegen( - codegen(vec![NativeType::Pointer, NativeType::Pointer], NativeType::U32), + codegen(vec![NativeType::Buffer, NativeType::Buffer], NativeType::U32), "extern uint32_t func(void* p0, void* p1);\n\n\ uint32_t func_trampoline(void* recv, struct FastApiTypedArray* p0, struct FastApiTypedArray* p1) {\ \n return func(p0->data, p1->data);\n\ @@ -217,13 +217,21 @@ mod tests { }\n\n", ); assert_codegen( - codegen(vec![NativeType::Pointer, NativeType::Pointer], NativeType::U64), + codegen(vec![NativeType::Buffer, NativeType::Buffer], NativeType::U64), "extern uint64_t func(void* p0, void* p1);\n\n\ void func_trampoline(void* recv, struct FastApiTypedArray* p0, struct FastApiTypedArray* p1, struct FastApiTypedArray* const p_ret) {\ \n uint64_t r = func(p0->data, p1->data);\ \n ((uint64_t*)p_ret->data)[0] = r;\n\ }\n\n", ); + assert_codegen( + codegen(vec![NativeType::Pointer, NativeType::Pointer], NativeType::U64), + "extern uint64_t func(void* p0, void* p1);\n\n\ + void func_trampoline(void* recv, void* p0, void* p1, struct FastApiTypedArray* const p_ret) {\ + \n uint64_t r = func(p0, p1);\ + \n ((uint64_t*)p_ret->data)[0] = r;\n\ + }\n\n", + ); } #[test] diff --git a/ext/ffi/lib.rs b/ext/ffi/lib.rs index 1d400069e8..70591411a8 100644 --- a/ext/ffi/lib.rs +++ b/ext/ffi/lib.rs @@ -266,6 +266,7 @@ enum NativeType { F32, F64, Pointer, + Buffer, Function, } @@ -285,8 +286,9 @@ impl From for libffi::middle::Type { NativeType::ISize => libffi::middle::Type::isize(), NativeType::F32 => libffi::middle::Type::f32(), NativeType::F64 => libffi::middle::Type::f64(), - NativeType::Pointer => libffi::middle::Type::pointer(), - NativeType::Function => libffi::middle::Type::pointer(), + NativeType::Pointer | NativeType::Buffer | NativeType::Function => { + libffi::middle::Type::pointer() + } } } } @@ -327,7 +329,9 @@ impl NativeValue { NativeType::ISize => Arg::new(&self.isize_value), NativeType::F32 => Arg::new(&self.f32_value), NativeType::F64 => Arg::new(&self.f64_value), - NativeType::Pointer | NativeType::Function => Arg::new(&self.pointer), + NativeType::Pointer | NativeType::Buffer | NativeType::Function => { + Arg::new(&self.pointer) + } } } @@ -375,7 +379,7 @@ impl NativeValue { } NativeType::F32 => Value::from(self.f32_value), NativeType::F64 => Value::from(self.f64_value), - NativeType::Pointer | NativeType::Function => { + NativeType::Pointer | NativeType::Function | NativeType::Buffer => { let value = self.pointer as usize; if value > MAX_SAFE_INTEGER as usize { json!(U32x2::from(value as u64)) @@ -479,7 +483,7 @@ impl NativeValue { v8::Number::new(scope, self.f64_value).into(); local_value.into() } - NativeType::Pointer | NativeType::Function => { + NativeType::Pointer | NativeType::Buffer | NativeType::Function => { let value = self.pointer as u64; let local_value: v8::Local = if value > MAX_SAFE_INTEGER as u64 { @@ -751,8 +755,10 @@ impl From<&NativeType> for fast_api::Type { NativeType::I64 => fast_api::Type::Int64, NativeType::U64 => fast_api::Type::Uint64, NativeType::ISize => fast_api::Type::Int64, - NativeType::USize | NativeType::Function => fast_api::Type::Uint64, - NativeType::Pointer => fast_api::Type::TypedArray(fast_api::CType::Uint8), + NativeType::USize | NativeType::Pointer | NativeType::Function => { + fast_api::Type::Uint64 + } + NativeType::Buffer => fast_api::Type::TypedArray(fast_api::CType::Uint8), } } } @@ -762,6 +768,7 @@ fn needs_unwrap(rv: NativeType) -> bool { rv, NativeType::Function | NativeType::Pointer + | NativeType::Buffer | NativeType::I64 | NativeType::ISize | NativeType::U64 @@ -1064,14 +1071,37 @@ fn ffi_parse_pointer_arg( arg: v8::Local, ) -> Result { // Order of checking: - // 1. ArrayBufferView: Common and not supported by Fast API, optimise this case. - // 2. BigInt: Uncommon and not supported by Fast API, optimise this case as second. - // 3. Number: Common and supported by Fast API, optimise the common case third. - // 4. ArrayBuffer: Fairly common and not supported by Fast API. + // 1. BigInt: Uncommon and not supported by Fast API, optimise this case. + // 2. Number: Common and supported by Fast API. + // 3. Null: Very uncommon / can be represented by a 0. + let pointer = if let Ok(value) = v8::Local::::try_from(arg) { + value.u64_value().0 as usize as *const u8 + } else if let Ok(value) = v8::Local::::try_from(arg) { + value.integer_value(scope).unwrap() as usize as *const u8 + } else if arg.is_null() { + ptr::null() + } else { + return Err(type_error( + "Invalid FFI pointer type, expected null, integer or BigInt", + )); + }; + Ok(NativeValue { pointer }) +} + +#[inline] +fn ffi_parse_buffer_arg( + scope: &mut v8::HandleScope, + arg: v8::Local, +) -> Result { + // Order of checking: + // 1. ArrayBuffer: Fairly common and not supported by Fast API, optimise this case. + // 2. ArrayBufferView: Common and supported by Fast API // 5. Null: Very uncommon / can be represented by a 0. - let pointer = if let Ok(value) = - v8::Local::::try_from(arg) - { + + let pointer = if let Ok(value) = v8::Local::::try_from(arg) { + let backing_store = value.get_backing_store(); + &backing_store[..] as *const _ as *const u8 + } else if let Ok(value) = v8::Local::::try_from(arg) { let byte_offset = value.byte_offset(); let backing_store = value .buffer(scope) @@ -1080,17 +1110,12 @@ fn ffi_parse_pointer_arg( })? .get_backing_store(); &backing_store[byte_offset..] as *const _ as *const u8 - } else if let Ok(value) = v8::Local::::try_from(arg) { - value.u64_value().0 as usize as *const u8 - } else if let Ok(value) = v8::Local::::try_from(arg) { - value.integer_value(scope).unwrap() as usize as *const u8 - } else if let Ok(value) = v8::Local::::try_from(arg) { - let backing_store = value.get_backing_store(); - &backing_store[..] as *const _ as *const u8 } else if arg.is_null() { ptr::null() } else { - return Err(type_error("Invalid FFI pointer type, expected null, integer, BigInt, ArrayBuffer, or ArrayBufferView")); + return Err(type_error( + "Invalid FFI buffer type, expected null, ArrayBuffer, or ArrayBufferView", + )); }; Ok(NativeValue { pointer }) } @@ -1174,6 +1199,9 @@ where NativeType::F64 => { ffi_args.push(ffi_parse_f64_arg(value)?); } + NativeType::Buffer => { + ffi_args.push(ffi_parse_buffer_arg(scope, value)?); + } NativeType::Pointer => { ffi_args.push(ffi_parse_pointer_arg(scope, value)?); } @@ -1247,6 +1275,9 @@ where NativeType::F64 => { ffi_args.push(ffi_parse_f64_arg(value)?); } + NativeType::Buffer => { + ffi_args.push(ffi_parse_buffer_arg(scope, value)?); + } NativeType::Pointer => { ffi_args.push(ffi_parse_pointer_arg(scope, value)?); } @@ -1302,9 +1333,11 @@ where 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), - }, + NativeType::Pointer | NativeType::Function | NativeType::Buffer => { + NativeValue { + pointer: cif.call::<*const u8>(*fun_ptr, &call_args), + } + } }) } } @@ -1368,9 +1401,11 @@ fn ffi_call( 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), - }, + NativeType::Pointer | NativeType::Function | NativeType::Buffer => { + NativeValue { + pointer: cif.call::<*const u8>(fun_ptr, &call_args), + } + } }) } } @@ -2004,7 +2039,7 @@ fn op_ffi_get_static<'scope>( let number: v8::Local = v8::Number::new(scope, result).into(); number.into() } - NativeType::Pointer | NativeType::Function => { + NativeType::Pointer | NativeType::Function | NativeType::Buffer => { let result = data_ptr as u64; let integer: v8::Local = if result > MAX_SAFE_INTEGER as u64 { v8::BigInt::new_from_u64(scope, result).into() diff --git a/test_ffi/tests/bench.js b/test_ffi/tests/bench.js index 5342f84ff5..c1a3d630fa 100644 --- a/test_ffi/tests/bench.js +++ b/test_ffi/tests/bench.js @@ -14,7 +14,7 @@ const dylib = Deno.dlopen(libPath, { "add_u32": { parameters: ["u32", "u32"], result: "u32" }, "add_u64": { parameters: ["u64", "u64"], result: "u64" }, "ffi_string": { parameters: [], result: "pointer" }, - "hash": { parameters: ["pointer", "u32"], result: "u32" }, + "hash": { parameters: ["buffer", "u32"], result: "u32" }, "nop_u8": { parameters: ["u8"], result: "void" }, "nop_i8": { parameters: ["i8"], result: "void" }, "nop_u16": { parameters: ["u16"], result: "void" }, @@ -27,7 +27,7 @@ const dylib = Deno.dlopen(libPath, { "nop_isize": { parameters: ["isize"], result: "void" }, "nop_f32": { parameters: ["f32"], result: "void" }, "nop_f64": { parameters: ["f64"], result: "void" }, - "nop_buffer": { parameters: ["pointer"], result: "void" }, + "nop_buffer": { parameters: ["buffer"], result: "void" }, "return_u8": { parameters: [], result: "u8" }, "return_i8": { parameters: [], result: "i8" }, "return_u16": { parameters: [], result: "u16" }, @@ -40,7 +40,7 @@ const dylib = Deno.dlopen(libPath, { "return_isize": { parameters: [], result: "isize" }, "return_f32": { parameters: [], result: "f32" }, "return_f64": { parameters: [], result: "f64" }, - "return_buffer": { parameters: [], result: "pointer" }, + "return_buffer": { parameters: [], result: "buffer" }, // Nonblocking calls "nop_nonblocking": { name: "nop", parameters: [], result: "void" }, "nop_u8_nonblocking": { name: "nop_u8", parameters: ["u8"], result: "void" }, @@ -97,7 +97,7 @@ const dylib = Deno.dlopen(libPath, { }, "nop_buffer_nonblocking": { name: "nop_buffer", - parameters: ["pointer"], + parameters: ["buffer"], result: "void", }, "return_u8_nonblocking": { name: "return_u8", parameters: [], result: "u8" }, @@ -155,7 +155,7 @@ const dylib = Deno.dlopen(libPath, { "return_buffer_nonblocking": { name: "return_buffer", parameters: [], - result: "pointer", + result: "buffer", }, // Parameter checking "nop_many_parameters": { @@ -172,7 +172,7 @@ const dylib = Deno.dlopen(libPath, { "isize", "f32", "f64", - "pointer", + "buffer", "u8", "i8", "u16", @@ -185,7 +185,7 @@ const dylib = Deno.dlopen(libPath, { "isize", "f32", "f64", - "pointer", + "buffer", ], result: "void", }, @@ -343,11 +343,6 @@ Deno.bench("nop_buffer()", () => { nop_buffer(buffer); }); -const buffer_ptr = Deno.UnsafePointer.of(buffer); -Deno.bench("nop_buffer() number", () => { - nop_buffer(buffer_ptr); -}); - const { return_u8 } = dylib.symbols; Deno.bench("return_u8()", () => { return_u8(); @@ -431,6 +426,7 @@ Deno.bench("nop_u32_nonblocking()", async () => { }); const { nop_i32_nonblocking } = dylib.symbols; + Deno.bench("nop_i32_nonblocking()", async () => { await nop_i32_nonblocking(100); }); @@ -469,11 +465,6 @@ const { nop_buffer_nonblocking } = dylib.symbols; Deno.bench("nop_buffer_nonblocking()", async () => { await nop_buffer_nonblocking(buffer); }); - -Deno.bench("nop_buffer_nonblocking() number", async () => { - await nop_buffer_nonblocking(buffer_ptr); -}); - const { return_u8_nonblocking } = dylib.symbols; Deno.bench("return_u8_nonblocking()", async () => { await return_u8_nonblocking(); @@ -574,38 +565,6 @@ Deno.bench("nop_many_parameters()", () => { ); }); -const buffer2_ptr = Deno.UnsafePointer.of(buffer2); -Deno.bench("nop_many_parameters() number", () => { - nop_many_parameters( - 135, - 47, - 356, - -236, - 7457, - -1356, - 16471468, - -1334748136, - 132658769535, - -42745856824, - 13567.26437, - 7.686234e-3, - buffer_ptr, - 64, - -42, - 83, - -136, - 3657, - -2376, - 3277918, - -474628146, - 344657895, - -2436732, - 135.26437e3, - 264.3576468623546834, - buffer2_ptr, - ); -}); - const { nop_many_parameters_nonblocking } = dylib.symbols; Deno.bench("nop_many_parameters_nonblocking()", () => { nop_many_parameters_nonblocking( diff --git a/test_ffi/tests/ffi_types.ts b/test_ffi/tests/ffi_types.ts index d4a90552f8..d319f7e220 100644 --- a/test_ffi/tests/ffi_types.ts +++ b/test_ffi/tests/ffi_types.ts @@ -38,6 +38,10 @@ const remote = Deno.dlopen( parameters: ["pointer"], result: "void", }, + method23: { + parameters: ["buffer"], + result: "void", + }, static1: { type: "usize" }, static2: { type: "pointer" }, static3: { type: "usize" }, @@ -132,6 +136,7 @@ remote.symbols.method14(0); // @ts-expect-error: Invalid argument remote.symbols.method15("foo"); +// @ts-expect-error: Invalid argument remote.symbols.method15(new Uint16Array(1)); remote.symbols.method15(0n); @@ -243,6 +248,17 @@ remote.symbols.method20(null); remote.symbols.method20(unsafe_callback_right2); remote.symbols.method20(unsafe_callback_right1.pointer); +remote.symbols.method23(new Uint8Array(1)); +remote.symbols.method23(new Uint32Array(1)); +remote.symbols.method23(new Uint8Array(1)); + +// @ts-expect-error: Cannot pass pointer values as buffer. +remote.symbols.method23(0); +// @ts-expect-error: Cannot pass pointer values as buffer. +remote.symbols.method23(0n); +// @ts-expect-error: Cannot pass pointer values as buffer. +remote.symbols.method23(null); + // @ts-expect-error: Invalid member type const static1_wrong: null = remote.symbols.static1; const static1_right: Deno.PointerValue = remote.symbols.static1; @@ -332,35 +348,47 @@ type __Tests__ = [ { symbols: { pushBuf: ( - ptr: number | bigint | TypedArray | null, + buf: TypedArray, + ptr: number | bigint | null, func: number | bigint | null, ) => void; }; close(): void; }, Deno.DynamicLibrary< - { pushBuf: { parameters: ["pointer", "function"]; result: "void" } } + { + pushBuf: { + parameters: ["buffer", "pointer", "function"]; + result: "void"; + }; + } > >, higher_order_returns: AssertEqual< { symbols: { pushBuf: ( - ptr: number | bigint | TypedArray | null, + buf: TypedArray, + ptr: number | bigint | null, func: number | bigint | null, ) => number | bigint; }; close(): void; }, Deno.DynamicLibrary< - { pushBuf: { parameters: ["pointer", "function"]; result: "pointer" } } + { + pushBuf: { + parameters: ["buffer", "pointer", "function"]; + result: "pointer"; + }; + } > >, non_exact_params: AssertEqual< { symbols: { foo: ( - ...args: (number | bigint | TypedArray | null)[] + ...args: (number | bigint | null)[] ) => number | bigint; }; close(): void; diff --git a/test_ffi/tests/test.js b/test_ffi/tests/test.js index 062dced761..7e8c41cfeb 100644 --- a/test_ffi/tests/test.js +++ b/test_ffi/tests/test.js @@ -42,12 +42,13 @@ const dylib = Deno.dlopen(libPath, { parameters: [], result: "void", }, - "print_buffer": { parameters: ["pointer", "usize"], result: "void" }, + "print_buffer": { parameters: ["buffer", "usize"], result: "void" }, + "print_pointer": { name: "print_buffer", parameters: ["pointer", "usize"], result: "void" }, "print_buffer2": { - parameters: ["pointer", "usize", "pointer", "usize"], + parameters: ["buffer", "usize", "buffer", "usize"], result: "void", }, - "return_buffer": { parameters: [], result: "pointer" }, + "return_buffer": { parameters: [], result: "buffer" }, "is_null_ptr": { parameters: ["pointer"], result: "u8" }, "add_u32": { parameters: ["u32", "u32"], result: "u32" }, "add_i32": { parameters: ["i32", "i32"], result: "i32" }, @@ -106,7 +107,7 @@ const dylib = Deno.dlopen(libPath, { result: "f64", nonblocking: true, }, - "fill_buffer": { parameters: ["u8", "pointer", "usize"], result: "void" }, + "fill_buffer": { parameters: ["u8", "buffer", "usize"], result: "void" }, "sleep_nonblocking": { name: "sleep_blocking", parameters: ["u64"], @@ -115,7 +116,7 @@ const dylib = Deno.dlopen(libPath, { }, "sleep_blocking": { parameters: ["u64"], result: "void" }, "nonblocking_buffer": { - parameters: ["pointer", "usize"], + parameters: ["buffer", "usize"], result: "void", nonblocking: true, }, @@ -214,7 +215,7 @@ if (!(status & (1 << 4))) { throw new Error("returnBuffer is not optimized"); } -dylib.symbols.print_buffer(ptr0, 8); +dylib.symbols.print_pointer(ptr0, 8); const ptrView = new Deno.UnsafePointerView(ptr0); const into = new Uint8Array(6); const into2 = new Uint8Array(3); @@ -239,9 +240,9 @@ console.log(Boolean(dylib.symbols.is_null_ptr(ptr0))); console.log(Boolean(dylib.symbols.is_null_ptr(null))); console.log(Boolean(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(into)))); const emptyBuffer = new BigUint64Array(0); -console.log(Boolean(dylib.symbols.is_null_ptr(emptyBuffer))); +console.log(Boolean(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptyBuffer)))); const emptySlice = into.subarray(6); -console.log(Boolean(dylib.symbols.is_null_ptr(emptySlice))); +console.log(Boolean(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptySlice)))); const addU32Ptr = dylib.symbols.get_add_u32_ptr(); const addU32 = new Deno.UnsafeFnPointer(addU32Ptr, {