lib,src: implement WebAssembly Web API #42701

tniessen · 2022-04-12T03:04:20Z

Now that we have fetch(), we may as well also have WebAssembly.compileStreaming() and WebAssembly.instantiateStreaming().

Unfortunately, our fetch() implementation cannot be interacted with easily from C++, which is why part of this implementation ended up in JavaScript. The C++ glue code allows controlling the v8::WasmStreaming instance from JavaScript.

This attempts to be a spec-compliant implementation with no node-specific extensions.

Refs: #41749
Fixes: #21130

nodejs-github-bot · 2022-04-12T03:04:24Z

Review requested:

@nodejs/gyp
@nodejs/startup

devsnek · 2022-04-12T03:06:01Z

src/node_wasm_web_api.cc

+  new WasmStreamingObject(env, args.This());
+}
+
+void WasmStreamingObject::Push(


unless i am mistaken, these methods do not allocate. would you be up for adding fast call wrappers? if not i can maybe add them in a followup pr.

Will try that tomorrow. So far, every time I tried using fast calls, I can into some V8 limitation, but Push might be a good candidate for once.

On second thought, given how web streams are performing so far and how slow everything is, that seems like premature optimization. I might create a follow-up PR to add that or I might push that as a separate commit later, but I'd like to get this landed rather sooner than later and not make it more complicated to review.

test/parallel/test-wasm-web-api.js

doc/api/errors.md

src/node_wasm_web_api.h

src/node_wasm_web_api.cc

src/node_wasm_web_api.h

aduh95

Should we add WTP tests? Maybe https://github.com/web-platform-tests/wpt/tree/HEAD/wasm/webapi or a subset of it?

doc/api/errors.md

tniessen · 2022-04-18T16:10:08Z

Should we add WTP tests?

That's a good idea. It's not straightforward because many WPT tests for this API rely on fetch and we currently mock fetch in WPT.

Of the ones I can run easily, wasm/webapi/empty-body.any.js is failing, presumably due to nodejs/undici#1345.

aduh95 · 2022-04-18T17:24:53Z

Should we add WTP tests?

That's a good idea. It's not straightforward because many WPT tests for this API rely on fetch and we currently mock fetch in WPT.

Of the ones I can run easily, wasm/webapi/empty-body.any.js is failing, presumably due to nodejs/undici#1345.

I think you can create a status file that lists all the failing tests, and that can be used as a TODO list for contributors who want to improve Node.js API compliance. But maybe that's a lot of work (I've never done that myself so not sure), and it probably should not block this from landing.

aduh95 · 2022-05-09T17:03:43Z

I think it's fine to land it on v16.x, because it's behind the --experimental-fetch flag.

Refs: #42701 Refs: nodejs/undici#1346 Refs: #42939 PR-URL: #42960 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>

Node landed `WebAssembly.compileStreaming` and `instantiateStreaming` in nodejs/node#42701 and made it available in Node 18, so we no longer need to exclude these properties from our WebAssembly endowment. This PR therefore removes the WebAssembly endowment factory, which will now be included in whatever form it exists on the execution environment root realm global.

juanarbol · 2022-05-31T03:55:05Z

I think it's fine to land it on v16.x, because it's behind the --experimental-fetch flag.

Yeap, it does, It lands fine; except for the bootstrap, tried to fix this by adding the binding wasm_web_api to lib/internal/bootstrap/loaders.js with no sucess, I will label this to request backport:

make[1]: Leaving directory '/home/juanarbol/GitHub/node/test/node-api/test_worker_terminate_finalization/build'                                     [487/5756]
/usr/bin/python3.10 tools/test.py  --mode=release \                                                                                                           
         \                                                                                                                                                    
        --skip-tests= \                                                                                                                                       
        default \                                                                                                                                             
        addons js-native-api node-api                                                                                                                         
=== release test-bootstrap-modules ===                                                                                                                        
Path: parallel/test-bootstrap-modules                                                                                                                         
node:assert:123                                                                                                                                               
  throw new AssertionError(obj);                                                                                                                              
  ^                                                                                                                                                           
                                                                                                                                                              
AssertionError [ERR_ASSERTION]: These modules were not loaded:                                                                                                
  Internal Binding wasm_web_api                                                                                                                               
                                                                                                                                                              
    at Object.<anonymous> (/home/juanarbol/GitHub/node/test/parallel/test-bootstrap-modules.js:216:8)                                                         
    at Module._compile (node:internal/modules/cjs/loader:1105:14)                                                                                             
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1159:10)                                                                               
    at Module.load (node:internal/modules/cjs/loader:981:32)                                                                                                  
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)                                                                                        
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:77:12)                                                                     
    at node:internal/main/run_main_module:17:47 {

Refs: #42660 Refs: #42701 PR-URL: #42836 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

Refs: nodejs/node#42660 Refs: nodejs/node#42701 PR-URL: nodejs/node#42836 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Akhil Marsonya <akhil.marsonya27@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Zeyu Yang <himself65@outlook.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com>

nodejs/node#42701

* chore: update to Node.js v18 * child_process: improve argument validation nodejs/node#41305 * bootstrap: support configure-time user-land snapshot nodejs/node#42466 * chore: update GN patch * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * src: use a typed array internally for process._exiting nodejs/node#43883 * chore: lib/internal/bootstrap -> lib/internal/process * src: disambiguate terms used to refer to builtins and addons nodejs/node#44135 * chore: remove redudant browserGlobals patch * chore: update BoringSSL patch * src: allow embedder-provided PageAllocator in NodePlatform nodejs/node#38362 * chore: fixup Node.js crypto tests - nodejs/node#44171 - nodejs/node#41600 * lib: add Promise methods to avoid-prototype-pollution lint rule nodejs/node#43849 * deps: update V8 to 10.1 nodejs/node#42657 * src: add kNoBrowserGlobals flag for Environment nodejs/node#40532 * chore: consolidate asar initialization patches * deps: update V8 to 10.1 nodejs/node#42657 * deps: update V8 to 9.8 nodejs/node#41610 * src,crypto: remove AllocatedBuffers from crypto_spkac nodejs/node#40752 * build: enable V8's shared read-only heap nodejs/node#42809 * src: fix ssize_t error from nghttp2.h nodejs/node#44393 * chore: fixup ESM patch * chore: fixup patch indices * src: merge NativeModuleEnv into NativeModuleLoader nodejs/node#43824 * [API] Pass OOMDetails to OOMErrorCallback https://chromium-review.googlesource.com/c/v8/v8/+/3647827 * src: iwyu in cleanup_queue.cc * src: return Maybe from a couple of functions nodejs/node#39603 * src: clean up embedder API nodejs/node#35897 * src: refactor DH groups to delete crypto_groups.h nodejs/node#43896 * deps,src: use SIMD for normal base64 encoding nodejs/node#39775 * chore: remove deleted source file * chore: update patches * chore: remove deleted source file * lib: add fetch nodejs/node#41749 * chore: remove nonexistent node specs * test: split report OOM tests nodejs/node#44389 * src: trace fs async api nodejs/node#44057 * http: trace http request / response nodejs/node#44102 * test: split test-crypto-dh.js nodejs/node#40451 * crypto: introduce X509Certificate API nodejs/node#36804 * src: split property helpers from node::Environment nodejs/node#44056 * nodejs/node#38905 bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob * lib,src: implement WebAssembly Web API nodejs/node#42701 * fixup! deps,src: use SIMD for normal base64 encoding * fixup! src: refactor DH groups to delete crypto_groups.h * chore: fixup base64 GN file * fix: check that node::InitializeContext() returns true * chore: delete _noBrowserGlobals usage * chore: disable fetch in renderer procceses * dns: default to verbatim=true in dns.lookup() nodejs/node#39987 Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>

Implemented in nodejs/node#42701. Manually tested that it's present in Node.js 18.1 and not in 18.0.

* Add data for `WebAssebly.*Streaming` in Node.js Implemented in nodejs/node#42701. Manually tested that it's present in Node.js 18.1 and not in 18.0. * Apply suggestions from code review Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <vinyldarkscratch@gmail.com> --------- Co-authored-by: Queen Vinyl Da.i'gyu-Kazotetsu <vinyldarkscratch@gmail.com>

tniessen added notable-change PRs with changes that should be highlighted in changelogs. wasm Issues and PRs related to WebAssembly. fetch Issues and PRs related to the Fetch API labels Apr 12, 2022

nodejs-github-bot added lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run. labels Apr 12, 2022

devsnek reviewed Apr 12, 2022

View reviewed changes

tniessen mentioned this pull request Apr 12, 2022

doc: add maintaining-webassembly.md #42660

Closed

devsnek reviewed Apr 12, 2022

View reviewed changes

test/parallel/test-wasm-web-api.js Show resolved Hide resolved

targos added the semver-minor PRs that contain new features and should be released in the next minor version. label Apr 12, 2022

aduh95 reviewed Apr 12, 2022

View reviewed changes

doc/api/errors.md Show resolved Hide resolved

RaisinTen reviewed Apr 12, 2022

View reviewed changes

tniessen force-pushed the wasm-web-api branch 4 times, most recently from f76bda6 to 8689723 Compare April 13, 2022 00:58

tniessen added the review wanted PRs that need reviews. label Apr 17, 2022

tniessen force-pushed the wasm-web-api branch from 3185f41 to 72b7298 Compare April 17, 2022 11:24

RaisinTen mentioned this pull request Apr 17, 2022

build: run clang-format on CI #42681

Merged

tniessen added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 17, 2022

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 17, 2022

This comment was marked as outdated.

Sign in to view

aduh95 reviewed Apr 17, 2022

View reviewed changes

doc/api/errors.md Show resolved Hide resolved

github-actions bot mentioned this pull request Apr 18, 2022

CI Reliability 2022-04-18 nodejs/reliability#255

Open

14 tasks

tniessen mentioned this pull request Apr 18, 2022

response.bodyUsed is undefined when body is null nodejs/undici#1345

Closed

This was referenced Apr 19, 2022

CI Reliability 2022-04-19 nodejs/reliability#256

Open

CI Reliability 2022-04-20 nodejs/reliability#257

Open

aduh95 removed the dont-land-on-v16.x label May 9, 2022

mizar mentioned this pull request May 10, 2022

WebAssemblyビルドの調整 yaneurao/YaneuraOu#236

Merged

rekmarks mentioned this pull request May 14, 2022

Remove WebAssembly endowment factory MetaMask/snaps#459

Merged

juanarbol added the backport-requested-v16.x label May 31, 2022

This was referenced May 31, 2022

lib: move WebAssembly Web API into separate file #42993

Merged

wasm: add missing init reported by coverity #42897

Closed

targos added dont-land-on-v16.x and removed backport-requested-v16.x labels Sep 16, 2022

juanarbol mentioned this pull request Sep 26, 2022

test: update hr-time and console wpt #44100

Closed

codebytere added a commit to electron/electron that referenced this pull request Oct 24, 2022

lib,src: implement WebAssembly Web API

1a6887f

nodejs/node#42701

codebytere added a commit to electron/electron that referenced this pull request Oct 24, 2022

lib,src: implement WebAssembly Web API

9b61756

nodejs/node#42701

codebytere added a commit to electron/electron that referenced this pull request Oct 26, 2022

lib,src: implement WebAssembly Web API

d8344cd

nodejs/node#42701

codebytere added a commit to electron/electron that referenced this pull request Nov 8, 2022

lib,src: implement WebAssembly Web API

1d482bd

nodejs/node#42701

codebytere mentioned this pull request Nov 8, 2022

Enable WebAssembly.{compileStreaming|instantiateStreaming} in Node.js electron/electron#36282

Closed

codebytere added a commit to electron/electron that referenced this pull request Nov 8, 2022

lib,src: implement WebAssembly Web API

41d7d26

nodejs/node#42701

nicolo-ribaudo added a commit to nicolo-ribaudo/browser-compat-data that referenced this pull request Sep 14, 2023

Add data for WebAssebly.*Streaming in Node.js

8461be3

Implemented in nodejs/node#42701. Manually tested that it's present in Node.js 18.1 and not in 18.0.

nicolo-ribaudo mentioned this pull request Sep 14, 2023

Add data for WebAssembly.*Streaming in Node.js mdn/browser-compat-data#20704

Merged

landonxjames mentioned this pull request Jan 29, 2024

Consider using compileStreaming and instantiateStreaming to enable WASM caching on Node bytecodealliance/jco#363

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lib,src: implement WebAssembly Web API #42701

lib,src: implement WebAssembly Web API #42701

tniessen commented Apr 12, 2022

nodejs-github-bot commented Apr 12, 2022

devsnek Apr 12, 2022

tniessen Apr 12, 2022

tniessen Apr 21, 2022

This comment was marked as outdated.

This comment was marked as outdated.

aduh95 left a comment

tniessen commented Apr 18, 2022

aduh95 commented Apr 18, 2022

aduh95 commented May 9, 2022

juanarbol commented May 31, 2022

lib,src: implement WebAssembly Web API #42701

lib,src: implement WebAssembly Web API #42701

Conversation

tniessen commented Apr 12, 2022

nodejs-github-bot commented Apr 12, 2022

devsnek Apr 12, 2022

Choose a reason for hiding this comment

tniessen Apr 12, 2022

Choose a reason for hiding this comment

tniessen Apr 21, 2022

Choose a reason for hiding this comment

This comment was marked as outdated.

This comment was marked as outdated.

aduh95 left a comment

Choose a reason for hiding this comment

tniessen commented Apr 18, 2022

aduh95 commented Apr 18, 2022

aduh95 commented May 9, 2022

juanarbol commented May 31, 2022