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

feat: Allow support for Wasm modules #402

Open
wants to merge 82 commits into
base: main
Choose a base branch
from

Conversation

bartlomieju
Copy link
Member

More generic solution than #344 and #357.

It allows embedders to load any module type they want.

Still some rough edges that need to be fixed. I will probably
split this PR into a few more.

core/modules/mod.rs Outdated Show resolved Hide resolved
@petamoriken
Copy link
Contributor

I disagree with adding type: "wasm" and type: "url".

WebAssembly Modules

WebAssembly Modules are just now being standardized. For now it is the same as regular JavaScript modules, import { fib } from "./fib.wasm";, and there is no consensus to add a special type attribute. Pleas see WebAssembly/esm-integration#42.

In addition, Stage 3 Source Phase Imports is in progress as a preliminary step. If it is to proceed, this should be implemented first.

import source fooModule from "./foo.wasm";

const fooInstance = WebAssembly.instantiate(fooModule, { /* imports */ });

URL Modules

This only resolves URLs, does not fetch anything, does not evaluate anything. It is beyond the scope of Import Attributes, which should be resolved in the proposed Stage 1 Asset References. Please refer to the Module Harmony note for more information on each phase of the import.

Import Phase Modifiers

@lucacasonato Any thoughts?

@bartlomieju
Copy link
Member Author

Thanks for the comment @petamoriken, we are still discussing wasm imports - "bytes" import will solve the biggest pain point of using WASM in Deno already - that is a need to fetch() the WASM file and it not being part of the module graph.

With "bytes" imports we can have a small helper function:

export async function wasmShim(wasmBytes) {
  const wasmMod = await WebAssembly.compile(wasmBytes);
  const requestedImports = WebAssembly.Module.imports(wasmMod);
  const importedModules = await Promise.all(
    requestedImports.map((i) => i.module).map((m) => import(m)),
  );
  const importsObject = {};
  for (let i = 0; i < requestedImports.length; i++) {
    const importedModule = importedModules[i];
    const requestedImport = requestedImports[i];
    if (typeof importsObject[requestedImport.module] === "undefined") {
      importsObject[requestedImport.module] = {};
    }
    const import_ = importedModule[requestedImport.name];
    importsObject[requestedImport.module][requestedImport.name] = import_;
  }
  const result = await WebAssembly.instantiate(wasmMod, importsObject);
  return result;
}

and then load a WASM file like so:

import wasmBytes from "./ffmpeg.wasm" with { type: "bytes" };
const ffmpeg = await wasmShim(wasmBytes);

This will make ffmpeg.wasm part of the module graph which will make this file be cached and very cheap to load.

@petamoriken
Copy link
Contributor

petamoriken commented Jan 7, 2024

I have no objections to type: "bytes" or type: "text".

However, for WebAssembly, I think the standard Source Phase Imports are sufficient.

import source wasmModule from "./ffmpeg.wasm";
const ffmpeg = await wasmShim(wasmModule);

@JOTSR
Copy link

JOTSR commented Jan 7, 2024

I think that using MIME type as module type assertion is a safer and more future-proof option than arbitrary string type. MIME type is more web-compliant and is more like future in-browser module assertion approach. Additionally, MIME type can prevent conflicts if a new alternative for an existing type emerges, and it can allow deno to verify the MIME type of remote imports and ensure host security.

Type MIME
wasm application/wasm
bytes application/octet-stream (any format), image/png (specific format)
text text/html, text/plain (eg: markdown)
url text/x-url

@bartlomieju
Copy link
Member Author

I have no objections to type: "bytes" or type: "text".

However, for WebAssembly, I think the standard Source Phase Imports are sufficient.

import source wasmModule from "./ffmpeg.wasm";
const ffmpeg = await wasmShim(wasmModule);

Yes, that would be the best solution, however this is still a proposal that is not implemented by V8. Once this is supported we can discourage the use of "bytes" import and suggest to use import source.

I think that using MIME type as module type assertion is a safer and more future-proof option than arbitrary string type. MIME type is more web-compliant and is more like future in-browser module assertion approach. Additionally, MIME type can prevent conflicts if a new alternative for an existing type emerges, and it can allow deno to verify the MIME type of remote imports and ensure host security.

I'm not sure I understand what you are suggesting here @JOTSR. This PR is just a proof-of-concept to get these types of imports working. The actual validation and decision if a particular import should work will be left to the embedder - in case of Deno CLI we already do checks against file extensions and MIME types.

@petamoriken
Copy link
Contributor

I have no objections to type: "bytes" or type: "text".
However, for WebAssembly, I think the standard Source Phase Imports are sufficient.

import source wasmModule from "./ffmpeg.wasm";
const ffmpeg = await wasmShim(wasmModule);

Yes, that would be the best solution, however this is still a proposal that is not implemented by V8. Once this is supported we can discourage the use of "bytes" import and suggest to use import source.

Can we achieve this somehow by transpiling using swc? Likely soon we will have source phase imports in swc.
swc-project/swc#8279

@bartlomieju
Copy link
Member Author

I have no objections to type: "bytes" or type: "text".
However, for WebAssembly, I think the standard Source Phase Imports are sufficient.

import source wasmModule from "./ffmpeg.wasm";
const ffmpeg = await wasmShim(wasmModule);

Yes, that would be the best solution, however this is still a proposal that is not implemented by V8. Once this is supported we can discourage the use of "bytes" import and suggest to use import source.

Can we achieve this somehow by transpiling using swc? Likely soon we will have source phase imports in swc. swc-project/swc#8279

That could work, but we never transpile JS sources in Deno. It would be a big change (and have a noticeable impact on performance) to transpile JS sources as well. I will raise this topic within the team.

@littledivy
Copy link
Member

littledivy commented Jan 8, 2024

type: "url" is quite useful for publishing FFI libraries.

import lib from "../libfoo.so" with { type: "url" };

const { symbols } = Deno.dlopen(new URL(lib).pathname, {});

@JOTSR
Copy link

JOTSR commented Jan 8, 2024

I'm not sure I understand what you are suggesting here @JOTSR. This PR is just a proof-of-concept to get these types of imports working. The actual validation and decision if a particular import should work will be left to the embedder - in case of Deno CLI we already do checks against file extensions and MIME types.

I said that one possible benefit of allowing the developer to specify the MIME type (mostly for remote URLs) can enhance security and clarity. This is also consistent with many web APIs that require the MIME type for linked resources.

Eg: like ejm.sh that allow embeding assets in the module.

import ffiLib from 'some_ffi_url' with { type: "application/octet-stream" }
//ffiLib Uint8Array

import wasmLib from 'some_wasm_url' with { type: "application/wasm" }
//wasmLib Uint8Array

Also using MIME type can allow "smart" imports in the future.

import htmlRaw from 'html' with { type: "text/plain" } //originally supported type
//htmlRaw as string

import htmlDoc from 'html' with { type: "text/html" } //a future support
//htmlDoc as Document

@jsejcksn
Copy link

jsejcksn commented Jan 8, 2024

Also using MIME type can allow "smart" imports in the future.

import htmlRaw from 'html' with { type: "text/plain" } //originally supported type
//htmlRaw as string

import htmlDoc from 'html' with { type: "text/html" } //a future support
//htmlDoc as Document

There's some prior discussion about that here:

core/modules/map.rs Outdated Show resolved Hide resolved
wasm_example/import.mjs Outdated Show resolved Hide resolved
core/modules/map.rs Outdated Show resolved Hide resolved
const modInstance = await import.meta.wasmInstantiate(wasmMod);"#,
);

let deps = WasmDeps {
Copy link
Contributor

Choose a reason for hiding this comment

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

We could probably store some basic WASM modules in the testdata and simplify these tests.

Copy link
Member Author

Choose a reason for hiding this comment

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

WasmDeps doesn't implement Serialize/Deserialize so there's no easy way to store intermediate representation. I agree that it could be improved, but maybe it's not a blocker for this PR?

let rendered = render_js_wasm_module("./foo.wasm", deps);
pretty_assertions::assert_eq!(
rendered,
r#"import wasmMod from "./foo.wasm" with { type: "$$deno-core-internal-wasm-module" };
Copy link
Contributor

Choose a reason for hiding this comment

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

Store as a .out file?

testing/integration/wasm_imports/add.wat Outdated Show resolved Hide resolved
const modInstance = await import.meta.wasmInstantiate(wasmMod);"#,
);

let deps = WasmDeps {
Copy link
Member Author

Choose a reason for hiding this comment

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

WasmDeps doesn't implement Serialize/Deserialize so there's no easy way to store intermediate representation. I agree that it could be improved, but maybe it's not a blocker for this PR?

@bartlomieju bartlomieju changed the title feat: Allow embedders to load custom module types feat: Allow support for Wasm modules Feb 5, 2024
@mmastrac mmastrac removed their request for review May 23, 2024 16:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants