Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deno 2 has a broken API for node:v8 when using serialize with Float16Array #26580

Closed
cu8code opened this issue Oct 27, 2024 · 4 comments · Fixed by #27285
Closed

Deno 2 has a broken API for node:v8 when using serialize with Float16Array #26580

cu8code opened this issue Oct 27, 2024 · 4 comments · Fixed by #27285
Labels
bug Something isn't working correctly node:v8 upstream Changes in upstream are required to solve these issues

Comments

@cu8code
Copy link
Contributor

cu8code commented Oct 27, 2024

Version: Deno 2.0.0

Here is an example code that I run

import { serialize } from "node:v8";

const float16Data = new Float16Array([1.0, 2.5, 3.14]);

try {
  const serialized = serialize(float16Data);
  console.log("Serialization successful!");
  console.log("Serialized data:", serialized);
} catch (error) {
  console.error("Serialization failed:", error.message);
}

I get the error

Serialization failed: Unserializable host object: 1,2.5,3.140625
@cu8code
Copy link
Contributor Author

cu8code commented Oct 27, 2024

The error is only with Float16Array

import { serialize, deserialize } from "node:v8";

function testSerialization(description, value) {
  console.log(`\nTesting ${description}:`);
  try {
    const serialized = serialize(value);
    const deserialized = deserialize(serialized);
    console.log("✓ Serialization successful");
    console.log("Original:", value);
    console.log("Deserialized:", deserialized);
  } catch (error) {
    console.error("✗ Serialization failed:", error.message);
  }
}


const arrays = {
  "Float16Array": new Float16Array([1.0, 2.5, 3.14]),
  "Float32Array": new Float32Array([1.0, 2.5, 3.14]),
  "Float64Array": new Float64Array([1.0, 2.5, 3.14]),
  "Int32Array": new Int32Array([1, 2, 3]),
  "Uint8Array": new Uint8Array([1, 2, 3])
};

console.log("Deno version:", Deno.version.deno);
console.log("V8 version:", Deno.version.v8);

for (const [type, array] of Object.entries(arrays)) {
  testSerialization(type, array);
}


testSerialization("Regular Array", [1.0, 2.5, 3.14]);

I get the output which indicated only Float16Array is broken

Deno version: 2.0.0
V8 version: 12.9.202.13-rusty

Testing Float16Array:
✗ Serialization failed: Unserializable host object: 1,2.5,3.140625

Testing Float32Array:
✓ Serialization successful
Original: Float32Array(3) [ 1, 2.5, 3.140000104904175 ]
Deserialized: Float32Array(3) [ 1, 2.5, 3.140000104904175 ]

Testing Float64Array:
✓ Serialization successful
Original: Float64Array(3) [ 1, 2.5, 3.14 ]
Deserialized: Float64Array(3) [ 1, 2.5, 3.14 ]

Testing Int32Array:
✓ Serialization successful
Original: Int32Array(3) [ 1, 2, 3 ]
Deserialized: Int32Array(3) [ 1, 2, 3 ]

Testing Uint8Array:
✓ Serialization successful
Original: Uint8Array(3) [ 1, 2, 3 ]
Deserialized: Uint8Array(3) [ 1, 2, 3 ]

Testing Regular Array:
✓ Serialization successful
Original: [ 1, 2.5, 3.14 ]
Deserialized: [ 1, 2.5, 3.14 ]

@cu8code
Copy link
Contributor Author

cu8code commented Oct 27, 2024

The error is occurring because, there is no support for Float16Array, in arrayBufferViewTypeToIndex function. I am not sure why ?

function arrayBufferViewTypeToIndex(abView: ArrayBufferView) {
const type = ObjectPrototypeToString(abView);
if (type === "[object Int8Array]") return 0;
if (type === "[object Uint8Array]") return 1;
if (type === "[object Uint8ClampedArray]") return 2;
if (type === "[object Int16Array]") return 3;
if (type === "[object Uint16Array]") return 4;
if (type === "[object Int32Array]") return 5;
if (type === "[object Uint32Array]") return 6;
if (type === "[object Float32Array]") return 7;
if (type === "[object Float64Array]") return 8;
if (type === "[object DataView]") return 9;
// Index 10 is FastBuffer.
if (type === "[object BigInt64Array]") return 11;
if (type === "[object BigUint64Array]") return 12;
return -1;
}

_writeHostObject(abView: any) {
// Keep track of how to handle different ArrayBufferViews. The default
// Serializer for Node does not use the V8 methods for serializing those
// objects because Node's `Buffer` objects use pooled allocation in many
// cases, and their underlying `ArrayBuffer`s would show up in the
// serialization. Because a) those may contain sensitive data and the user
// may not be aware of that and b) they are often much larger than the
// `Buffer` itself, custom serialization is applied.
let i = 10; // FastBuffer
if (abView.constructor !== Buffer) {
i = arrayBufferViewTypeToIndex(abView);
if (i === -1) {
throw new this._getDataCloneError(
`Unserializable host object: ${abView}`,
);
}
}
this.writeUint32(i);
this.writeUint32(abView.byteLength);
this.writeRawBytes(
new Uint8Array(abView.buffer, abView.byteOffset, abView.byteLength),
);
}

@marvinhagemeister marvinhagemeister added bug Something isn't working correctly node:v8 labels Oct 28, 2024
@bartlomieju bartlomieju added the good first issue Good for newcomers label Oct 28, 2024
@bartlomieju
Copy link
Member

FYI this doesn't work in Node.js either (v23.1) because Float16Array is not available in Node. This is not a "good first issue" after all. We'd need to wait for Node to implement it, or at least coordinate with them to ensure that we use correct number for the this type.

@bartlomieju bartlomieju added upstream Changes in upstream are required to solve these issues and removed good first issue Good for newcomers labels Oct 28, 2024
@bartlomieju
Copy link
Member

The support for serde of Float16Array was landed in Node.js in nodejs/node#55996, PR to Deno will follow.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node:v8 upstream Changes in upstream are required to solve these issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants