V3: Using JsBuffer to transfer AssetGraph to Nodejs #313
Conversation
alshdavid
force-pushed
the
alsh/v3-using-buffers-for-assetgraph-napi-transfer
branch
from
a1ad2fa to
5c05361
Compare
alshdavid
force-pushed
the
alsh/v3-using-buffers-for-assetgraph-napi-transfer
branch
from
5c05361 to
75c10cd
Compare
| }; | ||
|
|
||
| let node_bytes = serde_json::to_vec(&serialized_node)?; | ||
| let js_buffer = env.create_buffer_with_data(node_bytes)?; |
There was a problem hiding this comment.
why do we do this instead of just returning an object?
if this is better why not serialise the entire thing as JSON?
There was a problem hiding this comment.
why do we do this instead of just returning an object?
If you're referring to a napi_derive::napi(object) it's because the macros are broken in the IDE (though the build will pass). We can either move to napi_v3 (prerelease) or manually cast the properties to an object.
If you mean why convert to JSON in the first place, it's because creating the objects takes in Rust takes much longer
if this is better why not serialise the entire thing as JSON?
Returning a single JSON exceeds the max string length limit in Nodejs. Having each node be its own json allows for the total JSON size to be broken up into smaller pieces. Using buffers isn't necessary but allows the values to be moved rather than copied
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { |
There was a problem hiding this comment.
These tests were checking the max string length limitation of Nodejs was respected by the manual creation of the original JsString in Rust.
Because the new function uses Env directly it cannot be unit tested without a Nodejs environment. This circuit is covered by the Nodejs integration tests in v3 because failure to transfer the AssetGraph would result in a critical failure
alshdavid
deleted the
alsh/v3-using-buffers-for-assetgraph-napi-transfer
branch
To help with the determinism issue I am experimenting with changes to the serialization of the AssetGraph. This PR simplifies the transferral of the AssetGraph from
JsStringvalues toJsBufferto make it easier to work with when experimenting with changes to the serialization approach.It performs pretty much identically (slightly faster but not meaningfully) however does not have the same string size limitations and avoids the need to manually create a JSON string from Rust.