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

[littledivy]

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:https:///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

[littledivy]

Marked as breaking because of the type definition changes. It should not affect runtime usage since we didn't guarantee number | bigint at runtime

[aapoalas]

LGTM!

[aapoalas]

Actually, I wonder if we don't have some code on the Fast API path that is now irrelevant? I think we might be setting the Integer64Representation to Number, that should now change to BigInt

[littledivy]

Good catch.

Did we ever use kUint64 for u64 return types though? We have some needsUnwrapping logic where its passes a mutable RV as the last value of the fast call:

deno/ext/ffi/dlfcn.rs

Lines 55 to 60 in b21004b

	pub fn needs_unwrap(rv: &NativeType) -> bool {
	matches!(
	rv,
	NativeType::I64 | NativeType::ISize | NativeType::U64 | NativeType::USize
	)
	}

I updated the JS side where the unwrapping happens to always read it as BigInt.

[aapoalas]

Hmm, didn't I ever get around to supporting them (u64, i64 return values that is) in FFI? 🤔 They're nowadays supported in Fast API though, so we could get rid of the JS-side unwrapping.

[bartlomieju]

LGTM, good to see so much code removed.