Lost precision for large 64 bit numbers

[kevinkassimo]

Context: #12212

Currently all 64 bit numbers are forwarded to use v8::Number which results in lost of precision for e.g. large 64 values like u64::MAX:
https://github.com/denoland/serde_v8/blob/edd2994ac0732b56bad30f32655afd1e6250775c/src/ser.rs#L472-L474

While logically we have BigInt we can switch to, I am unsure how bad the performance implication could be.

[DjDeveloperr]

Tried giving this a try since I need this for FFI (returning pointers is basically impossible right now...), but for some reason tests fail as i32s are suddenly expected to be BigInts now.. but I just added implementations for serialize_(u|i)64 and removed from forward_to macro calls. Is there something else I need to do?

Here's what I changed btw: DjDeveloperr/serde_v8@07eb4b2

[AaronO]

@DjDeveloperr I had implemented this a while back, but I punted on it it since there was no immediate need and BigInts are incompatible with other Number values in JS, so this would be a breaking change.

> 3 + BigInt(2)
// Uncaught TypeError: Cannot mix BigInt and other types, use explicit conversions

[AaronO]

@DjDeveloperr Are you looking to pass around "opaque" pointers or u64s you want to do arithmetic on ?

[DjDeveloperr]

Oops, didn't think of the breaking change it would introduce... but I think it's quite important for FFI to support 64 bit integers. I sure can wrap the native library and make it write to buffer and read BigInt from it, but goal is to use without wrapping. So is it not possible to change these types to BigInts now?
And I'm trying to pass opaque pointers.

[DjDeveloperr]

I think another way without such breaking change would be for FFI to return bigints-as-strings (breaking but FFI is unstable) and we can parse them as BigInt in JS, not sure if that will be inefficient though.

[AaronO]

I think another way without such breaking change would be for FFI to return bigints-as-strings (breaking but FFI is unstable) and we can parse them as BigInt in JS, not sure if that will be inefficient though.

I think we can do it efficiently, likely without any serde_v8 changes. But I would prefer if pointers were opaque, not sure allowing pointer arithmetic in JS is a good idea ...

[DjDeveloperr]

IMO we should just allow pointer arithmetic, I don't think it's a good idea to hide pointer value from user

[eliassjogreen]

What if we return u/i64 as transmuted to f64, then the ffi code could use a DataView and convert it to BigInt64? I am not familiar enough with f64s to know if that will work for all numbers though...

[DjDeveloperr]

@eliassjogreen I think that would likely work.

[kevinkassimo]

I think another way without such breaking change would be for FFI to return bigints-as-strings (breaking but FFI is unstable) and we can parse them as BigInt in JS, not sure if that will be inefficient though.

How about we do not change any of the current forwarding logic, but then define a custom wrapper type (e.g. some like struct JSBigInt64(u64)) and could automatically be wrapped (with relevant trait impls) as long as user makes the return value type to be it, and any return value of such type would be converted to BigInt?

[eliassjogreen]

I think another way without such breaking change would be for FFI to return bigints-as-strings (breaking but FFI is unstable) and we can parse them as BigInt in JS, not sure if that will be inefficient though.

How about we do not change any of the current forwarding logic, but then define a custom wrapper type (e.g. some like struct JSBigInt64(u64)) and could automatically be wrapped (with relevant trait impls) as long as user makes the return value type to be it, and any return value of such type would be converted to BigInt?

Or we could add support for something already existing like the num_bigint crate for returning BigInts when that is required.

[DjDeveloperr]

What if we return u/i64 as transmuted to f64, then the ffi code could use a DataView and convert it to BigInt64? I am not familiar enough with f64s to know if that will work for all numbers though...

I think that would likely work.

Update: It doesn't work in my case unless the type is explicitly transmuted to f64 in native side - which I cannot do.

What about implementing serialize_i128 and serialize_u128 instead? Serde has built in support for these so it's easy to impl as well.