[Project516]

Under node, readAsync returns the Buffer from fs.readFileSync, and getWasmBinary wrapped it in new Uint8Array(...), which copies the whole file. On the web readAsync returns an ArrayBuffer, where the wrap only makes a view. WebAssembly.instantiate accepts either, so return what readAsync returns.

For a 40 MB module this takes about 50 ms and 40 MB of peak memory off startup under node.

test_node_wasm_binary_not_copied checks that WebAssembly.instantiate receives the Buffer under node. It fails without this change. Code size expectations are rebaselined: the JS output is slightly smaller, and no wasm size changed.

[gemini-code-assist]

Code Review

This pull request optimizes WebAssembly binary loading by returning the result of readAsync directly, which avoids copying the binary into a new Uint8Array under Node.js. However, this change causes getWasmBinary to return an ArrayBuffer instead of a Uint8Array on the web. This can lead to runtime errors for any downstream code expecting typed array properties or methods. To resolve this, the feedback suggests conditionally wrapping the response in a Uint8Array only if it is not already an instance of one.

[gemini-code-assist]

Returning readAsync(binaryFile) directly means that on the web, where readAsync returns an ArrayBuffer, getWasmBinary will now return an ArrayBuffer instead of a Uint8Array.

While WebAssembly.instantiate accepts both, other parts of the application or user-defined code might access Module.wasmBinary (or the returned value of getWasmBinary) and expect a Uint8Array (for example, checking .length or calling .subarray()). Since ArrayBuffer does not have these properties/methods, this change could introduce runtime errors or silent bugs on the web.

To avoid copying the Buffer under Node (since Buffer is a subclass of Uint8Array) while still ensuring a Uint8Array is returned on the web, we can check if the response is already an instance of Uint8Array and only wrap it in new Uint8Array() if it is not. This preserves the performance optimization under Node without changing the return type on the web.

      var response = await readAsync(binaryFile);
      return response instanceof Uint8Array ? response : new Uint8Array(response);

[Project516]

The return value never reaches Module.wasmBinary or user code. getWasmBinary has one caller, instantiateArrayBuffer, which passes the result straight to WebAssembly.instantiate(binary, imports), and that accepts an ArrayBuffer as well as a typed array. The function is internal (it is not in any exported runtime symbol list), and wasmBinary is a separate variable that this code only reads.

So on the web the value now stays an ArrayBuffer all the way into instantiate, and nothing else sees it. I kept the plain return because it is smaller in every codesize expectation. If you would rather keep a Uint8Array on the web, I can switch to response instanceof Uint8Array ? response : new Uint8Array(response).

[gemini-code-assist]

Returning readAsync(binaryFile) directly means that on the web, where readAsync returns an ArrayBuffer, getWasmBinary will now return an ArrayBuffer instead of a Uint8Array.

While WebAssembly.instantiate accepts both, other parts of the application or user-defined code might access Module.wasmBinary (or the returned value of getWasmBinary) and expect a Uint8Array (for example, checking .length or calling .subarray()). Since ArrayBuffer does not have these properties/methods, this change could introduce runtime errors or silent bugs on the web.

To avoid copying the Buffer under Node (since Buffer is a subclass of Uint8Array) while still ensuring a Uint8Array is returned on the web, we can check if the response is already an instance of Uint8Array and only wrap it in new Uint8Array() if it is not. This preserves the performance optimization under Node without changing the return type on the web.

      var response = await readAsync(binaryFile);
      return response instanceof Uint8Array ? response : new Uint8Array(response);

[sbc100]

Can I ask what motivated this change? Are you using Wasm on node in production and worried about peak memory?

[Project516]

I maintain a fork of ffmpeg.wasm (https://github.com/Project516/ffmpeg.wasm), which supports Node.js as well as browsers. Its core wasm is about 40 MB. While profiling startup of the Node build, the new Uint8Array(...) in getWasmBinary showed up as about 50 to 60 ms of a roughly 400 ms load, alongside the readFileSync itself. Skipping the copy also lowered peak RSS by about 20 MB in our benchmark runs.

So it is load time first and peak memory second. It matters most where each command starts a fresh process, which is how our benchmarks and some CLI-style uses run it. For small modules the copy is negligible, which is probably why it went unnoticed.

[sbc100]

Great, thats good to know! We don't really have good visibility on how many folks use emscripten-built modules under node in production.

[sbc100]

lgtm % comment