foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-23 07:44:48 -05:00
Code Issues Packages 1 Wiki Activity

BREAKING(ffi/unstable): always return u64 as bigint (#23981)

Browse source 
The mixed `number | bigint` representation was useful optimization for
pointers. Now, pointers are represented as V8 externals. As part of the
FFI stabilization effort we want to make `bigint` the only
representation for `u64` and `i64`.

BigInt representation performance is almost on par with mixed
representation with the added benefit that its less confusing and users
don't need manual checks and conversions for doing operations on the
value.

```
cpu: AMD Ryzen 5 7530U with Radeon Graphics
runtime: deno 1.43.6+92a8d09 (x86_64-unknown-linux-gnu)

file:///home/divy/gh/ffi/main.ts
benchmark                 time (avg)        iter/s             (min … max)       p75       p99      p995
-------------------------------------------------------------------------- -----------------------------
nop                        4.01 ns/iter 249,533,690.5     (3.97 ns … 10.8 ns) 3.97 ns 4.36 ns 9.03 ns
ret bigint                 7.74 ns/iter 129,127,186.8    (7.72 ns … 10.46 ns) 7.72 ns 8.11 ns 8.82 ns
ret i32                    7.81 ns/iter 128,087,100.5    (7.77 ns … 12.72 ns) 7.78 ns 8.57 ns 9.75 ns
ret bigint (add op)       15.02 ns/iter  66,588,253.2   (14.64 ns … 24.99 ns) 14.76 ns 19.13 ns 19.44 ns
ret i32    (add op)       12.02 ns/iter  83,209,131.8   (11.95 ns … 18.18 ns) 11.98 ns 13.11 ns 14.5 ns
```
This commit is contained in:
Divy Srivastava 2024-05-27 21:01:09 -07:00 committed by GitHub
parent d99c6c1ea4
commit 53606de634
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
9 changed files with 25 additions and 107 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
cli/tsc/dts/lib.deno.unstable.d.ts vendored
View file

@ -247,7 +247,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue
@ -292,7 +292,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue
@ -319,7 +319,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue

17
ext/ffi/00_ffi.js
View file

@ -46,7 +46,6 @@ const {
ObjectDefineProperty,
ObjectHasOwn,
ObjectPrototypeIsPrototypeOf,
Number,
NumberIsSafeInteger,
TypedArrayPrototypeGetBuffer,
TypedArrayPrototypeGetByteLength,
@ -348,10 +347,6 @@ function isReturnedAsBigInt(type) {
type === "usize" || type === "isize";
}
function isI64(type) {
return type === "i64" || type === "isize";
}
function isStruct(type) {
return typeof type === "object" && type !== null &&
typeof type.struct === "object";
@ -562,7 +557,6 @@ class DynamicLibrary {
const call = this.symbols[symbol];
const parameters = symbols[symbol].parameters;
const vi = new Int32Array(2);
const vui = new Uint32Array(TypedArrayPrototypeGetBuffer(vi));
const b = new BigInt64Array(TypedArrayPrototypeGetBuffer(vi));
const params = ArrayPrototypeJoin(
@ -572,22 +566,13 @@ class DynamicLibrary {
// Make sure V8 has no excuse to not optimize this function.
this.symbols[symbol] = new Function(
"vi",
"vui",
"b",
"call",
"NumberIsSafeInteger",
"Number",
`return function (${params}) {
call(${params}${parameters.length > 0 ? ", " : ""}vi);
${
isI64(resultType)
? `const n1 = Number(b[0])`
: `const n1 = vui[0] + 2 ** 32 * vui[1]` // Faster path for u64
};
if (NumberIsSafeInteger(n1)) return n1;
return b[0];
}`,
)(vi, vui, b, call, NumberIsSafeInteger, Number);
)(vi, b, call);
} else if (isStructResult && !isNonBlocking) {
const call = this.symbols[symbol];
const parameters = symbols[symbol].parameters;

15
ext/ffi/callback.rs
View file

@ -4,8 +4,6 @@ use crate::check_unstable;
use crate::symbol::NativeType;
use crate::FfiPermissions;
use crate::ForeignFunction;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::AnyError;
use deno_core::op2;
use deno_core::v8;
@ -243,20 +241,11 @@ unsafe fn do_ffi_callback(
}
NativeType::I64 | NativeType::ISize => {
let result = *((*val) as *const i64);
if result > MAX_SAFE_INTEGER as i64 || result < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
}
v8::BigInt::new_from_i64(scope, result).into()
}
NativeType::U64 | NativeType::USize => {
let result = *((*val) as *const u64);
if result > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
}
v8::BigInt::new_from_u64(scope, result).into()
}
NativeType::Pointer | NativeType::Buffer | NativeType::Function => {
let result = *((*val) as *const *mut c_void);

43
ext/ffi/ir.rs
View file

@ -1,8 +1,6 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
use crate::symbol::NativeType;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::v8;
@ -100,46 +98,13 @@ impl NativeValue {
v8::Integer::new_from_unsigned(scope, self.u32_value).into()
}
NativeType::I32 => v8::Integer::new(scope, self.i32_value).into(),
NativeType::U64 => {
let value = self.u64_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, value).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
}
NativeType::I64 => {
let value = self.i64_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as i64 || value < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, self.i64_value).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
}
NativeType::U64 => v8::BigInt::new_from_u64(scope, self.u64_value).into(),
NativeType::I64 => v8::BigInt::new_from_i64(scope, self.i64_value).into(),
NativeType::USize => {
let value = self.usize_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as usize {
v8::BigInt::new_from_u64(scope, value as u64).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
v8::BigInt::new_from_u64(scope, self.usize_value as u64).into()
}
NativeType::ISize => {
let value = self.isize_value;
let local_value: v8::Local<v8::Value> =
if !(MIN_SAFE_INTEGER..=MAX_SAFE_INTEGER).contains(&value) {
v8::BigInt::new_from_i64(scope, self.isize_value as i64).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
v8::BigInt::new_from_i64(scope, self.isize_value as i64).into()
}
NativeType::F32 => v8::Number::new(scope, self.f32_value as f64).into(),
NativeType::F64 => v8::Number::new(scope, self.f64_value).into(),

3
ext/ffi/lib.rs
View file

@ -39,9 +39,6 @@ const _: () = {
assert!(size_of::<*const ()>() == 8);
};
pub(crate) const MAX_SAFE_INTEGER: isize = 9007199254740991;
pub(crate) const MIN_SAFE_INTEGER: isize = -9007199254740991;
pub const UNSTABLE_FEATURE_NAME: &str = "ffi";
fn check_unstable(state: &OpState, api_name: &str) {

32
ext/ffi/static.rs
View file

@ -2,8 +2,6 @@
use crate::dlfcn::DynamicLibraryResource;
use crate::symbol::NativeType;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::op2;
@ -90,45 +88,29 @@ pub fn op_ffi_get_static<'scope>(
NativeType::U64 => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const u64) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_u64(scope, result).into();
integer
}
NativeType::I64 => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const i64) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as i64
|| result < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_i64(scope, result).into();
integer
}
NativeType::USize => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const usize) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as usize
{
v8::BigInt::new_from_u64(scope, result as u64).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_u64(scope, result as u64).into();
integer
}
NativeType::ISize => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const isize) };
let integer: v8::Local<v8::Value> =
if !(MIN_SAFE_INTEGER..=MAX_SAFE_INTEGER).contains(&result) {
v8::BigInt::new_from_i64(scope, result as i64).into()
} else {
v8::Number::new(scope, result as f64).into()
};
v8::BigInt::new_from_i64(scope, result as i64).into();
integer
}
NativeType::F32 => {

4
tests/ffi/tests/ffi_types.ts
View file

@ -160,7 +160,7 @@ let r_1: number | bigint = result;
const result2 = remote.symbols.method17();
// @ts-expect-error: Invalid argument
result2.then((_0: string) => {});
result2.then((_1: number | bigint) => {});
result2.then((_1: bigint) => {});
const result3 = remote.symbols.method18();
// @ts-expect-error: Invalid argument
@ -430,7 +430,7 @@ type __Tests__ = [
symbols: {
foo: (
...args: (number | Deno.PointerValue | null)[]
) => number | bigint;
) => bigint;
};
close(): void;
},

10
tests/ffi/tests/integration_tests.rs
View file

@ -76,10 +76,10 @@ fn basic() {
5\n\
5\n\
579\n\
8589934590\n\
-8589934590\n\
8589934590\n\
-8589934590\n\
8589934590n\n\
-8589934590n\n\
8589934590n\n\
-8589934590n\n\
9007199254740992n\n\
9007199254740992n\n\
-9007199254740992n\n\
@ -110,7 +110,7 @@ fn basic() {
Before\n\
After\n\
logCallback\n\
1 -1 2 -2 3 -3 4 -4 0.5 -0.5 1 2 3 4 5 6 7 8\n\
1 -1 2 -2 3 -3 4n -4n 0.5 -0.5 1 2 3 4 5 6 7 8\n\
u8: 8\n\
buf: [1, 2, 3, 4, 5, 6, 7, 8]\n\
logCallback\n\

2
tests/ffi/tests/test.js
View file

@ -637,7 +637,7 @@ await dylib.symbols.call_fn_ptr_return_u8_thread_safe(returnU8Callback.pointer);
// Test statics
assertEquals(dylib.symbols.static_u32, 42);
assertEquals(dylib.symbols.static_i64, -1242464576485);
assertEquals(dylib.symbols.static_i64, -1242464576485n);
assert(
typeof dylib.symbols.static_ptr === "object"
);