Skip to content

Conversation

devsnek
Copy link
Member

@devsnek devsnek commented Apr 24, 2022

this allows user code to take full advantage of v8's wasm streaming api by caching previously compiled modules.

Fixes: #36671

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/startup

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. 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 24, 2022
Copy link
Member

@tniessen tniessen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this would be a good candidate for an embedder API?

If we really want to expose this as a JavaScript API, the documentation should be very clear about the security implications. Loading compiled code from an untrusted origin defeats all security properties of WebAssembly.

@@ -245,6 +245,16 @@ function setupFetch() {
throw new ERR_WEBASSEMBLY_RESPONSE('body has already been used');
}

const handler = require('internal/v8').getWasmStreamingHandler();
const cached = await handler?.get(response.url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand this, this will only work when loading WebAssembly from a remote origin, and only if the URL is stable, and only if the WebAssembly module does not change.

if (streamState.setCompiledModuleBytes(cached)) {
return;
} else {
await handler.delete(response.url);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case, V8 has decided not to use the compiled code. That can happen, for example, when running node with certain V8 flags. However, actively deleting something from the cache means that two Node.js processes running with different command-line flags but using the same WebAssembly cache will keep filling and erasing the cache.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tniessen that's a really good point. Do you see a way to isolate this deletion?

env->wasm_streaming_compilation_callback()
->Call(
env->context(), Undefined(env->isolate()), arraysize(args), args)
.ToLocalChecked();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if this throws?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs related to general changes in the lib or src directory. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WebAssembly compiled module cache
5 participants